Merge pull request #43 from vmware-labs/42-relaxed-literals

42 relaxed literals

pkg/yamlpath/comparison.go

@@ -17,17 +17,6 @@ const (
 	compareIncomparable
 )
 
-func (c comparison) invertOrdering() comparison {
-	switch c {
-	case compareLessThan:
-		return compareGreaterThan
-	case compareGreaterThan:
-		return compareLessThan
-	default:
-		return c
-	}
-}
-
 type orderingOperator string
 
 const (
@@ -84,20 +73,21 @@ func compareFloat64(lhs, rhs float64) comparison {
 	return compareEqual
 }
 
-func compareNodeValues(lhs string, rhs string) comparison {
-	numeric := true
-	lhsFloat, err := strconv.ParseFloat(lhs, 64)
-	if err != nil {
-		numeric = false
+// compareNodeValues compares two values each of which may be a string, integer, or float
+func compareNodeValues(lhs, rhs typedValue) comparison {
+	if lhs.typ.isNumeric() && rhs.typ.isNumeric() {
+		return compareFloat64(mustParseFloat64(lhs.val), mustParseFloat64(rhs.val))
 	}
-	rhsFloat, err := strconv.ParseFloat(rhs, 64)
-	if err != nil {
-		numeric = false
+	if (lhs.typ != stringValueType && !lhs.typ.isNumeric()) || (rhs.typ != stringValueType && !rhs.typ.isNumeric()) {
+		panic("invalid type of value passed to compareNodeValues") // should never happen
 	}
-	if numeric {
-		return compareFloat64(lhsFloat, rhsFloat)
+	return compareStrings(lhs.val, rhs.val)
+}
+
+func mustParseFloat64(s string) float64 {
+	f, err := strconv.ParseFloat(s, 64)
+	if err != nil {
+		panic("invalid numeric value " + s) // should never happen
 	}
-	// it's ok to compare other values (such as strings, booleans, and nulls) as strings
-	// because the types of lhs and rhs will already have been checked to be equal
-	return compareStrings(lhs, rhs)
+	return f
 }

pkg/yamlpath/comparison_test.go

@@ -152,70 +152,70 @@ func TestComparators(t *testing.T) {
 			name:       "node values equal",
 			comparator: equal,
 			comparisons: map[comparison]bool{
-				compareNodeValues("a", "a"):   true,
-				compareNodeValues("a", "b"):   false,
-				compareNodeValues("1.0", "1"): true,
-				compareNodeValues("1.0", "a"): false,
-				compareNodeValues("a", "1.0"): false,
+				compareNodeValues(typedValueOfString("a"), typedValueOfString("a")):  true,
+				compareNodeValues(typedValueOfString("a"), typedValueOfString("b")):  false,
+				compareNodeValues(typedValueOfFloat("1.0"), typedValueOfInt("1")):    true,
+				compareNodeValues(typedValueOfFloat("1.0"), typedValueOfString("a")): false,
+				compareNodeValues(typedValueOfString("a"), typedValueOfFloat("1.0")): false,
 			},
 		},
 		{
 			name:       "node values not equal",
 			comparator: notEqual,
 			comparisons: map[comparison]bool{
-				compareNodeValues("a", "a"):   false,
-				compareNodeValues("a", "b"):   true,
-				compareNodeValues("1.0", "1"): false,
-				compareNodeValues("1.0", "a"): true,
-				compareNodeValues("a", "1.0"): true,
+				compareNodeValues(typedValueOfString("a"), typedValueOfString("a")):  false,
+				compareNodeValues(typedValueOfString("a"), typedValueOfString("b")):  true,
+				compareNodeValues(typedValueOfFloat("1.0"), typedValueOfInt("1")):    false,
+				compareNodeValues(typedValueOfFloat("1.0"), typedValueOfString("a")): true,
+				compareNodeValues(typedValueOfString("a"), typedValueOfFloat("1.0")): true,
 			},
 		},
 		{
 			name:       "node values greater than",
 			comparator: greaterThan,
 			comparisons: map[comparison]bool{
-				compareNodeValues("1.1", "1.2"): false,
-				compareNodeValues("1.1", "1.1"): false,
-				compareNodeValues("1.2", "1.1"): true,
-				compareNodeValues("a", "a"):     false, // should be excluded by lexer
-				compareNodeValues("1.0", "a"):   false, // should be excluded by lexer
-				compareNodeValues("a", "1.0"):   false, // should be excluded by lexer
+				compareNodeValues(typedValueOfFloat("1.1"), typedValueOfFloat("1.2")): false,
+				compareNodeValues(typedValueOfFloat("1.1"), typedValueOfFloat("1.1")): false,
+				compareNodeValues(typedValueOfFloat("1.2"), typedValueOfFloat("1.1")): true,
+				compareNodeValues(typedValueOfString("a"), typedValueOfString("a")):   false, // should be excluded by lexer
+				compareNodeValues(typedValueOfFloat("1.0"), typedValueOfString("a")):  false, // should be excluded by lexer
+				compareNodeValues(typedValueOfString("a"), typedValueOfFloat("1.0")):  false, // should be excluded by lexer
 			},
 		},
 		{
 			name:       "node values greater than or equal",
 			comparator: greaterThanOrEqual,
 			comparisons: map[comparison]bool{
-				compareNodeValues("1.1", "1.2"): false,
-				compareNodeValues("1.1", "1.1"): true,
-				compareNodeValues("1.2", "1.1"): true,
-				compareNodeValues("a", "a"):     true,  // should be excluded by lexer
-				compareNodeValues("1.0", "a"):   false, // should be excluded by lexer
-				compareNodeValues("a", "1.0"):   false, // should be excluded by lexer
+				compareNodeValues(typedValueOfFloat("1.1"), typedValueOfFloat("1.2")): false,
+				compareNodeValues(typedValueOfFloat("1.1"), typedValueOfFloat("1.1")): true,
+				compareNodeValues(typedValueOfFloat("1.2"), typedValueOfFloat("1.1")): true,
+				compareNodeValues(typedValueOfString("a"), typedValueOfString("a")):   true,  // should be excluded by lexer
+				compareNodeValues(typedValueOfFloat("1.0"), typedValueOfString("a")):  false, // should be excluded by lexer
+				compareNodeValues(typedValueOfString("a"), typedValueOfFloat("1.0")):  false, // should be excluded by lexer
 			},
 		},
 		{
 			name:       "node values less than",
 			comparator: lessThan,
 			comparisons: map[comparison]bool{
-				compareNodeValues("1.1", "1.2"): true,
-				compareNodeValues("1.1", "1.1"): false,
-				compareNodeValues("1.2", "1.1"): false,
-				compareNodeValues("a", "a"):     false, // should be excluded by lexer
-				compareNodeValues("1.0", "a"):   false, // should be excluded by lexer
-				compareNodeValues("a", "1.0"):   false, // should be excluded by lexer
+				compareNodeValues(typedValueOfFloat("1.1"), typedValueOfFloat("1.2")): true,
+				compareNodeValues(typedValueOfFloat("1.1"), typedValueOfFloat("1.1")): false,
+				compareNodeValues(typedValueOfFloat("1.2"), typedValueOfFloat("1.1")): false,
+				compareNodeValues(typedValueOfString("a"), typedValueOfString("a")):   false, // should be excluded by lexer
+				compareNodeValues(typedValueOfFloat("1.0"), typedValueOfString("a")):  false, // should be excluded by lexer
+				compareNodeValues(typedValueOfString("a"), typedValueOfFloat("1.0")):  false, // should be excluded by lexer
 			},
 		},
 		{
 			name:       "node values less than or equal",
 			comparator: lessThanOrEqual,
 			comparisons: map[comparison]bool{
-				compareNodeValues("1.1", "1.2"): true,
-				compareNodeValues("1.1", "1.1"): true,
-				compareNodeValues("1.2", "1.1"): false,
-				compareNodeValues("a", "a"):     true,  // should be excluded by lexer
-				compareNodeValues("1.0", "a"):   false, // should be excluded by lexer
-				compareNodeValues("a", "1.0"):   false, // should be excluded by lexer
+				compareNodeValues(typedValueOfFloat("1.1"), typedValueOfFloat("1.2")): true,
+				compareNodeValues(typedValueOfFloat("1.1"), typedValueOfFloat("1.1")): true,
+				compareNodeValues(typedValueOfFloat("1.2"), typedValueOfFloat("1.1")): false,
+				compareNodeValues(typedValueOfString("a"), typedValueOfString("a")):   true,  // should be excluded by lexer
+				compareNodeValues(typedValueOfFloat("1.0"), typedValueOfString("a")):  false, // should be excluded by lexer
+				compareNodeValues(typedValueOfString("a"), typedValueOfFloat("1.0")):  false, // should be excluded by lexer
 			},
 		},
 	}
@@ -243,54 +243,3 @@ func TestComparators(t *testing.T) {
 		t.Fatalf("testcase(s) still focussed")
 	}
 }
-
-func TestInvertOrdering(t *testing.T) {
-	cases := []struct {
-		name       string
-		comparison comparison
-		result     comparison
-		focus      bool // if true, run only tests with focus set to true
-	}{
-		{
-			name:       "compareLessThan",
-			comparison: compareLessThan,
-			result:     compareGreaterThan,
-		},
-		{
-			name:       "compareEqual",
-			comparison: compareEqual,
-			result:     compareEqual,
-		},
-		{
-			name:       "compareGreaterThan",
-			comparison: compareGreaterThan,
-			result:     compareLessThan,
-		},
-		{
-			name:       "compareIncomparable",
-			comparison: compareIncomparable,
-			result:     compareIncomparable,
-		},
-	}
-
-	focussed := false
-	for _, tc := range cases {
-		if tc.focus {
-			focussed = true
-			break
-		}
-	}
-
-	for _, tc := range cases {
-		if focussed && !tc.focus {
-			continue
-		}
-		t.Run(tc.name, func(t *testing.T) {
-			require.Equal(t, tc.result, tc.comparison.invertOrdering())
-		})
-	}
-
-	if focussed {
-		t.Fatalf("testcase(s) still focussed")
-	}
-}

pkg/yamlpath/filter.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"regexp"
 	"strconv"
+	"strings"
 
 	"gopkg.in/yaml.v3"
 )
@@ -75,22 +76,48 @@ func never(node, root *yaml.Node) bool {
 }
 
 func comparisonFilter(n *filterNode) filter {
-	return nodeToFilter(n, func(l, r string) bool {
-		return n.lexeme.comparator()(compareNodeValues(l, r))
+	compare := func(b bool) bool {
+		var c comparison
+		if b {
+			c = compareEqual
+		} else {
+			c = compareIncomparable
+		}
+		return n.lexeme.comparator()(c)
+	}
+	return nodeToFilter(n, func(l, r typedValue) bool {
+		if !l.typ.compatibleWith(r.typ) {
+			return compare(false)
+		}
+		switch l.typ {
+		case booleanValueType:
+			return compare(equalBooleans(l.val, r.val))
+
+		case nullValueType:
+			return compare(equalNulls(l.val, r.val))
+
+		default:
+			return n.lexeme.comparator()(compareNodeValues(l, r))
+		}
 	})
 }
 
-func nodeToFilter(n *filterNode, accept func(string, string) bool) filter {
+var x, y typedValue
+
+func init() {
+	x = typedValue{stringValueType, "x"}
+	y = typedValue{stringValueType, "y"}
+}
+
+func nodeToFilter(n *filterNode, accept func(typedValue, typedValue) bool) filter {
 	lhsPath := newFilterScanner(n.children[0])
 	rhsPath := newFilterScanner(n.children[1])
 	return func(node, root *yaml.Node) (result bool) {
+		// perform a set-wise comparison of the values in each path
 		match := false
 		for _, l := range lhsPath(node, root) {
 			for _, r := range rhsPath(node, root) {
-				if !l.typ.compatibleWith(r.typ) {
-					return accept("x", "y") // incompatible values should filter the same as unequal values which are not numerically comparable
-				}
-				if !accept(l.val, r.val) {
+				if !accept(l, r) {
 					return false
 				}
 				match = true
@@ -100,6 +127,16 @@ func nodeToFilter(n *filterNode, accept func(string, string) bool) filter {
 	}
 }
 
+func equalBooleans(l, r string) bool {
+	// Note: the YAML parser and our JSONPath lexer both rule out invalid boolean literals such as tRue.
+	return strings.EqualFold(l, r)
+}
+
+func equalNulls(l, r string) bool {
+	// Note: the YAML parser and our JSONPath lexer both rule out invalid null literals such as nUll.
+	return true
+}
+
 // filterScanner is a function that returns a slice of typed values from either a filter literal or a path expression
 // which refers to either the current node or the root node. It is used in filter comparisons.
 type filterScanner func(node, root *yaml.Node) []typedValue
@@ -210,6 +247,25 @@ func typedValueOfNode(node *yaml.Node) typedValue {
 	}
 }
 
+func newTypedValue(t valueType, v string) typedValue {
+	return typedValue{
+		typ: t,
+		val: v,
+	}
+}
+
+func typedValueOfString(s string) typedValue {
+	return newTypedValue(stringValueType, s)
+}
+
+func typedValueOfInt(i string) typedValue {
+	return newTypedValue(intValueType, i)
+}
+
+func typedValueOfFloat(f string) typedValue {
+	return newTypedValue(floatValueType, f)
+}
+
 func values(nodes []*yaml.Node, err error) []typedValue {
 	if err != nil {
 		panic(fmt.Errorf("unexpected error: %v", err)) // should never happen
@@ -232,7 +288,10 @@ func matchRegularExpression(parseTree *filterNode) filter {
 	return nodeToFilter(parseTree, stringMatchesRegularExpression)
 }
 
-func stringMatchesRegularExpression(s, expr string) bool {
-	re, _ := regexp.Compile(expr) // regex already compiled during lexing
-	return re.Match([]byte(s))
+func stringMatchesRegularExpression(s, expr typedValue) bool {
+	if s.typ != stringValueType || expr.typ != regularExpressionValueType {
+		panic("unexpected types") // should never happen
+	}
+	re, _ := regexp.Compile(expr.val) // regex already compiled during lexing
+	return re.Match([]byte(s.val))
 }

pkg/yamlpath/filter_test.go

@@ -468,6 +468,23 @@ x: "null"
 `,
 			match: false,
 		},
+		{
+			name:   "null comparison filter, path to literal, match on relaxed spelling",
+			filter: `@.x==null`,
+			yamlDoc: `---
+x: Null
+`,
+			match: true,
+		},
+		{
+			name:   "comparison filter, two paths to null, match on relaxed spelling",
+			filter: `@.x==@.y`,
+			yamlDoc: `---
+x: Null
+y: null
+`,
+			match: true,
+		},
 		{
 			name:   "existence || existence filter",
 			filter: "@.a || @.b",

pkg/yamlpath/fuzz/corpus/233d22ce8392691b1f441cf2da9d6261b328837a-6

@@ -0,0 +1 @@
+[?(@[?(@=~/\Q/=~/\Q/=~/\Q/=~/\Q/)]./=~/\Q/)]

pkg/yamlpath/fuzz/corpus/39601abd77c96dc2424c8c09a3f0df18ad4aa129

@@ -0,0 +1 @@
+$[?(==

pkg/yamlpath/fuzz/corpus/434a1e367d1b08b3714318f19b164f900ff554c9

@@ -0,0 +1 @@
+$.child[?(@.child

pkg/yamlpath/fuzz/corpus/5f706facc4deea313a104d433e15324b53863d00

@@ -0,0 +1 @@
+[3,4]

pkg/yamlpath/fuzz/corpus/67f78e8bdfc43ba168dff2cd3d73c90db19c7e54

@@ -0,0 +1 @@
+..1..c

pkg/yamlpath/fuzz/corpus/a0a045a8e195107ab2afe462ebe8ab66b986317b

@@ -0,0 +1 @@
+.store.book[::-1]

pkg/yamlpath/fuzz/corpus/db1b5e885cc72f13fcb2d4b862baf06d42b2b5c3

@@ -0,0 +1 @@
+..k

pkg/yamlpath/path_test.go

@@ -1044,6 +1044,12 @@ another: entry`,
 			path:            `$[?(true)]`,
 			expectedStrings: []string{"0\n"},
 		},
+		{
+			name:            "relaxed spelling of true, false, and null literals", // See https://yaml.org/spec/1.2/spec.html#id2805071
+			input:           `[FALSE, False, false, fAlse, TRUE, True, true, tRue, NULL, Null, null, nUll]`,
+			path:            `$[?(@==false || @==true || @==null)]`,
+			expectedStrings: []string{"FALSE\n", "False\n", "false\n", "TRUE\n", "True\n", "true\n", "NULL\n", "Null\n", "null\n"},
+		},
 	}
 
 	focussed := false