mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-29 06:46:49 +08:00
fix(ruby): a call reaches the class method or the instance method its receiver calls (#2834)
A Ruby class method (`def self.build`, `def Widget.build`, or a def inside
`class << self`) was indexed exactly like an instance method: is_static 0,
under the same qualified name as an instance method of that name
(Widget::build twice). So every call took whichever of the two the class
declared first. Widget.build could reach `def build`, and widget.build on an
instance could reach `def self.build`. In the repro for issue 2503,
`card = Billing::ChargeCard.new(...); card.call` linked to `def self.call`
rather than `def call`.
Fix:
- Extraction, both engines: the Ruby isStatic hook (isRubyClassMethod in
languages/ruby.ts, is_class_method in codegraph-kernel/src/ruby.rs) marks a
singleton_method, or a method whose parent past one body_statement is a
singleton_class, as static. Every other def is marked not static, the same
way on both, so the graph only changes for class methods.
- Resolution, Ruby only, where the call says which kind it is:
rubyMethodsOfKindFirst orders a call's candidates with the wanted kind
first. A call on a constant (Strategy 1, its inherited walk, and the
namespaced path in ruby-constant-paths.ts) wants the class method. A
receiver typed by `x = Widget.new` (resolveMethodOnType) wants the
instance method. A bare call splits only a real pair, one qualified name
among its candidates as both kinds, keeping the caller's kind: a block
may run on another object, so nothing else moves. A receiver known only
by its name (Strategies 2 and 3) says nothing of its kind and is left as
it was: rubocop's `cop.cop_name` runs over cop classes. With only one
kind present, every path picks what it did before.
Validation:
- New __tests__/ruby-singleton-methods.test.ts (6 cases, each on the
kernel and on wasm) and a parity case in kernel-ruby-parity.test.ts. On
main 9 of the 18 tests in the two files fail, every one that checks the
new behaviour; the guards and the old parity cases pass. All pass with
the fix.
- A/B against main f7820b58 (kernel path): nodes are unchanged except
is_static going 0 to 1 on the class methods, 167 on sinatra, 358 on
rubocop and 266 on mastodon's Ruby tree. Edges: none added or removed.
20 changed their target or confidence: sinatra 10, rubocop 0, mastodon
10. 19 are right: 13 moved from the wrong twin to the right one, such as
InlineRenderer.render to def self.render and sinatra's instance methods'
force_encoding to def force_encoding, and 6 kept their target with
higher confidence. One kept a target wrong in both arms: a call inside
sinatra's define_method block. Controls express and flask are
byte-identical.
- The A/B narrowed two rules. Preferring instance methods for a receiver
known only by its name moved rubocop's 19 right cop.cop_name edges, and
dropping other-kind methods from any bare call moved block-DSL calls on
sinatra. Both are gone: only a real pair is split.
- kernel-parity.mjs --lang ruby is the same on both arms: sinatra 147/147,
rubocop 1799/1801 (2 deferred), mastodon 3333/3333, 0 diffs. The fix
arm's kernel and wasm graphs are byte-identical on sinatra and rubocop.
- Index time, median of 3 interleaved runs: sinatra -3.7%, rubocop +0.5%,
mastodon +0.7%, controls express -2.7% and flask +1.0%.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
f7820b584f
commit
16a8e77c46
@@ -2,6 +2,20 @@ import type { Node as SyntaxNode } from 'web-tree-sitter';
|
||||
import { getNodeText, getChildByField } from '../tree-sitter-helpers';
|
||||
import type { LanguageExtractor } from '../tree-sitter-types';
|
||||
|
||||
/**
|
||||
* A class method: `def self.x` or `def Widget.x` (a singleton_method), or a
|
||||
* `def` inside `class << self`. It shares its qualified name with an instance
|
||||
* method of the same name (`Widget::build` for both), so only this tells the
|
||||
* two apart.
|
||||
*/
|
||||
function isRubyClassMethod(node: SyntaxNode): boolean {
|
||||
if (node.type === 'singleton_method') return true;
|
||||
if (node.type !== 'method') return false;
|
||||
let parent = node.parent;
|
||||
if (parent?.type === 'body_statement') parent = parent.parent;
|
||||
return parent?.type === 'singleton_class';
|
||||
}
|
||||
|
||||
export const rubyExtractor: LanguageExtractor = {
|
||||
functionTypes: ['method'],
|
||||
classTypes: ['class'],
|
||||
@@ -103,6 +117,7 @@ export const rubyExtractor: LanguageExtractor = {
|
||||
|
||||
return name;
|
||||
},
|
||||
isStatic: isRubyClassMethod,
|
||||
getVisibility: (node) => {
|
||||
// Ruby visibility is based on preceding visibility modifiers
|
||||
let sibling = node.previousNamedSibling;
|
||||
|
||||
@@ -7830,6 +7830,33 @@ function isRubyMethodInScope(method: Node, ref: UnresolvedRef, context: Resoluti
|
||||
return rubyAncestry(here.qualifiedName, context).has(method.qualifiedName.slice(0, cut));
|
||||
}
|
||||
|
||||
/**
|
||||
* Of a Ruby class's two methods of one name, `def build` and `def self.build`
|
||||
* (both `Widget::build`), the one a bare call means: a bare call is a call on
|
||||
* `self`, so in an instance method it is the instance method, and in a class
|
||||
* method (`def self.x`, `class << self`) the class method. Widget's `run`
|
||||
* calling `build` reaches `def build`, and its `def self.go` calling `build`
|
||||
* reaches `def self.build`. Only such pairs are split: inside a block that
|
||||
* runs on another object (`Module.new do … end`, a test's `it do … end`) the
|
||||
* caller's kind says nothing of `self`, and any other candidate stays as it
|
||||
* was. A call written anywhere but in a method splits nothing.
|
||||
*/
|
||||
function rubyMethodsOfCallerKind(candidates: Node[], ref: UnresolvedRef, context: ResolutionContext): Node[] {
|
||||
if (candidates.length < 2) return candidates;
|
||||
const caller = context.getNodeById?.(ref.fromNodeId);
|
||||
if (!caller || caller.kind !== 'method' || caller.language !== 'ruby') return candidates;
|
||||
const wantClass = caller.isStatic === true;
|
||||
const isRubyMethod = (n: Node): boolean => n.kind === 'method' && n.language === 'ruby';
|
||||
// Per qualified name, which kinds the candidates declare: 1 class, 2 instance.
|
||||
const kinds = new Map<string, number>();
|
||||
for (const n of candidates) {
|
||||
if (isRubyMethod(n)) kinds.set(n.qualifiedName, (kinds.get(n.qualifiedName) ?? 0) | (n.isStatic === true ? 1 : 2));
|
||||
}
|
||||
const twinOfOtherKind = (n: Node): boolean =>
|
||||
isRubyMethod(n) && kinds.get(n.qualifiedName) === 3 && (n.isStatic === true) !== wantClass;
|
||||
return candidates.some(twinOfOtherKind) ? candidates.filter((n) => !twinOfOtherKind(n)) : candidates;
|
||||
}
|
||||
|
||||
/** A Ruby class's qualified name, those of its superclasses, and of every module mixed into any of them. */
|
||||
export function rubyAncestry(qn: string, context: ResolutionContext): Set<string> {
|
||||
let memo = RUBY_ANCESTRY.get(context);
|
||||
@@ -11950,7 +11977,8 @@ export function matchByExactName(
|
||||
const ranked = dartBare ? nearestDartMembers(preferOwnDartLibrary(filtered, ref, context), ref, context)
|
||||
: dartTypeRef && dartPrefix === null ? preferOwnDartLibrary(filtered, ref, context)
|
||||
: swiftShape && swiftShape.shape !== 'chained' ? nearestSwiftMembers(filtered, ref, context)
|
||||
: kotlinBare ? lexicalKotlinMembers(filtered, ref, context) : cppSpecializationPool(filtered, ref, context);
|
||||
: kotlinBare ? lexicalKotlinMembers(filtered, ref, context)
|
||||
: rubyBare ? rubyMethodsOfCallerKind(filtered, ref, context) : cppSpecializationPool(filtered, ref, context);
|
||||
// (Above the ceiling below nothing is picked, so no translation unit is read.)
|
||||
const candidates = ranked.length <= AMBIGUOUS_NAME_CEILING ? withinCppTranslationUnit(ranked, ref, context, true) : ranked;
|
||||
|
||||
@@ -13125,7 +13153,11 @@ export function resolveMethodOnType(
|
||||
// intentionally in ANOTHER file (#314) — is unaffected: those blocks return
|
||||
// early whenever an import pins the class.
|
||||
if (ref.referenceKind === 'function_ref' && matches.length !== 1) return null;
|
||||
const ordered = preferCallSiteFile(withinCppTranslationUnit(cppVisibleFirst(matches, ref, context), ref, context), ref.filePath);
|
||||
// A Ruby receiver is typed only by `x = Widget.new`, so it is an instance:
|
||||
// `x.build` reaches `def build`, not `def self.build`, wherever each is.
|
||||
const ordered = rubyMethodsOfKindFirst(
|
||||
preferCallSiteFile(withinCppTranslationUnit(cppVisibleFirst(matches, ref, context), ref, context), ref.filePath),
|
||||
ref.language === 'ruby' ? 'instance' : undefined);
|
||||
return {
|
||||
original: ref,
|
||||
targetNodeId: ordered[0]!.id,
|
||||
@@ -18609,18 +18641,18 @@ function tsClassHead(lines: readonly string[], cls: Node): string {
|
||||
}
|
||||
|
||||
/** A method named `name` on a supertype of the given classes, nearest first. */
|
||||
function inheritedClassMethod(classes: Node[], name: string, context: ResolutionContext): Node | null {
|
||||
function inheritedClassMethod(classes: Node[], name: string, context: ResolutionContext, kind?: RubyMethodKind): Node | null {
|
||||
if (classes.length === 0) return null;
|
||||
let memo = INHERITED_METHODS.get(context);
|
||||
if (!memo) INHERITED_METHODS.set(context, (memo = new Map()));
|
||||
const key = `${classes.map((c) => c.id).join(',')}|${name}`;
|
||||
const key = `${classes.map((c) => c.id).join(',')}|${name}|${kind ?? ''}`;
|
||||
if (memo.has(key)) return memo.get(key)!;
|
||||
const found = findInheritedClassMethod(classes, name, context);
|
||||
const found = findInheritedClassMethod(classes, name, context, kind);
|
||||
memo.set(key, found);
|
||||
return found;
|
||||
}
|
||||
|
||||
function findInheritedClassMethod(classes: Node[], name: string, context: ResolutionContext): Node | null {
|
||||
function findInheritedClassMethod(classes: Node[], name: string, context: ResolutionContext, kind?: RubyMethodKind): Node | null {
|
||||
const seen = new Set<string>(classes.map((c) => c.id));
|
||||
let frontier = classes;
|
||||
for (let depth = 0; depth < 5 && frontier.length > 0; depth++) {
|
||||
@@ -18630,8 +18662,8 @@ function findInheritedClassMethod(classes: Node[], name: string, context: Resolu
|
||||
for (const decl of context.getNodesByName(sup)) {
|
||||
if (decl.language !== cls.language || !isMethodOwnerKind(decl) || seen.has(decl.id)) continue;
|
||||
seen.add(decl.id);
|
||||
const method = context.getNodesInFile(decl.filePath).find((n) => n.kind === 'method' && n.name === name &&
|
||||
n.qualifiedName.slice(0, Math.max(0, n.qualifiedName.lastIndexOf('::'))).split(/::|\./).pop() === decl.name);
|
||||
const method = rubyMethodsOfKindFirst(context.getNodesInFile(decl.filePath).filter((n) => n.kind === 'method' && n.name === name &&
|
||||
n.qualifiedName.slice(0, Math.max(0, n.qualifiedName.lastIndexOf('::'))).split(/::|\./).pop() === decl.name), kind)[0];
|
||||
if (method) return method;
|
||||
next.push(decl);
|
||||
}
|
||||
@@ -20242,7 +20274,9 @@ export function matchMethodCall(
|
||||
return { original: ref, targetNodeId: method.id, confidence: own ? 0.85 : 0.8, resolvedBy: 'qualified-name' };
|
||||
}
|
||||
|
||||
const methodNode = classStrategyMethod(classNode, methodName!, context);
|
||||
// A Ruby call on a class is a call of its class method: `Widget.build`
|
||||
// reaches `def self.build`, not `def build`, whichever comes first.
|
||||
const methodNode = classStrategyMethod(classNode, methodName!, context, ref.language === 'ruby' ? 'class' : undefined);
|
||||
|
||||
if (methodNode) {
|
||||
return {
|
||||
@@ -20255,7 +20289,8 @@ export function matchMethodCall(
|
||||
}
|
||||
// A class method the named class inherits — Horse's `THorse.Get(…)` is
|
||||
// THorseCore's, three `class(…)` heads up — before any guess by name.
|
||||
const inherited = inheritedClassMethod(classCandidates.filter((c) => c.language === ref.language), methodName!, context);
|
||||
const inherited = inheritedClassMethod(classCandidates.filter((c) => c.language === ref.language), methodName!, context,
|
||||
ref.language === 'ruby' ? 'class' : undefined);
|
||||
if (inherited) return { original: ref, targetNodeId: inherited.id, confidence: 0.8, resolvedBy: 'qualified-name' };
|
||||
return null;
|
||||
});
|
||||
@@ -20344,6 +20379,10 @@ export function matchMethodCall(
|
||||
if (!isLexicallyReachable(classNode, ref, context)) continue;
|
||||
if (productionCaller && isTestPath(classNode.filePath)) continue;
|
||||
|
||||
// A Ruby receiver named after a class may hold the class as well as an
|
||||
// instance of it (rubocop's `cop.cop_name` runs over its registry's
|
||||
// cop classes, so it is `def self.cop_name`), so neither of a class's
|
||||
// two methods of the name is preferred: the first declared is taken.
|
||||
const methodNode = classStrategyMethod(classNode, methodName!, context);
|
||||
|
||||
if (methodNode) {
|
||||
@@ -25688,9 +25727,12 @@ function tsFieldCall(
|
||||
* the caller's loop, so a nested `Outer::Logger` can take a top-level
|
||||
* `Logger.log()` (declared first, or the only one with the method). Choosing
|
||||
* among them needs each language's scope rules.
|
||||
*
|
||||
* `kind` picks between a Ruby class's two methods of the name, `def self.m`
|
||||
* and `def m`, which share one qualified name (rubyMethodsOfKindFirst).
|
||||
*/
|
||||
function findOwnMethod(nodesInFile: Node[], classNode: Node, methodName: string): Node | undefined {
|
||||
const methods = nodesInFile.filter((n) => n.kind === 'method' && n.name === methodName);
|
||||
function findOwnMethod(nodesInFile: Node[], classNode: Node, methodName: string, kind?: RubyMethodKind): Node | undefined {
|
||||
const methods = rubyMethodsOfKindFirst(nodesInFile.filter((n) => n.kind === 'method' && n.name === methodName), kind);
|
||||
const exact = `${classNode.qualifiedName}::${methodName}`;
|
||||
const byName = `${classNode.name}::${methodName}`;
|
||||
return (
|
||||
@@ -25706,9 +25748,30 @@ function findOwnMethod(nodesInFile: Node[], classNode: Node, methodName: string)
|
||||
* `Form` reaches `BaseForm::is_valid`, which the substring test this replaced
|
||||
* only found because `BaseForm` contains `Form`.
|
||||
*/
|
||||
function classStrategyMethod(classNode: Node, methodName: string, context: ResolutionContext): Node | undefined {
|
||||
return findOwnMethod(context.getNodesInFile(classNode.filePath), classNode, methodName) ??
|
||||
inheritedClassMethod([classNode], methodName, context) ?? undefined;
|
||||
function classStrategyMethod(classNode: Node, methodName: string, context: ResolutionContext, kind?: RubyMethodKind): Node | undefined {
|
||||
return findOwnMethod(context.getNodesInFile(classNode.filePath), classNode, methodName, kind) ??
|
||||
inheritedClassMethod([classNode], methodName, context, kind) ?? undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Which of a Ruby class's two methods of one name a call reaches: a call on
|
||||
* the class (`Widget.build`) its class method, `def self.build` or one in
|
||||
* `class << self`; a call on an instance (`widget.build`) its instance
|
||||
* method, `def build`. Both share one qualified name, `Widget::build`, and
|
||||
* only `isStatic` tells them apart.
|
||||
*/
|
||||
export type RubyMethodKind = 'class' | 'instance';
|
||||
|
||||
/**
|
||||
* `methods` with the ones of `kind` first, in their own order: a pick of the
|
||||
* first then takes one of the kind wherever there is one, and still the
|
||||
* first otherwise. Methods of other languages are never static-split.
|
||||
*/
|
||||
export function rubyMethodsOfKindFirst<T extends Node>(methods: T[], kind: RubyMethodKind | undefined): T[] {
|
||||
if (!kind || methods.length < 2) return methods;
|
||||
const fits = (m: T): boolean => m.language === 'ruby' && (m.isStatic === true) === (kind === 'class');
|
||||
const first = methods.filter(fits);
|
||||
return first.length === 0 || first.length === methods.length ? methods : [...first, ...methods.filter((m) => !fits(m))];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -20,7 +20,7 @@
|
||||
import type { Node } from '../types';
|
||||
import type { ResolutionContext, ResolvedRef, UnresolvedRef } from './types';
|
||||
import { isTestPath } from '../search/query-utils';
|
||||
import { rubyAncestry } from './name-matcher';
|
||||
import { rubyAncestry, rubyMethodsOfKindFirst } 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*)*)$/;
|
||||
@@ -98,14 +98,16 @@ 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).
|
||||
* Production code never reaches one a test declares.
|
||||
* 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.
|
||||
*/
|
||||
function methodOf(owner: string, name: string, ref: UnresolvedRef, context: ResolutionContext): { 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: ordered(methods, ref)[0]!, own: qn === owner };
|
||||
if (methods.length > 0) return { node: rubyMethodsOfKindFirst(ordered(methods, ref), 'class')[0]!, own: qn === owner };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user