libexpr: significantly improve error messages for bad attr paths

This commit makes Lix include the summarized content of the value being
indexed when it is bad.

lix/lix2 » nix eval --expr '{x.y = 2;}' 'x.y.z'
error: the value being indexed in the selection path 'x.y.z' at 'x.y' should be a set but is an integer: 2

lix/lix2 » nix eval --expr '{x.y = { a = 3; };}' 'x.y.z'
error: attribute 'z' in selection path 'x.y.z' not found inside path 'x.y', whose contents are: { a = 3; }
       Did you mean a?

lix/lix2 » nix eval --expr '{x.y = { a = 3; };}' 'x.y.1'
error: the expression selected by the selection path 'x.y.1' should be a list but is a set: { a = 3; }

Change-Id: I3202aba0e437e00b4c6d3ee287a2d9a7c6892dbf

doc/manual/rl-next/better-errors-attrpaths.md

@@ -0,0 +1,25 @@
+---
+synopsis: "Improved error messages for bad attr paths"
+category: Improvements
+cls: [2277, 2280]
+credits: jade
+---
+
+Lix now includes much more detail when a bad attribute path is accessed at the command line:
+
+```
+ » nix eval -f '<nixpkgs>' lixVersions.lix_2_92
+error: attribute 'lix_2_92' in selection path 'lixVersions.lix_2_92' not found
+       Did you mean one of lix_2_90 or lix_2_91?
+```
+
+After:
+
+```
+ » nix eval --impure -f '<nixpkgs>' lixVersions.lix_2_92
+error: attribute 'lix_2_92' in selection path 'lixVersions.lix_2_92' not found inside path 'lixVersions', whose contents are: { __unfix__ = «lambda @ /nix/store/hfz1qqd0z8amlgn8qwich1dvkmldik36-source/lib/fixed-points.nix:
+447:7»; buildLix = «thunk»; extend = «thunk»; latest = «thunk»; lix_2_90 = «thunk»; lix_2_91 = «thunk»; override = «thunk»; overrideDerivation = «thunk»; recurseForDerivations = true; stable = «thunk»; }
+       Did you mean one of lix_2_90 or lix_2_91?
+```
+
+This should avoid some unnecessary trips to the repl or to the debugger by giving some information about the value being selected on that was unexpected.

lix/libexpr/attr-path.cc

@@ -1,5 +1,6 @@
 #include "lix/libexpr/attr-path.hh"
 #include "lix/libexpr/eval-inline.hh"
+#include "print-options.hh"
 #include <algorithm>
 #include <sstream>
 
@@ -96,12 +97,18 @@ std::pair<Value *, PosIdx> findAlongAttrPath(EvalState & state, const std::strin
                 throw Error("empty attribute name in selection path '%1%'", attrPath);
 
             if (v->type() != nAttrs) {
-                auto pathPart = std::vector<std::string>(tokens.begin(), tokens.begin() + attrPathIdx);
+                auto pathPart =
-                state.ctx.errors.make<TypeError>(
+                    std::vector<std::string>(tokens.begin(), tokens.begin() + attrPathIdx);
-                    "the value being indexed in the selection path '%1%' at '%2%' should be a set but is %3%",
+                state.ctx.errors
+                    .make<TypeError>(
+                        "the value being indexed in the selection path '%1%' at '%2%' should be a "
+                        "set but is %3%: %4%",
                         attrPath,
                         unparseAttrPath(pathPart),
-                    showType(*v)).debugThrow();
+                        showType(*v),
+                        ValuePrinter(state, *v, errorPrintOptions)
+                    )
+                    .debugThrow();
             }
 
             Bindings::iterator a = v->attrs->find(state.ctx.symbols.create(attr));
@@ -111,21 +118,40 @@ std::pair<Value *, PosIdx> findAlongAttrPath(EvalState & state, const std::strin
                     attrNames.insert(state.ctx.symbols[attr.name]);
 
                 auto suggestions = Suggestions::bestMatches(attrNames, attr);
-                throw AttrPathNotFound(suggestions, "attribute '%1%' in selection path '%2%' not found", attr, attrPath);
+                auto pathPart =
+                    std::vector<std::string>(tokens.begin(), tokens.begin() + attrPathIdx);
+                throw AttrPathNotFound(
+                    suggestions,
+                    "attribute '%1%' in selection path '%2%' not found inside path '%3%', whose "
+                    "contents are: %4%",
+                    attr,
+                    attrPath,
+                    unparseAttrPath(pathPart),
+                    ValuePrinter(state, *v, errorPrintOptions)
+                );
             }
             v = &*a->value;
             pos = a->pos;
-        }
+        } else {
-
+            if (!v->isList()) {
-        else {
+                state.ctx.errors
-
+                    .make<TypeError>(
-            if (!v->isList())
+                        "the expression selected by the selection path '%1%' should be a list but "
-                state.ctx.errors.make<TypeError>(
+                        "is %2%: %3%",
-                    "the expression selected by the selection path '%1%' should be a list but is %2%",
                         attrPath,
-                    showType(*v)).debugThrow();
+                        showType(*v),
-            if (*attrIndex >= v->listSize())
+                        ValuePrinter(state, *v, errorPrintOptions)
-                throw AttrPathNotFound("list index %1% in selection path '%2%' is out of range", *attrIndex, attrPath);
+                    )
+                    .debugThrow();
+            }
+            if (*attrIndex >= v->listSize()) {
+                throw AttrPathNotFound(
+                    "list index %1% in selection path '%2%' is out of range for list %3%",
+                    *attrIndex,
+                    attrPath,
+                    ValuePrinter(state, *v, errorPrintOptions)
+                );
+            }
 
             v = v->listElems()[*attrIndex];
             pos = noPos;

tests/functional/misc.sh

@@ -24,13 +24,6 @@ eval_stdin_res=$(echo 'let a = {} // a; in a.foo' | nix-instantiate --eval -E -
 echo $eval_stdin_res | grep "at «stdin»:1:15:"
 echo $eval_stdin_res | grep "infinite recursion encountered"
 
-# Attribute path errors
-expectStderr 1 nix-instantiate --eval -E '{}' -A '"x' | grepQuiet "missing closing quote in selection path"
-expectStderr 1 nix-instantiate --eval -E '[]' -A 'x' | grepQuiet "should be a set"
-expectStderr 1 nix-instantiate --eval -E '{}' -A '1' | grepQuiet "should be a list"
-expectStderr 1 nix-instantiate --eval -E '{}' -A '.' | grepQuiet "empty attribute name"
-expectStderr 1 nix-instantiate --eval -E '[]' -A '1' | grepQuiet "out of range"
-
 # Unknown setting warning
 # NOTE(cole-h): behavior is different depending on the order, which is why we test an unknown option
 # before and after the `'{}'`!

tests/functional2/eval/test_attr_paths.py

@@ -0,0 +1,56 @@
+from functional2.testlib.fixtures import Nix
+import pytest
+from typing import NamedTuple
+from textwrap import dedent
+
+
+class ShouldError(NamedTuple):
+    expr: str
+    attr: str
+    error: str
+
+
+ERR_CASES: list[ShouldError] = [
+    # FIXME(jade): expect-test system for pytest that allows for updating these easily
+    ShouldError('{}', '"x',
+                """error: missing closing quote in selection path '"x'"""),
+    ShouldError(
+        '[]', 'x',
+        """error: the value being indexed in the selection path 'x' at '' should be a set but is a list: [ ]"""
+    ),
+    ShouldError(
+        '{}', '1',
+        """error: the expression selected by the selection path '1' should be a list but is a set: { }"""
+    ),
+    ShouldError('{}', '.',
+                """error: empty attribute name in selection path '.'"""),
+    ShouldError('{ x."" = 2; }', 'x.""',
+                """error: empty attribute name in selection path 'x.""'"""),
+    ShouldError('{ x."".y = 2; }', 'x."".y',
+                """error: empty attribute name in selection path 'x."".y'"""),
+    ShouldError(
+        '[]', '1',
+        """error: list index 1 in selection path '1' is out of range for list [ ]"""
+    ),
+    ShouldError(
+        '{ x.y = { z = 2; a = 3; }; }', 'x.y.c',
+        dedent("""\
+            error: attribute 'c' in selection path 'x.y.c' not found inside path 'x.y', whose contents are: { a = 3; z = 2; }
+                   Did you mean one of a or z?""")
+    ),
+]
+
+
+# I do not know why it does this, but I guess it makes sense as allowing a tool
+# to pass -A unconditionally and then allow a blank attribute to mean the whole
+# thing
+def test_attrpath_accepts_empty_attr_as_no_attr(nix: Nix):
+    assert nix.nix_instantiate(['--eval', '--expr', '{}', '-A',
+                                '']).run().ok().stdout_plain == '{ }'
+
+
+@pytest.mark.parametrize(['expr', 'attr', 'error'], ERR_CASES)
+def test_attrpath_error(nix: Nix, expr: str, attr: str, error: str):
+    res = nix.nix_instantiate(['--eval', '--expr', expr, '-A', attr]).run()
+
+    assert res.expect(1).stderr_plain == error

tests/functional2/eval/test_eval_trivial.py

@@ -1,4 +1,4 @@
-from .testlib.fixtures import Nix
+from functional2.testlib.fixtures import Nix
 
 def test_trivial_addition(nix: Nix):
     assert nix.eval('1 + 1').json() == 2