From 8e84a6b19102997e6c2f40884b03427af3c6bbff Mon Sep 17 00:00:00 2001 From: Tyler Prete Date: Wed, 11 Feb 2026 15:09:38 -0700 Subject: [PATCH] fix: filter non-importable (binary/test) targets from conflict resolution When both a scala_binary and scala_library exist in the same package with overlapping source globs, the binary could incorrectly win dependency resolution. While isImportable() correctly prevents binary targets from registering exports, the resolve phase's conflict resolution had no filtering for non-importable targets. Add filterNonImportableConflicts() which removes binary/test symbols from consideration during conflict resolution. Library targets always win over binary/test targets for the same symbol. Fixes stackb/scala-gazelle#159 Co-Authored-By: Claude Opus 4.6 --- language/scala/scala_rule.go | 55 +++++++++++++- language/scala/scala_rule_test.go | 115 ++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) diff --git a/language/scala/scala_rule.go b/language/scala/scala_rule.go index 5f52c13a..b518b9ab 100644 --- a/language/scala/scala_rule.go +++ b/language/scala/scala_rule.go @@ -125,7 +125,7 @@ func (r *scalaRule) ResolveImports(rctx *scalarule.ResolveContext) resolver.Impo continue } if symbol, ok := r.ResolveSymbol(rctx.Config, rctx.RuleIndex, rctx.From, scalaLangName, imp.Imp); ok { - imp.Symbol = symbol + imp.Symbol = filterNonImportableConflicts(symbol) if len(imp.Symbol.Conflicts) > 0 { if resolved, ok := sc.ResolveConflict(rctx.Rule, imports, imp, imp.Symbol); ok { imp.Symbol = resolved @@ -151,6 +151,59 @@ func (r *scalaRule) ResolveImports(rctx *scalarule.ResolveContext) resolver.Impo return imports } +// filterNonImportableConflicts removes binary/test symbols from conflict +// resolution. When both a scala_binary and scala_library provide the same +// symbol (e.g. due to overlapping srcs), the library should always win because +// binary targets are not importable. +func filterNonImportableConflicts(sym *resolver.Symbol) *resolver.Symbol { + if len(sym.Conflicts) == 0 { + return sym + } + + // Collect all candidates (primary + conflicts), keeping only importable ones. + importable := make([]*resolver.Symbol, 0, 1+len(sym.Conflicts)) + if !isBinaryRule(sym.Provider) { + importable = append(importable, sym) + } + for _, c := range sym.Conflicts { + if !isBinaryRule(c.Provider) { + importable = append(importable, c) + } + } + + // If nothing was filtered, return the original symbol unchanged. + if len(importable) == 1+len(sym.Conflicts) { + return sym + } + + // If no importable symbols exist, return the original (e.g. binary-only package). + if len(importable) == 0 { + return sym + } + + // Single importable symbol remaining: return a copy without conflicts. + if len(importable) == 1 { + return &resolver.Symbol{ + Type: importable[0].Type, + Name: importable[0].Name, + Label: importable[0].Label, + Provider: importable[0].Provider, + Requires: importable[0].Requires, + } + } + + // Multiple importable symbols: return the first with the rest as conflicts. + // Allocate a new Symbol to avoid mutating the shared trie state. + return &resolver.Symbol{ + Type: importable[0].Type, + Name: importable[0].Name, + Label: importable[0].Label, + Provider: importable[0].Provider, + Conflicts: importable[1:], + Requires: importable[0].Requires, + } +} + // ResolveSymbol implements the resolver.SymbolResolver interface. func (r *scalaRule) ResolveSymbol(c *config.Config, ix *resolve.RuleIndex, from label.Label, lang string, imp string) (*resolver.Symbol, bool) { return r.ctx.resolver.ResolveSymbol(c, ix, from, lang, imp) diff --git a/language/scala/scala_rule_test.go b/language/scala/scala_rule_test.go index bad992c9..24c8b535 100644 --- a/language/scala/scala_rule_test.go +++ b/language/scala/scala_rule_test.go @@ -338,6 +338,121 @@ func (m *mockGlobalScope) GetSymbols(prefix string) []*resolver.Symbol { return m.Global.GetSymbols(prefix) } +func TestFilterNonImportableConflicts(t *testing.T) { + libSymbol := &resolver.Symbol{ + Type: sppb.ImportType_CLASS, + Name: "com.foo.MyClass", + Provider: "scala_library", + Label: label.Label{Pkg: "com/foo", Name: "mylib"}, + } + binSymbol := &resolver.Symbol{ + Type: sppb.ImportType_CLASS, + Name: "com.foo.MyClass", + Provider: "scala_binary", + Label: label.Label{Pkg: "com/foo", Name: "mybin"}, + } + testSymbol := &resolver.Symbol{ + Type: sppb.ImportType_CLASS, + Name: "com.foo.MyClass", + Provider: "scala_test", + Label: label.Label{Pkg: "com/foo", Name: "mytest"}, + } + lib2Symbol := &resolver.Symbol{ + Type: sppb.ImportType_CLASS, + Name: "com.foo.MyClass", + Provider: "scala_library", + Label: label.Label{Pkg: "com/bar", Name: "otherlib"}, + } + + for name, tc := range map[string]struct { + symbol *resolver.Symbol + wantLabel label.Label + wantConflicts int + }{ + "no conflicts - returns unchanged": { + symbol: libSymbol, + wantLabel: libSymbol.Label, + wantConflicts: 0, + }, + "library primary with binary conflict - binary filtered out": { + symbol: &resolver.Symbol{ + Type: libSymbol.Type, + Name: libSymbol.Name, + Provider: libSymbol.Provider, + Label: libSymbol.Label, + Conflicts: []*resolver.Symbol{binSymbol}, + }, + wantLabel: libSymbol.Label, + wantConflicts: 0, + }, + "binary primary with library conflict - library wins": { + symbol: &resolver.Symbol{ + Type: binSymbol.Type, + Name: binSymbol.Name, + Provider: binSymbol.Provider, + Label: binSymbol.Label, + Conflicts: []*resolver.Symbol{libSymbol}, + }, + wantLabel: libSymbol.Label, + wantConflicts: 0, + }, + "test primary with library conflict - library wins": { + symbol: &resolver.Symbol{ + Type: testSymbol.Type, + Name: testSymbol.Name, + Provider: testSymbol.Provider, + Label: testSymbol.Label, + Conflicts: []*resolver.Symbol{libSymbol}, + }, + wantLabel: libSymbol.Label, + wantConflicts: 0, + }, + "binary primary with binary conflict - no importable alternative, returns original": { + symbol: &resolver.Symbol{ + Type: binSymbol.Type, + Name: binSymbol.Name, + Provider: binSymbol.Provider, + Label: binSymbol.Label, + Conflicts: []*resolver.Symbol{testSymbol}, + }, + wantLabel: binSymbol.Label, + wantConflicts: 1, + }, + "two libraries conflict with binary - binary filtered, libraries remain": { + symbol: &resolver.Symbol{ + Type: libSymbol.Type, + Name: libSymbol.Name, + Provider: libSymbol.Provider, + Label: libSymbol.Label, + Conflicts: []*resolver.Symbol{binSymbol, lib2Symbol}, + }, + wantLabel: libSymbol.Label, + wantConflicts: 1, + }, + "binary primary with two library conflicts - first library wins": { + symbol: &resolver.Symbol{ + Type: binSymbol.Type, + Name: binSymbol.Name, + Provider: binSymbol.Provider, + Label: binSymbol.Label, + Conflicts: []*resolver.Symbol{libSymbol, lib2Symbol}, + }, + wantLabel: libSymbol.Label, + wantConflicts: 1, + }, + } { + t.Run(name, func(t *testing.T) { + got := filterNonImportableConflicts(tc.symbol) + if got.Label != tc.wantLabel { + t.Errorf("label: want %v, got %v", tc.wantLabel, got.Label) + } + if len(got.Conflicts) != tc.wantConflicts { + t.Errorf("conflicts: want %d, got %d", tc.wantConflicts, len(got.Conflicts)) + } + }) + } +} + func NewTestScalaConfig(t *testing.T, universe resolver.Universe, rel string, dd ...rule.Directive) (*scalaconfig.Config, error) { c := config.New() sc := scalaconfig.New(zerolog.New(os.Stderr), universe, c, rel)