From e27d6da6b2ce3d7a3d40430241265d0d2404ffac Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sun, 27 Sep 2026 13:32:17 +0000 Subject: [PATCH] fix(scala): resolve companion inheritance to type definitions (#1824) (#2029) * fix(scala): resolve extends/with to the trait, not its companion object The Scala trait + companion object idiom puts two same-named symbols in one file. Both were indexed as kind 'class', so an 'extends X' reference tied between the trait and its companion and the winner was arbitrary. When the companion won, every subtype was detached from the trait's inheritance chain, and impact analysis on a widely-used trait stopped at depth 1. - classify Scala object_definition as 'module' (a singleton value, not a type - 'extends' can never target it) - add 'module' to the classifyClassNode contract and handle it in the extractor dispatch - in name matching, bias extends/implements references toward actual type definitions and penalize 'module' candidates, so the trait wins the tie deterministically Regression test: trait + companion in one file, subclass in another - the extends edge must land on the trait and impact must reach the subtype through it. * fix(scala): resolve companion inheritance to type definitions (#1824) Scala objects and their companion types could tie during inheritance resolution, disconnecting subtype impact paths. Classify objects as modules in both extraction engines and reject Scala singleton inheritance targets across resolution strategies. Preserve object method lookup and inherited receiver methods, with declaration-order, impact, and native/WASM parity regressions. Co-authored-by: Hubert Tarnacki Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Hubert Tarnacki Co-authored-by: Claude Opus 5.5 --- __tests__/extraction.test.ts | 30 +++-- .../kernel-parity/ScalaCompanions.scala | 23 ++++ __tests__/kernel-scala-parity.test.ts | 19 ++++ __tests__/resolution.test.ts | 104 ++++++++++++++++++ codegraph-kernel/src/scala.rs | 12 +- src/extraction/languages/scala.ts | 10 +- src/extraction/tree-sitter-types.ts | 2 +- src/extraction/tree-sitter.ts | 3 + src/resolution/index.ts | 8 +- src/resolution/name-matcher.ts | 8 +- src/resolution/types.ts | 6 + 11 files changed, 203 insertions(+), 22 deletions(-) create mode 100644 __tests__/fixtures/kernel-parity/ScalaCompanions.scala diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 57c6a11e7..29d48477f 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -4462,7 +4462,7 @@ class APXCharacter { // the one real definition const scala = extractFromSource('M.scala', 'trait Marker\ncase object Red\nclass Foo\n'); const scalaNames = scala.nodes - .filter((n) => ['class', 'trait', 'interface'].includes(n.kind)) + .filter((n) => ['class', 'trait', 'interface', 'module'].includes(n.kind)) .map((n) => n.name); expect(scalaNames).toEqual(expect.arrayContaining(['Marker', 'Red', 'Foo'])); }); @@ -8681,15 +8681,27 @@ class UserService(private val repo: UserRepository) { expect(cls?.language).toBe('scala'); }); - it('should extract object definitions as class kind', () => { - const code = ` -object DatabaseConfig { - val url = "jdbc:postgresql://localhost/mydb" -} -`; + it.each(['\n', '\r\n'])('extracts objects as modules with method ownership (%j)', (eol) => { + const code = [ + 'object DatabaseConfig {', + ' val url = "jdbc:postgresql://localhost/mydb"', + ' def connect(): String = url', + '}', + 'def scope(): Int = {', + ' object Local { def value(): Int = 1 }', + ' Local.value()', + '}', + 'case object Empty', + ].join(eol); const result = extractFromSource('Config.scala', code); - const obj = result.nodes.find((n) => n.kind === 'class' && n.name === 'DatabaseConfig'); - expect(obj).toBeDefined(); + for (const [name, method] of [['DatabaseConfig', 'connect'], ['Local', 'value']]) { + const obj = result.nodes.find((n) => n.kind === 'module' && n.name === name)!; + const member = result.nodes.find((n) => n.kind === 'method' && n.name === method)!; + expect(obj).toBeDefined(); + expect(member).toBeDefined(); + expect(result.edges.some((e) => e.kind === 'contains' && e.source === obj.id && e.target === member.id)).toBe(true); + } + expect(result.nodes.some((n) => n.kind === 'module' && n.name === 'Empty')).toBe(true); }); it('should extract trait definitions as trait kind', () => { diff --git a/__tests__/fixtures/kernel-parity/ScalaCompanions.scala b/__tests__/fixtures/kernel-parity/ScalaCompanions.scala new file mode 100644 index 000000000..099abcc5c --- /dev/null +++ b/__tests__/fixtures/kernel-parity/ScalaCompanions.scala @@ -0,0 +1,23 @@ +object ObjectFirst { + def create(): ObjectFirst = new ObjectFirst +} +class ObjectFirst { + def instance(): Int = 1 +} +trait TraitFirst { + def value(): Int = 1 +} +object TraitFirst { + def factory(): Int = 2 +} +object Live extends TraitFirst { + def run(): Int = value() +} +case object Empty + +def scope(): Int = { + object Local { + def localMethod(): Int = 3 + } + Local.localMethod() +} diff --git a/__tests__/kernel-scala-parity.test.ts b/__tests__/kernel-scala-parity.test.ts index d596a4a96..1dd9633e0 100644 --- a/__tests__/kernel-scala-parity.test.ts +++ b/__tests__/kernel-scala-parity.test.ts @@ -104,6 +104,7 @@ describe.skipIf(!kernelBuilt)('kernel Scala extraction parity', () => { const FIXTURES = [ ['torture.scala', 40], + ['ScalaCompanions.scala', 14], ['TortureDocs.scala', 3], ['TortureVref.scala', 4], ['TortureFnref.scala', 4], @@ -127,6 +128,24 @@ describe.skipIf(!kernelBuilt)('kernel Scala extraction parity', () => { }); } + it.each(['LF', 'CRLF'])('companion kinds and method ownership (%s)', (eol) => { + let source = fs.readFileSync(path.join(FIXTURE_DIR, 'ScalaCompanions.scala'), 'utf8'); + if (eol === 'CRLF') source = source.replace(/\n/g, '\r\n'); + const result = assertParity('ScalaCompanions.scala', source, 14); + for (const [owner, method] of [ + ['ObjectFirst', 'create'], ['TraitFirst', 'factory'], ['Live', 'run'], ['Local', 'localMethod'], + ]) { + const obj = result.nodes.find((n) => n.kind === 'module' && n.name === owner)!; + const member = result.nodes.find((n) => n.kind === 'method' && n.name === method)!; + expect(obj).toBeDefined(); + expect(member).toBeDefined(); + expect(result.edges.some((e) => e.kind === 'contains' && e.source === obj.id && e.target === member.id)).toBe(true); + } + expect(result.nodes.some((n) => n.kind === 'class' && n.name === 'ObjectFirst')).toBe(true); + expect(result.nodes.some((n) => n.kind === 'trait' && n.name === 'TraitFirst')).toBe(true); + expect(result.nodes.some((n) => n.kind === 'module' && n.name === 'Empty')).toBe(true); + }); + it('torture pins: import first-segment names, companion pairs, value-ref edges', () => { const src = fs.readFileSync(path.join(FIXTURE_DIR, 'torture.scala'), 'utf8'); const result = assertParity('fixtures/torture.scala', src, 40); diff --git a/__tests__/resolution.test.ts b/__tests__/resolution.test.ts index 96d9fc932..ba055293f 100644 --- a/__tests__/resolution.test.ts +++ b/__tests__/resolution.test.ts @@ -5049,6 +5049,110 @@ object Main { }); }); + describe('Scala companion object vs extends resolution', () => { + for (const parentKind of ['trait', 'class'] as const) { + for (const objectFirst of [true, false]) { + it.each([false, true])( + `resolves ${parentKind} companions (objectFirst=${objectFirst}, imported=%s) through every impact depth`, + async (imported) => { + const typeDef = `${parentKind} ExtAgreement { + def extId: String = "x" +} +`; + const objectDef = `object ExtAgreement { + val Kind = "agreement" +} +`; + fs.writeFileSync(path.join(tempDir, 'ExtAgreement.scala'), + 'package contracts\n' + (objectFirst ? objectDef + typeDef : typeDef + objectDef)); + fs.writeFileSync(path.join(tempDir, 'Audit.scala'), + 'package contracts\nobject Audit {}\ntrait Audit { def audit(): String = "ok" }\n'); + fs.writeFileSync(path.join(tempDir, 'MExtAgreement.scala'), + (imported ? 'package model\nimport contracts.ExtAgreement\nimport contracts.Audit\n' : 'package contracts\n') + + 'class MExtAgreement extends ExtAgreement with Audit {\n def render(): String = extId\n}\n'); + fs.writeFileSync(path.join(tempDir, 'LeafAgreement.scala'), + (imported ? 'package model\n' : 'package contracts\n') + + 'class LeafAgreement extends MExtAgreement {\n def leaf(): String = render()\n}\n'); + cg = await CodeGraph.init(tempDir, { index: true }); + + const parent = cg.getNodesByKind(parentKind).find((n) => n.name === 'ExtAgreement'); + const companion = cg.getNodesByKind('module').find((n) => n.name === 'ExtAgreement'); + expect(parent).toBeDefined(); + expect(companion).toBeDefined(); + expect(cg.getIncomingEdges(parent!.id).filter((e) => e.kind === 'extends')).toHaveLength(1); + for (const name of ['ExtAgreement', 'Audit']) { + const obj = cg.getNodesByKind('module').find((n) => n.name === name)!; + expect(cg.getIncomingEdges(obj.id).filter((e) => e.kind === 'extends')).toEqual([]); + } + const audit = cg.getNodesByKind('trait').find((n) => n.name === 'Audit')!; + expect(cg.getIncomingEdges(audit.id).filter((e) => e.kind === 'extends')).toHaveLength(1); + + // Impact must traverse THROUGH the type to descendants and their methods. + const impactNames = [...cg.getImpactRadius(parent!.id, 5).nodes.values()].map((n) => n.name); + expect(impactNames).toEqual(expect.arrayContaining(['MExtAgreement', 'render', 'LeafAgreement', 'leaf'])); + } + ); + } + } + + it.each([false, true])('rejects a sole singleton parent (imported=%s)', async (imported) => { + fs.writeFileSync(path.join(tempDir, 'OnlyObject.scala'), + 'package contracts\nobject OnlyObject { def value(): Int = 1 }\n'); + fs.writeFileSync(path.join(tempDir, 'Invalid.scala'), + (imported ? 'package model\nimport contracts.OnlyObject\n' : 'package contracts\n') + + 'class Invalid extends OnlyObject {}\ntrait AlsoInvalid extends OnlyObject {}\n'); + cg = await CodeGraph.init(tempDir, { index: true }); + const obj = cg.getNodesByKind('module').find((n) => n.name === 'OnlyObject')!; + expect(obj).toBeDefined(); + expect(cg.getIncomingEdges(obj.id).filter((e) => e.kind === 'extends' || e.kind === 'implements')).toEqual([]); + }); + + it('keeps singleton objects as inheritance sources and owners of methods', async () => { + fs.writeFileSync(path.join(tempDir, 'Service.scala'), + 'trait Service { def inherited(): Int = 1 }\nobject LiveService extends Service { def run(): Int = this.inherited() }\n'); + cg = await CodeGraph.init(tempDir, { index: true }); + const service = cg.getNodesByKind('trait').find((n) => n.name === 'Service')!; + const obj = cg.getNodesByKind('module').find((n) => n.name === 'LiveService')!; + expect(cg.getIncomingEdges(service.id).some((e) => e.kind === 'extends' && e.source === obj.id)).toBe(true); + const run = cg.getNodesByKind('method').find((n) => n.qualifiedName === 'LiveService::run')!; + expect(cg.getOutgoingEdges(obj.id).some((e) => e.kind === 'contains' && e.target === run.id)).toBe(true); + const inherited = cg.getNodesByKind('method').find((n) => n.qualifiedName === 'Service::inherited')!; + expect(cg.getIncomingEdges(inherited.id).some((e) => e.kind === 'calls' && e.source === run.id)).toBe(true); + }); + + it('resolves inherited methods through a singleton receiver', async () => { + fs.writeFileSync(path.join(tempDir, 'Service.scala'), + 'trait Service { def inherited(): Int = 1 }\n' + + 'object LiveService extends Service {}\n' + + 'object Unrelated { def inherited(): Int = 2 }\n' + + 'object Client { def use(): Int = { val receiver = LiveService; receiver.inherited() } }\n'); + cg = await CodeGraph.init(tempDir, { index: true }); + const inherited = cg.getNodesByKind('method').find((n) => n.qualifiedName === 'Service::inherited')!; + const use = cg.getNodesByKind('method').find((n) => n.qualifiedName === 'Client::use')!; + expect(cg.getIncomingEdges(inherited.id).some((e) => e.kind === 'calls' && e.source === use.id)).toBe(true); + }); + + it('keeps object method calls anchored to their receiver', async () => { + fs.writeFileSync(path.join(tempDir, 'Api.scala'), + 'class API { def send(): Int = 2 }\n' + + 'object Api { def send(): Int = 1 }\n' + + 'object Client { def run(): Int = Api.send() }\n'); + cg = await CodeGraph.init(tempDir, { index: true }); + const send = cg.getNodesByKind('method').find((n) => n.qualifiedName === 'Api::send')!; + const run = cg.getNodesByKind('method').find((n) => n.qualifiedName === 'Client::run')!; + expect(cg.getIncomingEdges(send.id).some((e) => e.kind === 'calls' && e.source === run.id)).toBe(true); + }); + + it('preserves Ruby module inclusion', async () => { + fs.writeFileSync(path.join(tempDir, 'trackable.rb'), + 'module Trackable\n def track; end\nend\nclass Record\n include Trackable\nend\n'); + cg = await CodeGraph.init(tempDir, { index: true }); + const mod = cg.getNodesByKind('module').find((n) => n.name === 'Trackable')!; + expect(cg.getIncomingEdges(mod.id).some((e) => + (e.kind === 'extends' || e.kind === 'implements') && cg.getNode(e.source)?.name === 'Record')).toBe(true); + }); + }); + describe('Dart chained static-factory / factory-constructor call resolution (#645/#608 mechanism)', () => { function callerNamesOf(qualifiedName: string): string[] { const target = cg.getNodesByKind('method').find((n) => n.qualifiedName === qualifiedName); diff --git a/codegraph-kernel/src/scala.rs b/codegraph-kernel/src/scala.rs index a5ef94978..27968f0eb 100644 --- a/codegraph-kernel/src/scala.rs +++ b/codegraph-kernel/src/scala.rs @@ -548,10 +548,14 @@ impl<'t> Walker<'t> { self.extract_method_or_function(node); return; // skipChildren } - "class_definition" | "object_definition" => { + "class_definition" => { self.extract_class(node, "class"); return; } + "object_definition" => { + self.extract_class(node, "module"); + return; + } "trait_definition" => { self.extract_class(node, "trait"); return; @@ -1193,10 +1197,14 @@ impl<'t> Walker<'t> { // the inverse of kotlin). Body-local classes/objects/traits/enums DO // extract fully. match kind { - "class_definition" | "object_definition" => { + "class_definition" => { self.extract_class(node, "class"); return; } + "object_definition" => { + self.extract_class(node, "module"); + return; + } "trait_definition" => { self.extract_class(node, "trait"); return; diff --git a/src/extraction/languages/scala.ts b/src/extraction/languages/scala.ts index b0d995faa..bd0d327b3 100644 --- a/src/extraction/languages/scala.ts +++ b/src/extraction/languages/scala.ts @@ -104,6 +104,11 @@ export const scalaExtractor: LanguageExtractor = { classifyClassNode: (node: SyntaxNode) => { if (node.type === 'trait_definition') return 'trait'; + // A Scala `object` is a singleton (the companion-object idiom), not a + // type: `extends X` can never target it. Classifying it as `module` + // keeps it distinguishable from the same-named trait/class so the + // resolver can prefer the type node for extends/implements refs. + if (node.type === 'object_definition') return 'module'; return 'class'; }, @@ -140,9 +145,8 @@ export const scalaExtractor: LanguageExtractor = { // idiom for `static final` — `object Config { val Timeout = 30 }`), so // emit them as `constant`/`variable` like a top-level val, which lets // value-reference edges target them. A `class`/`trait`/`enum`/`given` val - // is a per-instance immutable field. Both an `object` and a `class` - // extract as `class` kind, so the AST node type of the enclosing - // definition — not the parent node's kind — is what distinguishes them. + // is a per-instance immutable field. Use the AST node type of the enclosing + // definition to distinguish them, including given/enum scopes. let enclosingDef: string | null = null; for (let p = node.parent; p; p = p.parent) { if ( diff --git a/src/extraction/tree-sitter-types.ts b/src/extraction/tree-sitter-types.ts index e344d545a..6477b0273 100644 --- a/src/extraction/tree-sitter-types.ts +++ b/src/extraction/tree-sitter-types.ts @@ -236,7 +236,7 @@ export interface LanguageExtractor { * Classify a class_declaration node when the grammar reuses one node type * for multiple concepts (e.g. Swift uses class_declaration for classes, structs, and enums). */ - classifyClassNode?: (node: SyntaxNode) => 'class' | 'struct' | 'enum' | 'interface' | 'trait'; + classifyClassNode?: (node: SyntaxNode) => 'class' | 'struct' | 'enum' | 'interface' | 'trait' | 'module'; /** * Classify a methodTypes node when the grammar reuses one node type for diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 1b3f7edf0..f3c654350 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -1095,6 +1095,8 @@ export class TreeSitterExtractor { this.extractInterface(node); } else if (classification === 'trait') { this.extractClass(node, 'trait'); + } else if (classification === 'module') { + this.extractClass(node, 'module'); } else { this.extractClass(node); } @@ -5876,6 +5878,7 @@ export class TreeSitterExtractor { else if (classification === 'enum') this.extractEnum(node); else if (classification === 'interface') this.extractInterface(node); else if (classification === 'trait') this.extractClass(node, 'trait'); + else if (classification === 'module') this.extractClass(node, 'module'); else this.extractClass(node); return; } diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 252fe8cca..3dadf624c 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -15,7 +15,7 @@ import { ResolutionContext, FrameworkResolver, ImportMapping, - SUPERTYPE_TARGET_KINDS, + isSupertypeTarget, isInheritanceRef, isImportableKind, } from './types'; @@ -634,7 +634,9 @@ export class ReferenceResolver { if (hit && hit.gen === this.supertypeGen) return hit.supers; const typeNodes = this.context .getNodesByName(typeName) - .filter((n) => SUPERTYPE_BEARING_KINDS.has(n.kind) && n.language === language); + // Scala singletons can inherit members even though they cannot be parents. + .filter((n) => n.language === language && (SUPERTYPE_BEARING_KINDS.has(n.kind) || + (n.language === 'scala' && n.kind === 'module'))); let supers: string[]; if (typeNodes.length === 0) { supers = []; @@ -2664,7 +2666,7 @@ export class ReferenceResolver { if (!isInheritanceRef(ref)) return result; const target = this.queries.getNodeById(result.targetNodeId); - if (target && !SUPERTYPE_TARGET_KINDS.has(target.kind)) return null; + if (target && !isSupertypeTarget(target)) return null; if (isBoundToOutOfRepoImport(ref, this.context)) return null; return result; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index a1404be2b..95b2a552d 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -6,7 +6,7 @@ import * as path from 'path'; import { Language, Node } from '../types'; -import { UnresolvedRef, ResolvedRef, ResolutionContext, SUPERTYPE_TARGET_KINDS, isInheritanceRef, isImportableKind } from './types'; +import { UnresolvedRef, ResolvedRef, ResolutionContext, isSupertypeTarget, isInheritanceRef, isImportableKind } from './types'; import { blankStringContents, stripCommentsForRegex } from './strip-comments'; import { JS_BUILT_INS, JS_BUILTIN_METHODS, TS_PRIMITIVE_TYPES } from './js-builtins'; @@ -792,7 +792,7 @@ export function matchByExactName( // the real `trait`, and as the sole candidate was adopted outright by the // single-match shortcut. Restricting the pool BEFORE ranking lets the // legitimate supertype win instead of merely dropping the false edge. - .filter((n) => !isInheritanceRef(ref) || SUPERTYPE_TARGET_KINDS.has(n.kind)) + .filter((n) => !isInheritanceRef(ref) || isSupertypeTarget(n)) // Likewise for `imports`: a member that only exists inside a type is not // importable, so it is not a candidate. Without this a `path`/`id`/`url` // import resolved to some interface's same-named property. @@ -2555,7 +2555,7 @@ export function matchMethodCall( ); for (const classNode of classCandidates) { - if (classNode.kind === 'class' || classNode.kind === 'struct' || classNode.kind === 'union' || classNode.kind === 'interface') { + if (classNode.kind === 'class' || classNode.kind === 'struct' || classNode.kind === 'union' || classNode.kind === 'interface' || (classNode.language === 'scala' && classNode.kind === 'module')) { // Skip cross-language class matches if (classNode.language !== ref.language) continue; @@ -2599,7 +2599,7 @@ export function matchMethodCall( ref.filePath, ); for (const classNode of fuzzyClassCandidates) { - if (classNode.kind === 'class' || classNode.kind === 'struct' || classNode.kind === 'union' || classNode.kind === 'interface') { + if (classNode.kind === 'class' || classNode.kind === 'struct' || classNode.kind === 'union' || classNode.kind === 'interface' || (classNode.language === 'scala' && classNode.kind === 'module')) { // Skip cross-language class matches if (classNode.language !== ref.language) continue; diff --git a/src/resolution/types.ts b/src/resolution/types.ts index a5bf7a2c9..42f689a56 100644 --- a/src/resolution/types.ts +++ b/src/resolution/types.ts @@ -329,6 +329,12 @@ export const SUPERTYPE_TARGET_KINDS = new Set([ 'type_alias', 'component', 'module', 'namespace', ]); +/** Scala singleton objects are values, unlike inheritable Ruby modules. */ +export function isSupertypeTarget(node: Node): boolean { + return SUPERTYPE_TARGET_KINDS.has(node.kind) && + !(node.language === 'scala' && node.kind === 'module'); +} + /** True for the reference kinds that assert an inheritance/conformance relation. */ export function isInheritanceRef(ref: UnresolvedRef): boolean { return ref.referenceKind === 'extends' || ref.referenceKind === 'implements';