Skip to content

Commit 2fb9408

Browse files
authored
[syntax] Refactor YAML editing functionality (#411)
Replace the CLI's `yamlNode` type and methods with equivalents on `syntax.YAMLNode`. This is a bit duplicative of functionality already in the syntax package, which allows consumers to edit syntax trees without knowledge of the concrete syntax. However, this is the lowest-lift option for refactoring this functionality with high confidence in the results. We should replace the `YAMLNode` methods with higher-level methods that are concrete-syntax-agnostic in the future.
1 parent 78952f4 commit 2fb9408

File tree

27 files changed

+324
-161
lines changed

27 files changed

+324
-161
lines changed

cmd/esc/cli/env_get.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/pulumi/esc"
2323
"github.com/pulumi/esc/cmd/esc/cli/client"
2424
"github.com/pulumi/esc/cmd/esc/cli/style"
25+
"github.com/pulumi/esc/syntax/encoding"
2526
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
2627
"github.com/pulumi/pulumi/sdk/v3/go/common/util/cmdutil"
2728
)
@@ -281,7 +282,7 @@ func (get *envGetCommand) getEnvironmentMember(
281282
}
282283

283284
if len(path) != 0 && path[0] == "imports" {
284-
node, _ := yamlNode{&docNode}.get(path)
285+
node, _ := encoding.YAMLSyntax{Node: &docNode}.Get(path)
285286
if node == nil {
286287
return nil, nil
287288
}
@@ -313,8 +314,8 @@ func (get *envGetCommand) getEnvironmentMember(
313314
}
314315

315316
definitionYAML := ""
316-
if valuesNode, ok := (yamlNode{&docNode}.get(resource.PropertyPath{"values"})); ok {
317-
if node, _ := (yamlNode{valuesNode}.get(path)); node != nil {
317+
if valuesNode, ok := (encoding.YAMLSyntax{Node: &docNode}.Get(resource.PropertyPath{"values"})); ok {
318+
if node, _ := (encoding.YAMLSyntax{Node: valuesNode}.Get(path)); node != nil {
318319
expr, ok := getEnvExpr(esc.Expr{Object: env.Exprs}, path)
319320
if !ok {
320321
return nil, fmt.Errorf("internal error: no expr for path %v", path)

cmd/esc/cli/env_rm.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/spf13/cobra"
1111
"gopkg.in/yaml.v3"
1212

13+
"github.com/pulumi/esc/syntax/encoding"
1314
"github.com/pulumi/pulumi/sdk/v3/go/common/diag/colors"
1415
pulumienv "github.com/pulumi/pulumi/sdk/v3/go/common/env"
1516
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
@@ -84,11 +85,11 @@ func newEnvRmCmd(env *envCommand) *cobra.Command {
8485
if docNode.Kind != yaml.DocumentNode {
8586
return nil
8687
}
87-
valuesNode, ok := yamlNode{&docNode}.get(resource.PropertyPath{"values"})
88+
valuesNode, ok := encoding.YAMLSyntax{Node: &docNode}.Get(resource.PropertyPath{"values"})
8889
if !ok {
8990
return nil
9091
}
91-
err = yamlNode{valuesNode}.delete(nil, path)
92+
err = encoding.YAMLSyntax{Node: valuesNode}.Delete(nil, path)
9293
if err != nil {
9394
return err
9495
}

cmd/esc/cli/env_set.go

+5-155
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/spf13/cobra"
1313
"gopkg.in/yaml.v3"
1414

15+
"github.com/pulumi/esc/syntax/encoding"
1516
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
1617
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
1718
)
@@ -107,18 +108,18 @@ func newEnvSetCmd(env *envCommand) *cobra.Command {
107108
}
108109

109110
if path[0] == "imports" {
110-
_, err = yamlNode{&docNode}.set(nil, path, yamlValue)
111+
_, err = encoding.YAMLSyntax{Node: &docNode}.Set(nil, path, yamlValue)
111112
} else {
112-
valuesNode, ok := yamlNode{&docNode}.get(resource.PropertyPath{"values"})
113+
valuesNode, ok := encoding.YAMLSyntax{Node: &docNode}.Get(resource.PropertyPath{"values"})
113114
if !ok {
114-
valuesNode, err = yamlNode{&docNode}.set(nil, resource.PropertyPath{"values"}, yaml.Node{
115+
valuesNode, err = encoding.YAMLSyntax{Node: &docNode}.Set(nil, resource.PropertyPath{"values"}, yaml.Node{
115116
Kind: yaml.MappingNode,
116117
})
117118
if err != nil {
118119
return fmt.Errorf("internal error: %w", err)
119120
}
120121
}
121-
_, err = yamlNode{valuesNode}.set(nil, path, yamlValue)
122+
_, err = encoding.YAMLSyntax{Node: valuesNode}.Set(nil, path, yamlValue)
122123
}
123124
if err != nil {
124125
return err
@@ -187,154 +188,3 @@ func looksLikeSecret(path resource.PropertyPath, n yaml.Node) bool {
187188
return info.Entropy >= entropyThreshold ||
188189
(info.Entropy >= (entropyThreshold/2) && entropyPerChar >= entropyPerCharThreshold)
189190
}
190-
191-
type yamlNode struct {
192-
*yaml.Node
193-
}
194-
195-
func (n yamlNode) get(path resource.PropertyPath) (_ *yaml.Node, ok bool) {
196-
if n.Kind == yaml.DocumentNode {
197-
return yamlNode{n.Content[0]}.get(path)
198-
}
199-
200-
if len(path) == 0 {
201-
return n.Node, true
202-
}
203-
204-
switch n.Kind {
205-
case yaml.SequenceNode:
206-
index, ok := path[0].(int)
207-
if !ok || index < 0 || index >= len(n.Content) {
208-
return nil, false
209-
}
210-
return yamlNode{n.Content[index]}.get(path[1:])
211-
case yaml.MappingNode:
212-
key, ok := path[0].(string)
213-
if !ok {
214-
return nil, false
215-
}
216-
for i := 0; i < len(n.Content); i += 2 {
217-
keyNode, valueNode := n.Content[i], n.Content[i+1]
218-
if keyNode.Value == key {
219-
return yamlNode{valueNode}.get(path[1:])
220-
}
221-
}
222-
return nil, false
223-
default:
224-
return nil, false
225-
}
226-
}
227-
228-
func (n yamlNode) set(prefix, path resource.PropertyPath, new yaml.Node) (*yaml.Node, error) {
229-
if n.Kind == yaml.DocumentNode {
230-
return yamlNode{n.Content[0]}.set(prefix, path, new)
231-
}
232-
233-
if len(path) == 0 {
234-
n.Content = new.Content
235-
n.Kind = new.Kind
236-
n.Tag = new.Tag
237-
n.Value = new.Value
238-
return n.Node, nil
239-
}
240-
241-
prefix = append(prefix, path[0])
242-
switch n.Kind {
243-
case 0:
244-
switch accessor := path[0].(type) {
245-
case int:
246-
n.Kind, n.Tag = yaml.SequenceNode, "!!seq"
247-
case string:
248-
n.Kind, n.Tag = yaml.MappingNode, "!!map"
249-
default:
250-
contract.Failf("unexpected accessor kind %T", accessor)
251-
return nil, nil
252-
}
253-
return n.set(prefix[:len(prefix)-1], path, new)
254-
case yaml.SequenceNode:
255-
index, ok := path[0].(int)
256-
if !ok {
257-
return nil, fmt.Errorf("%v: key for an array must be an int", prefix)
258-
}
259-
if index < 0 || index > len(n.Content) {
260-
return nil, fmt.Errorf("%v: array index out of range", prefix)
261-
}
262-
if index == len(n.Content) {
263-
n.Content = append(n.Content, &yaml.Node{})
264-
}
265-
elem := n.Content[index]
266-
return yamlNode{elem}.set(prefix, path[1:], new)
267-
case yaml.MappingNode:
268-
key, ok := path[0].(string)
269-
if !ok {
270-
return nil, fmt.Errorf("%v: key for a map must be a string", prefix)
271-
}
272-
273-
var valueNode *yaml.Node
274-
for i := 0; i < len(n.Content); i += 2 {
275-
keyNode, value := n.Content[i], n.Content[i+1]
276-
if keyNode.Value == key {
277-
valueNode = value
278-
break
279-
}
280-
}
281-
if valueNode == nil {
282-
n.Content = append(n.Content, &yaml.Node{
283-
Kind: yaml.ScalarNode,
284-
Value: key,
285-
Tag: "!!str",
286-
})
287-
n.Content = append(n.Content, &yaml.Node{})
288-
valueNode = n.Content[len(n.Content)-1]
289-
}
290-
return yamlNode{valueNode}.set(prefix, path[1:], new)
291-
default:
292-
return nil, fmt.Errorf("%v: expected an array or an object", prefix)
293-
}
294-
}
295-
296-
func (n yamlNode) delete(prefix, path resource.PropertyPath) error {
297-
if n.Kind == yaml.DocumentNode {
298-
return yamlNode{n.Content[0]}.delete(prefix, path)
299-
}
300-
301-
prefix = append(prefix, path[0])
302-
switch n.Kind {
303-
case yaml.SequenceNode:
304-
index, ok := path[0].(int)
305-
if !ok {
306-
return fmt.Errorf("%v: key for an array must be an int", prefix)
307-
}
308-
if index < 0 || index >= len(n.Content) {
309-
return fmt.Errorf("%v: array index out of range", prefix)
310-
}
311-
if len(path) == 1 {
312-
n.Content = append(n.Content[:index], n.Content[index+1:]...)
313-
return nil
314-
}
315-
elem := n.Content[index]
316-
return yamlNode{elem}.delete(prefix, path[1:])
317-
case yaml.MappingNode:
318-
key, ok := path[0].(string)
319-
if !ok {
320-
return fmt.Errorf("%v: key for a map must be a string", prefix)
321-
}
322-
323-
i := 0
324-
for ; i < len(n.Content); i += 2 {
325-
if n.Content[i].Value == key {
326-
break
327-
}
328-
}
329-
if len(path) == 1 {
330-
if i != len(n.Content) {
331-
n.Content = append(n.Content[:i], n.Content[i+2:]...)
332-
}
333-
return nil
334-
}
335-
valueNode := n.Content[i+1]
336-
return yamlNode{valueNode}.delete(prefix, path[1:])
337-
default:
338-
return fmt.Errorf("%v: expected an array or an object", prefix)
339-
}
340-
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
values:
2+
foo: bar
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
edits:
2+
- values.bar: baz
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
values:
2+
foo: bar
3+
bar: baz
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
values:
2+
array:
3+
- hello
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
edits:
2+
- 'values.array[1]': world
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
values:
2+
array:
3+
- hello
4+
- world

syntax/encoding/testdata/yaml-edit/many/doc.yaml

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
edits:
2+
- 'object["foo"]': baz
3+
- 'object["bar"]': qux
4+
- 'array[0]': 1
5+
- 'array[1].nested': 2
6+
- '["dotted.property"]': [list, of, strings]
7+
- 'environmentVariables': {HELLO: world}
8+
- '["deeply.nested"].path.with.an["\"array[]\""][0]': neat
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
object:
2+
foo: baz
3+
bar: qux
4+
array:
5+
- 1
6+
- nested: 2
7+
dotted.property:
8+
- list
9+
- of
10+
- strings
11+
environmentVariables:
12+
HELLO: world
13+
deeply.nested:
14+
path:
15+
with:
16+
an:
17+
'"array[]"':
18+
- neat
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
values:
2+
array:
3+
- hello
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
edits:
2+
- 'values.array[0]': null
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
values:
2+
array: []
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
values:
2+
foo: bar
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
edits:
2+
- values.foo: null
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
values: {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
values:
2+
array:
3+
- hello
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
edits:
2+
- 'values.array[0]': bonjour
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
values:
2+
array:
3+
- bonjour
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
values:
2+
foo: bar
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
edits:
2+
- values.foo: baz
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
values:
2+
foo: baz

0 commit comments

Comments
 (0)