diff --git a/lix/libexpr/attr-path.cc b/lix/libexpr/attr-path.cc index a3d5d1ca5..62ebf0193 100644 --- a/lix/libexpr/attr-path.cc +++ b/lix/libexpr/attr-path.cc @@ -1,45 +1,83 @@ #include "lix/libexpr/attr-path.hh" #include "lix/libexpr/eval-inline.hh" +#include +#include namespace nix { -Strings parseAttrPath(std::string_view s) +std::vector parseAttrPath(std::string_view const s) { - Strings res; + std::vector res; std::string cur; + bool haveData = false; auto i = s.begin(); while (i != s.end()) { if (*i == '.') { res.push_back(cur); + haveData = false; cur.clear(); } else if (*i == '"') { + // If there is a quote there *will* be a named term even if it is empty. ++i; + haveData = true; + while (1) { if (i == s.end()) throw ParseError("missing closing quote in selection path '%1%'", s); if (*i == '"') break; cur.push_back(*i++); } - } else + } else { cur.push_back(*i); + haveData = true; + } ++i; } - if (!cur.empty()) res.push_back(cur); + if (haveData) res.push_back(cur); return res; } +std::string unparseAttrPath(std::vector const & attrPath) +{ + // FIXME(jade): can probably be rewritten with ranges once libc++ has a + // fully featured implementation + // https://github.com/llvm/llvm-project/pull/65536 + auto ret = std::ostringstream{}; + bool first = true; + + for (auto const & part : attrPath) { + if (!first) { + ret << "."; + } + first = false; + + bool mustQuote = std::ranges::any_of(part, [](char c) -> bool { + return c == '"' || c == '.' || c == ' '; + }); + + if (mustQuote || part.empty()) { + ret << '"' << part << '"'; + } else { + ret << part; + } + } + + return ret.str(); +} + + std::pair findAlongAttrPath(EvalState & state, const std::string & attrPath, Bindings & autoArgs, Value & vIn) { - Strings tokens = parseAttrPath(attrPath); + auto tokens = parseAttrPath(attrPath); Value * v = &vIn; PosIdx pos = noPos; - for (auto & attr : tokens) { + for (auto [attrPathIdx, attr] : enumerate(tokens)) { /* Is i an index (integer) or a normal attribute name? */ auto attrIndex = string2Int(attr); @@ -54,15 +92,18 @@ std::pair findAlongAttrPath(EvalState & state, const std::strin according to what is specified in the attrPath. */ if (!attrIndex) { - - if (v->type() != nAttrs) - state.ctx.errors.make( - "the expression selected by the selection path '%1%' should be a set but is %2%", - attrPath, - showType(*v)).debugThrow(); if (attr.empty()) throw Error("empty attribute name in selection path '%1%'", attrPath); + if (v->type() != nAttrs) { + auto pathPart = std::vector(tokens.begin(), tokens.begin() + attrPathIdx); + state.ctx.errors.make( + "the value being indexed in the selection path '%1%' at '%2%' should be a set but is %3%", + attrPath, + unparseAttrPath(pathPart), + showType(*v)).debugThrow(); + } + Bindings::iterator a = v->attrs->find(state.ctx.symbols.create(attr)); if (a == v->attrs->end()) { std::set attrNames; diff --git a/lix/libexpr/attr-path.hh b/lix/libexpr/attr-path.hh index 894899f87..0c5480ada 100644 --- a/lix/libexpr/attr-path.hh +++ b/lix/libexpr/attr-path.hh @@ -22,6 +22,18 @@ std::pair findAlongAttrPath( */ std::pair findPackageFilename(EvalState & state, Value & v, std::string what); -Strings parseAttrPath(std::string_view s); +/** + * Parses an attr path (as used in nix-build -A foo.bar.baz) into a list of tokens. + * + * Such an attr path is a dot-separated sequence of attribute names, which are possibly quoted. + * No escaping is performed; attribute names containing double quotes are unrepresentable. + */ +std::vector parseAttrPath(std::string_view const s); + +/** + * Converts an attr path from a list of strings into a string once more. + * The result returned is an attr path and is *not necessarily valid nix syntax*. + */ +std::string unparseAttrPath(std::vector const & attrPath); } diff --git a/lix/libexpr/eval-cache.cc b/lix/libexpr/eval-cache.cc index 1287c5e4d..443177afd 100644 --- a/lix/libexpr/eval-cache.cc +++ b/lix/libexpr/eval-cache.cc @@ -528,7 +528,7 @@ ref AttrCursor::getAttr(EvalState & state, const std::string & name) return ref(p); } -OrSuggestions> AttrCursor::findAlongAttrPath(EvalState & state, const Strings & attrPath) +OrSuggestions> AttrCursor::findAlongAttrPath(EvalState & state, const std::vector & attrPath) { auto res = shared_from_this(); for (auto & attr : attrPath) { diff --git a/lix/libexpr/eval-cache.hh b/lix/libexpr/eval-cache.hh index bd6ed0128..42a74d246 100644 --- a/lix/libexpr/eval-cache.hh +++ b/lix/libexpr/eval-cache.hh @@ -128,7 +128,7 @@ public: * Get an attribute along a chain of attrsets. Note that this does * not auto-call functors or functions. */ - OrSuggestions> findAlongAttrPath(EvalState & state, const Strings & attrPath); + OrSuggestions> findAlongAttrPath(EvalState & state, const std::vector & attrPath); std::string getString(EvalState & state); diff --git a/lix/libutil/types.hh b/lix/libutil/types.hh index 05523b9ed..89d277448 100644 --- a/lix/libutil/types.hh +++ b/lix/libutil/types.hh @@ -123,6 +123,9 @@ struct MaintainCount * A Rust/Python-like enumerate() iterator adapter. * * Borrowed from http://reedbeta.com/blog/python-like-enumerate-in-cpp17. + * + * FIXME(jade): remove once P2164R9 is implemented in libc++ and replace with + * std::views::enumerate: https://libcxx.llvm.org/Status/Cxx23.html */ template ())), diff --git a/tests/unit/libexpr-support/tests/libexpr.hh b/tests/unit/libexpr-support/tests/libexpr.hh index aab674942..915a38525 100644 --- a/tests/unit/libexpr-support/tests/libexpr.hh +++ b/tests/unit/libexpr-support/tests/libexpr.hh @@ -8,7 +8,6 @@ #include "lix/libexpr/nixexpr.hh" #include "lix/libexpr/eval.hh" #include "lix/libexpr/eval-inline.hh" -#include "lix/libstore/store-api.hh" #include "tests/libstore.hh" @@ -20,6 +19,10 @@ namespace nix { initLibExpr(); } + EvalState & evalState() { + return state; + } + protected: LibExprTest() : LibStoreTest() diff --git a/tests/unit/libexpr/attr-path.cc b/tests/unit/libexpr/attr-path.cc new file mode 100644 index 000000000..2eb219dbb --- /dev/null +++ b/tests/unit/libexpr/attr-path.cc @@ -0,0 +1,46 @@ +#include "lix/libexpr/attr-path.hh" +#include "lix/libexpr/attr-set.hh" +#include "tests/libexpr.hh" +#include +#include +#include +#include +#include + +namespace nix { + +class AttrPathEval : public LibExprTest +{ +public: + std::pair testFindAlongAttrPath(std::string expr, std::string path); +}; + +RC_GTEST_PROP(AttrPath, prop_round_trip, ()) +{ + auto strings = *rc::gen::container>( + rc::gen::container(rc::gen::distinctFrom('"')) + ); + auto const unparsed = unparseAttrPath(strings); + auto const unparsedReparsed = parseAttrPath(unparsed); + + RC_ASSERT(strings == unparsedReparsed); +} + +std::pair AttrPathEval::testFindAlongAttrPath(std::string expr, std::string path) +{ + auto v = eval(expr); + auto bindings = evalState().ctx.buildBindings(0).finish(); + return findAlongAttrPath(state, path, *bindings, v); +} + +// n.b. I do not know why we throw for empty attrs but they are apparently +// disallowed. +TEST_F(AttrPathEval, emptyAttrsThrows) +{ + std::string expr = "{a.\"\".b = 2;}"; + ASSERT_NO_THROW(testFindAlongAttrPath(expr, "a")); + ASSERT_THROW(testFindAlongAttrPath(expr, "a.\"\".b"), Error); + ASSERT_THROW(testFindAlongAttrPath(expr, "a.\"\""), Error); +} + +} diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 5b09e418f..8ab196e7f 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -193,6 +193,7 @@ liblixexpr_test_support = declare_dependency( ) libexpr_tests_sources = files( + 'libexpr/attr-path.cc', 'libexpr/derived-path.cc', 'libexpr/error_traces.cc', 'libexpr/flakeref.cc',