diff --git a/CHANGELOG.md b/CHANGELOG.md index a36bbe48a5..767a5d1ab4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- In Ruby, a call on a namespaced class such as `Billing::ChargeCard.call(...)` or `::Billing::ChargeCard.call(...)` now links the caller to that class, the way `ChargeCard.call(...)` already did, and `Users::Create.call` links to `Users::Create`, not to a `Billing::Create` of the same name. Before, a class used only through such calls looked like it had no dependents. Thanks @james-jang for the report. (#2503) - In TypeScript, an awaited call with type arguments like `await fetchApi("/me")` or `await api.get(url)` now links to the function it calls. Before, the call was dropped, so typed fetch wrappers looked like they had no callers. (#2586) - In React, a component rendered at module scope, in a top-level constant or inside a test's `it(…)` callback, like `createRoot(el).render()` or `render()`, now counts as rendered there, so an app's entry reaches its root component and a component's tests show among its callers. (#2587) - In Python, `import a.b as X` written inside a function now links its calls like `X.f()`, as the same import at module level already did. Thanks @Egrn for the report. (#2600) diff --git a/__tests__/kernel-ruby-parity.test.ts b/__tests__/kernel-ruby-parity.test.ts index 70390e9925..92e9ce4484 100644 --- a/__tests__/kernel-ruby-parity.test.ts +++ b/__tests__/kernel-ruby-parity.test.ts @@ -107,6 +107,22 @@ describe.skipIf(!kernelBuilt)('kernel Ruby extraction parity', () => { assertParity('fixtures/torture.rb (crlf)', crlf, 30); }); + // Namespaced and `::`-rooted constant receivers (#2503): both backends emit + // the same references ref, under the qualified name minus a leading `::`. + it('namespaced and ::-rooted constant receivers', () => { + const src = [ + 'class Callers', + ' def a = Billing::Create.call', + ' def b = Users::Create&.call', + ' def c = ::Billing::Create.call', + ' def d = ::Foo.call', + ' def e = Deep::Op.call', + 'end', + '', + ].join('\n'); + assertParity('src/callers.rb', src); + }); + it('mixin implements refs carry filePath through the v2 ref-flag wire path', () => { const src = 'class Widget\n include Comparable\nend\n'; process.env.CODEGRAPH_KERNEL_LANGS = 'all'; diff --git a/__tests__/ruby-namespaced-receiver.test.ts b/__tests__/ruby-namespaced-receiver.test.ts new file mode 100644 index 0000000000..30a6a39823 --- /dev/null +++ b/__tests__/ruby-namespaced-receiver.test.ts @@ -0,0 +1,168 @@ +/** + * A Ruby call on a namespaced constant (`Billing::ChargeCard.call`) is a + * dependency on that class, the same as the bare `ChargeCard.call` (#1110). + * The reference carries the qualified name, so same-named classes in other + * namespaces (`Billing::Create` / `Users::Create`) and external constants + * (`ActiveRecord::Base`) are not mistaken for it. + * + * Runs against the native kernel (skipped when none is built) and the wasm + * extractor. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { getKernel } from '../src/extraction/kernel/loader'; + +const FILES: Record = { + 'app/services/billing/charge_card.rb': `module Billing + class ChargeCard + def self.call(**kw) = new(**kw).call + def initialize(amount:) = @amount = amount + def call = @amount + end +end +`, + 'app/services/checkout/complete.rb': `module Checkout + class Complete + def call + Billing::ChargeCard.call(amount: 1) + end + end +end +`, + 'app/services/billing/refund.rb': `module Billing + class Refund + def call + ChargeCard.call(amount: 2) + end + end +end +`, + // Same class name in two namespaces; the caller names which one it means. + 'app/billing/create.rb': `module Billing + class Create + def self.call = 1 + end +end +`, + 'app/users/create.rb': `module Users + class Create + def self.call = 2 + end +end +`, + 'app/callers.rb': `class Callers + def billing = Billing::Create.call + def users = Users::Create.call + def users_safe = Users::Create&.call + def rooted = ::Billing::Create.call + def rooted_top = ::Foo.call + def external = ActiveRecord::Base.transaction { } +end +`, + 'app/foo.rb': `class Foo + def self.call = 3 +end +`, + // A partly qualified receiver, resolved from inside the enclosing module. + 'app/svc/deep/op.rb': `module Svc + module Deep + class Op + def self.call = 4 + end + end +end +`, + 'app/svc/runner.rb': `module Svc + class Runner + def run = Deep::Op.call + end +end +`, + // A project class that merely shares the last segment of an external constant. + 'app/base.rb': `class Base +end +`, +}; + +let root = ''; +let cg: CodeGraph | undefined; +let kernel: string | undefined; + +afterEach(() => { + cg?.close(); + cg = undefined; + if (root) fs.rmSync(root, { recursive: true, force: true }); + if (kernel === undefined) delete process.env.CODEGRAPH_KERNEL; + else process.env.CODEGRAPH_KERNEL = kernel; +}); + +async function index(backend: string): Promise { + kernel = process.env.CODEGRAPH_KERNEL; + if (backend === 'wasm') process.env.CODEGRAPH_KERNEL = '0'; + else delete process.env.CODEGRAPH_KERNEL; + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ruby-nsrecv-')); + for (const [rel, content] of Object.entries(FILES)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); + return cg; +} + +/** `file:caller` of every node with a `references` edge into the class `name` defined in `file`. */ +function referencedBy(graph: CodeGraph, name: string, file: string): string[] { + const cls = graph.searchNodes(name).map((r) => r.node).find((n) => n.kind === 'class' && n.filePath === file)!; + return graph + .getIncomingEdgesTo([cls.id]) + .filter((e) => e.kind === 'references') + .map((e) => { const n = graph.getNode(e.source)!; return `${n.filePath}:${n.name}`; }) + .sort(); +} + +describe('Ruby namespaced-constant receiver', () => { + const backends = [ + ['kernel', getKernel() === null], + ['wasm', false], + ] as const; + + describe.each(backends)('%s', (backend, skip) => { + it.skipIf(skip)('records a references edge to the class, like a bare constant receiver', async () => { + const graph = await index(backend); + const files = referencedBy(graph, 'ChargeCard', 'app/services/billing/charge_card.rb').map((r) => r.split(':')[0]); + expect([...new Set(files)]).toEqual([ + 'app/services/billing/refund.rb', + 'app/services/checkout/complete.rb', + ]); + }); + + it.skipIf(skip)('sends each namespaced caller to the class of its own namespace', async () => { + const graph = await index(backend); + expect(referencedBy(graph, 'Create', 'app/billing/create.rb')).toEqual([ + 'app/callers.rb:billing', + 'app/callers.rb:rooted', + ]); + expect(referencedBy(graph, 'Create', 'app/users/create.rb')).toEqual([ + 'app/callers.rb:users', + 'app/callers.rb:users_safe', + ]); + }); + + it.skipIf(skip)('links a ::-rooted top-level receiver to its class', async () => { + const graph = await index(backend); + expect(referencedBy(graph, 'Foo', 'app/foo.rb')).toEqual(['app/callers.rb:rooted_top']); + }); + + it.skipIf(skip)('links a partly qualified receiver written inside its enclosing module', async () => { + const graph = await index(backend); + expect(referencedBy(graph, 'Op', 'app/svc/deep/op.rb')).toEqual(['app/svc/runner.rb:run']); + }); + + it.skipIf(skip)('does not link an external namespaced constant to a project class of the same last name', async () => { + const graph = await index(backend); + expect(referencedBy(graph, 'Base', 'app/base.rb')).toEqual([]); + }); + }); +}); diff --git a/codegraph-kernel/src/ruby.rs b/codegraph-kernel/src/ruby.rs index 3e4e33cb08..fd4b6a25f1 100644 --- a/codegraph-kernel/src/ruby.rs +++ b/codegraph-kernel/src/ruby.rs @@ -821,15 +821,26 @@ impl<'t> Walker<'t> { self.push_ref(caller, &callee, calls_kind, line, col); // Capitalized constant receiver (`Klass.static_call`, `RETRY_MAX.times`) - // → an ADDITIONAL references ref at the RECEIVER's position. - // (scope_resolution receivers get none — type ≠ constant.) - if !skip && receiver.kind() == "constant" { - self.push_ref_at( - caller, - &receiver_name.to_string(), - edge_kind_index("references").unwrap(), - receiver, - ); + // → an ADDITIONAL references ref at the RECEIVER's position. A namespaced + // receiver (`Billing::ChargeCard.call`) references the class under its + // qualified name (a leading `::` dropped), so `Billing::Create` and + // `Users::Create` resolve apart; it needs a capitalized last segment. + if !skip { + let constant_name = receiver_name.trim_start_matches("::"); + let last = constant_name.rsplit("::").next().unwrap_or(constant_name); + let is_constant = match receiver.kind() { + "constant" => true, + "scope_resolution" => last.starts_with(|c: char| c.is_ascii_uppercase()), + _ => false, + }; + if is_constant { + self.push_ref_at( + caller, + &constant_name.to_string(), + edge_kind_index("references").unwrap(), + receiver, + ); + } } } diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index ee757ab08d..79205f3701 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -5276,10 +5276,17 @@ export class TreeSitterExtractor { // — is itself a dependency on that constant; emit a `references` ref so a // class used only via its class methods still records a dependent (the edge // the old receiver-only callee happened to provide, now made explicit). - if (!skip && receiverNode.type === 'constant') { + // A namespaced receiver (`Billing::ChargeCard.call`) references the class under + // its qualified name (a leading `::` dropped), so `Billing::Create` and + // `Users::Create` resolve apart; it needs a capitalized last segment. + const constantName = receiverName.replace(/^::/, ''); + const isConstant = receiverNode.type === 'constant' + || (receiverNode.type === 'scope_resolution' + && /^[A-Z]/.test(constantName.split('::').pop()!)); + if (!skip && isConstant) { this.unresolvedReferences.push({ fromNodeId: callerId, - referenceName: receiverName, + referenceName: constantName, referenceKind: 'references', line: receiverNode.startPosition.row + 1, column: receiverNode.startPosition.column,