Skip to content

Commit

Permalink
tools/fix: rewrite list addition and multiplication
Browse files Browse the repository at this point in the history 
Arithmetic on lists hasn't been part of the spec since 2021. We want to
remove support for it.

Teach `cue fix` to detect and rewrite:
- list addition, which becomes a call to `list.Concat`
- list multiplication, which becomes a call to `list.Repeat`

These fixes are not perfect because `cue fix` has no access to type
information and so only cases where there is a literal list can be
fixed.

Fixes #2237.

Signed-off-by: Matthew Sackman <matthew@cue.works>
Change-Id: I99946833277cda5de8004a30d5e4f909cfcc1016
  • Loading branch information
@cuematthew
cuematthew committed Aug 29, 2024
1 parent a4ebf4d commit 540f282
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 100 deletions.
81 changes: 40 additions & 41 deletions tools/fix/fix.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,59 +44,58 @@ func File(f *ast.File, o ...Option) *ast.File {
f(&options)
}

// Rewrite integer division operations to use builtins.
f = astutil.Apply(f, func(c astutil.Cursor) bool {
n := c.Node()
switch x := n.(type) {
switch n := n.(type) {
case *ast.BinaryExpr:
switch x.Op {
switch n.Op {
case token.IDIV, token.IMOD, token.IQUO, token.IREM:
ast.SetRelPos(x.X, token.NoSpace)
// Rewrite integer division operations to use builtins.
ast.SetRelPos(n.X, token.NoSpace)
c.Replace(&ast.CallExpr{
// Use the __foo version to prevent accidental shadowing.
Fun: ast.NewIdent("__" + x.Op.String()),
Args: []ast.Expr{x.X, x.Y},
Fun: ast.NewIdent("__" + n.Op.String()),
Args: []ast.Expr{n.X, n.Y},
})

case token.ADD, token.MUL:
// The fix here only works when at least one argument is a
// literal list. It would be better to be able to use CUE
// to infer type information, and then apply the fix to
// all places where we infer a list argument.
x, y := n.X, n.Y
_, xIsList := x.(*ast.ListLit)
_, yIsList := y.(*ast.ListLit)
if !(xIsList || yIsList) {
break
}
pkg := c.Import("list")
if pkg == nil {
break
}
if n.Op == token.ADD {
// Rewrite list addition to use list.Concat
ast.SetRelPos(x, token.NoSpace)
c.Replace(&ast.CallExpr{
Fun: ast.NewSel(pkg, "Concat"),
Args: []ast.Expr{ast.NewList(x, y)},
})
} else {
// Rewrite list multiplication to use list.Repeat
if !xIsList {
x, y = y, x
}
ast.SetRelPos(x, token.NoSpace)
c.Replace(&ast.CallExpr{
Fun: ast.NewSel(pkg, "Repeat"),
Args: []ast.Expr{x, y},
})
}
}
}
return true
}, nil).(*ast.File)

// TODO: we are probably reintroducing slices. Disable for now.
//
// Rewrite slice expression.
// f = astutil.Apply(f, func(c astutil.Cursor) bool {
// n := c.Node()
// getVal := func(n ast.Expr) ast.Expr {
// if n == nil {
// return nil
// }
// if id, ok := n.(*ast.Ident); ok && id.Name == "_" {
// return nil
// }
// return n
// }
// switch x := n.(type) {
// case *ast.SliceExpr:
// ast.SetRelPos(x.X, token.NoRelPos)

// lo := getVal(x.Low)
// hi := getVal(x.High)
// if lo == nil { // a[:j]
// lo = mustParseExpr("0")
// astutil.CopyMeta(lo, x.Low)
// }
// if hi == nil { // a[i:]
// hi = ast.NewCall(ast.NewIdent("len"), x.X)
// astutil.CopyMeta(lo, x.High)
// }
// if pkg := c.Import("list"); pkg != nil {
// c.Replace(ast.NewCall(ast.NewSel(pkg, "Slice"), x.X, lo, hi))
// }
// }
// return true
// }, nil).(*ast.File)

if options.simplify {
f = simplify(f)
}
Expand Down
117 changes: 58 additions & 59 deletions tools/fix/fix_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,84 +27,83 @@ func TestFile(t *testing.T) {
in string
out string
simplify bool
}{{
name: "rewrite integer division",
in: `package foo
}{
{
name: "rewrite integer division",
in: `package foo
a: 1 div 2
b: 3 mod 5
c: 2 quo 9
d: 1.0 rem 1.0 // pass on illegal values.
`,
out: `package foo
out: `package foo
a: __div(1, 2)
b: __mod(3, 5)
c: __quo(2, 9)
d: __rem(1.0, 1.0) // pass on illegal values.
`,
}, {
simplify: true,
in: `
x1: 3 & _
x2: _ | {[string]: int}
x3: 4 & (9 | _)
x4: (_ | 9) & 4
x5: (_ & 9) & 4
x6: 4 & (_ & 9)
`,
out: `x1: 3
},

{
name: "simplify literal tops",
simplify: true,
in: `
x1: 3 & _
x2: _ | {[string]: int}
x3: 4 & (9 | _)
x4: (_ | 9) & 4
x5: (_ & 9) & 4
x6: 4 & (_ & 9)
`,
out: `x1: 3
x2: _
x3: 4
x4: 4
x5: 9 & 4
x6: 4 & 9
`,
},

{
name: "rewrite list addition",
in: `a: [7]
b: a + a
c: a + [8]
d: [9] + a
e: [0] + [1]
`,
out: `import list6c6973 "list"
// }, {
// name: "slice",
// in: `package foo

// // keep comment
// l[3:4] // and this one

// a: len(l[3:4])
// b: len(l[a:_])
// c: len(l[_:x])
// d: len(l[_:_])
// `,
// out: `package foo

// import list6c6973 "list"

// // keep comment
// list6c6973.Slice(l, 3, 4)// and this one

// a: len(list6c6973.Slice(l, 3, 4))
// b: len(list6c6973.Slice(l, a, len(l)))
// c: len(list6c6973.Slice(l, 0, x))
// d: len(list6c6973.Slice(l, 0, len(l)))
// `,
// }, {
// name: "slice2",
// in: `package foo

// import "list"

// a: list.Contains("foo")
// b: len(l[_:_])
// `,
// out: `package foo

// import (
// "list"
// list6c6973 "list"
// )

// a: list.Contains("foo")
// b: len(list6c6973.Slice(l, 0, len(l)))
// `,
}}
a: [7]
b: a + a
c: list6c6973.Concat([a, [8]])
d: list6c6973.Concat([[9], a])
e: list6c6973.Concat([[0], [1]])
`,
},

{
name: "rewrite list multiplication",
in: `a: [7]
b: a * 3
c: 4
d: [7] * c
e: c * [8]
f: [9] * 5
`,
out: `import list6c6973 "list"
a: [7]
b: a * 3
c: 4
d: list6c6973.Repeat([7], c)
e: list6c6973.Repeat([8], c)
f: list6c6973.Repeat([9], 5)
`,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
f, err := parser.ParseFile("", tc.in, parser.ParseComments)
Expand Down

0 comments on commit 540f282

Please sign in to comment.