diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d6dc8d3f..8efec5781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - TypeScript and JavaScript module-level declarations now name themselves as the caller of whatever their initializer runs. `const cfg = loadConfig()` recorded the *file* as loadConfig's caller, which is no use for callers or impact; it now records `cfg`. And an object literal that wasn't exported — `const handlers = { onSave: () => persist() }` — was skipped entirely, so nothing inside it reached the graph at all. Re-index with `codegraph index -f` after upgrading to pick up the new edges. - `codegraph_explore` again lists a dynamic-dispatch link when the same two symbols are also joined by an ordinary call, instead of dropping it from the summary. - Java fields initialized with a lambda or an anonymous class — `private final Runnable r = () -> doWork();`, `new LocationListener() { … }`, `Parcelable.Creator` — now contribute call edges, and the anonymous class and its overrides become real symbols instead of being invisible. Previously everything inside a field initializer was dropped, so a method reached only from one looked like it had no callers. Re-index with `codegraph index -f` after upgrading to pick up the new edges. -- A Kotlin property whose accessor is written on the same line — `val url: String get() = build(host)` — now contributes its accessor's calls too, instead of dropping them. +- Kotlin `init { }` blocks and destructuring declarations no longer swallow their code: `init { val cfg = load() }` and `val (a, b) = makePair()` contributed no call edge at all, and now attribute to the enclosing class or file. +- A Kotlin property's accessor body now belongs to the property whichever line it is written on, instead of being dropped (same line) or handed to the enclosing class (own line). - Kotlin properties that hold a lambda, a SAM callback or an anonymous object — `private val frameListener = CameraFrameListener { … }`, the way Android and MSDK callbacks are almost always declared — now contribute call edges. Previously everything inside such an initializer was dropped, so a function reached only through one of these callbacks looked like it had no callers at all and its blast radius came back far too small. Delegated properties (`by lazy { … }`) and plain initializers (`val x = compute()`) were affected the same way and are fixed too. Re-index with `codegraph index -f` after upgrading to pick up the new edges. - The blast-radius section of `codegraph_explore` flagged "no covering tests found" whenever no test called a symbol directly — falsely branding helpers that tests exercise through their callers as untested (about 40% of flagged symbols in a measured sample). The check now follows caller chains up to 3 hops and reports indirect coverage as "tested via callers"; when nothing is found it states exactly what was checked instead of an unconditional warning. Thanks @inth3shadows for measuring the false-positive rate. (#1475) diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 68940ee06..fb42fc1ed 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -2062,11 +2062,11 @@ object Holder { expect(callersOf('hit')).toEqual(['topLevelLambda']); }); - it('a same-line accessor body is walked too; a next-line one still is not', () => { - // `val x: T get() = …` nests the accessor UNDER the declaration, so the - // hook consumed it and its calls vanished. Written on its own line the - // accessor parses as a SIBLING of the property — out of reach from here, - // and still attributed to the enclosing class as before. + it('an accessor body belongs to its property, written on either line', () => { + // `val x: T get() = …` nests the accessor UNDER the declaration; written + // on its own line the grammar makes it a following SIBLING instead. Both + // used to lose their calls (the nested one) or hand them to the enclosing + // class (the sibling); both now attribute to the property. const src = ` package p @@ -2074,19 +2074,51 @@ class C { val sameLine: Int get() = compute() val nextLine: Int get() = compute() + var written: Int = 0 + set(v) { store(v) } private fun compute(): Int = 1 + private fun store(v: Int) {} } `; const result = extractFromSource('C.kt', src); const byId = new Map(result.nodes.map((n) => [n.id, n])); - const owners = result.unresolvedReferences - .filter((u) => u.referenceKind === 'calls' && u.referenceName === 'compute') - .map((u) => { - const n = byId.get(u.fromNodeId); - return n ? `${n.kind}:${n.name}` : '?'; - }) - .sort(); - expect(owners).toEqual(['class:C', 'field:sameLine']); + const ownersOf = (name: string) => + result.unresolvedReferences + .filter((u) => u.referenceKind === 'calls' && u.referenceName === name) + .map((u) => { + const n = byId.get(u.fromNodeId); + return n ? `${n.kind}:${n.name}` : '?'; + }) + .sort(); + expect(ownersOf('compute')).toEqual(['field:nextLine', 'field:sameLine']); + expect(ownersOf('store')).toEqual(['field:written']); + }); + + it('an `init` block and a destructuring RHS no longer vanish', () => { + // Both mint no symbol of their own, so the hook consumed them and their + // code disappeared entirely; they now attribute to the enclosing scope. + const src = ` +package p + +class C { + init { val q = initCall() } + val (a, b) = makePair() +} + +val (t1, t2) = topMakePair() +`; + const result = extractFromSource('C.kt', src); + const byId = new Map(result.nodes.map((n) => [n.id, n])); + const owner = (name: string) => { + const u = result.unresolvedReferences.find( + (r) => r.referenceKind === 'calls' && r.referenceName === name + ); + const n = u ? byId.get(u.fromNodeId) : undefined; + return n ? `${n.kind}:${n.name}` : undefined; + }; + expect(owner('initCall')).toBe('class:C'); + expect(owner('makePair')).toBe('class:C'); + expect(owner('topMakePair')).toBe('namespace:p'); }); }); }); diff --git a/__tests__/fixtures/kernel-parity/torture.kt b/__tests__/fixtures/kernel-parity/torture.kt index a916cc6d8..c5531c55e 100644 --- a/__tests__/fixtures/kernel-parity/torture.kt +++ b/__tests__/fixtures/kernel-parity/torture.kt @@ -278,3 +278,14 @@ class AccessorK { var sameLinePair: Int get() = compute() set(v) { draw(v) } } + +class SiblingAccessorK { + var nextLine: Int = 0 + get() = compute() + set(v) { draw(v) } + val (localA, localB) = makePair() + init { + val fromInit = compute() + register(fromInit) + } +} diff --git a/codegraph-kernel/src/kotlin.rs b/codegraph-kernel/src/kotlin.rs index b89969fe4..366705700 100644 --- a/codegraph-kernel/src/kotlin.rs +++ b/codegraph-kernel/src/kotlin.rs @@ -117,6 +117,36 @@ fn property_initializers<'t>(node: Node<'t>) -> Vec> { out } +/// Accessors written on their OWN line parse as SIBLINGS of the property, not +/// as children of it (same-line ones nest — see property_initializers). Walking +/// back over any accessors between us and the declaration finds the property an +/// accessor belongs to; None when this accessor stands alone. +fn accessor_owner<'t>(node: Node<'t>) -> Option> { + let mut p = node.prev_named_sibling(); + while let Some(n) = p { + if matches!(n.kind(), "getter" | "setter") { + p = n.prev_named_sibling(); + continue; + } + return if n.kind() == "property_declaration" { Some(n) } else { None }; + } + None +} + +/// The sibling accessors that follow a property declaration, in source order. +fn following_accessors<'t>(node: Node<'t>) -> Vec> { + let mut out = Vec::new(); + let mut n = node.next_named_sibling(); + while let Some(c) = n { + if !matches!(c.kind(), "getter" | "setter") { + break; + } + out.push(c); + n = c.next_named_sibling(); + } + out +} + struct Scope { row: u32, kind: &'static str, @@ -613,25 +643,23 @@ impl<'t> Walker<'t> { // --- the visitNode hook (property branch ONLY — fun-interface recovery is // defer-shielded and not ported) ------------------------------------------------ - fn try_visit_hook(&mut self, node: Node<'t>) -> bool { - if node.kind() != "property_declaration" { - return false; - } + /// A property's node kind, or None when the declaration mints no node at + /// all: destructuring, an unreadable name, or a local (inside a function + /// body / `init` block / lambda / accessor). Kind by enclosing scope — a + /// singleton `object` / `companion object` (and a top-level property) holds + /// SHARED values (`val`→constant, `var`→variable, the Scala-object rule; a + /// `const val` is just a val); a class/interface/enum instance `val`/`var` + /// is per-instance state → `field`. + fn property_kind(&self, node: Node<'t>) -> Option<&'static str> { let var_decl = (0..node.named_child_count()) .filter_map(|i| node.named_child(i)) - .find(|c| c.kind() == "variable_declaration"); - let name_node = var_decl.and_then(|vd| { - (0..vd.named_child_count()) - .filter_map(|i| vd.named_child(i)) - .find(|c| c.kind() == "simple_identifier") - }); - let Some(name_node) = name_node else { return false }; // destructuring → decline - let name = self.text(name_node).to_string(); - if name.is_empty() { - return false; + .find(|c| c.kind() == "variable_declaration")?; + let name_node = (0..var_decl.named_child_count()) + .filter_map(|i| var_decl.named_child(i)) + .find(|c| c.kind() == "simple_identifier")?; + if self.text(name_node).is_empty() { + return None; } - - // Scope walk up the parent chain — first match wins. let mut scope: &str = "const"; let mut p = node.parent(); while let Some(pn) = p { @@ -654,19 +682,67 @@ impl<'t> Walker<'t> { p = pn.parent(); } if scope == "local" { - return true; // a local — extract nothing, subtree still scanned + return None; } - let binding = (0..node.named_child_count()) .filter_map(|i| node.named_child(i)) .find(|c| c.kind() == "binding_pattern_kind"); let is_val = binding.map(|b| self.text(b) == "val").unwrap_or(false); - let kind: &'static str = if scope == "instance" { + Some(if scope == "instance" { "field" } else if is_val { "constant" } else { "variable" + }) + } + + fn try_visit_hook(&mut self, node: Node<'t>) -> bool { + // An own-line accessor already walked by its owning property below. The + // ownership test re-derives the property's kind rather than remembering + // it: a destructured or local declaration mints no node, so its + // accessors were NOT consumed and must keep falling through. + if matches!(node.kind(), "getter" | "setter") { + return accessor_owner(node) + .and_then(|owner| self.property_kind(owner)) + .is_some(); + } + if node.kind() != "property_declaration" { + return false; + } + let var_decl = (0..node.named_child_count()) + .filter_map(|i| node.named_child(i)) + .find(|c| c.kind() == "variable_declaration"); + let name_node = var_decl.and_then(|vd| { + (0..vd.named_child_count()) + .filter_map(|i| vd.named_child(i)) + .find(|c| c.kind() == "simple_identifier") + }); + // Destructuring (`val (a, b) = makePair()`): NEITHER arm mints a symbol + // for the destructured names — declining just routes the node to + // extractField/extractVariable, which both find nothing for kotlin and + // end in the same fn-ref scan. But the RHS is CODE, and it was vanishing + // whole. Consume the node here and walk it at the ENCLOSING scope (no + // symbol of its own to attribute to). + let Some(name_node) = name_node else { + for init in property_initializers(node) { + self.visit_function_body(init); + } + return true; + }; + let name = self.text(name_node).to_string(); + if name.is_empty() { + return false; + } + let Some(kind) = self.property_kind(node) else { + // A local — no node is minted, but the initializer is still code. + // Walk it at the ENCLOSING scope: an `init { }` block's + // `val q = load()` is the CLASS calling load, and it used to + // disappear entirely (only the block's bare statements survived). + for init in property_initializers(node) { + self.visit_function_body(init); + } + return true; }; // The `type`-field signature read is dead (zero fields) → signature // undefined; NO docstring/visibility/isStatic — the modifiers merge in @@ -676,11 +752,17 @@ impl<'t> Walker<'t> { // fix, ported): without this the subtree is only fn-ref-scanned, so a // lambda / SAM / object initializer (`val cb = Runnable { target() }` — // the idiomatic Android callback field) contributed NO call edge at all. + // The property also OWNS any accessor written on its own line, which the + // grammar makes a following SIBLING rather than a child; those bodies + // used to attribute to the enclosing class. if let Some(row) = row { self.stack.push(Scope { row, kind, name: name.clone() }); for init in property_initializers(node) { self.visit_function_body(init); } + for acc in following_accessors(node) { + self.visit_function_body(acc); + } self.stack.pop(); } true diff --git a/src/extraction/languages/kotlin.ts b/src/extraction/languages/kotlin.ts index 4ae71c966..144d52318 100644 --- a/src/extraction/languages/kotlin.ts +++ b/src/extraction/languages/kotlin.ts @@ -73,6 +73,68 @@ function kotlinPropertyInitializers(node: SyntaxNode): SyntaxNode[] { return out; } + +/** + * A property's node kind, or null when the declaration mints no node at all: + * destructuring (`val (a, b) = …`), an unreadable name, or a local (one inside + * a function body / `init` block / lambda / accessor). Kind by enclosing scope: + * a singleton `object` / `companion object` — and a top-level property — holds + * *shared* values, so `val`→`constant` and `var`→`variable` (the Scala-object + * rule; a `const val` is just a val). A `class`/`interface`/`enum` instance + * `val`/`var` is per-instance state → `field` (never a value-ref target, like a + * Java instance `final`). + */ +function kotlinPropertyKind( + node: SyntaxNode, + source: string +): 'field' | 'constant' | 'variable' | null { + const varDecl = node.namedChildren.find((c) => c.type === 'variable_declaration'); + const nameNode = varDecl?.namedChildren.find((c) => c.type === 'simple_identifier'); + if (!nameNode || !getNodeText(nameNode, source)) return null; + + let scope: 'local' | 'const' | 'instance' = 'const'; + for (let p = node.parent; p; p = p.parent) { + const pt = p.type; + if ( + pt === 'function_body' || pt === 'function_declaration' || + pt === 'lambda_literal' || pt === 'anonymous_initializer' || + pt === 'control_structure_body' || pt === 'getter' || pt === 'setter' + ) { scope = 'local'; break; } + if (pt === 'companion_object' || pt === 'object_declaration') { scope = 'const'; break; } + if (pt === 'class_declaration') { scope = 'instance'; break; } + } + if (scope === 'local') return null; + + const binding = node.namedChildren.find((c) => c.type === 'binding_pattern_kind'); + const isVal = binding != null && getNodeText(binding, source) === 'val'; + return scope === 'instance' ? 'field' : isVal ? 'constant' : 'variable'; +} + +/** + * Accessors written on their OWN line parse as SIBLINGS of the property, not as + * children of it (same-line ones nest — see kotlinPropertyInitializers). Walking + * back over any accessors between us and the declaration finds the property an + * accessor belongs to; null when this accessor stands alone (a grammar + * accident, or an accessor on a destructured/local declaration). + */ +function kotlinAccessorOwner(node: SyntaxNode): SyntaxNode | null { + for (let p = node.previousNamedSibling; p; p = p.previousNamedSibling) { + if (p.type === 'getter' || p.type === 'setter') continue; + return p.type === 'property_declaration' ? p : null; + } + return null; +} + +/** The sibling accessors that follow a property declaration, in source order. */ +function kotlinFollowingAccessors(node: SyntaxNode): SyntaxNode[] { + const out: SyntaxNode[] = []; + for (let n = node.nextNamedSibling; n; n = n.nextNamedSibling) { + if (n.type !== 'getter' && n.type !== 'setter') break; + out.push(n); + } + return out; +} + /** Check if a node matches the `fun interface` misparse pattern */ function isFunInterfaceNode(node: SyntaxNode): boolean { let hasFun = false; @@ -119,40 +181,35 @@ export const kotlinExtractor: LanguageExtractor = { // Kotlin properties (`val` / `var` / `const val`). The name nests as // property_declaration → variable_declaration → simple_identifier, which the // generic variable/field path can't read — so nothing was extracted before. - // Kind by enclosing scope: a singleton `object` / `companion object` (and a - // top-level property) holds *shared* values — `val`→`constant`, - // `var`→`variable` (the Scala-object rule; a `const val` is a `val`). A - // `class`/`interface`/`enum` instance `val`/`var` is per-instance state → - // `field` (never a value-ref target, like a Java instance `final`). A - // property inside a function body / `init` block / lambda is a local and is - // skipped entirely. + // Kind comes from kotlinPropertyKind. if (node.type === 'property_declaration') { const varDecl = node.namedChildren.find((c) => c.type === 'variable_declaration'); const nameNode = varDecl?.namedChildren.find((c) => c.type === 'simple_identifier'); - if (!nameNode) return false; // destructuring `val (a,b)` etc. — leave to default + // Destructuring (`val (a, b) = makePair()`): no symbol is minted for the + // destructured names either way — declining just routes the node to + // extractField/extractVariable, which both find nothing for Kotlin and + // end in the same fn-ref scan. But the RHS is CODE, and it was vanishing + // whole. Consume the node here and walk it at the ENCLOSING scope (there + // is no symbol of its own to attribute it to). + if (!nameNode) { + for (const init of kotlinPropertyInitializers(node)) ctx.visitFunctionBody(init, ''); + return true; + } const name = getNodeText(nameNode, ctx.source); if (!name) return false; - // Walk to the nearest enclosing definition: a function body / init / lambda - // means it's a local; `object`/`companion object` is a constant scope; a - // `class_declaration` (covers class/interface/enum) is an instance scope. - let scope: 'local' | 'const' | 'instance' = 'const'; - for (let p = node.parent; p; p = p.parent) { - const pt = p.type; - if ( - pt === 'function_body' || pt === 'function_declaration' || - pt === 'lambda_literal' || pt === 'anonymous_initializer' || - pt === 'control_structure_body' || pt === 'getter' || pt === 'setter' - ) { scope = 'local'; break; } - if (pt === 'companion_object' || pt === 'object_declaration') { scope = 'const'; break; } - if (pt === 'class_declaration') { scope = 'instance'; break; } + const kind = kotlinPropertyKind(node, ctx.source); + if (kind == null) { + // A local — no node is minted, but the initializer is still code. Walk + // it at the ENCLOSING scope: an `init { }` block's `val q = load()` is + // the CLASS calling load, and it used to disappear entirely (only the + // block's bare statements survived). + for (const init of kotlinPropertyInitializers(node)) ctx.visitFunctionBody(init, ''); + return true; } - if (scope === 'local') return true; // a local — don't extract const binding = node.namedChildren.find((c) => c.type === 'binding_pattern_kind'); const isVal = binding != null && getNodeText(binding, ctx.source) === 'val'; - const kind = scope === 'instance' ? 'field' : isVal ? 'constant' : 'variable'; - const typeNode = node.childForFieldName('type'); const sig = typeNode ? `${isVal ? 'val' : 'var'} ${name}: ${getNodeText(typeNode, ctx.source)}` @@ -164,7 +221,13 @@ export const kotlinExtractor: LanguageExtractor = { // (`private val cb = Runnable { target() }` — the idiomatic Android // callback field) contributed NO call edge at all, and everything reached // only through such a callback looked like it had no callers. - const inits = created ? kotlinPropertyInitializers(node) : []; + // The property also OWNS any accessor written on its own line, which the + // grammar makes a following SIBLING rather than a child; those bodies used + // to attribute to the enclosing class. Consumed here so the accessor + // branch below can skip them without any cross-node state. + const inits = created + ? [...kotlinPropertyInitializers(node), ...kotlinFollowingAccessors(node)] + : []; if (created && inits.length > 0) { ctx.pushScope(created.id); for (const init of inits) ctx.visitFunctionBody(init, created.id); @@ -173,6 +236,15 @@ export const kotlinExtractor: LanguageExtractor = { return true; } + // An own-line accessor already walked by its owning property above. The + // ownership test re-derives the property's kind rather than remembering it: + // a destructured or local declaration mints no node, so its accessors were + // NOT consumed and must keep falling through to the normal recursion. + if (node.type === 'getter' || node.type === 'setter') { + const owner = kotlinAccessorOwner(node); + return owner != null && kotlinPropertyKind(owner, ctx.source) != null; + } + // Handle Kotlin `fun interface` declarations. // Tree-sitter-kotlin doesn't support `fun interface` syntax (Kotlin 1.4+). // It produces two different misparse patterns: