mvdan.cc / cue
this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

cue/format: fix incorrect quote removal in fmt -s

Fix https://cuelang.org/cl/1193386 introduced a regression in which
certain labels will have their quotes removed, resulting in modifying
the semantics of the original CUE.

The current algorithm is split into two:

1. It identifies, for each string in the AST, whether its quotes can
be removed without altering the original semantics of the CUE.
2. Based on the collected data, it then strips quotes from strings
where applicable.

The above CL incorrectly caused the string replacement step (2) to walk
each ast node entirely, instead of only handling the node label. This
caused the scope information to be incorrectly interpreted, since it is
only meant for a specific label and not the entire subtree.

Take for example the following CUE:

foo: {}
bar: "foo": foo

When the algorithm reaches the node representing "bar", it will now
descend into both the label node (ident bar) and the value node
(the {"foo": foo} struct). But the scoping information is built in
such a way that it's applicable only to the bar label - not the entire
tree located under bar. The data then causes "foo" to incorrectly be
replaced with foo.

Note that this introduces a minor "regression" in which fields are not
simplified inside of complex label, so that the following expression
will no longer be simplified:

[!={"a": 1}.a]: {}

It is now clear to me that the original algorithm intentionally did not
take such cases into account, and they were simplified because of the
above bug.

Fixes #3189.

Signed-off-by: Noam Dolovich <noam.tzvi.dolovich@gmail.com>
Change-Id: I84c2db55f62054950ad77c0fa69a777218fcc86d
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1195742
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>

authored by

Noam Dolovich and committed by
Daniel Martí b0537804 2eab1b70

+16 -15
+4 -13
cue/format/simplify.go
··· 52 52 for _, d := range decls { 53 53 switch x := d.(type) { 54 54 case *ast.Field: 55 - astutil.Apply(x, sc.replace, nil) 55 + if _, ok := x.Label.(*ast.BasicLit); ok { 56 + x.Label = astutil.Apply(x.Label, nil, sc.replace).(ast.Label) 57 + } 56 58 } 57 59 } 58 60 } ··· 104 106 func (s *labelSimplifier) replace(c astutil.Cursor) bool { 105 107 switch x := c.Node().(type) { 106 108 case *ast.BasicLit: 107 - // We do not want to replace all nodes of type *ast.BasicList, 108 - // only those that are labels. 109 - // Here we only replace x if it is a field label, 110 - // for example {"foo": 1} as opposed to [!="foo"]. 111 - if c.Parent() == nil { 112 - return false 113 - } 114 - if f, ok := c.Parent().Node().(*ast.Field); !ok || f.Label != x { 115 - return false 116 - } 117 - 118 109 str, err := strconv.Unquote(x.Value) 119 110 if err == nil && s.scope[str] && !internal.IsDefOrHidden(str) { 120 111 c.Replace(ast.NewIdent(str)) 121 112 } 122 113 } 123 - return true 114 + return false 124 115 }
+7 -2
cue/format/testdata/simplify.golden
··· 66 66 bar: "bar" 67 67 baz: "baz" 68 68 [!={ 69 - a: "baz" 69 + "a": "baz" 70 70 }.a & !="bar"]: {} 71 71 } 72 72 y: { 73 73 bar: "a-value" 74 74 ({ 75 - a: "bar" 75 + "a": "bar" 76 76 }.a): {} 77 77 } 78 78 } 79 79 } 80 + 81 + { 82 + foo: {} 83 + bar: "foo": foo // removing the quotes would cause a cyclic reference 84 + }
+5
cue/format/testdata/simplify.input
··· 81 81 } 82 82 } 83 83 } 84 + 85 + { 86 + foo: {} 87 + bar: "foo": foo // removing the quotes would cause a cyclic reference 88 + }