Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixed

- A def calling another instruction with too few arguments is refused instead of binding the missing one to the empty string. `file.write(path)` inside a def overwrote the file with nothing and reported `ok.done` — a file destroyed under a success verdict. A plan-level call was always checked on both bounds; only the def side was not (#633).

## [0.13.0] - 2026-09-09

### Changed
Expand Down
14 changes: 12 additions & 2 deletions internal/lang/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -702,8 +702,18 @@ func (ev *evaluator) evalCall(c Call) value {
if !ok {
ev.fail("unknown instruction %q", c.Name)
}
if len(c.Args) > len(def.Params) {
ev.fail("%s takes %d argument(s), got %d", c.Name, len(def.Params), len(c.Args))
// Both bounds, as a plan-level call has always been checked (parser.go). Only the
// upper one was tested here, so a call missing an argument bound the parameter to the
// empty string: `file.write(path)` overwrote a file with nothing and reported
// `ok.done` — a file destroyed under a success verdict (#633).
//
// The lower bound is the count of parameters with no default, since omitting a
// defaulted one is what defaults are for.
if req := requiredCount(def); len(c.Args) < req || len(c.Args) > len(def.Params) {
if req == len(def.Params) {
ev.fail("%s takes %d argument(s), got %d", c.Name, req, len(c.Args))
}
ev.fail("%s takes %d–%d argument(s), got %d", c.Name, req, len(def.Params), len(c.Args))
}
// Positional arguments, evaluated in THIS def's scope, then handed over as the
// callee's own params. Nothing else of this def crosses over.
Expand Down
44 changes: 44 additions & 0 deletions internal/lang/text_primitives_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,47 @@ func TestText_Arity(t *testing.T) {
})
}
}

// #633. A def calling another instruction with too few arguments was accepted: the missing
// parameter bound to the empty string, so `file.write(p)` overwrote a file with nothing and
// reported `ok.done`. Reproduced on a real target before this test existed.
//
// The plan path checks both bounds (parser.go). This one checked only the upper one, which
// is why the same mistake is refused in a plan and silent in a def — where defs compose and
// nobody reads the call site.
func TestCallArity_TooFewIsRefused(t *testing.T) {
src := `def callee(a: str, b: str) { check { return ok.done } }
def caller(p: str) { check { callee(p) return ok.done } }`
_, err := evalWithFetch(t, src, "caller", map[string]string{"p": "x"}, nil)
if err == nil {
t.Fatal("a call missing a required argument must be refused, not bound to empty")
}
for _, want := range []string{"callee", "2", "1"} {
if !strings.Contains(err.Error(), want) {
t.Fatalf("the refusal must name the callee and both counts, missing %q: %v", want, err)
}
}
}

// The other bound still holds, and its message is unchanged.
func TestCallArity_TooManyIsStillRefused(t *testing.T) {
src := `def callee(a: str) { check { return ok.done } }
def caller(p: str) { check { callee(p, p) return ok.done } }`
if _, err := evalWithFetch(t, src, "caller", map[string]string{"p": "x"}, nil); err == nil {
t.Fatal("too many arguments must stay refused")
}
}

// A parameter with a default may be omitted — that is what defaults are for, and four
// stdlib defs rely on it (`docker.prune`, `dir.copy`, `dir.sync`, `docker.compose-restart`).
func TestCallArity_ADefaultedParameterMayBeOmitted(t *testing.T) {
src := `def callee(a: str, b: str = "fallback") { check { if b == "fallback" { return ok.defaulted } return err.bound } }
def caller(p: str) { check { c = callee(p) if c { return ok.defaulted } return err.bound } }`
res, err := evalWithFetch(t, src, "caller", map[string]string{"p": "x"}, nil)
if err != nil {
t.Fatalf("omitting a defaulted parameter must stay legal: %v", err)
}
if got := res.String(); got != "ok.defaulted" {
t.Fatalf("got %s, want ok.defaulted — the default must reach the callee", got)
}
}
Loading