From 50e7cffc3052035653f70542ff358424db5d5c05 Mon Sep 17 00:00:00 2001 From: Paul Cody Johnston Date: Wed, 13 Dec 2023 20:29:45 -0700 Subject: [PATCH] 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. --- language/scala/scala_rule.go | 17 ++++++++++++-- pkg/provider/java_provider.go | 18 ++++++++++++++- pkg/provider/protobuf_provider.go | 38 ------------------------------- 3 files changed, 32 insertions(+), 41 deletions(-) diff --git a/language/scala/scala_rule.go b/language/scala/scala_rule.go index 4318170..8d097a6 100644 --- 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 index 0b29717..abadebc 100644 --- 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 index 514af1f..840315a 100644 --- 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