diff --git a/language/scala/scala_rule.go b/language/scala/scala_rule.go index 5f52c13..b518b9a 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 bad992c..24c8b53 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)