Repository: goccy/go-yaml
Pull request #593 (2024-12-15 17:58:42)

Quote the numbers even if they are out of range
Closes https://github.com/goccy/go-yaml/issues/586

The `IsNeedQuoted` function previously returned `false` when a given string was out of range, as `ToNumber` failed to parse such strings. I updated the `IsNeedQuoted` function to check the error returned by `ToNumber` and, if it is `strconv.ErrRange`, ensure the string is quoted instead.

Before submitting your PR, please confirm the following.

- [x] Describe the purpose for which you created this PR.  
- [x] Create test code that corresponds to the modification

codecov-commenter: :warning: Please install the !['codecov app svg image'](https://github.com/codecov/engineering-team/assets/152432831/e90313f4-9d3a-4b63-8b54-cfe14e7ec20d) to ensure uploads and comments are reliably processed by Codecov.

## [Codecov](https://app.codecov.io/gh/goccy/go-yaml/pull/593?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Masaaki+Goshima) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 77.45%. Comparing base [(`e1d8782`)](https://app.codecov.io/gh/goccy/go-yaml/commit/e1d8782b74a021e8b6ebce59fc2491bb3c0017e0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Masaaki+Goshima) to head [(`b5d5b31`)](https://app.codecov.io/gh/goccy/go-yaml/commit/b5d5b311c89ea89cd30a155d6757a3826df39a8b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Masaaki+Goshima).

:exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.

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


```diff
@@            Coverage Diff             @@
##           master     #593      +/-   ##
==========================================
+ Coverage   77.43%   77.45%   +0.02%     
==========================================
  Files          21       21              
  Lines        7772     7779       +7     
==========================================
+ Hits         6018     6025       +7     
  Misses       1342     1342              
  Partials      412      412              
```

</details>
shuheiktgw: I’ve migrated most of the logic from `ToNumber` to a new function called `toNumber`, preserved the original interface of `ToNumber`, and created a new function called `isNumber` to check if a given string is a number 🙂 

Patch:
diff --git a/ast/ast.go b/ast/ast.go
index 1ad8f15..53ad0da 100644
--- a/ast/ast.go
+++ b/ast/ast.go
@@ -301,12 +301,12 @@ func checkLineBreak(t *token.Token) bool {
 			}
 			// Due to the way that comment parsing works its assumed that when a null value does not have new line in origin
 			// it was squashed therefore difference is ignored.
-			//foo:
+			// foo:
 			//  bar:
 			//  # comment
 			//  baz: 1
-			//becomes
-			//foo:
+			// becomes
+			// foo:
 			//  bar: null # comment
 			//
 			//  baz: 1
diff --git a/token/token.go b/token/token.go
index b880bd2..21464cf 100644
--- a/token/token.go
+++ b/token/token.go
@@ -1,6 +1,7 @@
 package token
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -539,15 +540,35 @@ type NumberValue struct {
 }
 
 func ToNumber(value string) *NumberValue {
-	if len(value) == 0 {
+	num, err := toNumber(value)
+	if err != nil {
 		return nil
 	}
+	return num
+}
+
+func isNumber(value string) bool {
+	num, err := toNumber(value)
+	if err != nil {
+		var numErr *strconv.NumError
+		if errors.As(err, &numErr) && errors.Is(numErr.Err, strconv.ErrRange) {
+			return true
+		}
+		return false
+	}
+	return num != nil
+}
+
+func toNumber(value string) (*NumberValue, error) {
+	if len(value) == 0 {
+		return nil, nil
+	}
 	if strings.HasPrefix(value, "_") {
-		return nil
+		return nil, nil
 	}
 	dotCount := strings.Count(value, ".")
 	if dotCount > 1 {
-		return nil
+		return nil, nil
 	}
 
 	isNegative := strings.HasPrefix(value, "-")
@@ -589,19 +610,19 @@ func ToNumber(value string) *NumberValue {
 	if typ == NumberTypeFloat {
 		f, err := strconv.ParseFloat(text, 64)
 		if err != nil {
-			return nil
+			return nil, err
 		}
 		v = f
 	} else if isNegative {
 		i, err := strconv.ParseInt(text, base, 64)
 		if err != nil {
-			return nil
+			return nil, err
 		}
 		v = i
 	} else {
 		u, err := strconv.ParseUint(text, base, 64)
 		if err != nil {
-			return nil
+			return nil, err
 		}
 		v = u
 	}
@@ -610,7 +631,7 @@ func ToNumber(value string) *NumberValue {
 		Type:  typ,
 		Value: v,
 		Text:  text,
-	}
+	}, nil
 }
 
 // This is a subset of the formats permitted by the regular expression
@@ -635,7 +656,7 @@ func isTimestamp(value string) bool {
 	return false
 }
 
-// IsNeedQuoted whether need quote for passed string or not
+// IsNeedQuoted checks whether the value needs quote for passed string or not
 func IsNeedQuoted(value string) bool {
 	if value == "" {
 		return true
@@ -643,7 +664,7 @@ func IsNeedQuoted(value string) bool {
 	if _, exists := reservedEncKeywordMap[value]; exists {
 		return true
 	}
-	if num := ToNumber(value); num != nil {
+	if isNumber(value) {
 		return true
 	}
 	first := value[0]


Test patch:
diff --git a/token/token_test.go b/token/token_test.go
index e5e5ba8..3b339b4 100644
--- a/token/token_test.go
+++ b/token/token_test.go
@@ -81,6 +81,10 @@ func TestIsNeedQuoted(t *testing.T) {
 		"",
 		"true",
 		"1.234",
+		"0b11111111111111111111111111111111111111111111111111111111111111111",
+		"0o7777777777777777777777777777777777777777",
+		"999999999999999999999999999999999999999999",
+		"0xffffffffffffffffffffffffffffffffffffffff",
 		"1:1",
 		"2001-12-15T02:59:43.1Z",
 		"2001-12-14t21:59:43.10-05:00",