deprecated-features/broken-string-escape: Improve warning message
Explicitly catch common errors (trying to escape line breaks, badly escaping interpolations) to provide better messages and user guidance for these cases. Change-Id: I3dd1b2ad3bca33be393e65be5e72f4fb9544a46a
This commit is contained in:
piegames
@@ -698,7 +698,7 @@ template<> struct BuildAST<grammar::v1::string::escape> {
|
||||
if (!ps.featureSettings.isEnabled(Dep::BrokenStringEscape) && c != '\\' && c != '$'
|
||||
&& c != '"' && c != 'r' && c != 'n' && c != 't')
|
||||
{
|
||||
ps.badEscapeFound(ps.at(in), c, "\\");
|
||||
ps.badEscapeFound(ps.at(in), c, false);
|
||||
}
|
||||
s.append(ps.at(in), "\\"); // FIXME compat with old parser
|
||||
s.append(ps.at(in), in.string_view());
|
||||
@@ -769,7 +769,7 @@ template<> struct BuildAST<grammar::v1::ind_string::escape> {
|
||||
KJ_FALLTHROUGH;
|
||||
default:
|
||||
if (!ps.featureSettings.isEnabled(Dep::BrokenStringEscape)) {
|
||||
ps.badEscapeFound(ps.at(in), c, "''\\");
|
||||
ps.badEscapeFound(ps.at(in), c, true);
|
||||
}
|
||||
|
||||
s.lines.back().parts.emplace_back(ps.at(in), in.string_view());
|
||||
|
||||
@@ -38,7 +38,7 @@ struct State
|
||||
void badLineEndingFound(const PosIdx pos, bool warnOnly);
|
||||
void badFirstLineIndStringFound(const PosIdx pos);
|
||||
void badSingleLineIndStringFound(const PosIdx pos);
|
||||
void badEscapeFound(const PosIdx pos, char found, std::string escape);
|
||||
void badEscapeFound(const PosIdx pos, char found, bool isIndented);
|
||||
void nulFound(const PosIdx pos);
|
||||
void recSetMergeFound(const AttrPath & attrPath, const PosIdx pos);
|
||||
void recSetDynamicAttrFound(const PosIdx pos);
|
||||
@@ -129,17 +129,61 @@ inline void State::badFirstLineIndStringFound(const PosIdx pos)
|
||||
});
|
||||
}
|
||||
// Added 2024-12-12, equally used in the wild.
|
||||
inline void State::badEscapeFound(const PosIdx pos, char found, std::string escape)
|
||||
inline void State::badEscapeFound(const PosIdx pos, char found, bool isIndented)
|
||||
{
|
||||
logWarning({
|
||||
.msg = HintFmt(
|
||||
"%s is an ill-defined escape. You can drop the %s and simply write %s instead. Use %s "
|
||||
"to silence this warning.",
|
||||
auto escape = std::string(isIndented ? "''\\" : "\\");
|
||||
auto interpolEscape = std::string(isIndented ? "''${" : "\\${");
|
||||
HintFmt msg = HintFmt(
|
||||
"%s is an ill-defined escape. You can drop the %s and simply write %s instead. "
|
||||
"Use %s to silence this warning.",
|
||||
escape + found,
|
||||
escape,
|
||||
found,
|
||||
"--extra-deprecated-features broken-string-escape"
|
||||
);
|
||||
/* Special case some common escapes to provide better messages */
|
||||
if (found == '$' || found == '{') {
|
||||
/* Someone possibly tried to escape an interpolation but used the wrong sequence.
|
||||
* We don't have the full context within this function to know for sure (extracting that information
|
||||
* would require changing some parser rules), but we can at least add a hint about this to the default
|
||||
* message.
|
||||
*/
|
||||
msg = HintFmt(
|
||||
"%s is an ill-defined escape. You can drop the %s and simply write %s instead. "
|
||||
"If you meant to escape an interpolation, write %s instead. "
|
||||
"Use %s to silence this warning.",
|
||||
escape + found,
|
||||
escape,
|
||||
found,
|
||||
interpolEscape,
|
||||
"--extra-deprecated-features broken-string-escape"
|
||||
),
|
||||
);
|
||||
} else if (found == '\r' || found == '\n') {
|
||||
/* Someone tried to escape a line break */
|
||||
msg = HintFmt(
|
||||
"%s at the end of a line is an ill-defined escape. Escaping line endings has no effect. "
|
||||
"You can either drop the trailing %s, or use an explicit string concatenation instead. "
|
||||
"Use %s to silence this warning.",
|
||||
escape,
|
||||
escape,
|
||||
"--extra-deprecated-features broken-string-escape"
|
||||
);
|
||||
} else if (!isprint(found) || isspace(found)) {
|
||||
/* Generic error message for all non-printable escape characters */
|
||||
msg = HintFmt(
|
||||
"Found an ill-defined escape. You can drop the %s, as it has no effect. %s"
|
||||
"Use %s to silence this warning.",
|
||||
escape,
|
||||
isIndented
|
||||
? "(Note that an escaped space at the beginning of a string line may influence the string's "
|
||||
"indentation, however deliberately using this in strings is not supported.) "
|
||||
: "",
|
||||
"--extra-deprecated-features broken-string-escape"
|
||||
);
|
||||
}
|
||||
|
||||
logWarning({
|
||||
.msg = msg,
|
||||
.pos = positions[pos],
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1 +0,0 @@
|
||||
"a\n\nf{$\"'\rv\\\t\n'\${\"x\"}\n"
|
||||
@@ -1 +0,0 @@
|
||||
[ "This is an indented multi-line string\nliteral. An amount of whitespace at\nthe start of each line matching the minimum\nindentation of all lines in the string\nliteral together will be removed. Thus,\nin this case four spaces will be\nstripped from each line, even though\n THIS LINE is indented six spaces.\n\nAlso, empty lines don't count in the\ndetermination of the indentation level (the\nprevious empty line has indentation 0, but\nit doesn't matter).\n" "If the string starts with whitespace\n followed by a newline, it's stripped, but\n that's not the case here. Two spaces are\n stripped because of the \" \" at the start. \n" "This line is indented\na bit further.\n" "Anti-quotations, like so, are\nalso allowed.\n" " The \\ is not special here.\n' can be followed by any character except another ', e.g. 'x'.\nLikewise for $, e.g. $$ or $varName.\nBut ' followed by ' is special, as is $ followed by {.\nIf you want them, use anti-quotations: '', \${.\n" " Tabs are not interpreted as whitespace (since we can't guess\n what tab settings are intended), so don't use them.\n\tThis line starts with a space and a tab, so only one\n space will be stripped from each line.\n" "Also note that if the last line (just before the closing ' ')\nconsists only of whitespace, it's ignored. But here there is\nsome non-whitespace stuff, so the line isn't removed. " "\nThis shows a hacky way to preserve an empty line after the start.\nBut there's no reason to do so: you could just repeat the empty\nline.\n" " Similarly you can force an indentation level,\n in this case to 2 spaces. This works because the anti-quote\n is significant (not whitespace).\n" "" "" "" "start on network-interfaces\n\nstart script\n\n rm -f /var/run/opengl-driver\n ln -sf 123 /var/run/opengl-driver\n\n rm -f /var/log/slim.log\n \nend script\n\nenv SLIM_CFGFILE=abc\nenv SLIM_THEMESDIR=def\nenv FONTCONFIG_FILE=/etc/fonts/fonts.conf \t\t\t\t# !!! cleanup\nenv XKB_BINDIR=foo/bin \t\t\t\t# Needed for the Xkb extension.\nenv LD_LIBRARY_PATH=libX11/lib:libXext/lib:/usr/lib/ # related to xorg-sys-opengl - needed to load libglx for (AI)GLX support (for compiz)\n\nenv XORG_DRI_DRIVER_PATH=nvidiaDrivers/X11R6/lib/modules/drivers/ \n\nexec slim/bin/slim\n" "Escaping of ' followed by ': ''\nEscaping of $ followed by {: \${\nAnd finally to interpret \\n etc. as in a string: \n, \r, \t.\n" "foo\n'bla'\nbar\n" "cut -d $'\\t' -f 1\n" "ending dollar $$\n" " Lines without any indentation effectively disable the indentation\n stripping for the entire string:\n\n cat >$out/foo/data <<EOF\n lasjdöaxnasd\nasdom 12398\nä\"§Æẞ¢«»”alsd\nEOF\n" "Empty lines with a bit of whitespace don't affect the indentation calculation:\n\nAnd empty lines with more whitespace will have whitespace in the string:\n \nUnless it's the last line:\n" " Indentation stripping\n must not be impressed by\nthe last line not being empty" "\t Nor by people\n weirdly mixing tabs\n\tand spaces\n\t" ]
|
||||
@@ -0,0 +1,5 @@
|
||||
warning: ''\ at the end of a line is an ill-defined escape. Escaping line endings has no effect. You can either drop the trailing ''\, or use an explicit string concatenation instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
at /pwd/in.nix:1:7:
|
||||
1| ''a''\
|
||||
| ^
|
||||
2| b''
|
||||
+14
-2
@@ -10,13 +10,13 @@ warning: ''\f is an ill-defined escape. You can drop the ''\ and simply write f
|
||||
3| ''\f''\{''\$''\"''\'''\r''\v''\\''\t
|
||||
| ^
|
||||
4| ''\'''${"x"}
|
||||
warning: ''\{ is an ill-defined escape. You can drop the ''\ and simply write { instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
warning: ''\{ is an ill-defined escape. You can drop the ''\ and simply write { instead. If you meant to escape an interpolation, write ''${ instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
at /pwd/in.nix:3:8:
|
||||
2| ''\a''\n
|
||||
3| ''\f''\{''\$''\"''\'''\r''\v''\\''\t
|
||||
| ^
|
||||
4| ''\'''${"x"}
|
||||
warning: ''\$ is an ill-defined escape. You can drop the ''\ and simply write $ instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
warning: ''\$ is an ill-defined escape. You can drop the ''\ and simply write $ instead. If you meant to escape an interpolation, write ''${ instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
at /pwd/in.nix:3:12:
|
||||
2| ''\a''\n
|
||||
3| ''\f''\{''\$''\"''\'''\r''\v''\\''\t
|
||||
@@ -40,3 +40,15 @@ warning: ''\\ is an ill-defined escape. You can drop the ''\ and simply write \
|
||||
3| ''\f''\{''\$''\"''\'''\r''\v''\\''\t
|
||||
| ^
|
||||
4| ''\'''${"x"}
|
||||
warning: ''\$ is an ill-defined escape. You can drop the ''\ and simply write $ instead. If you meant to escape an interpolation, write ''${ instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
at /pwd/in.nix:7:4:
|
||||
6| ''${interpolation}
|
||||
7| ''\${bad interpolation 1}
|
||||
| ^
|
||||
8| $''\{bad interpolation 2}
|
||||
warning: ''\{ is an ill-defined escape. You can drop the ''\ and simply write { instead. If you meant to escape an interpolation, write ''${ instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
at /pwd/in.nix:8:5:
|
||||
7| ''\${bad interpolation 1}
|
||||
8| $''\{bad interpolation 2}
|
||||
| ^
|
||||
9| ''
|
||||
@@ -1 +1 @@
|
||||
"a\n\nf{$\"'\rv\\\t\n'\${\"x\"}\n"
|
||||
"a\n\nf{$\"'\rv\\\t\n'\${\"x\"}\n''\n\${interpolation}\n\${bad interpolation 1}\n\${bad interpolation 2}\n"
|
||||
|
||||
@@ -2,4 +2,8 @@
|
||||
''\a''\n
|
||||
''\f''\{''\$''\"''\'''\r''\v''\\''\t
|
||||
''\'''${"x"}
|
||||
'''
|
||||
''${interpolation}
|
||||
''\${bad interpolation 1}
|
||||
$''\{bad interpolation 2}
|
||||
''
|
||||
|
||||
@@ -1,13 +1,6 @@
|
||||
[[test]]
|
||||
runner = "eval-okay"
|
||||
matrix = true
|
||||
flags = ["--extra-deprecated-features", "broken-string-indentation broken-string-escape"]
|
||||
|
||||
[[test]]
|
||||
name = "depr-warning"
|
||||
runner = "eval-okay"
|
||||
matrix = true
|
||||
in = ["in.nix", "in-escapes.nix"]
|
||||
|
||||
[[test]]
|
||||
runner = "parse-okay"
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
warning: \ at the end of a line is an ill-defined escape. Escaping line endings has no effect. You can either drop the trailing \, or use an explicit string concatenation instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
at /pwd/in.nix:1:4:
|
||||
1| "a\
|
||||
| ^
|
||||
2| b"
|
||||
@@ -3,7 +3,7 @@ warning: \a is an ill-defined escape. You can drop the \ and simply write a inst
|
||||
1| "\a\n\{\$\"\'\r\v\\\t
|
||||
| ^
|
||||
2| \f"
|
||||
warning: \{ is an ill-defined escape. You can drop the \ and simply write { instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
warning: \{ is an ill-defined escape. You can drop the \ and simply write { instead. If you meant to escape an interpolation, write \${ instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
at /pwd/in.nix:1:7:
|
||||
1| "\a\n\{\$\"\'\r\v\\\t
|
||||
| ^
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
warning: \{ is an ill-defined escape. You can drop the \ and simply write { instead. If you meant to escape an interpolation, write \${ instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
|
||||
at /pwd/in.nix:7:33:
|
||||
6| "escape: \"quote\" \n \\"
|
||||
7| "\${escaped interpolation}, $\{wrongly escaped interpolation}"
|
||||
| ^
|
||||
8| "end
|
||||
@@ -1 +1 @@
|
||||
[ "foobar" "/a/b/c/d" "/foo/xyzzy/foo.txt" "/../foo/x/y" "escape: \"quote\" \n \\" "end\nof\nline" "foobarblaat" "foo$bar" "$\"$\"" "$" ]
|
||||
[ "foobar" "/a/b/c/d" "/foo/xyzzy/foo.txt" "/../foo/x/y" "escape: \"quote\" \n \\" "\${escaped interpolation}, \${wrongly escaped interpolation}" "end\nof\nline" "foobarblaat" "foo$bar" "$\"$\"" "$" ]
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
(toString (/foo/bar + "/../xyzzy/." + "/foo.txt"))
|
||||
("/../foo" + toString /x/y)
|
||||
"escape: \"quote\" \n \\"
|
||||
"\${escaped interpolation}, $\{wrongly escaped interpolation}"
|
||||
"end
|
||||
of
|
||||
line"
|
||||
|
||||
@@ -1,9 +0,0 @@
|
||||
[[test]]
|
||||
runner = "eval-okay"
|
||||
matrix = true
|
||||
flags = ["--extra-deprecated-features", "broken-string-escape"]
|
||||
in = ["in.nix", "in-backslash-newline.nix"]
|
||||
|
||||
[[test]]
|
||||
runner = "eval-okay"
|
||||
in = "in-escapes.nix"
|
||||
Reference in New Issue
Block a user