diff --git a/doc/manual/rl-next/deprecated-features.md b/doc/manual/rl-next/deprecated-features.md new file mode 100644 index 000000000..b800e24f0 --- /dev/null +++ b/doc/manual/rl-next/deprecated-features.md @@ -0,0 +1,11 @@ +--- +synopsis: 'more deprecated features' +issues: [] +cls: [2092] +category: Breaking Changes +credits: [piegames, commentator2.0] +--- +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. diff --git a/lix/libexpr/parser/grammar.hh b/lix/libexpr/parser/grammar.hh index cd549f150..09eedb89e 100644 --- a/lix/libexpr/parser/grammar.hh +++ b/lix/libexpr/parser/grammar.hh @@ -247,6 +247,7 @@ struct string : _string, seq< > {}; struct _ind_string { + struct strip_first_line : seq>, one<'\n'>> {}; struct line_start : semantic, star> {}; template struct literal : semantic, seq {}; @@ -264,7 +265,7 @@ struct _ind_string { struct ind_string : _ind_string, seq< TAO_PEGTL_STRING("''"), // Strip first line completely if empty - opt>, one<'\n'>>, + opt<_ind_string::strip_first_line>, list< seq< // Start a line with some indentation diff --git a/lix/libexpr/parser/parser-impl1.inc.cc b/lix/libexpr/parser/parser-impl1.inc.cc index 885f83c81..d134e9bdb 100644 --- a/lix/libexpr/parser/parser-impl1.inc.cc +++ b/lix/libexpr/parser/parser-impl1.inc.cc @@ -656,9 +656,20 @@ template<> struct BuildAST : change_head { struct IndStringState : SubexprState { using SubexprState::SubexprState; + // If the first line (after the '') is empty it gets completely removed. + // We track that in the grammar because no need to process it any further, + // but we still require the information to know the actual number of lines + // in the string. + bool firstLineStripped = false; std::vector lines; }; +template<> struct BuildAST { + static void apply(const auto & in, IndStringState & s, State & ps) { + s.firstLineStripped = true; + } +}; + template<> struct BuildAST { static void apply(const auto & in, IndStringState & s, State & ps) { s.lines.push_back(IndStringLine { in.string_view(), ps.at(in) }); @@ -717,6 +728,16 @@ template<> struct BuildAST { template<> struct BuildAST : change_head { static void success(const auto & in, IndStringState & s, ExprState & e, State & ps) { + if (!ps.featureSettings.isEnabled(Dep::BrokenStringIndent)) { + /* Check for semantically incorrect code: Single-line string with indentation */ + if (s.lines.size() == 1 && !s.firstLineStripped && s.lines.front().indentation.size() > 0) { + ps.badSingleLineIndStringFound(ps.at(in)); + } + /* Check for semantically incorrect code: Multi-line string with text on the first line */ + if (s.lines.size() > 1 && !s.firstLineStripped) { + ps.badFirstLineIndStringFound(ps.at(in)); + } + } e.pushExpr(noPos, ps.stripIndentation(ps.at(in), std::move(s.lines))); } }; diff --git a/lix/libexpr/parser/state.hh b/lix/libexpr/parser/state.hh index 57051e39a..b2e2f67e0 100644 --- a/lix/libexpr/parser/state.hh +++ b/lix/libexpr/parser/state.hh @@ -36,6 +36,8 @@ struct State void dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos); void overridesFound(const PosIdx pos); void badLineEndingFound(const PosIdx pos, bool warnOnly); + void badFirstLineIndStringFound(const PosIdx pos); + void badSingleLineIndStringFound(const PosIdx pos); void nulFound(const PosIdx pos); void addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr e, const PosIdx pos); void mergeAttrs(AttrPath & attrPath, ExprSet * source, ExprSet * target); @@ -95,6 +97,31 @@ inline void State::overridesFound(const PosIdx pos) { }); } +// Both added 2026-01-30. Probably won't turn this one into an error for a while, + +// as this has quite a lot of use in the wild. But it's clearly wrong code, +// so we should warn users about it. +// See the documentation on deprecated features for more details. +inline void State::badSingleLineIndStringFound(const PosIdx pos) +{ + logWarning({ + .msg = HintFmt( + "Whitespace in a ''-string will be stripped even if the string only has a single line, which is most likely not the intent of the code. To fix this, remove the whitespace or replace the string with \" instead. Use %s to silence this warning.", + "--extra-deprecated-features broken-string-indentation" + ), + .pos = positions[pos], + }); +} +inline void State::badFirstLineIndStringFound(const PosIdx pos) +{ + logWarning({ + .msg = HintFmt( + "Whitespace calculations for indentation stripping in a multiline ''-string include the first line, so putting text on it will effectively disable all indentation stripping. To fix this, simply break the line right after the string starts. Use %s to silence this warning.", + "--extra-deprecated-features broken-string-indentation" + ), + .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) { diff --git a/lix/libutil/deprecated-features/broken-string-indentation.md b/lix/libutil/deprecated-features/broken-string-indentation.md new file mode 100644 index 000000000..34073844e --- /dev/null +++ b/lix/libutil/deprecated-features/broken-string-indentation.md @@ -0,0 +1,23 @@ +--- +name: broken-string-indentation +internalName: BrokenStringIndent +timeline: + - date: 2026-01-30 + release: 2.95.0 + cls: [2092] + message: Introduced as a warning. +--- +Allow indented strings (those starting with `''`) even when the indentation stripping will produce incorrect and probably unintended results. +Affected strings are: + +- Single line indented strings that start with whitespace: `'' foo''` will be stripped of its leading space to `foo`. + To fix this, convert the string to `"` or manually concatenate in the leading whitespace. +- Multi line indented strings with text on the first line: + ``` + ''foo + bar + '' + + ``` + Having text on the first line here will completely disable the indentation stripping, which is unlikely desired. + To fix this, move the contents of the first line down by one line. diff --git a/lix/libutil/meson.build b/lix/libutil/meson.build index bcb24e717..2cc007090 100644 --- a/lix/libutil/meson.build +++ b/lix/libutil/meson.build @@ -175,6 +175,7 @@ experimental_feature_definitions = files( deprecated_feature_definitions = files( # keep-sorted start 'deprecated-features/ancient-let.md', + 'deprecated-features/broken-string-indentation.md', 'deprecated-features/cr-line-endings.md', 'deprecated-features/nix-path-shadow.md', 'deprecated-features/nul-bytes.md', diff --git a/tests/functional2/lang/string-indented/depr-warning.err.exp b/tests/functional2/lang/string-indented/depr-warning.err.exp new file mode 100644 index 000000000..52fc0ae1c --- /dev/null +++ b/tests/functional2/lang/string-indented/depr-warning.err.exp @@ -0,0 +1,18 @@ +warning: Whitespace calculations for indentation stripping in a multiline ''-string include the first line, so putting text on it will effectively disable all indentation stripping. To fix this, simply break the line right after the string starts. Use --extra-deprecated-features broken-string-indentation to silence this warning. + at /pwd/in.nix:19:8: + 18| + 19| s2 = '' If the string starts with whitespace + | ^ + 20| followed by a newline, it's stripped, but +warning: Whitespace calculations for indentation stripping in a multiline ''-string include the first line, so putting text on it will effectively disable all indentation stripping. To fix this, simply break the line right after the string starts. Use --extra-deprecated-features broken-string-indentation to silence this warning. + at /pwd/in.nix:55:8: + 54| + 55| s8 = '' ${""} + | ^ + 56| This shows a hacky way to preserve an empty line after the start. +warning: Whitespace in a ''-string will be stripped even if the string only has a single line, which is most likely not the intent of the code. To fix this, remove the whitespace or replace the string with " instead. Use --extra-deprecated-features broken-string-indentation to silence this warning. + at /pwd/in.nix:102:9: + 101| + 102| s12 = '' ''; + | ^ + 103| diff --git a/tests/functional2/lang/string-indented/depr-warning.out.exp b/tests/functional2/lang/string-indented/depr-warning.out.exp new file mode 100644 index 000000000..726cbc0bf --- /dev/null +++ b/tests/functional2/lang/string-indented/depr-warning.out.exp @@ -0,0 +1 @@ +[ "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 <