Skip to content

Commit

Permalink
tools/fix: correct the ast walker so sub-expressions get fixed
Browse files Browse the repository at this point in the history
Originally, the ast walker in fix was using the "before" function. But
this would mean recursion would stop as soon as it does any replacement.
This means it wouldn't work for nested expressions.

By simply switching to the "after" function, we ensure we don't do any
replacement until we've bottomed out in the AST.

Signed-off-by: Matthew Sackman <[email protected]>
Change-Id: I5372b319429f48c222b2a8b4a6507873f19250c1
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200495
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
cuematthew committed Sep 2, 2024
1 parent 5936450 commit d0617b5
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
3 changes: 1 addition & 2 deletions cmd/cue/cmd/testdata/script/fix.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ exec cue fix ./...
# Make sure we fix all files in a directory, even if they're a mix of packages (or no packages).
cmp p/one.cue p/one.cue.fixed
cmp p/two.cue p/two.cue.fixed
# TODO: the following cmp fails because of a bug in fix/fix.go
! cmp p/three.cue p/three.cue.fixed
cmp p/three.cue p/three.cue.fixed
# TODO: the added imports have unnecessary name-mangling. https://cuelang.org/issue/3408
-- p/one.cue --
package one
Expand Down
7 changes: 5 additions & 2 deletions tools/fix/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ func File(f *ast.File, o ...Option) *ast.File {
f(&options)
}

f = astutil.Apply(f, func(c astutil.Cursor) bool {
// Make sure we use the "after" function, and not the "before",
// because "before" will stop recursion early which creates
// problems with nested expressions.
f = astutil.Apply(f, nil, func(c astutil.Cursor) bool {
n := c.Node()
switch n := n.(type) {
case *ast.BinaryExpr:
Expand Down Expand Up @@ -94,7 +97,7 @@ func File(f *ast.File, o ...Option) *ast.File {
}
}
return true
}, nil).(*ast.File)
}).(*ast.File)

if options.simplify {
f = simplify(f)
Expand Down

0 comments on commit d0617b5

Please sign in to comment.