(redo)Only re-encode subst value if it smells like json and target do…

…c is yaml (#796)

* Improve handling of substitution values wrt json or yaml encoding.

If target is json then of course value  must be json.   Not much to talk about there.

However if target is yaml and value is json then we would like to convert the json to yaml first so we don't have a target that looks like a mix of yaml and json.

That said, if value and target are both yaml then we would like to preserve whatever is in the value.   e.g. anchors and refs, string styles etc.

* In SubstituteByData, only re-encode value to yaml if the value bytes look like json and if the target file is not json.

* Only perform json sniff if target isn't json

* Fix small typo in sniffJson regex

---------

Co-authored-by: Hilmar Falkenberg <[email protected]>

pkg/utils/subst/subst.go

@@ -3,6 +3,7 @@ package subst
 import (
 	"bytes"
 	"container/list"
+	"regexp"
 	"sync"
 
 	"github.com/mandelsoft/goutils/errors"
@@ -120,39 +121,34 @@ func (f *fileinfo) Content() ([]byte, error) {
 	}
 }
 
+var sniffJson = regexp.MustCompile(`^\s*(\{|\[|")`)
+
 func (f *fileinfo) SubstituteByData(path string, value []byte) error {
-	var node interface{}
-	err := runtime.DefaultYAMLEncoding.Unmarshal(value, &node)
-	if err != nil {
-		return err
-	}
-	if f.json {
-		value, err = runtime.DefaultJSONEncoding.Marshal(node)
-	} else {
-		value, err = runtime.DefaultYAMLEncoding.Marshal(node)
-	}
-	if err != nil {
-		return err
+	var err error
+
+	if !f.json && sniffJson.Match(value) {
+		// yaml is generally a superset of json so we could just insert the json value
+		// into a yaml file and have a valid yaml.
+		// However having a yaml file that looks like a mix of yaml and json is off putting.
+		// So if the value looks like json and the target file is yaml we will first
+		// attempt to re-enode the value as yaml before inserting into the target document.
+		// However... we don't want to perform re-encoding for everything because if the
+		// value is actually yaml with some snippets in json style for readability
+		// purposes we don't want to unecessarily lose that styling.  Hence the initial
+		// sniff test for json instead of always re-encoding.
+		var valueData interface{}
+		if err = runtime.DefaultJSONEncoding.Unmarshal(value, &valueData); err == nil {
+			if value, err = runtime.DefaultYAMLEncoding.Marshal(valueData); err != nil {
+				return err
+			}
+		}
 	}
+
 	m := &yaml.Node{}
-	err = yaml.Unmarshal(value, m)
-	if err != nil {
+	if err = yaml.Unmarshal(value, m); err != nil {
 		return err
 	}
 
-	if !f.json {
-		var replaceFlowStyle func(*yaml.Node)
-		replaceFlowStyle = func(nd *yaml.Node) {
-			if nd.Style == yaml.FlowStyle {
-				nd.Style = yaml.LiteralStyle
-			}
-			for _, chld := range nd.Content {
-				replaceFlowStyle(chld)
-			}
-		}
-		replaceFlowStyle(m)
-	}
-
 	nd := &yqlib.CandidateNode{}
 	nd.SetDocument(0)
 	nd.SetFilename("value")
@@ -161,6 +157,7 @@ func (f *fileinfo) SubstituteByData(path string, value []byte) error {
 	if err = nd.UnmarshalYAML(m.Content[0], map[string]*yqlib.CandidateNode{}); err != nil {
 		return err
 	}
+
 	return f.substituteByValue(path, nd)
 }
 

pkg/utils/subst/subst_test.go

@@ -1,6 +1,8 @@
 package subst
 
 import (
+	"strings"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -347,6 +349,59 @@ single: 'foo\nbar'`
   value2: orig2`))
 	})
 
+	It("stores string that is compatible with zero style into yaml using zero style", func() {
+		value := `{"foo": "bar"}`
+		data := `data: ~`
+
+		content, err := Parse([]byte(data))
+		Expect(err).To(Succeed())
+
+		Expect(content.SubstituteByData("data", []byte(value))).To(Succeed())
+
+		result, err := content.Content()
+		Expect(err).To(Succeed())
+		expected := `data:
+  foo: bar`
+
+		Expect(strings.TrimSpace(string(result))).To(Equal(expected))
+	})
+
+	It("stores string that requires double quote style with yaml using double quote style", func() {
+		value := `{"foo": "true"}`
+		data := `data: ~`
+
+		content, err := Parse([]byte(data))
+		Expect(err).To(Succeed())
+
+		Expect(content.SubstituteByData("data", []byte(value))).To(Succeed())
+
+		result, err := content.Content()
+		Expect(err).To(Succeed())
+		expected := `data:
+  foo: "true"`
+
+		Expect(strings.TrimSpace(string(result))).To(Equal(expected))
+	})
+
+	It("stores string that is compatible with literal style into yaml using literal style", func() {
+		value := `{"foo": "bar\ncar"}`
+		data := `data: ~`
+
+		content, err := Parse([]byte(data))
+		Expect(err).To(Succeed())
+
+		Expect(content.SubstituteByData("data", []byte(value))).To(Succeed())
+
+		result, err := content.Content()
+		Expect(err).To(Succeed())
+		expected := `data:
+  foo: |-
+    bar
+    car`
+
+		Expect(strings.TrimSpace(string(result))).To(Equal(expected))
+	})
+
 	It("handles non-string scalar", func() {
 		value := `2`