Skip to content

Commit 3d36560

Browse files
cuematthewcuematthew
committed
tools/fix: rewrite list addition and multiplication
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 <[email protected]> Change-Id: I99946833277cda5de8004a30d5e4f909cfcc1016 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200212 TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Daniel Martí <[email protected]> Unity-Result: CUE porcuepine <[email protected]>
1 parent b123f2b commit 3d36560

File tree

2 files changed

+98
-100
lines changed

2 files changed

+98
-100
lines changed

‎tools/fix/fix.go

+40-41
Original file line numberDiff line numberDiff line change
@@ -44,59 +44,58 @@ func File(f *ast.File, o ...Option) *ast.File {
4444
f(&options)
4545
}
4646

47-
// Rewrite integer division operations to use builtins.
4847
f = astutil.Apply(f, func(c astutil.Cursor) bool {
4948
n := c.Node()
50-
switch x := n.(type) {
49+
switch n := n.(type) {
5150
case *ast.BinaryExpr:
52-
switch x.Op {
51+
switch n.Op {
5352
case token.IDIV, token.IMOD, token.IQUO, token.IREM:
54-
ast.SetRelPos(x.X, token.NoSpace)
53+
// Rewrite integer division operations to use builtins.
54+
ast.SetRelPos(n.X, token.NoSpace)
5555
c.Replace(&ast.CallExpr{
5656
// Use the __foo version to prevent accidental shadowing.
57-
Fun: ast.NewIdent("__" + x.Op.String()),
58-
Args: []ast.Expr{x.X, x.Y},
57+
Fun: ast.NewIdent("__" + n.Op.String()),
58+
Args: []ast.Expr{n.X, n.Y},
5959
})
60+
61+
case token.ADD, token.MUL:
62+
// The fix here only works when at least one argument is a
63+
// literal list. It would be better to be able to use CUE
64+
// to infer type information, and then apply the fix to
65+
// all places where we infer a list argument.
66+
x, y := n.X, n.Y
67+
_, xIsList := x.(*ast.ListLit)
68+
_, yIsList := y.(*ast.ListLit)
69+
if !(xIsList || yIsList) {
70+
break
71+
}
72+
pkg := c.Import("list")
73+
if pkg == nil {
74+
break
75+
}
76+
if n.Op == token.ADD {
77+
// Rewrite list addition to use list.Concat
78+
ast.SetRelPos(x, token.NoSpace)
79+
c.Replace(&ast.CallExpr{
80+
Fun: ast.NewSel(pkg, "Concat"),
81+
Args: []ast.Expr{ast.NewList(x, y)},
82+
})
83+
} else {
84+
// Rewrite list multiplication to use list.Repeat
85+
if !xIsList {
86+
x, y = y, x
87+
}
88+
ast.SetRelPos(x, token.NoSpace)
89+
c.Replace(&ast.CallExpr{
90+
Fun: ast.NewSel(pkg, "Repeat"),
91+
Args: []ast.Expr{x, y},
92+
})
93+
}
6094
}
6195
}
6296
return true
6397
}, nil).(*ast.File)
6498

65-
// TODO: we are probably reintroducing slices. Disable for now.
66-
//
67-
// Rewrite slice expression.
68-
// f = astutil.Apply(f, func(c astutil.Cursor) bool {
69-
// n := c.Node()
70-
// getVal := func(n ast.Expr) ast.Expr {
71-
// if n == nil {
72-
// return nil
73-
// }
74-
// if id, ok := n.(*ast.Ident); ok && id.Name == "_" {
75-
// return nil
76-
// }
77-
// return n
78-
// }
79-
// switch x := n.(type) {
80-
// case *ast.SliceExpr:
81-
// ast.SetRelPos(x.X, token.NoRelPos)
82-
83-
// lo := getVal(x.Low)
84-
// hi := getVal(x.High)
85-
// if lo == nil { // a[:j]
86-
// lo = mustParseExpr("0")
87-
// astutil.CopyMeta(lo, x.Low)
88-
// }
89-
// if hi == nil { // a[i:]
90-
// hi = ast.NewCall(ast.NewIdent("len"), x.X)
91-
// astutil.CopyMeta(lo, x.High)
92-
// }
93-
// if pkg := c.Import("list"); pkg != nil {
94-
// c.Replace(ast.NewCall(ast.NewSel(pkg, "Slice"), x.X, lo, hi))
95-
// }
96-
// }
97-
// return true
98-
// }, nil).(*ast.File)
99-
10099
if options.simplify {
101100
f = simplify(f)
102101
}

‎tools/fix/fix_test.go

+58-59
Original file line numberDiff line numberDiff line change
@@ -27,84 +27,83 @@ func TestFile(t *testing.T) {
2727
in string
2828
out string
2929
simplify bool
30-
}{{
31-
name: "rewrite integer division",
32-
in: `package foo
30+
}{
31+
{
32+
name: "rewrite integer division",
33+
in: `package foo
3334
3435
a: 1 div 2
3536
b: 3 mod 5
3637
c: 2 quo 9
3738
d: 1.0 rem 1.0 // pass on illegal values.
3839
`,
39-
out: `package foo
40+
out: `package foo
4041
4142
a: __div(1, 2)
4243
b: __mod(3, 5)
4344
c: __quo(2, 9)
4445
d: __rem(1.0, 1.0) // pass on illegal values.
4546
`,
46-
}, {
47-
simplify: true,
48-
in: `
49-
x1: 3 & _
50-
x2: _ | {[string]: int}
51-
x3: 4 & (9 | _)
52-
x4: (_ | 9) & 4
53-
x5: (_ & 9) & 4
54-
x6: 4 & (_ & 9)
55-
`,
56-
out: `x1: 3
47+
},
48+
49+
{
50+
name: "simplify literal tops",
51+
simplify: true,
52+
in: `
53+
x1: 3 & _
54+
x2: _ | {[string]: int}
55+
x3: 4 & (9 | _)
56+
x4: (_ | 9) & 4
57+
x5: (_ & 9) & 4
58+
x6: 4 & (_ & 9)
59+
`,
60+
out: `x1: 3
5761
x2: _
5862
x3: 4
5963
x4: 4
6064
x5: 9 & 4
6165
x6: 4 & 9
6266
`,
67+
},
68+
69+
{
70+
name: "rewrite list addition",
71+
in: `a: [7]
72+
b: a + a
73+
c: a + [8]
74+
d: [9] + a
75+
e: [0] + [1]
76+
`,
77+
out: `import list6c6973 "list"
6378
64-
// }, {
65-
// name: "slice",
66-
// in: `package foo
67-
68-
// // keep comment
69-
// l[3:4] // and this one
70-
71-
// a: len(l[3:4])
72-
// b: len(l[a:_])
73-
// c: len(l[_:x])
74-
// d: len(l[_:_])
75-
// `,
76-
// out: `package foo
77-
78-
// import list6c6973 "list"
79-
80-
// // keep comment
81-
// list6c6973.Slice(l, 3, 4)// and this one
82-
83-
// a: len(list6c6973.Slice(l, 3, 4))
84-
// b: len(list6c6973.Slice(l, a, len(l)))
85-
// c: len(list6c6973.Slice(l, 0, x))
86-
// d: len(list6c6973.Slice(l, 0, len(l)))
87-
// `,
88-
// }, {
89-
// name: "slice2",
90-
// in: `package foo
91-
92-
// import "list"
93-
94-
// a: list.Contains("foo")
95-
// b: len(l[_:_])
96-
// `,
97-
// out: `package foo
98-
99-
// import (
100-
// "list"
101-
// list6c6973 "list"
102-
// )
103-
104-
// a: list.Contains("foo")
105-
// b: len(list6c6973.Slice(l, 0, len(l)))
106-
// `,
107-
}}
79+
a: [7]
80+
b: a + a
81+
c: list6c6973.Concat([a, [8]])
82+
d: list6c6973.Concat([[9], a])
83+
e: list6c6973.Concat([[0], [1]])
84+
`,
85+
},
86+
87+
{
88+
name: "rewrite list multiplication",
89+
in: `a: [7]
90+
b: a * 3
91+
c: 4
92+
d: [7] * c
93+
e: c * [8]
94+
f: [9] * 5
95+
`,
96+
out: `import list6c6973 "list"
97+
98+
a: [7]
99+
b: a * 3
100+
c: 4
101+
d: list6c6973.Repeat([7], c)
102+
e: list6c6973.Repeat([8], c)
103+
f: list6c6973.Repeat([9], 5)
104+
`,
105+
},
106+
}
108107
for _, tc := range testCases {
109108
t.Run(tc.name, func(t *testing.T) {
110109
f, err := parser.ParseFile("", tc.in, parser.ParseComments)

0 commit comments

Comments
 (0)