From d8dc220963e972da562312cfd21d81586127ab44 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Thu, 29 Oct 2026 10:57:14 -0500 Subject: [PATCH] fix(ruby): calls to `!`/`?` methods and calls on what `.new` made link their method Two kinds of Ruby call linked nothing. A call to a method whose name ends in `!` or `?` on a receiver (`writer.flush!`, `writer.ready?` after `writer = CacheWriter.new`, `CacheWriter.create!(1)`) failed the method-call pattern, whose method part took no `!` or `?`. A method called straight on a new object (`CacheWriter.new(key).write_cache`, `Billing::Ledger.new.charge!`) was recorded by both extractors with the receiver's raw text, arguments and blocks included, which no strategy matched. Both extractors now record a call on `Const.new(...)`, `A::B.new` or `::Foo.new` as `new Const().m`, the constructed-receiver shape TS/JS, Java and C# already use; the wasm extractor and the kernel's ruby.rs emit the same refs. ruby-constant-paths resolves that shape before the name pre-filter: the class by Ruby's constant lookup from the namespaces around the call, the method as the class's own instance method or the first its ancestry declares, and nothing when either is missing, so rack-protection's `Request.new(env)` (rack's Rack::Request) links nothing. The Ruby dot pattern in matchMethodCall takes a trailing `!` or `?`; the receiver-word scoring among several project methods of the name stays off for such names on a value receiver, where it was wrong on 55 of 74 new edges (`options.key?` on a Hash -> InheritableOptions#key?). rubyAncestry no longer reads a module a call returns (`include RouteSet.new.tap {...}.url_helpers`) as a mixin of the constant the call starts at. Validation, kernel engine, main 37052b18 vs this change: - sinatra cb22afd +12, jekyll 97c3bdc +53, activesupport +398, actionpack +97 (2 removed: `ActionController::Base.with` had reached RoutesHelpers.with through a misread mixin), activerecord +1012 (rails 793d744 subtrees). About 21 of 1,572 added edges are wrong, all from the class-name and single-method guesses that plain names get too. - Controls requests (Python) and cobra (Go): dump-graph byte-identical. - kernel-parity.mjs: fixtures 10/10, sinatra 147/147, jekyll + rails 1044/1044 files byte-parity, none deferred. - New tests: 10 fail with main's kernel and TS, all pass with the fix, in both engines. Dropping the scoring gate or the mixin fix fails its guard test. - Index time, median of 3 interleaved: activesupport 5.35 vs 4.63 s, jekyll 2.66 vs 2.73 s (noise). Reimplements PR #684 by @ricobl, who found both gaps. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/kernel-ruby-parity.test.ts | 45 +++ __tests__/ruby-constructed-receivers.test.ts | 294 +++++++++++++++++++ codegraph-kernel/src/ruby.rs | 19 ++ src/extraction/tree-sitter.ts | 21 +- src/resolution/name-matcher.ts | 19 +- src/resolution/ruby-constant-paths.ts | 39 ++- 7 files changed, 428 insertions(+), 10 deletions(-) create mode 100644 __tests__/ruby-constructed-receivers.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a8212dae68..ee561ad181 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -198,6 +198,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a base class like `Message` in `class DynamicMessage final : public Message` now links to the class C++ itself would pick: the one declared in the namespaces around the class, or brought in by the file's `using` declarations and namespace aliases, and the class a `typedef` or `using` alias names. Before, a base class was matched by its name alone, so it could land on a class of the same name in another namespace or another language: in protocolbuffers/protobuf, `DynamicMessage` derived from the JSON parser's internal `Message` class, and in google/leveldb and RocksDB, iterators derived from an unrelated nested `Iterator` class such as the skip list's. The wrong base showed up in `codegraph_explore`'s type hierarchy and in impact, and tied the base's methods to the wrong overrides. A template parameter used as a base no longer links to a class that shares its name, and a base written from the global scope, like `public ::testing::Test`, now links to its class. Re-index C++ projects after upgrading. - In C++ and Rust, a call written with a qualifier, like `Status::OK()`, now links to a method of the class it names, never to one of a class whose name only ends the same way. Before, facebook/rocksdb's calls to `Status::OK()`, `Status::InvalidArgument(…)` and `Status::Corruption(…)` linked to `IOStatus`'s methods of those names, a subclass calling its base, like `return FileSystemWrapper::NewWritableFile(…)` or google/leveldb's `return DB::Put(…)`, linked to another subclass or to the calling method itself, and BurntSushi/ripgrep's `Path::new(…)` on the standard library's `Path` linked to its own `PrinterPath::new`. When classes in several languages share the name, the one in the call's own language is taken, so protocolbuffers/protobuf's C++ `DescriptorPool::generated_pool()` calls, which met its Ruby namesake first and linked to nothing, now link to the C++ method. A C++ call through a template parameter, like `CacheShard::ComputeHash(key)` in rocksdb's `template class ShardedCache`, no longer links to a class that merely shares words with the parameter's name, unless that class is the parameter's default, like `TcParser` in `template `. A template's value parameter, like `const PrefetchOpts& kOpts`, is no longer mistaken for a type parameter, so references to `PrefetchOpts` inside it now link. Re-index C++ and Rust projects after upgrading. - In Ruby, a call on a class written with its namespace, like `Billing::ChargeCard.call(…)` or `::Billing::ChargeCard.call(…)`, now links the caller to that class and to the class method it calls, as `ChargeCard.call(…)` already linked to the class. Before, a class used only through such calls looked like it had no callers. The name is looked up the way Ruby does it, from the namespaces around the call outward, so `Users::Create.call` reaches `Users::Create` and never a `Billing::Create`, a call inside `module Api; module V2` reaches `Api::V2`'s class before another version's, and a library's class like `ActiveRecord::Base` is never taken for a project class of the same last name. Re-index Ruby projects after upgrading. Thanks @james-jang for the report and @drakeo338 for the pull request. (#2503) +- In Ruby, a call to a method whose name ends in `!` or `?` now links when its receiver's class is known, like `writer.flush!` or `writer.ready?` after `writer = CacheWriter.new`, or `CacheWriter.create!(…)` on the class. A method called straight on a new object, like `CacheWriter.new(key).write_cache` or `Billing::Ledger.new.charge!`, now links to that class's method or to one it inherits. Before, all of these linked nothing. The class is looked up the way Ruby does it, from the namespaces around the call outward, and one from outside your project, like rack's `Request` used inside `Rack::Protection`, links nothing rather than a project class of the same name. Re-index Ruby projects after upgrading. Thanks @ricobl. - In PHP, a class written through a namespace alias in a different case than its `use` statement, like `new field\FirstName()` after `use App\Fields as Field;`, now links to that class, since PHP reads namespace names without regard to case. Before, the name was read relative to the file's own namespace and stayed unlinked. Re-index PHP projects after upgrading. Thanks @danusha2345. (#2369) - In C++, a call through a class template's specialization written with its arguments, like fmtlib/fmt's `formatter::format(42, ctx)`, no longer links to a member defined for another specialization, like a test's `formatter::format`, and when the template declares no such member, as for `formatter::parse(ctx)`, the call is no longer matched to another class's method of that name, like `native_formatter::parse`. A call to the specialization a member is defined for, and to a template's own members, links as before. Re-index C++ projects after upgrading. - In C++, a function is now named by its declarator, never by something written in the clauses after its parameters. nlohmann/json's constructor `basic_json(CompatibleType&& val) noexcept(noexcept(JSONSerializer::to_json(…)))` was missing from the index, replaced by a method `JSONSerializer::to_json` that doesn't exist and that other `JSONSerializer::to_json(…)` calls linked to; a method declared with `throw(std::bad_alloc)` was indexed as `std::bad_alloc`, and a function with a trailing `requires std::integral` as `std::integral`. Re-index C++ projects after upgrading. diff --git a/__tests__/kernel-ruby-parity.test.ts b/__tests__/kernel-ruby-parity.test.ts index 72a25297e8..fcf02984df 100644 --- a/__tests__/kernel-ruby-parity.test.ts +++ b/__tests__/kernel-ruby-parity.test.ts @@ -148,6 +148,51 @@ describe.skipIf(!kernelBuilt)('kernel Ruby extraction parity', () => { ]); }); + // A call on what `.new` made (`CacheWriter.new(key).write_cache`, #684) is + // recorded as `new CacheWriter().write_cache` on both backends: the class as + // written, without the arguments the receiver's text carried. A `new` on a + // value (`klass.new.run`, `self.class.new`) or a later link of a chain keeps + // the receiver's text, as before. + it('calls on a constructed receiver', () => { + const src = [ + 'class Callers', + ' def a(key, klass)', + ' CacheWriter.new.write_cache(key)', + ' CacheWriter.new(key, ttl: 5).flush!', + ' Billing::Ledger.new { |l| l }.post', + ' ::CacheWriter.new.ready?', + ' CacheWriter.new&.write_cache(key)', + ' klass.new.run', + ' self.class.new(key).run', + ' Widget.create(key).save', + ' CacheWriter.new.first.second', + ' end', + 'end', + '', + ].join('\n'); + assertParity('src/callers.rb', src); + process.env.CODEGRAPH_KERNEL = '0'; + const calls = extractFromSource('src/callers.rb', src, 'ruby').unresolvedReferences + .filter((r) => r.referenceKind === 'calls' && /[.(]/.test(r.referenceName)) + .map((r) => `${r.referenceName} ${r.line}`); + delete process.env.CODEGRAPH_KERNEL; + expect(calls.sort()).toEqual([ + 'CacheWriter.new.first.second 11', + 'Widget.create 10', + 'Widget.create(key).save 10', + 'klass.new 8', + 'klass.new.run 8', + 'new ::CacheWriter().ready? 6', + 'new Billing::Ledger().post 5', + 'new CacheWriter().first 11', + 'new CacheWriter().flush! 4', + 'new CacheWriter().write_cache 3', + 'new CacheWriter().write_cache 7', + 'self.class.new 9', + 'self.class.new(key).run 9', + ]); + }); + // A class method (`def self.x`, `def Widget.x`, a `def` in `class << self`) // is static on both backends, every other def is not: they share a // qualified name with any instance method of the name. diff --git a/__tests__/ruby-constructed-receivers.test.ts b/__tests__/ruby-constructed-receivers.test.ts new file mode 100644 index 0000000000..48a67e1147 --- /dev/null +++ b/__tests__/ruby-constructed-receivers.test.ts @@ -0,0 +1,294 @@ +/** + * Ruby calls on a typed receiver whose method name ends in `!` or `?`, and + * calls on what `.new` made (#684). + * + * `writer.flush!` and `writer.ready?` after `writer = CacheWriter.new` linked + * nothing, while `writer.write_cache` linked: the method-call pattern took no + * `!` or `?`. `CacheWriter.create!` linked nothing where `CacheWriter.create` + * did, for the same reason. + * + * `CacheWriter.new(key).write_cache` was recorded with the receiver's text, + * arguments and all, so it never linked. It is now `new CacheWriter() + * .write_cache`, and links an instance method of the class Ruby's constant + * lookup finds for `CacheWriter` where the call is written, its own or one + * its ancestry declares. A class the project doesn't declare links nothing: + * rack-protection's `Request.new(env)` is rack's `Rack::Request`, not the + * project's `Sinatra::Request`. + * + * Runs on 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/writer.rb': `class CacheWriter + def self.create(key) + new + end + + def self.create!(key) + new + end + + def write_cache(key) + key + end + + def flush! + nil + end + + def ready? + true + end +end + +class OtherWriter + def write_cache(key) + key + end + + def flush! + nil + end + + def ready? + false + end +end +`, + 'app/billing.rb': `module Billing + class Card + def charge!(amount) + amount + end + end + + class Ledger < Card + def post + Card.new.charge!(1) + end + end +end + +module Admin + class Card + def charge!(amount) + amount + end + end +end +`, + 'app/counter.rb': `class Counter + def bang + writer = CacheWriter.new + writer.flush! + end + + def question + writer = CacheWriter.new + writer.ready? + end + + def chained(key) + CacheWriter.new.write_cache(key) + end + + def chained_args(key) + CacheWriter.new(key).write_cache(key) + end + + def chained_bang + CacheWriter.new(1).flush! + end + + def namespaced + Billing::Ledger.new.charge!(2) + end + + def top_level + ::CacheWriter.new.ready? + end + + def class_bang + CacheWriter.create!(1) + end + + def outside(env) + Request.new(env).preferred_type + end +end +`, + 'lib/rack/show_exceptions.rb': `module Rack + module Protection + class ShowExceptions + def call(env) + Request.new(env).preferred_type("text/html") + end + end + end +end +`, + 'lib/sinatra/request.rb': `module Sinatra + class Request + def preferred_type(*types) + types.first + end + end +end +`, + // A module a call returns is no mixin of the constant the call starts at: + // UrlForHelper doesn't get RouteSet's methods (actionpack's url_for tests). + 'app/url_helper.rb': `class RouteSet + def url_for(options) + options + end + + def url_helpers + Module.new + end +end + +class UrlForHelper + include RouteSet.new.url_helpers +end + +class UrlForTest + def test_url + UrlForHelper.new.url_for(1) + end +end +`, + // Two project methods named `key?`: a `?` call on an untyped receiver is not + // guessed between them by the receiver's words. + 'lib/options.rb': `class InheritableOptions + def key?(key) + true + end +end + +class FlashHash + def key?(name) + false + end +end + +class Store + def merged(options) + options.key?(:namespace) + end +end +`, +}; + +let root = ''; +let cg: CodeGraph | undefined; +let savedKernel: string | undefined; + +afterEach(() => { + cg?.close(); + cg = undefined; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = ''; + if (savedKernel === undefined) delete process.env.CODEGRAPH_KERNEL; + else process.env.CODEGRAPH_KERNEL = savedKernel; +}); + +function write(files: Record): void { + 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); + } +} + +async function index(backend: string, files: Record = FILES): Promise { + savedKernel = 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-made-')); + write(files); + cg = await CodeGraph.init(root, { index: true }); + return cg; +} + +/** `:` of every `calls` edge written on `line` of `file`. */ +function callsAt(graph: CodeGraph, file: string, line: number): string[] { + const ids = graph.getNodesInFile(file).map((n) => n.id); + return graph + .getOutgoingEdgesFrom(ids) + .filter((e) => e.kind === 'calls' && e.line === line) + .map((e) => { + const t = graph.getNode(e.target)!; + return `${t.qualifiedName}:${t.startLine}`; + }) + .sort(); +} + +const backends = [ + ['kernel', getKernel() === null], + ['wasm', false], +] as const; + +describe.each(backends)('Ruby bang and question methods, and calls on .new (%s, #684)', (backend, skip) => { + it.skipIf(skip)('links a `!` or `?` method called on a typed local', async () => { + const graph = await index(backend); + expect(callsAt(graph, 'app/counter.rb', 4)).toEqual(['CacheWriter::flush!:14']); + expect(callsAt(graph, 'app/counter.rb', 9)).toEqual(['CacheWriter::ready?:18']); + }); + + it.skipIf(skip)('links a `!` class method', async () => { + const graph = await index(backend); + expect(callsAt(graph, 'app/counter.rb', 33)).toEqual(['CacheWriter::create!:6']); + }); + + it.skipIf(skip)('links a call on what `.new` made, with or without arguments', async () => { + const graph = await index(backend); + expect(callsAt(graph, 'app/counter.rb', 13)).toEqual(['CacheWriter::write_cache:10']); + expect(callsAt(graph, 'app/counter.rb', 17)).toEqual(['CacheWriter::write_cache:10']); + expect(callsAt(graph, 'app/counter.rb', 21)).toEqual(['CacheWriter::flush!:14']); + expect(callsAt(graph, 'app/counter.rb', 29)).toEqual(['CacheWriter::ready?:18']); + }); + + it.skipIf(skip)('finds the class by Ruby constant lookup, and the method in its ancestry', async () => { + const graph = await index(backend); + // `Card` inside `Billing::Ledger` is Billing::Card, not Admin::Card. + expect(callsAt(graph, 'app/billing.rb', 10)).toEqual(['Billing::Card::charge!:3']); + // Billing::Ledger has no `charge!`; its superclass Billing::Card does. + expect(callsAt(graph, 'app/counter.rb', 25)).toEqual(['Billing::Card::charge!:3']); + }); + + it.skipIf(skip)('links nothing for a class the project does not declare', async () => { + const graph = await index(backend); + expect(callsAt(graph, 'app/counter.rb', 37)).toEqual([]); + expect(callsAt(graph, 'lib/rack/show_exceptions.rb', 5)).toEqual([]); + }); + + it.skipIf(skip)("finds no method in a module a call returns, which the class's ancestry doesn't name", async () => { + const graph = await index(backend); + expect(callsAt(graph, 'app/url_helper.rb', 17)).toEqual([]); + }); + + it.skipIf(skip)('guesses no `?` method by the receiver word among several of the name', async () => { + const graph = await index(backend); + // `options` shares a word with InheritableOptions, but a Hash answers `key?` too. + expect(callsAt(graph, 'lib/options.rb', 15)).toEqual([]); + }); +}); + +describe('Ruby call on .new, after a sync (#684)', () => { + it('links the call once a sync adds the class it constructs', async () => { + const { 'app/writer.rb': writer, ...rest } = FILES; + const graph = await index('default', rest); + expect(callsAt(graph, 'app/counter.rb', 17)).toEqual([]); + + write({ 'app/writer.rb': writer! }); + expect((await graph.sync()).filesAdded).toBe(1); + // counter.rb did not change: only the retry of its parked refs links it. + expect(callsAt(graph, 'app/counter.rb', 17)).toEqual(['CacheWriter::write_cache:10']); + }, 60_000); +}); diff --git a/codegraph-kernel/src/ruby.rs b/codegraph-kernel/src/ruby.rs index 96e004e59a..cf568ef218 100644 --- a/codegraph-kernel/src/ruby.rs +++ b/codegraph-kernel/src/ruby.rs @@ -212,6 +212,21 @@ impl<'t> Walker<'t> { fn text(&self, node: Node) -> &'t str { &self.src[node.byte_range()] } + /// rubyConstructedClass (tree-sitter.ts): the class a call receiver + /// constructs, as written — `Foo` for `Foo.new(key)`, `Billing::Card` for + /// `Billing::Card.new`, `::Foo` for `::Foo.new`; None for any other + /// receiver, a `new` on a value (`klass.new`) among them. + fn constructed_class(&self, receiver: Node) -> Option<&'t str> { + if receiver.kind() != "call" { + return None; + } + let method = receiver.child_by_field_name("method")?; + let made = receiver.child_by_field_name("receiver")?; + if self.text(method) != "new" || !is_constant_path(made) { + return None; + } + Some(self.text(made)) + } fn line_of(&self, node: Node) -> u32 { node.start_position().row as u32 + 1 } @@ -826,6 +841,10 @@ impl<'t> Walker<'t> { let skip = matches!(receiver_name, "self" | "super"); let callee = if skip { method_name.to_string() + } else if let Some(class_name) = self.constructed_class(receiver) { + // A call on what `Foo.new(…)` made: `new Foo().m`, not the + // receiver's text with its arguments in it (#684). + format!("new {class_name}().{method_name}") } else { format!("{receiver_name}.{method_name}") }; diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 16678aa5eb..6afc7ba652 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -336,6 +336,20 @@ function isRubyConstantPath(node: SyntaxNode): boolean { return getChildByField(node, 'name')?.type === 'constant' && (!scope || isRubyConstantPath(scope)); } +/** + * The class a Ruby call receiver constructs, as written: `Foo` for + * `Foo.new(key)`, `Billing::Card` for `Billing::Card.new`, `::Foo` for + * `::Foo.new`. Null for any other receiver, a `new` on a value (`klass.new`) + * among them. + */ +function rubyConstructedClass(receiver: SyntaxNode, source: string): string | null { + if (receiver.type !== 'call') return null; + const method = getChildByField(receiver, 'method'); + const made = getChildByField(receiver, 'receiver'); + if (!method || getNodeText(method, source) !== 'new' || !made || !isRubyConstantPath(made)) return null; + return getNodeText(made, source); +} + /** True when `node` is (transitively) inside a C function body — i.e. a local, * not a file/namespace-scope declaration. Walks the parent chain to the root. */ function hasFunctionAncestor(node: SyntaxNode): boolean { @@ -5599,9 +5613,14 @@ export class TreeSitterExtractor { } const SKIP_RECEIVERS = new Set(['self', 'super']); const skip = SKIP_RECEIVERS.has(receiverName); + // A call on what `Foo.new(…)` / `Billing::Card.new` made is a call on an + // instance of that class: recorded as `new Foo().bar`, the shape a + // constructed receiver has in every language, rather than the receiver's + // text with its arguments in it (#684). + const constructed = rubyConstructedClass(receiverNode, this.source); this.unresolvedReferences.push({ fromNodeId: callerId, - referenceName: skip ? methodName : `${receiverName}.${methodName}`, + referenceName: skip ? methodName : constructed ? `new ${constructed}().${methodName}` : `${receiverName}.${methodName}`, referenceKind: 'calls', line, column, diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index d1fe6f7dfa..19a07d2578 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -8397,7 +8397,9 @@ export function rubyAncestry(qn: string, context: ResolutionContext): Set 1 || (targetMethods.length === 1 && (scopedOut || pythonChain) && !narrowed)) { + // Multiple methods: score by receiver name word overlap with class name. + // Not for a Ruby name ending in `!` or `?` called on a value (#684): + // `key?`, `empty?`, `respond_to?`, `is_a?` and `merge!` are what the core + // classes answer on nearly every receiver, and the receiver's words can't + // tell which project method of the name, if any, is meant: activesupport's + // `options.key?` on a Hash went to `InheritableOptions#key?`, actionpack's + // `body.respond_to?` to `RackBody#respond_to?`. A constant receiver + // (`MarshalWithFallback.dumped?`) still names its owner. + const rubyBangOrPredicate = ref.language === 'ruby' && /[!?]$/.test(methodName!) && /^[a-z_]/.test(receiverLink(objectOrClass!)); + if (!rubyBangOrPredicate && + (targetMethods.length > 1 || (targetMethods.length === 1 && (scopedOut || pythonChain) && !narrowed))) { // What the receiver is named after is its last link: `builder.tokeniser` // is a Tokeniser, `table.Columns` no kind of table. In a python chain the // last link as written: Django's `request.META` and `request.GET` are diff --git a/src/resolution/ruby-constant-paths.ts b/src/resolution/ruby-constant-paths.ts index 8d6925912c..44bdb4035e 100644 --- a/src/resolution/ruby-constant-paths.ts +++ b/src/resolution/ruby-constant-paths.ts @@ -16,16 +16,25 @@ * somewhere the index can't see, never from another class that declares one. * Production code never names a class a test declares. A bare constant * (`ChargeCard`) is left to the name strategies, as before. + * + * A call on what `.new` made (`ChargeCard.new(card).charge!`, recorded as + * `new ChargeCard().charge!`) resolves here too, bare constant or path: an + * instance method of the class the lookup finds, or nothing (#684). */ import type { Node } from '../types'; import type { ResolutionContext, ResolvedRef, UnresolvedRef } from './types'; import { isTestPath } from '../search/query-utils'; -import { rubyAncestry, rubyMethodsOfKindFirst } from './name-matcher'; +import { rubyAncestry, rubyMethodsOfKindFirst, type RubyMethodKind } from './name-matcher'; /** `A::B`, `::A`, `::A::B`: a constant path. Group 1 is a leading `::`. */ const CONSTANT_PATH = /^(::)?([A-Z]\w*(?:::[A-Z]\w*)*)$/; /** `A::B.m`, `::A.m`: a call on one. Group 3 is the method. */ const CONSTANT_PATH_CALL = /^(::)?([A-Z]\w*(?:::[A-Z]\w*)*)\.([A-Za-z_]\w*[?!=]?)$/; +/** + * `new Foo().m`, `new A::B().m!`: a call on what `Foo.new(…)` or `A::B.new` + * made, as the extractor records it (#684). Group 3 is the method. + */ +const CONSTRUCTED_CALL = /^new (::)?([A-Z]\w*(?:::[A-Z]\w*)*)\(\)\.([A-Za-z_]\w*[?!=]?)$/; /** What a Ruby constant is in the index: a class, a module, or a value (`RATES = {…}` is a `variable`). */ const NAMESPACE_KINDS: ReadonlySet = new Set(['class', 'module']); @@ -48,6 +57,17 @@ export function matchRubyConstantPathRef(ref: UnresolvedRef, context: Resolution return target ? { original: ref, targetNodeId: target.id, confidence: 0.9, resolvedBy: 'qualified-name' } : null; } if (ref.referenceKind === 'calls') { + // A call on an instance `Foo.new(…)` made, bare or namespaced: an instance + // method of the class Ruby's lookup finds for `Foo` there, its own or one + // its ancestry declares — never a namesake's by the method name alone. A + // class the project doesn't declare (rack-protection's `Request.new(env)` + // is rack's) has no method here. + const made = CONSTRUCTED_CALL.exec(ref.referenceName); + if (made) { + const cls = constantsNamed(made[2]!, made[1] !== undefined, ref, context)[0]; + const method = cls?.kind === 'class' ? methodOf(cls.qualifiedName, made[3]!, ref, context, 'instance') : null; + return method ? { original: ref, targetNodeId: method.node.id, confidence: method.own ? 0.9 : 0.85, resolvedBy: 'instance-method' } : null; + } const m = CONSTANT_PATH_CALL.exec(ref.referenceName); if (!m || !isNamespaced(m)) return undefined; const owner = constantsNamed(m[2]!, m[1] !== undefined, ref, context)[0]; @@ -98,16 +118,23 @@ function lexicalScope(ref: UnresolvedRef, context: ResolutionContext): string { /** * The method `name` of the class or module `owner`: its own, else the first * of its superclasses and mixed-in modules that declares one (rubyAncestry). - * Of a class's `def self.m` and `def m`, which share one qualified name, the - * call on the class reaches the class method. Production code never reaches - * one a test declares. + * Of a class's `def self.m` and `def m`, which share one qualified name, a + * call on the class reaches the class method, and a call on an instance + * (`kind` 'instance') the instance method. Production code never reaches one + * a test declares. */ -function methodOf(owner: string, name: string, ref: UnresolvedRef, context: ResolutionContext): { node: Node; own: boolean } | null { +function methodOf( + owner: string, + name: string, + ref: UnresolvedRef, + context: ResolutionContext, + kind: RubyMethodKind = 'class', +): { node: Node; own: boolean } | null { const production = !isTestPath(ref.filePath); for (const qn of rubyAncestry(owner, context)) { const methods = context.getNodesByQualifiedName(`${qn}::${name}`).filter((n) => n.kind === 'method' && n.language === 'ruby' && !(production && isTestPath(n.filePath))); - if (methods.length > 0) return { node: rubyMethodsOfKindFirst(ordered(methods, ref), 'class')[0]!, own: qn === owner }; + if (methods.length > 0) return { node: rubyMethodsOfKindFirst(ordered(methods, ref), kind)[0]!, own: qn === owner }; } return null; }