mirror of
https://github.com/NixOS/nix.git
synced 2024-11-21 14:22:29 +00:00
libexpr: deprecate the bogus "or"-as-variable
As a prelude to making "or" work like a normal variable, emit a warning any time the "fn or" production is used in a context that will change how it is parsed when that production is refactored. In detail: in the future, OR_KW will be moved to expr_simple, and the cursed ExprCall production that is currently part of the expr_select nonterminal will be generated "normally" in expr_app instead. Any productions that accept an expr_select will be affected, except for the expr_app nonterminal itself (because, while expr_app has a production accepting a bare expr_select, its other production will continue to accept "fn or" expressions). So all we need to do is emit an appropriate warning when an expr_simple representing a cursed ExprCall is accepted in one of those productions without first going through expr_app. As the warning message describes, users can suppress the warning by wrapping their problematic "fn or" expressions in parentheses. For example, "f g or" can be made future-proof by rewriting it as "f (g or)"; similarly "[ x y or ]" can be rewritten as "[ x (y or) ]", etc. The parentheses preserve the current grouping behavior, as in the future "f g or" will be parsed as "(f g) or", just like "f g anything-else" is grouped. (Mechanically, this suppresses the warning because the problem ExprCalls go through the "expr_app : expr_select" production, which resets the cursed status on the ExprCall.)
This commit is contained in:
parent
68ba6ff470
commit
da332d678e
@ -663,4 +663,32 @@ std::string DocComment::getInnerText(const PosTable & positions) const {
|
||||
return docStr;
|
||||
}
|
||||
|
||||
|
||||
|
||||
/* ‘Cursed or’ handling.
|
||||
*
|
||||
* In parser.y, every use of expr_select in a production must call one of the
|
||||
* two below functions.
|
||||
*
|
||||
* To be removed by https://github.com/NixOS/nix/pull/11121
|
||||
*/
|
||||
|
||||
void ExprCall::resetCursedOr()
|
||||
{
|
||||
cursedOrEndPos.reset();
|
||||
}
|
||||
|
||||
void ExprCall::warnIfCursedOr(const SymbolTable & symbols, const PosTable & positions)
|
||||
{
|
||||
if (cursedOrEndPos.has_value()) {
|
||||
std::ostringstream out;
|
||||
out << "at " << positions[pos] << ": "
|
||||
"This expression uses `or` as an identifier in a way that will change in a future Nix release.\n"
|
||||
"Wrap this entire expression in parentheses to preserve its current meaning:\n"
|
||||
" (" << positions[pos].getSnippetUpTo(positions[*cursedOrEndPos]).value_or("could not read expression") << ")\n"
|
||||
"Give feedback at https://github.com/NixOS/nix/pull/11121";
|
||||
warn(out.str());
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -96,6 +96,10 @@ struct Expr
|
||||
virtual void setName(Symbol name);
|
||||
virtual void setDocComment(DocComment docComment) { };
|
||||
virtual PosIdx getPos() const { return noPos; }
|
||||
|
||||
// These are temporary methods to be used only in parser.y
|
||||
virtual void resetCursedOr() { };
|
||||
virtual void warnIfCursedOr(const SymbolTable & symbols, const PosTable & positions) { };
|
||||
};
|
||||
|
||||
#define COMMON_METHODS \
|
||||
@ -354,10 +358,16 @@ struct ExprCall : Expr
|
||||
Expr * fun;
|
||||
std::vector<Expr *> args;
|
||||
PosIdx pos;
|
||||
std::optional<PosIdx> cursedOrEndPos; // used during parsing to warn about https://github.com/NixOS/nix/issues/11118
|
||||
ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args)
|
||||
: fun(fun), args(args), pos(pos)
|
||||
: fun(fun), args(args), pos(pos), cursedOrEndPos({})
|
||||
{ }
|
||||
ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args, PosIdx && cursedOrEndPos)
|
||||
: fun(fun), args(args), pos(pos), cursedOrEndPos(cursedOrEndPos)
|
||||
{ }
|
||||
PosIdx getPos() const override { return pos; }
|
||||
virtual void resetCursedOr() override;
|
||||
virtual void warnIfCursedOr(const SymbolTable & symbols, const PosTable & positions) override;
|
||||
COMMON_METHODS
|
||||
};
|
||||
|
||||
|
@ -264,19 +264,28 @@ expr_op
|
||||
;
|
||||
|
||||
expr_app
|
||||
: expr_app expr_select { $$ = makeCall(CUR_POS, $1, $2); }
|
||||
| expr_select
|
||||
: expr_app expr_select { $$ = makeCall(CUR_POS, $1, $2); $2->warnIfCursedOr(state->symbols, state->positions); }
|
||||
| /* Once a ‘cursed or’ reaches this nonterminal, it is no longer cursed,
|
||||
because the uncursed parse would also produce an expr_app. But we need
|
||||
to remove the cursed status in order to prevent valid things like
|
||||
`f (g or)` from triggering the warning. */
|
||||
expr_select { $$ = $1; $$->resetCursedOr(); }
|
||||
;
|
||||
|
||||
expr_select
|
||||
: expr_simple '.' attrpath
|
||||
{ $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), nullptr); delete $3; }
|
||||
| expr_simple '.' attrpath OR_KW expr_select
|
||||
{ $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), $5); delete $3; }
|
||||
| /* Backwards compatibility: because Nixpkgs has a rarely used
|
||||
function named ‘or’, allow stuff like ‘map or [...]’. */
|
||||
{ $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), $5); delete $3; $5->warnIfCursedOr(state->symbols, state->positions); }
|
||||
| /* Backwards compatibility: because Nixpkgs has a function named ‘or’,
|
||||
allow stuff like ‘map or [...]’. This production is problematic (see
|
||||
https://github.com/NixOS/nix/issues/11118) and will be refactored in the
|
||||
future by treating `or` as a regular identifier. The refactor will (in
|
||||
very rare cases, we think) change the meaning of expressions, so we mark
|
||||
the ExprCall with data (establishing that it is a ‘cursed or’) that can
|
||||
be used to emit a warning when an affected expression is parsed. */
|
||||
expr_simple OR_KW
|
||||
{ $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, state->s.or_)}); }
|
||||
{ $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, state->s.or_)}, state->positions.add(state->origin, @$.endOffset)); }
|
||||
| expr_simple
|
||||
;
|
||||
|
||||
@ -472,7 +481,7 @@ string_attr
|
||||
;
|
||||
|
||||
expr_list
|
||||
: expr_list expr_select { $$ = $1; $1->elems.push_back($2); /* !!! dangerous */ }
|
||||
: expr_list expr_select { $$ = $1; $1->elems.push_back($2); /* !!! dangerous */; $2->warnIfCursedOr(state->symbols, state->positions); }
|
||||
| { $$ = new ExprList; }
|
||||
;
|
||||
|
||||
|
12
tests/functional/lang/eval-okay-deprecate-cursed-or.err.exp
Normal file
12
tests/functional/lang/eval-okay-deprecate-cursed-or.err.exp
Normal file
@ -0,0 +1,12 @@
|
||||
warning: at /pwd/lang/eval-okay-deprecate-cursed-or.nix:3:47: This expression uses `or` as an identifier in a way that will change in a future Nix release.
|
||||
Wrap this entire expression in parentheses to preserve its current meaning:
|
||||
((x: x) or)
|
||||
Give feedback at https://github.com/NixOS/nix/pull/11121
|
||||
warning: at /pwd/lang/eval-okay-deprecate-cursed-or.nix:4:39: This expression uses `or` as an identifier in a way that will change in a future Nix release.
|
||||
Wrap this entire expression in parentheses to preserve its current meaning:
|
||||
((x: x + 1) or)
|
||||
Give feedback at https://github.com/NixOS/nix/pull/11121
|
||||
warning: at /pwd/lang/eval-okay-deprecate-cursed-or.nix:5:44: This expression uses `or` as an identifier in a way that will change in a future Nix release.
|
||||
Wrap this entire expression in parentheses to preserve its current meaning:
|
||||
((x: x) or)
|
||||
Give feedback at https://github.com/NixOS/nix/pull/11121
|
1
tests/functional/lang/eval-okay-deprecate-cursed-or.exp
Normal file
1
tests/functional/lang/eval-okay-deprecate-cursed-or.exp
Normal file
@ -0,0 +1 @@
|
||||
0
|
11
tests/functional/lang/eval-okay-deprecate-cursed-or.nix
Normal file
11
tests/functional/lang/eval-okay-deprecate-cursed-or.nix
Normal file
@ -0,0 +1,11 @@
|
||||
let
|
||||
# These are cursed and should warn
|
||||
cursed0 = builtins.length (let or = 1; in [ (x: x) or ]);
|
||||
cursed1 = let or = 1; in (x: x * 2) (x: x + 1) or;
|
||||
cursed2 = let or = 1; in { a = 2; }.a or (x: x) or;
|
||||
|
||||
# These are uses of `or` as an identifier that are not cursed
|
||||
allowed0 = let or = (x: x); in map or [];
|
||||
allowed1 = let f = (x: x); or = f; in f (f or);
|
||||
in
|
||||
0
|
Loading…
Reference in New Issue
Block a user