mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-15 03:08:20 +08:00
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 <noreply@anthropic.com>
295 lines
7.8 KiB
TypeScript
295 lines
7.8 KiB
TypeScript
/**
|
|
* 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<string, string> = {
|
|
'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<string, string>): 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<string, string> = FILES): Promise<CodeGraph> {
|
|
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;
|
|
}
|
|
|
|
/** `<qualified name>:<line>` 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);
|
|
});
|