libexpr: Soft-deprecate __overrides

Change-Id: I787e69e1dad6edc5ccdb747b74a9ccd6e8e13bb3

doc/manual/rl-next/deprecated-features.md

@@ -1,12 +1,16 @@
 ---
-synopsis: Deprecated URL literals
+synopsis: Deprecated language features
 issues: [fj#437]
-cls: [1736, 1735, 1744]
+cls: [1785, 1736, 1735, 1744]
 category: Breaking Changes
 credits: [piegames, horrors]
 ---
 
-URL literals have long been obsolete and discouraged of use, and now they are officially deprecated.
-This means that all URLs must be properly put within quotes like all other strings.
+A system for deprecation (and then the planned removal) of undesired language features has been put into place.
+It is controlled via feature flags much like experimental features, except that the deprecations are enabled default,
+and can be disabled via the flags for backwards compatibility (opt-out with `--extra-deprecated-features` or the Nix configuration file).
 
-To ease migration, they can still be enabled with `--extra-deprecated-features url-literals` for now.
+- `url-literals`: **URL literals** have long been obsolete and discouraged of use, and now they are officially deprecated.
+  This means that all URLs must be properly put within quotes like all other strings.
+- `rec-set-overrides`: **__overrides** is an old arcane syntax which has not been in use for more than a decade.
+  It is soft-deprecated with a warning only, with the plan to turn that into an error in a future release.

src/libexpr/eval.cc

@@ -249,6 +249,7 @@ EvalState::EvalState(
         .findFile = symbols.create("__findFile"),
         .nixPath = symbols.create("__nixPath"),
         .body = symbols.create("body"),
+        .overrides = symbols.create("__overrides"),
     }
     , repair(NoRepair)
     , emptyBindings(0)

src/libexpr/nixexpr.hh

@@ -48,7 +48,7 @@ protected:
 
 public:
     struct AstSymbols {
-        Symbol sub, lessThan, mul, div, or_, findFile, nixPath, body;
+        Symbol sub, lessThan, mul, div, or_, findFile, nixPath, body, overrides;
     };
 
 

src/libexpr/parser/parser.cc

@@ -676,6 +676,12 @@ template<> struct BuildAST<grammar::expr::ancient_let> : change_head<BindingsSta
 
 template<> struct BuildAST<grammar::expr::rec_set> : change_head<BindingsState> {
     static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) {
+        // Before inserting new attrs, check for __override and throw an error
+        // (the error will initially be a warning to ease migration)
+        if (!featureSettings.isEnabled(Dep::RecSetOverrides) && b.attrs.attrs.contains(ps.s.overrides)) {
+            ps.overridesFound(ps.at(in));
+        }
+
         b.attrs.pos = ps.at(in);
         b.attrs.recursive = true;
         s.pushExpr<ExprAttrs>(b.attrs.pos, std::move(b.attrs));

src/libexpr/parser/state.hh

@@ -2,6 +2,7 @@
 ///@file
 
 #include "eval.hh"
+#include "logging.hh"
 
 namespace nix::parser {
 
@@ -23,6 +24,7 @@ struct State
 
     void dupAttr(const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos);
     void dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos);
+    void overridesFound(const PosIdx pos);
     void addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr<Expr> e, const PosIdx pos);
     std::unique_ptr<Formals> validateFormals(std::unique_ptr<Formals> formals, PosIdx pos = noPos, Symbol arg = {});
     std::unique_ptr<Expr> stripIndentation(const PosIdx pos,
@@ -58,6 +60,17 @@ inline void State::dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos)
     });
 }
 
+inline void State::overridesFound(const PosIdx pos) {
+    // Added 2024-09-18. Turn into an error at some point in the future.
+    // See the documentation on deprecated features for more details.
+    warn(
+        "%s found at %s. This feature is deprecated and will be removed in the future. Use %s to silence this warning.",
+        "__overrides",
+        positions[pos],
+        "--extra-deprecated-features rec-set-overrides"
+    );
+}
+
 inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr<Expr> e, const PosIdx pos)
 {
     AttrPath::iterator i;
@@ -123,6 +136,12 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_
                 dupAttr(attrPath, pos, j->second.pos);
             }
         } else {
+            // Before inserting new attrs, check for __override and throw an error
+            // (the error will initially be a warning to ease migration)
+            if (attrs->recursive && !featureSettings.isEnabled(Dep::RecSetOverrides) && i->symbol == s.overrides) {
+                overridesFound(pos);
+            }
+
             // This attr path is not defined. Let's create it.
             e->setName(i->symbol);
             attrs->attrs.emplace(std::piecewise_construct,

src/libutil/deprecated-features.cc

@@ -27,6 +27,15 @@ struct DeprecatedFeatureDetails
 constexpr size_t numDepFeatures = 1 + static_cast<size_t>(Dep::UrlLiterals);
 
 constexpr std::array<DeprecatedFeatureDetails, numDepFeatures> depFeatureDetails = {{
+    {
+        .tag = Dep::RecSetOverrides,
+        .name = "rec-set-overrides",
+        .description = R"(
+            Allow `__overrides` in recursive attribute sets.
+
+            Use fix point functions (e.g. `lib.fix` in Nixpkgs) instead.
+        )",
+    },
     {
         .tag = Dep::UrlLiterals,
         .name = "url-literals",

src/libutil/deprecated-features.hh

@@ -18,6 +18,7 @@ namespace nix {
  */
 enum struct DeprecatedFeature
 {
+    RecSetOverrides,
     UrlLiterals,
 };
 

tests/functional/lang.sh

@@ -53,10 +53,11 @@ done
 for i in lang/parse-okay-*.nix; do
     echo "parsing $i (should succeed)";
     i=$(basename "$i" .nix)
-    # Hard-code that these two files are allowed to use url literals (because they test them)
-    if [[ "$i" == "parse-okay-url" || "$i" == "parse-okay-regression-20041027" ]]
-    then
-        extraArgs="--extra-deprecated-features url-literals"
+
+    if [ -e "lang/$i.flags" ]; then
+        extraArgs="$(cat "lang/$i.flags")"
+    else
+        extraArgs=""
     fi
     if
         expect 0 nix-instantiate --parse ${extraArgs-} - < "lang/$i.nix" \
@@ -75,8 +76,14 @@ done
 for i in lang/eval-fail-*.nix; do
     echo "evaluating $i (should fail)";
     i=$(basename "$i" .nix)
+
+    if [ -e "lang/$i.flags" ]; then
+        extraArgs="$(cat "lang/$i.flags")"
+    else
+        extraArgs=""
+    fi
     if
-        expectStderr 1 nix-instantiate --eval --strict --show-trace "lang/$i.nix" \
+        expectStderr 1 nix-instantiate --eval --strict --show-trace ${extraArgs-} "lang/$i.nix" \
             | sed "s!$(pwd)!/pwd!g" > "lang/$i.err"
     then
         diffAndAccept "$i" err err.exp
@@ -90,8 +97,14 @@ for i in lang/eval-okay-*.nix; do
     echo "evaluating $i (should succeed)";
     i=$(basename "$i" .nix)
 
+    if [ -e "lang/$i.flags" ]; then
+        extraArgs="$(cat "lang/$i.flags")"
+    else
+        extraArgs=""
+    fi
+
     if test -e "lang/$i.exp.xml"; then
-        if expect 0 nix-instantiate --eval --xml --no-location --strict \
+        if expect 0 nix-instantiate --eval --xml --no-location --strict ${extraArgs-} \
                 "lang/$i.nix" > "lang/$i.out.xml"
         then
             diffAndAccept "$i" out.xml exp.xml

tests/functional/lang/eval-fail-set-override.flags

@@ -0,0 +1 @@
+--extra-deprecated-features rec-set-overrides

tests/functional/lang/eval-okay-attrs6.flags

@@ -0,0 +1 @@
+--extra-deprecated-features rec-set-overrides

tests/functional/lang/eval-okay-inherit-from.flags

@@ -0,0 +1 @@
+--extra-deprecated-features rec-set-overrides

tests/functional/lang/eval-okay-overrides.flags

@@ -0,0 +1 @@
+--extra-deprecated-features rec-set-overrides

tests/functional/lang/parse-okay-rec-set-override-nowarning.exp

@@ -0,0 +1 @@
+parse-okay-rec-set-override-warning.exp

tests/functional/lang/parse-okay-rec-set-override-nowarning.flags

@@ -0,0 +1 @@
+--extra-deprecated-features rec-set-overrides

tests/functional/lang/parse-okay-rec-set-override-nowarning.nix

@@ -0,0 +1 @@
+parse-okay-rec-set-override-warning.nix

tests/functional/lang/parse-okay-rec-set-override-warning.err.exp

@@ -0,0 +1,2 @@
+warning: __overrides found at «stdin»:3:16. This feature is deprecated and will be removed in the future. Use --extra-deprecated-features rec-set-overrides to silence this warning.
+warning: __overrides found at «stdin»:4:2. This feature is deprecated and will be removed in the future. Use --extra-deprecated-features rec-set-overrides to silence this warning.

tests/functional/lang/parse-okay-rec-set-override-warning.exp

@@ -0,0 +1 @@
+[ ({ a = rec { __overrides = { }; }; }) (rec { __overrides = { }; }) ({ __overrides = { }; }) (rec { "${("__overrides" + "")}" = { }; }) ]

tests/functional/lang/parse-okay-rec-set-override-warning.nix

@@ -0,0 +1,9 @@
+[
+	# Should warn
+	{ a = rec {}; a.__overrides = {}; }
+	rec { __overrides = {}; }
+	# Should not warn: Not recursive
+	{ __overrides = {}; }
+	# Should not warn: Dynamic
+	rec { ${"__overrides" + ""} = {}; }
+]

tests/functional/lang/parse-okay-regression-20041027.flags

@@ -0,0 +1 @@
+--extra-deprecated-features url-literals

tests/functional/lang/parse-okay-url.flags

@@ -0,0 +1 @@
+--extra-deprecated-features url-literals