guiwf/lix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libexpr: Warn on incorrect string escapes

Browse Source 
In Nixpkgs, there are several strings like "\d\.\d" which attempt to be
a regex but are just literally "d.d". The escaping rules are silly and
we should warn our users about that.

Co-authored-by: Commentator2.0 <lix@crystal-cavern.systems>
Change-Id: I779b0757358fc9adc34dc140e1670b83abc93b67
This commit is contained in:
piegames
2026-01-31 15:32:27 +01:00
co-authored by Commentator2.0
parent f1fbd1d095
commit 56dee9186f
18 changed files with 158 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
doc/manual/rl-next/deprecated-features.md
+2 -1
View File
@@ -1,7 +1,7 @@
---
synopsis: 'more deprecated features'
issues: []
cls: [2092]
cls: [2092, 2310]
category: Breaking Changes
credits: [piegames, commentator2.0]
---
@@ -9,3 +9,4 @@ This release cycle features a new batch of deprecated (anti-)features.
You can opt in into the old behavior with `--extra-deprecated-features` or any equivalent configuration option.
- `broken-string-indentation` indented strings (those starting with `''`) might produce unintended results due to how the whitespace stripping is done. Those cases will now warn the user.
- `broken-string-escape` "escaped" characters without a properly defined escape sequence evaluate to "themselves". This is in most cases unintended behaviour, both for writing regexes, and using legacy or uncommon escape sequences like `\f`. The user will now be warned, if those are present.
lix/libexpr/parser/parser-impl1.inc.cc
+29 -5
View File
@@ -580,14 +580,17 @@ struct StringState : SubexprState {
if (c == 'n') *t = '\n';
else if (c == 'r') *t = '\r';
else if (c == 't') *t = '\t';
else *t = c;
else {
*t = c;
}
}
else if (c == '\r') {
/* Normalise CR and CR/LF into LF. */
*t = '\n';
if (*s == '\n') s++; /* cr/lf */
} else {
*t = c;
}
else *t = c;
t++;
}
if (!ps.featureSettings.isEnabled(Dep::NulBytes) && size_t(s - str.data() - 1) != str.size())
@@ -640,8 +643,15 @@ template<> struct BuildAST<grammar::v1::string::interpolation> {
template<> struct BuildAST<grammar::v1::string::escape> {
static void apply(const auto & in, StringState & s, State & ps) {
if (!ps.featureSettings.isEnabled(Dep::NulBytes) && *in.begin() == '\0')
char c = *in.begin();
if (!ps.featureSettings.isEnabled(Dep::NulBytes) && c == '\0') {
ps.nulFound(ps.at(in));
}
if (!ps.featureSettings.isEnabled(Dep::BrokenStringEscape) && c != '\\' && c != '$'
&& c != '"' && c != 'r' && c != 'n' && c != 't')
{
ps.badEscapeFound(ps.at(in), c, "\\");
}
s.append(ps.at(in), "\\"); // FIXME compat with old parser
s.append(ps.at(in), in.string_view());
}
@@ -691,17 +701,31 @@ template<> struct BuildAST<grammar::v1::ind_string::interpolation> {
template<> struct BuildAST<grammar::v1::ind_string::escape> {
static void apply(const auto & in, IndStringState & s, State & ps) {
switch (*in.begin()) {
auto c = *in.begin();
switch (c) {
case 'n': s.lines.back().parts.emplace_back(ps.at(in), "\n"); break;
case 'r': s.lines.back().parts.emplace_back(ps.at(in), "\r"); break;
case 't': s.lines.back().parts.emplace_back(ps.at(in), "\t"); break;
// TODO merge with below
// `''\'` must escape to itself even though one can just write `'` instead, because of
// shit like
// `''\'''${` to express the string `'${` (remember that `'''` escapes to `''`)
case '\'':
s.lines.back().parts.emplace_back(ps.at(in), "'");
break;
case 0:
if (!ps.featureSettings.isEnabled(Dep::NulBytes)) {
ps.nulFound(ps.at(in));
break;
}
KJ_FALLTHROUGH;
default: s.lines.back().parts.emplace_back(ps.at(in), in.string_view()); break;
default:
if (!ps.featureSettings.isEnabled(Dep::BrokenStringEscape)) {
ps.badEscapeFound(ps.at(in), c, "''\\");
}
s.lines.back().parts.emplace_back(ps.at(in), in.string_view());
break;
}
}
};
lix/libexpr/parser/state.hh
+16
View File
@@ -38,6 +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 nulFound(const PosIdx pos);
void addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr<Expr> e, const PosIdx pos);
void mergeAttrs(AttrPath & attrPath, ExprSet * source, ExprSet * target);
@@ -122,6 +123,21 @@ inline void State::badFirstLineIndStringFound(const PosIdx pos)
.pos = positions[pos],
});
}
// Added 2024-12-12, equally used in the wild.
inline void State::badEscapeFound(const PosIdx pos, char found, std::string escape)
{
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.",
escape + found,
escape,
found,
"--extra-deprecated-features broken-string-escape"
),
.pos = positions[pos],
});
}
// Added 2025-02-05. This is unlikely to ever occur in the wild, given how broken it is
inline void State::badLineEndingFound(const PosIdx pos, bool warnOnly)
{
lix/libutil/deprecated-features/broken-string-escape.md
+17
View File
@@ -0,0 +1,17 @@
---
name: broken-string-escape
internalName: BrokenStringEscape
timeline:
- date: 2024-12-12
release: 2.95.0
cls: [2310]
message: Introduced as a warning.
---
In Nix, string literals define syntax for escaping special characters like `\n`.
Only a limited set of escape rules are defined.
All characters without defined escape sequence escape "as themselves", e.g. `"\f"` becomes simply `f` instead of a form feed character.
Using these fallback escape sequences is now deprecated, because all usage sites in the wild found so far have been proven to be erroneous,
where the string did not end up the way the author likely intended.
For example when writing a regex in Nix, `"\."` will evaluate to the pattern `.` instead of the probably intended `\.` for matching a literal dot character, for which `"\\."` would have been correct instead.
To fix this, carefully evaluate each usage site for its intended usage and either remove the backslash or add a second backslash depending on the context.
lix/libutil/meson.build
+1
View File
@@ -175,6 +175,7 @@ experimental_feature_definitions = files(
deprecated_feature_definitions = files(
# keep-sorted start
'deprecated-features/ancient-let.md',
'deprecated-features/broken-string-escape.md',
'deprecated-features/broken-string-indentation.md',
'deprecated-features/cr-line-endings.md',
'deprecated-features/nix-path-shadow.md',
tests/functional2/lang/builtins.fromJSON/eval-okay.err.exp
View File
tests/functional2/lang/builtins.fromJSON/in.nix
+1 -1
View File
@@ -33,7 +33,7 @@ builtins.fromJSON
};
Animated = false;
IDs = [ 116 943 234 38793 true false null (0-100) ];
Escapes = "\"\\\/\t\n\r\t"; # supported in JSON but not Nix: \b\f
Escapes = "\"\\/\t\n\r\t"; # supported in JSON but not Nix: \b\f
Subtitle = false;
Latitude = 37.7668;
Longitude = -122.3959;
tests/functional2/lang/string-indented/depr-warning-escapes.err.exp
+42
View File
@@ -0,0 +1,42 @@
warning: ''\a is an ill-defined escape. You can drop the ''\ and simply write a instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
at /pwd/in.nix:2:4:
1| ''
2| ''\a''\n
| ^
3| ''\f''\{''\$''\"''\'''\r''\v''\\''\t
warning: ''\f is an ill-defined escape. You can drop the ''\ and simply write f instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
at /pwd/in.nix:3:4:
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.
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.
at /pwd/in.nix:3:12:
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.
at /pwd/in.nix:3:16:
2| ''\a''\n
3| ''\f''\{''\$''\"''\'''\r''\v''\\''\t
| ^
4| ''\'''${"x"}
warning: ''\v is an ill-defined escape. You can drop the ''\ and simply write v instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
at /pwd/in.nix:3:28:
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.
at /pwd/in.nix:3:32:
2| ''\a''\n
3| ''\f''\{''\$''\"''\'''\r''\v''\\''\t
| ^
4| ''\'''${"x"}
tests/functional2/lang/string-indented/depr-warning-escapes.out.exp
+1
View File
@@ -0,0 +1 @@
"a\n\nf{$\"'\rv\\\t\n'\${\"x\"}\n"
tests/functional2/lang/string-indented/eval-okay-escapes.out.exp
+1
View File
@@ -0,0 +1 @@
"a\n\nf{$\"'\rv\\\t\n'\${\"x\"}\n"
tests/functional2/lang/string-indented/in-escapes.nix
+5
View File
@@ -0,0 +1,5 @@
''
''\a''\n
''\f''\{''\$''\"''\'''\r''\v''\\''\t
''\'''${"x"}
''
tests/functional2/lang/string-indented/test.toml
+3 -2
View File
@@ -1,12 +1,13 @@
[[test]]
runner = "eval-okay"
matrix = true
flags = ["--extra-deprecated-features", "broken-string-indentation"]
flags = ["--extra-deprecated-features", "broken-string-indentation broken-string-escape"]
[[test]]
name = "depr-warning"
runner = "eval-okay"
in = "in.nix"
matrix = true
in = ["in.nix", "in-escapes.nix"]
[[test]]
runner = "parse-okay"
tests/functional2/lang/string/eval-okay-escapes.err.exp
+26
View File
@@ -0,0 +1,26 @@
warning: \a is an ill-defined escape. You can drop the \ and simply write a instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
at /pwd/in.nix:1:3:
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.
at /pwd/in.nix:1:7:
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.
at /pwd/in.nix:1:13:
1| "\a\n\{\$\"\'\r\v\\\t
| ^
2| \f"
warning: \v is an ill-defined escape. You can drop the \ and simply write v instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
at /pwd/in.nix:1:17:
1| "\a\n\{\$\"\'\r\v\\\t
| ^
2| \f"
warning: \f is an ill-defined escape. You can drop the \ and simply write f instead. Use --extra-deprecated-features broken-string-escape to silence this warning.
at /pwd/in.nix:2:2:
1| "\a\n\{\$\"\'\r\v\\\t
2| \f"
| ^
3|
tests/functional2/lang/string/eval-okay-escapes.out.exp
+1
View File
@@ -0,0 +1 @@
"a\n{$\"'\rv\\\t\nf"
tests/functional2/lang/string/in-escapes.nix
+2
View File
@@ -0,0 +1,2 @@
"\a\n\{\$\"\'\r\v\\\t
\f"
tests/functional2/lang/string/test.toml
+9
View File
@@ -0,0 +1,9 @@
[[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"
tests/nixpkgs/eval.nix
+1
View File
@@ -12,6 +12,7 @@ let
# FIXME: All of these are fixed in Nixpkgs already, so clear the list on the next `nixpkgs-regression` bump
deprecatedFeatures = [
"broken-string-indentation"
"broken-string-escape"
];
in
tests/nixpkgs/lib.nix
+1
View File
@@ -21,6 +21,7 @@ let
# FIXME: All of these are fixed in Nixpkgs already, so clear the list on the next `nixpkgs` bump
deprecatedFeatures = [
"broken-string-indentation"
"broken-string-escape"
];
env.NIX_CONFIG = "extra-deprecated-features = ${concatStringsSep " " deprecatedFeatures}";