From 8f91da1f1de4ed6f45447af1890f6eeb893d8d8a Mon Sep 17 00:00:00 2001 From: bufdev Date: Tue, 21 Nov 2023 19:04:43 -0500 Subject: [PATCH 1/3] Add functionality to slicesext --- private/buf/bufwire/module_config_reader.go | 4 +-- .../command/beta/studioagent/studioagent.go | 2 +- private/bufpkg/bufimage/util.go | 4 +-- .../bufmodule/bufmoduleconfig/config.go | 2 +- private/bufpkg/bufmodule/targeting_module.go | 4 +-- private/pkg/bandeps/state.go | 4 +-- private/pkg/git/lister.go | 2 +- .../pkg/normalpath/normalpath_unix_test.go | 8 ++--- .../pkg/normalpath/normalpath_windows_test.go | 8 ++--- private/pkg/slicesext/slicesext.go | 36 ++++++++++++++++--- private/pkg/slicesext/slicesext_test.go | 23 ++++++++++++ private/pkg/stringutil/stringutil.go | 6 ++-- 12 files changed, 77 insertions(+), 26 deletions(-) diff --git a/private/buf/bufwire/module_config_reader.go b/private/buf/bufwire/module_config_reader.go index 7c49bad449..113d814374 100644 --- a/private/buf/bufwire/module_config_reader.go +++ b/private/buf/bufwire/module_config_reader.go @@ -268,8 +268,8 @@ func (m *moduleConfigReader) getProtoFileModuleSourceConfigSet( if err != nil { return nil, err } - workspaceConfigs := slicesext.ToMap(bufwork.AllConfigFilePaths) - moduleConfigs := slicesext.ToMap(bufconfig.AllConfigFilePaths) + workspaceConfigs := slicesext.ToStructMap(bufwork.AllConfigFilePaths) + moduleConfigs := slicesext.ToStructMap(bufconfig.AllConfigFilePaths) terminateFileProvider := readBucketCloser.TerminateFileProvider() var workspaceConfigDirectory string var moduleConfigDirectory string diff --git a/private/buf/cmd/buf/command/beta/studioagent/studioagent.go b/private/buf/cmd/buf/command/beta/studioagent/studioagent.go index 98b1e8409c..3fec928b4b 100644 --- a/private/buf/cmd/buf/command/beta/studioagent/studioagent.go +++ b/private/buf/cmd/buf/command/beta/studioagent/studioagent.go @@ -186,7 +186,7 @@ func run( container.Logger(), flags.Origin, clientTLSConfig, - slicesext.ToMap(flags.DisallowedHeaders), + slicesext.ToStructMap(flags.DisallowedHeaders), flags.ForwardHeaders, flags.PrivateNetwork, ) diff --git a/private/bufpkg/bufimage/util.go b/private/bufpkg/bufimage/util.go index 238cfba990..009d8f4348 100644 --- a/private/bufpkg/bufimage/util.go +++ b/private/bufpkg/bufimage/util.go @@ -44,7 +44,7 @@ func imageWithOnlyPaths(image Image, fileOrDirPaths []string, excludeFileOrDirPa if err := normalpath.ValidatePathsNormalizedValidatedUnique(excludeFileOrDirPaths); err != nil { return nil, err } - excludeFileOrDirPathMap := slicesext.ToMap(excludeFileOrDirPaths) + excludeFileOrDirPathMap := slicesext.ToStructMap(excludeFileOrDirPaths) // These are the files that fileOrDirPaths actually reference and will // result in the non-imports in our resulting Image. The Image will also include // the ImageFiles that the nonImportImageFiles import @@ -132,7 +132,7 @@ func imageWithOnlyPaths(image Image, fileOrDirPaths []string, excludeFileOrDirPa // make a map of the directory paths // note that we do not make this a map to begin with as maps are unordered, // and we want to make sure we iterate over the paths in a deterministic order - potentialDirPathMap := slicesext.ToMap(potentialDirPaths) + potentialDirPathMap := slicesext.ToStructMap(potentialDirPaths) // map of all paths based on the imageFiles // the map of paths within potentialDirPath that matches a file in image.Files() diff --git a/private/bufpkg/bufmodule/bufmoduleconfig/config.go b/private/bufpkg/bufmodule/bufmoduleconfig/config.go index 0a0a7025b6..f0f7de15f5 100644 --- a/private/bufpkg/bufmodule/bufmoduleconfig/config.go +++ b/private/bufpkg/bufmodule/bufmoduleconfig/config.go @@ -76,7 +76,7 @@ func newConfigV1Beta1(externalConfig ExternalConfigV1Beta1, deps ...string) (*Co } // verify that all excludes are within a root - rootMap := slicesext.ToMap(roots) + rootMap := slicesext.ToStructMap(roots) for _, fullExclude := range fullExcludes { switch matchingRoots := normalpath.MapAllEqualOrContainingPaths(rootMap, fullExclude, normalpath.Relative); len(matchingRoots) { case 0: diff --git a/private/bufpkg/bufmodule/targeting_module.go b/private/bufpkg/bufmodule/targeting_module.go index 706c8d31b1..156c0ffd45 100644 --- a/private/bufpkg/bufmodule/targeting_module.go +++ b/private/bufpkg/bufmodule/targeting_module.go @@ -56,7 +56,7 @@ func (m *targetingModule) TargetFileInfos(ctx context.Context) (fileInfos []bufm bufmoduleref.SortFileInfos(fileInfos) } }() - excludePathMap := slicesext.ToMap(m.excludePaths) + excludePathMap := slicesext.ToStructMap(m.excludePaths) // We start by ensuring that no paths have been duplicated between target and exclude pathes. for _, targetPath := range m.targetPaths { if _, ok := excludePathMap[targetPath]; ok { @@ -150,7 +150,7 @@ func (m *targetingModule) TargetFileInfos(ctx context.Context) (fileInfos []bufm } // We have potential directory paths, do the expensive operation to // make a map of the directory paths. - potentialDirPathMap := slicesext.ToMap(potentialDirPaths) + potentialDirPathMap := slicesext.ToStructMap(potentialDirPaths) // The map of paths within potentialDirPath that matches a file. // This needs to contain all paths in potentialDirPathMap at the end for us to // have had matches for every targetPath input. diff --git a/private/pkg/bandeps/state.go b/private/pkg/bandeps/state.go index 32b08e56c2..a4775b905f 100644 --- a/private/pkg/bandeps/state.go +++ b/private/pkg/bandeps/state.go @@ -172,7 +172,7 @@ func (s *state) packagesForPackageExpressionUncached( span.SetStatus(codes.Error, err.Error()) return nil, err } - return slicesext.ToMap(getNonEmptyLines(string(data))), nil + return slicesext.ToStructMap(getNonEmptyLines(string(data))), nil } func (s *state) depsForPackage( @@ -234,7 +234,7 @@ func (s *state) depsForPackageUncached( span.SetStatus(codes.Error, err.Error()) return nil, err } - return slicesext.ToMap(getNonEmptyLines(string(data))), nil + return slicesext.ToStructMap(getNonEmptyLines(string(data))), nil } type packagesResult struct { diff --git a/private/pkg/git/lister.go b/private/pkg/git/lister.go index f09742a7fe..7ef3b9b833 100644 --- a/private/pkg/git/lister.go +++ b/private/pkg/git/lister.go @@ -82,7 +82,7 @@ func (l *lister) ListFilesAndUnstagedFiles( // stringSliceExcept returns all elements in source that are not in except. func stringSliceExcept(source []string, except []string) []string { - exceptMap := slicesext.ToMap(except) + exceptMap := slicesext.ToStructMap(except) result := make([]string, 0, len(source)) for _, s := range source { if _, ok := exceptMap[s]; !ok { diff --git a/private/pkg/normalpath/normalpath_unix_test.go b/private/pkg/normalpath/normalpath_unix_test.go index 56d55ed54b..dd11b897a7 100644 --- a/private/pkg/normalpath/normalpath_unix_test.go +++ b/private/pkg/normalpath/normalpath_unix_test.go @@ -342,7 +342,7 @@ func TestMapHasEqualOrContainingPath(t *testing.T) { } func testMapHasEqualOrContainingPath(t *testing.T, expected bool, path string, keys ...string) { - keyMap := slicesext.ToMap(keys) + keyMap := slicesext.ToStructMap(keys) assert.Equal(t, expected, MapHasEqualOrContainingPath(keyMap, path, Relative), fmt.Sprintf("%s %v", path, keys)) } @@ -367,7 +367,7 @@ func testMapAllEqualOrContainingPaths(t *testing.T, expected []string, path stri expected = make([]string, 0) } sort.Strings(expected) - keyMap := slicesext.ToMap(keys) + keyMap := slicesext.ToStructMap(keys) assert.Equal(t, expected, MapAllEqualOrContainingPaths(keyMap, path, Relative), fmt.Sprintf("%s %v", path, keys)) } @@ -424,7 +424,7 @@ func TestMapHasEqualOrContainingPathAbs(t *testing.T) { } func testMapHasEqualOrContainingPathAbs(t *testing.T, expected bool, path string, keys ...string) { - keyMap := slicesext.ToMap(keys) + keyMap := slicesext.ToStructMap(keys) assert.Equal(t, expected, MapHasEqualOrContainingPath(keyMap, path, Absolute), fmt.Sprintf("%s %v", path, keys)) } @@ -449,6 +449,6 @@ func testMapAllEqualOrContainingPathsAbs(t *testing.T, expected []string, path s expected = make([]string, 0) } sort.Strings(expected) - keyMap := slicesext.ToMap(keys) + keyMap := slicesext.ToStructMap(keys) assert.Equal(t, expected, MapAllEqualOrContainingPaths(keyMap, path, Absolute), fmt.Sprintf("%s %v", path, keys)) } diff --git a/private/pkg/normalpath/normalpath_windows_test.go b/private/pkg/normalpath/normalpath_windows_test.go index 8d19f0eb01..d85f7f7a8f 100644 --- a/private/pkg/normalpath/normalpath_windows_test.go +++ b/private/pkg/normalpath/normalpath_windows_test.go @@ -362,7 +362,7 @@ func TestMapHasEqualOrContainingPath(t *testing.T) { } func testMapHasEqualOrContainingPath(t *testing.T, expected bool, path string, keys ...string) { - keyMap := slicesext.ToMap(keys) + keyMap := slicesext.ToStructMap(keys) assert.Equal(t, expected, MapHasEqualOrContainingPath(keyMap, path, Relative), fmt.Sprintf("%s %v", path, keys)) } @@ -386,7 +386,7 @@ func testMapAllEqualOrContainingPaths(t *testing.T, expected []string, path stri expected = make([]string, 0) } sort.Strings(expected) - keyMap := slicesext.ToMap(keys) + keyMap := slicesext.ToStructMap(keys) assert.Equal(t, expected, MapAllEqualOrContainingPaths(keyMap, path, Relative), fmt.Sprintf("%s %v", path, keys)) } @@ -497,7 +497,7 @@ func TestMapHasEqualOrContainingPathAbs(t *testing.T) { } func testMapHasEqualOrContainingPathAbs(t *testing.T, expected bool, path string, keys ...string) { - keyMap := slicesext.ToMap(keys) + keyMap := slicesext.ToStructMap(keys) assert.Equal(t, expected, MapHasEqualOrContainingPath(keyMap, path, Absolute), fmt.Sprintf("%s %v", path, keys)) } @@ -527,6 +527,6 @@ func testMapAllEqualOrContainingPathsAbs(t *testing.T, expected []string, path s expected = make([]string, 0) } sort.Strings(expected) - keyMap := slicesext.ToMap(keys) + keyMap := slicesext.ToStructMap(keys) assert.Equal(t, expected, MapAllEqualOrContainingPaths(keyMap, path, Absolute), fmt.Sprintf("%s %v", path, keys)) } diff --git a/private/pkg/slicesext/slicesext.go b/private/pkg/slicesext/slicesext.go index fb7ccce3ba..d98817f25d 100644 --- a/private/pkg/slicesext/slicesext.go +++ b/private/pkg/slicesext/slicesext.go @@ -138,8 +138,8 @@ func Copy[T any](s []T) []T { return sc } -// ToMap converts the slice to a map. -func ToMap[T comparable](s []T) map[T]struct{} { +// ToStructMap converts the slice to a map with struct{} values. +func ToStructMap[T comparable](s []T) map[T]struct{} { m := make(map[T]struct{}, len(s)) for _, e := range s { m[e] = struct{}{} @@ -147,6 +147,17 @@ func ToMap[T comparable](s []T) map[T]struct{} { return m } +// ToValuesMap transforms the input slice into a map from f(V) -> V. +// +// Duplicate values of type K will result in a single map entry. +func ToValuesMapV[K comparable, V any](s []V, f func(V) K) map[K]V { + m := make(map[K]V) + for _, e := range s { + m[f(e)] = e + } + return m +} + // MapKeysToSortedSlice converts the map's keys to a sorted slice. func MapKeysToSortedSlice[M ~map[K]V, K Ordered, V any](m M) []K { s := MapKeysToSlice(m) @@ -171,7 +182,24 @@ func MapKeysToSlice[K comparable, V any](m map[K]V) []K { // ToUniqueSorted returns a sorted copy of s with no duplicates. func ToUniqueSorted[S ~[]T, T Ordered](s S) S { - return MapKeysToSortedSlice(ToMap(s)) + return MapKeysToSortedSlice(ToStructMap(s)) +} + +// Duplicates returns the duplicate values in s. +// +// Values are returned in the order they are found in S. +func Duplicates[T comparable](s []T) []T { + count := make(map[T]int, len(s)) + // Needed instead of var declaration to make tests pass. + duplicates := make([]T, 0) + for _, e := range s { + count[e] = count[e] + 1 + if count[e] == 2 { + // Only insert the first time this is found. + duplicates = append(duplicates, e) + } + } + return duplicates } // ToChunks splits s into chunks of the given chunk size. @@ -214,7 +242,7 @@ func ElementsEqual[T comparable](one []T, two []T) bool { // // Nil and empty slices are treated as equals. func ElementsContained(superset []string, subset []string) bool { - m := ToMap(superset) + m := ToStructMap(superset) for _, elem := range subset { if _, ok := m[elem]; !ok { return false diff --git a/private/pkg/slicesext/slicesext_test.go b/private/pkg/slicesext/slicesext_test.go index f916f2b33b..ee0f333c1f 100644 --- a/private/pkg/slicesext/slicesext_test.go +++ b/private/pkg/slicesext/slicesext_test.go @@ -49,6 +49,29 @@ func TestElementsContained(t *testing.T) { assert.False(t, ElementsContained([]string{"two"}, []string{"one", "two"})) } +func TestDuplicats(t *testing.T) { + assert.Equal( + t, + []string{}, + Duplicates([]string{"a", "b", "c", "d", "e"}), + ) + assert.Equal( + t, + []string{"a"}, + Duplicates([]string{"a", "b", "c", "a", "e"}), + ) + assert.Equal( + t, + []string{"a"}, + Duplicates([]string{"a", "a", "c", "a", "e"}), + ) + assert.Equal( + t, + []string{"b", "a"}, + Duplicates([]string{"a", "b", "b", "a", "e"}), + ) +} + func TestToChunks(t *testing.T) { t.Parallel() testToChunks( diff --git a/private/pkg/stringutil/stringutil.go b/private/pkg/stringutil/stringutil.go index 5502de60cd..8f7c4036c6 100644 --- a/private/pkg/stringutil/stringutil.go +++ b/private/pkg/stringutil/stringutil.go @@ -72,9 +72,9 @@ func MapToSlice(m map[string]struct{}) []string { // SliceToMap transforms s to a map. // -// Deprecated: Use slicesext.ToMap instead. +// Deprecated: Use slicesext.ToStructMap instead. func SliceToMap(s []string) map[string]struct{} { - return slicesext.ToMap(s) + return slicesext.ToStructMap(s) } // SliceToUniqueSortedSlice returns a sorted copy of s with no duplicates. @@ -88,7 +88,7 @@ func SliceToUniqueSortedSlice(s []string) []string { // // Strings with only spaces are considered empty. func SliceToUniqueSortedSliceFilterEmptyStrings(s []string) []string { - m := slicesext.ToMap(s) + m := slicesext.ToStructMap(s) for key := range m { if strings.TrimSpace(key) == "" { delete(m, key) From d56da718f0187e375e58db70b68837a1652f1bfa Mon Sep 17 00:00:00 2001 From: bufdev Date: Tue, 21 Nov 2023 19:07:55 -0500 Subject: [PATCH 2/3] commit --- private/pkg/slicesext/slicesext_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/private/pkg/slicesext/slicesext_test.go b/private/pkg/slicesext/slicesext_test.go index ee0f333c1f..0a4c426d24 100644 --- a/private/pkg/slicesext/slicesext_test.go +++ b/private/pkg/slicesext/slicesext_test.go @@ -49,7 +49,8 @@ func TestElementsContained(t *testing.T) { assert.False(t, ElementsContained([]string{"two"}, []string{"one", "two"})) } -func TestDuplicats(t *testing.T) { +func TestDuplicates(t *testing.T) { + t.Parallel() assert.Equal( t, []string{}, From c99ed833c164c45a3470f5102248edaccb19dd38 Mon Sep 17 00:00:00 2001 From: bufdev Date: Tue, 21 Nov 2023 19:09:11 -0500 Subject: [PATCH 3/3] commit --- private/pkg/slicesext/slicesext_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/private/pkg/slicesext/slicesext_test.go b/private/pkg/slicesext/slicesext_test.go index 0a4c426d24..cc5dac528a 100644 --- a/private/pkg/slicesext/slicesext_test.go +++ b/private/pkg/slicesext/slicesext_test.go @@ -71,6 +71,11 @@ func TestDuplicates(t *testing.T) { []string{"b", "a"}, Duplicates([]string{"a", "b", "b", "a", "e"}), ) + assert.Equal( + t, + []string{"b", "a"}, + Duplicates([]string{"a", "b", "b", "a", "b"}), + ) } func TestToChunks(t *testing.T) {