Repository: alecthomas/kong
Pull request #348 (2023-01-25 08:27:39)

fix: handle contents of tags properly by unquoting them when necessary
So far bare tags did not get unquoted, causing several problems. E.g., using backslashs in tags doesn't work as expected (I noticed when having a `default` tag containing a RegExp).

This also led to the issue described in #324. 

The PR related to that issue (#330) is not really tackling the problem, it's just trying to fix the most obvious symptoms.

I added a test to ensure things are working.

alecthomas: Nice, thanks!

You can disable the linter with `//nolint:<linter>` on the line that it reported.
FlorianLoch: You are welcome! :)

Ignored the linter and amended my commit, please try again. 

Patch:
diff --git a/tag.go b/tag.go
index 6a94b2d..cac4074 100644
--- a/tag.go
+++ b/tag.go
@@ -56,11 +56,13 @@ func (t *Tag) String() string {
 
 type tagChars struct {
 	sep, quote, assign rune
+	needsUnquote       bool
 }
 
-var kongChars = tagChars{sep: ',', quote: '\'', assign: '='}
-var bareChars = tagChars{sep: ' ', quote: '"', assign: ':'}
+var kongChars = tagChars{sep: ',', quote: '\'', assign: '=', needsUnquote: false}
+var bareChars = tagChars{sep: ' ', quote: '"', assign: ':', needsUnquote: true}
 
+// nolint:gocyclo
 func parseTagItems(tagString string, chr tagChars) (map[string][]string, error) {
 	d := map[string][]string{}
 	key := []rune{}
@@ -68,11 +70,25 @@ func parseTagItems(tagString string, chr tagChars) (map[string][]string, error)
 	quotes := false
 	inKey := true
 
-	add := func() {
-		d[string(key)] = append(d[string(key)], string(value))
+	add := func() error {
+		// Bare tags are quoted, therefore we need to unquote them in the same fashion reflect.Lookup() (implicitly)
+		// unquotes "kong tags".
+		s := string(value)
+
+		if chr.needsUnquote && s != "" {
+			if unquoted, err := strconv.Unquote(fmt.Sprintf(`"%s"`, s)); err == nil {
+				s = unquoted
+			} else {
+				return fmt.Errorf("unquoting tag value `%s`: %w", s, err)
+			}
+		}
+
+		d[string(key)] = append(d[string(key)], s)
 		key = []rune{}
 		value = []rune{}
 		inKey = true
+
+		return nil
 	}
 
 	runes := []rune(tagString)
@@ -86,7 +102,10 @@ func parseTagItems(tagString string, chr tagChars) (map[string][]string, error)
 			eof = true
 		}
 		if !quotes && r == chr.sep {
-			add()
+			if err := add(); err != nil {
+				return nil, err
+			}
+
 			continue
 		}
 		if r == chr.assign && inKey {
@@ -96,6 +115,12 @@ func parseTagItems(tagString string, chr tagChars) (map[string][]string, error)
 		if r == '\\' {
 			if next == chr.quote {
 				idx++
+
+				// We need to keep the backslashes, otherwise subsequent unquoting cannot work
+				if chr.needsUnquote {
+					value = append(value, r)
+				}
+
 				r = chr.quote
 			}
 		} else if r == chr.quote {
@@ -119,7 +144,9 @@ func parseTagItems(tagString string, chr tagChars) (map[string][]string, error)
 		return nil, fmt.Errorf("%v is not quoted properly", tagString)
 	}
 
-	add()
+	if err := add(); err != nil {
+		return nil, err
+	}
 
 	return d, nil
 }
@@ -242,7 +269,7 @@ func hydrateTag(t *Tag, typ reflect.Type) error { // nolint: gocyclo
 	}
 	t.PlaceHolder = t.Get("placeholder")
 	t.Enum = t.Get("enum")
-	scalarType := (typ == nil || !(typ.Kind() == reflect.Slice || typ.Kind() == reflect.Map || typ.Kind() == reflect.Ptr))
+	scalarType := typ == nil || !(typ.Kind() == reflect.Slice || typ.Kind() == reflect.Map || typ.Kind() == reflect.Ptr)
 	if t.Enum != "" && !(t.Required || t.HasDefault) && scalarType {
 		return fmt.Errorf("enum value is only valid if it is either required or has a valid default value")
 	}


Test patch:
diff --git a/kong_test.go b/kong_test.go
index 53e064b..2dd7cd3 100644
--- a/kong_test.go
+++ b/kong_test.go
@@ -8,8 +8,9 @@ import (
 	"testing"
 
 	"github.com/alecthomas/assert/v2"
-	"github.com/alecthomas/kong"
 	"github.com/alecthomas/repr"
+
+	"github.com/alecthomas/kong"
 )
 
 func mustNew(t *testing.T, cli interface{}, options ...kong.Option) *kong.Kong {
diff --git a/tag_test.go b/tag_test.go
index a34f7da..f648edc 100644
--- a/tag_test.go
+++ b/tag_test.go
@@ -5,17 +5,18 @@ import (
 	"testing"
 
 	"github.com/alecthomas/assert/v2"
+
 	"github.com/alecthomas/kong"
 )
 
 func TestDefaultValueForOptionalArg(t *testing.T) {
 	var cli struct {
-		Arg string `kong:"arg,optional,default='👌'"`
+		Arg string `kong:"arg,optional,default='\"\\'👌\\'\"'"`
 	}
 	p := mustNew(t, &cli)
 	_, err := p.Parse(nil)
 	assert.NoError(t, err)
-	assert.Equal(t, "👌", cli.Arg)
+	assert.Equal(t, "\"'👌'\"", cli.Arg)
 }
 
 func TestNoValueInTag(t *testing.T) {
@@ -66,6 +67,18 @@ func TestEscapedQuote(t *testing.T) {
 	assert.Equal(t, "i don't know", cli.DoYouKnow)
 }
 
+func TestEscapingInQuotedTags(t *testing.T) {
+	var cli struct {
+		Regex1 string `kong:"default='\\d+\n'"`
+		Regex2 string `default:"\\d+\n"`
+	}
+	p := mustNew(t, &cli)
+	_, err := p.Parse(nil)
+	assert.NoError(t, err)
+	assert.Equal(t, "\\d+\n", cli.Regex1)
+	assert.Equal(t, "\\d+\n", cli.Regex2)
+}
+
 func TestBareTags(t *testing.T) {
 	var cli struct {
 		Cmd struct {