Repository: goccy/go-yaml
Pull request #354 (2023-03-07 00:53:24)

Quote YAML 1.1 bools at encoding time for compatibility with other legacy parsers
**This is rather a feature request with a proposed implementation!**

Hey @goccy, thank you so much for your awesome work on this library ☺️ 

We sometimes cannot produce YAML documents using this awesome library when the consumer of the produced YAML documents uses legacy YAML parsers.

Apparently, the issue is- We have no way to encode a string `on` as a string in terms of YAML 1.1.

I'm afraid you might say why you need to care about the legacy YAML versions that this library is NOT intended to support. Indeed... but please read on!

A simple failure scenario is this:

- `goccy/go-yaml` encodes a string (not bool) "on" as `on` in the YAML data
- Another legacy yaml parser decodes it as a bool `true`.

This change addresses that, by quoting YAML 1.1 bool-like strings "on", "off", and so on at the encoding time so that the YAML data produced by the `goccy/go-yaml`'s can be fed into other legacy YAML parsers that support YAML up to 1.1.

I do know go-yaml is intended to support YAML 1.2 only. However, this change does not break that (I think). This just enhances compatibility with legacy parsers without quitting to be a YAML 1.2 only library. I presume YAML 1.2 spec says a string `on` can be encoded as either `on` or `"on`" in the YAML document, and we just prefer to use the latter for compatibility.

mumoshu: @goccy We've been using [c14e882](https://github.com/goccy/go-yaml/pull/354/commits/c14e882d2162b4128291029e7b23c111c8080a01) in [helmfile](https://github.com/helmfile/helmfile) for a while, and it has been doing a great job so far!

Also, your CI discovered some failures due to tests not being up-to-date with the change- I've fixed that in 506b55e.
codecov-commenter: # [Codecov](https://codecov.io/gh/goccy/go-yaml/pull/354?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Masaaki+Goshima) Report
> Merging [#354](https://codecov.io/gh/goccy/go-yaml/pull/354?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Masaaki+Goshima) (506b55e) into [master](https://codecov.io/gh/goccy/go-yaml/commit/11ad39b5c5ae6e36fc172b0a58feff816cca6efe?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Masaaki+Goshima) (11ad39b) will **decrease** coverage by `0.04%`.
> The diff coverage is `87.50%`.

:mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Masaaki+Goshima)

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #354      +/-   ##
==========================================
- Coverage   75.37%   75.34%   -0.04%     
==========================================
  Files          13       13              
  Lines        4545     4551       +6     
==========================================
+ Hits         3426     3429       +3     
- Misses        864      866       +2     
- Partials      255      256       +1     
```

</details>
goccy: First, Thank you for creating a great PR. Also, thank you for the sponsor registration.

I understand what you are saying. Indeed, in order to provide this functionality, we need to support encoding to quoted values.
One point I was wondering is what tools or libraries are intended to be used to read the yaml exported by go-yaml. For example, if you use go-yaml for decoding, you should be fine, but what other parser do you expect to need to use ?
yxxhero: /cc @mumoshu 
goccy: Thank you for your contribution !! LGTM

Patch:
diff --git a/token/token.go b/token/token.go
index 796b522..4943023 100644
--- a/token/token.go
+++ b/token/token.go
@@ -283,6 +283,28 @@ var (
 		"False",
 		"FALSE",
 	}
+	// For compatibility with other YAML 1.1 parsers
+	// Note that we use these solely for encoding the bool value with quotes.
+	// go-yaml should not treat these as reserved keywords at parsing time.
+	// as go-yaml is supposed to be compliant only with YAML 1.2.
+	reservedLegacyBoolKeywords = []string{
+		"y",
+		"Y",
+		"yes",
+		"Yes",
+		"YES",
+		"n",
+		"N",
+		"no",
+		"No",
+		"NO",
+		"on",
+		"On",
+		"ON",
+		"off",
+		"Off",
+		"OFF",
+	}
 	reservedInfKeywords = []string{
 		".inf",
 		".Inf",
@@ -297,6 +319,11 @@ var (
 		".NAN",
 	}
 	reservedKeywordMap = map[string]func(string, string, *Position) *Token{}
+	// reservedEncKeywordMap contains is the keyword map used at encoding time.
+	// This is supposed to be a superset of reservedKeywordMap,
+	// and used to quote legacy keywords present in YAML 1.1 or lesser for compatibility reasons,
+	// even though this library is supposed to be YAML 1.2-compliant.
+	reservedEncKeywordMap = map[string]func(string, string, *Position) *Token{}
 )
 
 func reservedKeywordToken(typ Type, value, org string, pos *Position) *Token {
@@ -317,7 +344,14 @@ func init() {
 		}
 	}
 	for _, keyword := range reservedBoolKeywords {
-		reservedKeywordMap[keyword] = func(value, org string, pos *Position) *Token {
+		f := func(value, org string, pos *Position) *Token {
+			return reservedKeywordToken(BoolType, value, org, pos)
+		}
+		reservedKeywordMap[keyword] = f
+		reservedEncKeywordMap[keyword] = f
+	}
+	for _, keyword := range reservedLegacyBoolKeywords {
+		reservedEncKeywordMap[keyword] = func(value, org string, pos *Position) *Token {
 			return reservedKeywordToken(BoolType, value, org, pos)
 		}
 	}
@@ -581,7 +615,7 @@ func IsNeedQuoted(value string) bool {
 	if value == "" {
 		return true
 	}
-	if _, exists := reservedKeywordMap[value]; exists {
+	if _, exists := reservedEncKeywordMap[value]; exists {
 		return true
 	}
 	if stat := getNumberStat(value); stat.isNum {


Test patch:
diff --git a/encode_test.go b/encode_test.go
index db36028..3aeddc6 100644
--- a/encode_test.go
+++ b/encode_test.go
@@ -440,7 +440,7 @@ func TestEncoder(t *testing.T) {
 		},
 
 		{
-			"a:\n  y: \"\"\n",
+			"a:\n  \"y\": \"\"\n",
 			struct {
 				A *struct {
 					X string `yaml:"x,omitempty"`
@@ -702,7 +702,7 @@ func TestEncodeStructIncludeMap(t *testing.T) {
 	if err != nil {
 		t.Fatalf("%+v", err)
 	}
-	expect := "a:\n  m:\n    x: y\n"
+	expect := "a:\n  m:\n    x: \"y\"\n"
 	actual := string(bytes)
 	if actual != expect {
 		t.Fatalf("unexpected output. expect:[%s] actual:[%s]", expect, actual)
@@ -720,7 +720,7 @@ func TestEncodeDefinedTypeKeyMap(t *testing.T) {
 	if err != nil {
 		t.Fatalf("%+v", err)
 	}
-	expect := "m:\n  x: y\n"
+	expect := "m:\n  x: \"y\"\n"
 	actual := string(bytes)
 	if actual != expect {
 		t.Fatalf("unexpected output. expect:[%s] actual:[%s]", expect, actual)
diff --git a/token/token_test.go b/token/token_test.go
index 9db464e..3e14870 100644
--- a/token/token_test.go
+++ b/token/token_test.go
@@ -101,6 +101,22 @@ func TestIsNeedQuoted(t *testing.T) {
 		`"a b`,
 		"a:",
 		"a: b",
+		"y",
+		"Y",
+		"yes",
+		"Yes",
+		"YES",
+		"n",
+		"N",
+		"no",
+		"No",
+		"NO",
+		"on",
+		"On",
+		"ON",
+		"off",
+		"Off",
+		"OFF",
 	}
 	for i, test := range needQuotedTests {
 		if !token.IsNeedQuoted(test) {
diff --git a/yaml_test.go b/yaml_test.go
index 0404794..dfe6df1 100644
--- a/yaml_test.go
+++ b/yaml_test.go
@@ -95,7 +95,7 @@ a:
   b: hello
   c: true
   d:
-    x: y
+    x: "y"
 b:
   a: 2
   b: world