Merge pull request #23 from inteon/improve_linter_diff

Rewrite and fix linter diffing logic

Author: jetstack-bot

go.mod

@@ -19,6 +19,7 @@ require (
 	github.com/go-openapi/swag v0.22.3 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
+	github.com/google/go-cmp v0.5.8 // indirect
 	github.com/google/uuid v1.5.0 // indirect
 	github.com/huandu/xstrings v1.4.0 // indirect
 	github.com/imdario/mergo v0.3.16 // indirect

go.sum

@@ -20,8 +20,9 @@ github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw
 github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
 github.com/google/gnostic-models v0.6.8 h1:yo/ABAfM5IMRsS1VnXjTBvUb61tFIHozhlYvRgGre9I=
 github.com/google/gnostic-models v0.6.8/go.mod h1:5n7qKqH0f5wFt+aWF8CW6pZLLNOfYuF5OpfBSENuI8U=
-github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
+github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g=
 github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
@@ -103,8 +104,6 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn
 golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 h1:H2TDz8ibqkAF6YGhCdN3jS9O0/s90v0rJh3X/OLHEUk=
-golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2/go.mod h1:K8+ghG5WaK9qNqU5K3HdILfMLy1f3aNYFI/wnl100a8=
 google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
 google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
 google.golang.org/protobuf v1.27.1 h1:SnqbnDw1V7RiZcXPx5MEeqPv2s79L9i7BJUlG/+RurQ=

linter/diff.go

@@ -17,72 +17,25 @@ limitations under the License.
 package linter
 
 import (
-	"sort"
-	"strings"
+	"github.com/cert-manager/helm-tool/linter/sets"
 )
 
-func DiffPaths(pathsA []string, pathsB []string) ([]string, []string) {
-	sort.Strings(pathsA)
-	sort.Strings(pathsB)
-
-	missingA := []string{}
-	missingB := []string{}
-
-	prefix := "<NOT A PREFIX>"
-	var i, j int
-	for i < len(pathsA) && j < len(pathsB) {
-		pathA := pathsA[i]
-		pathB := pathsB[j]
-
-		pathAHasPrefix := strings.HasPrefix(pathA, prefix)
-		pathBHasPrefix := strings.HasPrefix(pathB, prefix)
-
-		if pathA == pathB {
-			prefix = pathA
-			i++
-			j++
-			continue
-		}
-
-		if pathBHasPrefix && pathAHasPrefix {
-			prefix = "<NOT A PREFIX>"
-			continue
-		}
-
-		if pathA < pathB {
-			if !pathAHasPrefix {
-				missingB = append(missingB, pathA)
-			}
-			i++
-		} else {
-			if !pathBHasPrefix {
-				missingA = append(missingA, pathB)
-			}
-			j++
-		}
-	}
-
-	for i < len(pathsA) {
-		pathA := pathsA[i]
-
-		pathAHasPrefix := strings.HasPrefix(pathA, prefix)
-		if !pathAHasPrefix {
-			missingB = append(missingB, pathA)
-		}
-
-		i++
-	}
-
-	for j < len(pathsB) {
-		pathB := pathsB[j]
-
-		pathBHasPrefix := strings.HasPrefix(pathB, prefix)
-		if !pathBHasPrefix {
-			missingA = append(missingA, pathB)
-		}
-
-		j++
-	}
+// DiffPaths returns the paths that are missing from each set.
+// We consider a path to be missing if it is not present in the
+// other set, and if it is not a prefix or an extension of another
+// path in the other set. We consider a string a prefix of another
+// string if the other string starts with the first string followed
+// by a period or an opening square bracket.
+func DiffPaths(pathsA sets.Set[string], pathsB sets.Set[string]) (sets.Set[string], sets.Set[string]) {
+	pathsA, pathsB = sets.RemovePrefixes(pathsA), sets.RemovePrefixes(pathsB)
+
+	missingA := sets.Remove(pathsB, pathsA)
+	missingA = sets.RemovePrefixes(missingA, pathsA)
+	missingA = sets.RemoveExtensions(missingA, pathsA)
+
+	missingB := sets.Remove(pathsA, pathsB)
+	missingB = sets.RemovePrefixes(missingB, pathsB)
+	missingB = sets.RemoveExtensions(missingB, pathsB)
 
 	return missingA, missingB
 }

linter/diff_test.go

@@ -19,78 +19,85 @@ package linter
 import (
 	"testing"
 
+	"github.com/cert-manager/helm-tool/linter/sets"
 	"github.com/stretchr/testify/require"
 )
 
 func TestDiffPaths(t *testing.T) {
 	type testcase struct {
-		a            []string
-		b            []string
-		wantMissingA []string
-		wantMissingB []string
+		a            sets.Set[string]
+		b            sets.Set[string]
+		wantMissingA sets.Set[string]
+		wantMissingB sets.Set[string]
 	}
 
 	testcases := []testcase{
 		{
-			a:            []string{".$", ".$.a", ".$.b", ".$.c"},
-			b:            []string{".$", ".$.a", ".$.b", ".$.c"},
-			wantMissingA: []string{},
-			wantMissingB: []string{},
+			a:            sets.New("a", "b", "c"),
+			b:            sets.New("a", "b", "c"),
+			wantMissingA: sets.New[string](),
+			wantMissingB: sets.New[string](),
 		},
 		{
-			a:            []string{".$", ".$.a", ".$.b", ".$.c"},
-			b:            []string{".$", ".$.a", ".$.b"},
-			wantMissingA: []string{},
-			wantMissingB: []string{".$.c"},
+			a:            sets.New("a", "b", "c"),
+			b:            sets.New("a", "b"),
+			wantMissingA: sets.New[string](),
+			wantMissingB: sets.New("c"),
 		},
 		{
-			a:            []string{".$", ".$.a", ".$.b"},
-			b:            []string{".$", ".$.a", ".$.b", ".$.c"},
-			wantMissingA: []string{".$.c"},
-			wantMissingB: []string{},
+			a:            sets.New("a", "b"),
+			b:            sets.New("a", "b", "c"),
+			wantMissingA: sets.New("c"),
+			wantMissingB: sets.New[string](),
 		},
 		{
-			a:            []string{".$", ".$.a", ".$.a.b"},
-			b:            []string{".$", ".$.a"},
-			wantMissingA: []string{},
-			wantMissingB: []string{},
+			a:            sets.New("a.b"),
+			b:            sets.New("a"),
+			wantMissingA: sets.New[string](),
+			wantMissingB: sets.New[string](),
 		},
 		{
-			a:            []string{".$", ".$.a", ".$.a.b"},
-			b:            []string{".$", ".$.a", ".$.a.c"},
-			wantMissingA: []string{".$.a.c"},
-			wantMissingB: []string{".$.a.b"},
+			a:            sets.New("a.b"),
+			b:            sets.New("a.c"),
+			wantMissingA: sets.New("a.c"),
+			wantMissingB: sets.New("a.b"),
 		},
 		{
-			a:            []string{".$", ".$.a", ".$.a.b.d"},
-			b:            []string{".$", ".$.a", ".$.a.c"},
-			wantMissingA: []string{".$.a.c"},
-			wantMissingB: []string{".$.a.b.d"},
+			a:            sets.New("a.b.d"),
+			b:            sets.New("a.c"),
+			wantMissingA: sets.New("a.c"),
+			wantMissingB: sets.New("a.b.d"),
 		},
 		{
-			a:            []string{".$", ".$.a", ".$.a.b.d"},
-			b:            []string{".$", ".$.b", ".$.c"},
-			wantMissingA: []string{".$.b", ".$.c"},
-			wantMissingB: []string{".$.a", ".$.a.b.d"},
+			a:            sets.New("a.b.d"),
+			b:            sets.New("b", "c"),
+			wantMissingA: sets.New("b", "c"),
+			wantMissingB: sets.New("a.b.d"),
 		},
 		{
-			a:            []string{".$", ".$.b", ".$.d.e.f"},
-			b:            []string{".$", ".$.a", ".$.b", ".$.b.c", ".$.d.g"},
-			wantMissingA: []string{".$.a", ".$.d.g"},
-			wantMissingB: []string{".$.d.e.f"},
+			a:            sets.New("b", "d.e.f"),
+			b:            sets.New("a", "b.c", "d.g"),
+			wantMissingA: sets.New("a", "d.g"),
+			wantMissingB: sets.New("d.e.f"),
 		},
 		{
-			a:            []string{".$", ".$.a", ".$.a.b", ".$.a.b.c3"},
-			b:            []string{".$", ".$.a", ".$.a.b", ".$.a.b.c1", ".$.a.b.c2", ".$.a.b.c3", ".$.a.b.c4"},
-			wantMissingA: []string{".$.a.b.c1", ".$.a.b.c2", ".$.a.b.c4"},
-			wantMissingB: []string{},
+			a:            sets.New("a.b.c3"),
+			b:            sets.New("a.b.c1", "a.b.c2", "a.b.c3", "a.b.c4"),
+			wantMissingA: sets.New("a.b.c1", "a.b.c2", "a.b.c4"),
+			wantMissingB: sets.New[string](),
+		},
+		{
+			a:            sets.New("app.logLevel", "app.test", "app"),
+			b:            sets.New("app.logLevel", "app.logLevela", "app.name"),
+			wantMissingA: sets.New("app.logLevela", "app.name"),
+			wantMissingB: sets.New("app.test"),
 		},
 	}
 
 	for _, tc := range testcases {
 		diffA, diffB := DiffPaths(tc.a, tc.b)
 
-		require.EqualValues(t, tc.wantMissingA, diffA)
-		require.EqualValues(t, tc.wantMissingB, diffB)
+		require.ElementsMatch(t, tc.wantMissingA.UnsortedList(), diffA.UnsortedList())
+		require.ElementsMatch(t, tc.wantMissingB.UnsortedList(), diffB.UnsortedList())
 	}
 }

linter/linter.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	"github.com/cert-manager/helm-tool/linter/parsetemplates"
+	"github.com/cert-manager/helm-tool/linter/sets"
 	"github.com/cert-manager/helm-tool/parser"
 )
 
@@ -36,17 +37,13 @@ func Lint(
 		return err
 	}
 
-	valuePathsDict := map[string]struct{}{}
+	valuePaths := sets.Set[string]{}
 	for _, section := range document.Sections {
 		for _, property := range section.Properties {
-			valuePathsDict[property.Path.PatternString()] = struct{}{}
+			valuePaths.Insert(property.Path.PatternString())
 		}
 	}
-	valuePathsDict = parsetemplates.MakeUniform(valuePathsDict)
-	valuePaths := []string{}
-	for path := range valuePathsDict {
-		valuePaths = append(valuePaths, path)
-	}
+	valuePaths = sets.RemovePrefixes(valuePaths)
 
 	exceptionStrings := []string{}
 	if exceptionsPath != "" {
@@ -61,7 +58,7 @@ func Lint(
 	missingValues, missingTemplates := DiffPaths(valuePaths, templatePaths)
 
 	succeeded := true
-	for _, missingValue := range missingValues {
+	for missingValue := range missingValues {
 		exceptionString := fmt.Sprintf("value missing from values.yaml: %s", missingValue)
 
 		if !slices.Contains(exceptionStrings, exceptionString) {
@@ -70,7 +67,7 @@ func Lint(
 		}
 	}
 
-	for _, missingTemplate := range missingTemplates {
+	for missingTemplate := range missingTemplates {
 		exceptionString := fmt.Sprintf("value missing from templates: %s", missingTemplate)
 
 		if !slices.Contains(exceptionStrings, exceptionString) {