diff options
| -rw-r--r-- | CHANGELOG.md | 4 | ||||
| -rw-r--r-- | src/buildtool/build_engine/expression/evaluator.cpp | 25 | ||||
| -rw-r--r-- | test/buildtool/build_engine/expression/expression.test.cpp | 15 |
3 files changed, 29 insertions, 15 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index d4f194c3..dace2933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,10 @@ local build root on upgrade. - The built-in rule `"generic"` now properly enforces that the obtained outputs form a well-formed artifact stage; a conflicting arrangement of artifacts was possilbe beforehand. +- The built-in expression functions `"join"` and `"join_cmd"` + now properly enforce that the argument is a list of strings. + So far, they used to accept a single string, treating it as a + singleton list. - A bug was fixed that cased `just serve` to fail with an internal error when building against ignore-special roots. - `just` now accurately reports internal errors that occurred on diff --git a/src/buildtool/build_engine/expression/evaluator.cpp b/src/buildtool/build_engine/expression/evaluator.cpp index 3e129539..dfae8499 100644 --- a/src/buildtool/build_engine/expression/evaluator.cpp +++ b/src/buildtool/build_engine/expression/evaluator.cpp @@ -347,14 +347,16 @@ auto ShellQuote(std::string arg) -> std::string { return fmt::format("'{}'", arg); } -template <bool kDoQuote = false> +template <bool kDoQuote = false, bool kAllowString = false> auto Join(ExpressionPtr const& expr, std::string const& sep) -> ExpressionPtr { - if (expr->IsString()) { - auto string = expr->String(); - if constexpr (kDoQuote) { - string = ShellQuote(std::move(string)); + if constexpr (kAllowString) { + if (expr->IsString()) { + auto string = expr->String(); + if constexpr (kDoQuote) { + string = ShellQuote(std::move(string)); + } + return ExpressionPtr{std::move(string)}; } - return ExpressionPtr{std::move(string)}; } if (expr->IsList()) { auto const& list = expr->List(); @@ -370,8 +372,10 @@ auto Join(ExpressionPtr const& expr, std::string const& sep) -> ExpressionPtr { }); return ExpressionPtr{ss.str()}; } - throw Evaluator::EvaluationError{fmt::format( - "Join expects string or list but got: {}.", expr->ToString())}; + throw Evaluator::EvaluationError{ + fmt::format("Join expects a list of strings{}, but got: {}.", + kAllowString ? " or a single string" : "", + expr->ToString())}; } template <bool kDisjoint = false> @@ -787,7 +791,7 @@ auto JoinCmdExpr(SubExprEvaluator&& eval, ExpressionPtr const& expr, Configuration const& env) -> ExpressionPtr { auto const& list = eval(expr->Get("$1", list_t{}), env); - return Join</*kDoQuote=*/true>(list, " "); + return Join</*kDoQuote=*/true, /*kAllowString=*/false>(list, " "); } auto JsonEncodeExpr(SubExprEvaluator&& eval, @@ -1120,7 +1124,8 @@ auto ConcatTargetNameExpr(SubExprEvaluator&& eval, Configuration const& env) -> ExpressionPtr { auto p1 = eval(expr->Get("$1", ""s), env); auto p2 = eval(expr->Get("$2", ""s), env); - return ConcatTargetName(p1, Join(p2, "")); + return ConcatTargetName( + p1, Join</*kDoQuote=*/false, /*kAllowString=*/true>(p2, "")); } auto ContextExpr(SubExprEvaluator&& eval, diff --git a/test/buildtool/build_engine/expression/expression.test.cpp b/test/buildtool/build_engine/expression/expression.test.cpp index aaca1c71..3a34702d 100644 --- a/test/buildtool/build_engine/expression/expression.test.cpp +++ b/test/buildtool/build_engine/expression/expression.test.cpp @@ -1042,14 +1042,11 @@ TEST_CASE("Expression Evaluation", "[expression]") { // NOLINT REQUIRE(multi->IsString()); CHECK(multi == Expression::FromJson(R"("foo;bar;baz")"_json)); + // only list of strings are allowed expr = Replace(expr, "$1", foo); REQUIRE(expr); - auto string = expr.Evaluate(env, fcts); - REQUIRE(string); - REQUIRE(string->IsString()); - CHECK(string == Expression::FromJson(R"("foo")"_json)); + CHECK_FALSE(expr.Evaluate(env, fcts)); - // only list of strings or string is allowed expr = Replace(expr, "$1", list_t{foo, ExpressionPtr{number_t{}}}); REQUIRE(expr); CHECK_FALSE(expr.Evaluate(env, fcts)); @@ -1070,6 +1067,14 @@ TEST_CASE("Expression Evaluation", "[expression]") { // NOLINT REQUIRE(result->IsString()); CHECK(result == Expression::FromJson(R"("'foo' 'bar'\\''s' 'baz'")"_json)); + + expr = Expression::FromJson(R"( + {"type": "join_cmd" + , "$1": "not a list" + } + )"_json); + REQUIRE(expr); + CHECK_FALSE(expr.Evaluate(env, fcts)); } SECTION("escape_chars expression") { |