Ensure range over map is deterministic (#112)

Audited all uses of the "range" keyword and checked if the subject is a map. In those cases, range over the keys instead, sort keys, and iterate that.
stackb · Dec 14, 2023 · 50e7cff · 50e7cff
1 parent 7316bb2
commit 50e7cff
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 41 deletions.
diff --git a/language/scala/scala_rule.go b/language/scala/scala_rule.go
@@ -227,7 +227,9 @@ func (r *scalaRule) fileExports(file *sppb.File, exports resolver.ImportMap) {
 
 	// resolve extends clauses in the file.  While these are probably duplicated
 	// in the 'Names' slice, do it anyway.
-	for token, extends := range file.Extends {
+	tokens := extendsKeysSorted(file.Extends)
+	for _, token := range tokens {
+		extends := file.Extends[token]
 		parts := strings.SplitN(token, " ", 2)
 		if len(parts) != 2 {
 			log.Fatalf("invalid extends token: %q: should have form '(class|interface|object) com.foo.Bar' ", token)
@@ -323,7 +325,9 @@ func (r *scalaRule) fileImports(file *sppb.File, imports resolver.ImportMap) {
 
 	// resolve extends clauses in the file.  While these are probably duplicated
 	// in the 'Names' slice, do it anyway.
-	for token, extends := range file.Extends {
+	tokens := extendsKeysSorted(file.Extends)
+	for _, token := range tokens {
+		extends := file.Extends[token]
 		parts := strings.SplitN(token, " ", 2)
 		if len(parts) != 2 {
 			log.Fatalf("invalid extends token: %q: should have form '(class|interface|object) com.foo.Bar' ", token)
@@ -474,3 +478,12 @@ func (s *importSymbols) Pop() (*importSymbol, bool) {
 
 	return x, true
 }
+
+func extendsKeysSorted(collection map[string]*sppb.ClassList) []string {
+	keys := make([]string, 0, len(collection))
+	for token := range collection {
+		keys = append(keys, token)
+	}
+	sort.Strings(keys)
+	return keys
+}
diff --git a/pkg/provider/java_provider.go b/pkg/provider/java_provider.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log"
 	"path/filepath"
+	"sort"
 
 	"github.com/bazelbuild/bazel-gazelle/config"
 	"github.com/bazelbuild/bazel-gazelle/label"
@@ -66,7 +67,9 @@ func (p *JavaProvider) CheckFlags(fs *flag.FlagSet, c *config.Config, scope reso
 
 // OnResolve implements part of the resolver.SymbolProvider interface.
 func (p *JavaProvider) OnResolve() error {
-	for classFile, symbol := range p.classSymbols {
+	classFiles := sortedSymbolClassfiles(p.classSymbols)
+	for _, classFile := range classFiles {
+		symbol := p.classSymbols[classFile]
 		for _, superclass := range append(classFile.Superclasses, classFile.Interfaces...) {
 			if resolved, ok := p.scope.GetSymbol(superclass); ok {
 				symbol.Require(resolved)
@@ -161,3 +164,16 @@ func (p *JavaProvider) putSymbol(impType sppb.ImportType, imp string, from label
 	p.scope.PutSymbol(symbol)
 	return symbol
 }
+
+func sortedSymbolClassfiles(classSymbols map[*jipb.ClassFile]*resolver.Symbol) []*jipb.ClassFile {
+	classFiles := make([]*jipb.ClassFile, 0, len(classSymbols))
+	for classFile := range classSymbols {
+		classFiles = append(classFiles, classFile)
+	}
+	sort.Slice(classFiles, func(i, j int) bool {
+		a := classFiles[i]
+		b := classFiles[j]
+		return a.Name < b.Name
+	})
+	return classFiles
+}
diff --git a/pkg/provider/protobuf_provider.go b/pkg/provider/protobuf_provider.go
@@ -50,25 +50,9 @@ func (p *ProtobufProvider) RegisterFlags(fs *flag.FlagSet, cmd string, c *config
 func (p *ProtobufProvider) CheckFlags(fs *flag.FlagSet, c *config.Config, scope resolver.Scope) error {
 	p.scope = scope
 
-	// if !filepath.IsAbs(p.indexFile) {
-	// 	p.indexFile = filepath.Join(c.WorkDir, p.indexFile)
-	// }
-	// if err := p.readProtobufIndex(p.indexFile); err != nil {
-	// 	return err
-	// }
-
 	return nil
 }
 
-// func (p *ProtobufProvider) readProtobufIndex(filename string) error {
-// 	var index jipb.JarIndex
-// 	if err := protobuf.ReadFile(filename, &index); err != nil {
-// 		return fmt.Errorf("reading %s: %v", filename, err)
-// 	}
-
-// 	return nil
-// }
-
 // OnResolve implements part of the resolver.SymbolProvider interface.
 func (p *ProtobufProvider) OnResolve() error {
 	// messageRequires := p.resolveScalapbMessageRequires()
@@ -107,28 +91,6 @@ func (p *ProtobufProvider) OnResolve() error {
 	return nil
 }
 
-// func (p *ProtobufProvider) resolveScalapbMessageRequires() (requires []*resolver.Symbol) {
-// 	if sym, ok := p.scope.GetSymbol("scalapb.GeneratedMessage"); ok {
-// 		requires = append(requires, sym)
-// 	} else {
-// 		log.Println("warning: scalapb.GeneratedMessage not found.  Transitive scalapb dependencies may not resolve fully.")
-// 	}
-
-// 	if sym, ok := p.scope.GetSymbol("scalapb.GeneratedMessageCompanion"); ok {
-// 		requires = append(requires, sym)
-// 	} else {
-// 		log.Println("warning: scalapb.GeneratedMessageCompanion not found.  Transitive scalapb dependencies may not resolve fully.")
-// 	}
-
-// 	if sym, ok := p.scope.GetSymbol("scalapb.lenses.Updatable"); ok {
-// 		requires = append(requires, sym)
-// 	} else {
-// 		log.Println("warning: scalapb.lenses.Updatable not found.  Transitive scalapb dependencies may not resolve fully.")
-// 	}
-
-// 	return
-// }
-
 // OnEnd implements part of the resolver.SymbolProvider interface.
 func (p *ProtobufProvider) OnEnd() error {
 	return nil