Better escaping of String literals #126
Conversation
|
CC @lrhn |
json_serializable/lib/src/utils.dart
Outdated
| if (value.contains('\n')) { | ||
| return "r'''\n$value'''"; | ||
| } | ||
| value = value.replaceAll('\\', r'\\'); |
There was a problem hiding this comment.
this is for an old commit, I think...
json_serializable/lib/src/utils.dart
Outdated
|
|
||
| /// A [RegExp] that matches whitespace characters that should be escaped. | ||
| final _escapeRegExp = new RegExp( | ||
| '[\\x00-\\x07\\x0E-\\x1F${_escapeMap.keys.map(_getHexLiteral).join()}]'); |
There was a problem hiding this comment.
That seems a little too clever for what is just: new RegExp(r'[\x00-\x1f\x7f]').
I can't promise that it generates as efficient a RegExp (it might, if we recognize that it actually is a contiguous range, but I can't promise it).
There was a problem hiding this comment.
Nevermind, you extended the map in a later change, then it does make sense. I'd still probably start with just `\x00-\x1f' and not mind the overlap.
json_serializable/lib/src/utils.dart
Outdated
| return value; | ||
| } | ||
|
|
||
| canBeRaw = false; | ||
| var mapped = _escapeMap[match[0]]; | ||
| if (mapped != null) return mapped; | ||
| return _getHexLiteral(match[0]); |
There was a problem hiding this comment.
Those three lines could be just:
return _escapeMap[value] ?? _getHexLiteral(value);I'd even do:
return _escapeMap.putIfAbsent(value, _getHexLiteral);(and make the map not constant, then you will cache the hex literals the first time you compute them instead of creating a new string each time).
There was a problem hiding this comment.
Ooo...not a bad idea...
json_serializable/lib/src/utils.dart
Outdated
|
|
||
| // The only safe way to wrap the content is to escape all of the | ||
| // problematic characters - `$`, `'`, and `"` | ||
| var string = value.replaceAll(new RegExp(r"""(?=[$'"])"""), r'\'); |
There was a problem hiding this comment.
Cache the regexp in a final variable, like for _escapeRegExp.
| } else if (value == '"') { | ||
| hasDoubleQuote = true; | ||
| return value; | ||
| } else if (value == r'$') { |
There was a problem hiding this comment.
You could treat \ the same as $ wrt. "canBeRaw", a string with content "$\ can be raw as r'"$\'.
I can see that it doesn't work with the way this operation currently works - you can't decide later that the string can't be raw, because then you have to go back and fix previous \ characters, so I guess this is a good enough trade-off since backslashes are rare.
The alternative is to scan the string once first to figure out what it contains, and then fix it up in one second operation.
json_serializable/lib/src/utils.dart
Outdated
| } | ||
| } | ||
|
|
||
| // The only safe way to wrap the content is to escape all of the |
There was a problem hiding this comment.
I take offense with the word "only" :)
You could potentially use tripple-quotes in some cases (you just don't have the information here to know when).
No description provided.