mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-25 12:00:02 +08:00
fix(resolution): a lone same-file definition scores as a strong match
matchByExactName scored a winner among several same-named candidates by directory proximity alone: 0.7 when the files share two folders, else 0.4. findBestMatch already ranks the definition in the reference's own file first, so the target was right, but a shallow file's own function read as an uncertain guess (below 0.6), and because the resolver keeps the most confident strategy, the same-file definition lost to a framework's naming guess in another file (ASP.NET's PascalCase and Service rules at 0.7 and 0.85). isDecisiveSameFileMatch now scores it 0.9, as a lone candidate, when it is the only candidate in the file, is not the caller itself, and, as a member of a type, module or namespace, belongs to one the caller is in. The PR checked that last rule for methods only. We extended it to every member kind whose owner the file declares, other than a function, so a sibling enum's entry or a sibling class's field is not raised. Validation, main vs this commit, 21 repos in 11 languages (flask, click, fastapi, gin, cobra, etcd, ripgrep, jq, Slim, serilog, eShopOnWeb, nowinandroid, okhttp, http4s, playframework, spring-modulith, express, zod, bulletproof-react, excalidraw, vben): - no edge added or removed; - 9 targets moved, all in eShopOnWeb and all right: C# properties and fields named after their type that the code reads (Color Color), which ASP.NET's naming rule had linked to the class in another project; - 6,927 edges keep their target and gain confidence, 535 of them from 0.4; in 874 a framework rule's 0.7-0.85 now loses to the same target by exact name; 30 end in another file, Go type positions the type gate moves to the package's own type; - as written, the PR also raised 77 edges onto another type's member to 0.9, about 48 of them wrong (okhttp's InterceptorOverridesTest enum entries, playframework's sibling providers' `get`); they keep main's confidence. Index CPU, median of interleaved runs: okhttp 32.3 vs 32.4 s, etcd 33.4 vs 33.1 s. Tests: 3 of 7 fail on main; the PR as written fails the sibling-member case. js-class-expressions pinned the old 0.4 for `this.run()` reaching the class's own `run` in a shallow file; it now expects 0.9. Original PR #1938 by @danusha2345. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Colby Mchenry
co-authored by
Claude Opus 5.5
parent
0b238452a3
commit
cde4cd74eb
@@ -209,6 +209,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
|||||||
- In C++, a variable initialized from a call of a member function template with explicit template arguments now has the type that template returns for them, so calls on it link to that class's methods: protocolbuffers/protobuf's `const auto& rep = reflection->GetRaw<RepeatedPtrField<…>>(…)` is a `RepeatedPtrField`, and facebook/rocksdb's `auto bbto = factory->GetOptions<BlockBasedTableOptions>()` a `BlockBasedTableOptions`. Re-index C++ projects after upgrading.
|
- In C++, a variable initialized from a call of a member function template with explicit template arguments now has the type that template returns for them, so calls on it link to that class's methods: protocolbuffers/protobuf's `const auto& rep = reflection->GetRaw<RepeatedPtrField<…>>(…)` is a `RepeatedPtrField`, and facebook/rocksdb's `auto bbto = factory->GetOptions<BlockBasedTableOptions>()` a `BlockBasedTableOptions`. Re-index C++ projects after upgrading.
|
||||||
- Framework support now switches on when an app's `package.json` sits three or more folders deep, as with an ASP.NET solution's Angular or React app in `src/Web/ClientApp/`, or a React app in a workspace folder like `web/ui/mantine-ui/`. Before, only the project root and the first two folder levels were read, so frameworks such as Angular, React Router, Vue Router, SvelteKit and TanStack Router could go undetected there: an Angular app's routes, its `routerLink` and `router.navigate(…)` navigation and its template's event bindings were missing, so a handler used only from a `(click)` binding had no callers, and a React app's `navigate('/login')` calls didn't reach their route. Re-index projects laid out this way after upgrading.
|
- Framework support now switches on when an app's `package.json` sits three or more folders deep, as with an ASP.NET solution's Angular or React app in `src/Web/ClientApp/`, or a React app in a workspace folder like `web/ui/mantine-ui/`. Before, only the project root and the first two folder levels were read, so frameworks such as Angular, React Router, Vue Router, SvelteKit and TanStack Router could go undetected there: an Angular app's routes, its `routerLink` and `router.navigate(…)` navigation and its template's event bindings were missing, so a handler used only from a `(click)` binding had no callers, and a React app's `navigate('/login')` calls didn't reach their route. Re-index projects laid out this way after upgrading.
|
||||||
- In a repository that mixes languages, a web framework's naming conventions now link only code written in that framework's own languages. Before, Express's also linked Go and Python code: in etcd a Go method's `*zap.Logger` result type linked to the method itself, and in Harbor a Python `self.jaeger.validate()` call linked to another class's `validate`. Links that cross languages on purpose, like React Native and Expo native modules, Swift and Objective-C, Drupal's `routing.yml`, SvelteKit's `$lib/…` imports in `.ts` files and a Razor page's `@model`, are unchanged.
|
- In a repository that mixes languages, a web framework's naming conventions now link only code written in that framework's own languages. Before, Express's also linked Go and Python code: in etcd a Go method's `*zap.Logger` result type linked to the method itself, and in Harbor a Python `self.jaeger.validate()` call linked to another class's `validate`. Links that cross languages on purpose, like React Native and Expo native modules, Swift and Objective-C, Drupal's `routing.yml`, SvelteKit's `$lib/…` imports in `.ts` files and a Razor page's `@model`, are unchanged.
|
||||||
|
- A name that only one definition in its own file can mean, like a call to a function the file defines, is no longer marked as an uncertain match when the file sits near the top of the project, and it now wins over a framework's weaker same-named guess in another file: in an ASP.NET app, `ToastService.OnShow += …` now reads the component's own `ToastService` property instead of linking to the `ToastService` class. Thanks @danusha2345.
|
||||||
- In C and C++, a call written from the global scope, like `::leveldb::RepairDB(…)` or `::hpb::CreateMessage(…)`, now links to the function of exactly that name, and one through a namespace alias, like protobuf's `::_pbi::…` calls, to the function in the aliased namespace. Before, these calls were dropped unless the repository also held Swift and Objective-C files, and even then a name spelled out in full, like `::google::protobuf::internal::TSanWrite(…)`, linked to nothing, while `::operator delete(…)` linked to one class's own `operator delete`. A global call never links to a function or method of the same name inside a namespace or class, such as a wrapper `open()` that calls the C library's `::open(…)`, and `::std::…` and `::testing::…` calls stay unlinked unless that library's source is part of your project. Re-index C and C++ projects after upgrading.
|
- In C and C++, a call written from the global scope, like `::leveldb::RepairDB(…)` or `::hpb::CreateMessage(…)`, now links to the function of exactly that name, and one through a namespace alias, like protobuf's `::_pbi::…` calls, to the function in the aliased namespace. Before, these calls were dropped unless the repository also held Swift and Objective-C files, and even then a name spelled out in full, like `::google::protobuf::internal::TSanWrite(…)`, linked to nothing, while `::operator delete(…)` linked to one class's own `operator delete`. A global call never links to a function or method of the same name inside a namespace or class, such as a wrapper `open()` that calls the C library's `::open(…)`, and `::std::…` and `::testing::…` calls stay unlinked unless that library's source is part of your project. Re-index C and C++ projects after upgrading.
|
||||||
- In Go, an interface that embeds another, like gin's `IRouter` embedding `IRoutes`, now links to it, so `codegraph_explore` and impact show the embedded interface as its base. Before, every embedded interface was dropped. An embedded field written through another package of your project, like `*refresh.Discovery` or `storage.Appender`, or with type arguments, like `List[int]`, now links too. One from the standard library or another module, like `io.Closer` or `sync.Mutex`, links to nothing rather than to a project type that shares its name, and a type-set constraint like `~int | ~float64` is not an embedding. An embedded type written without a package now always links to its own package's type: before, etcd's `Client`, which embeds its own `Lease` interface, was linked to a `Lease` struct in another package. Re-index Go projects after upgrading.
|
- In Go, an interface that embeds another, like gin's `IRouter` embedding `IRoutes`, now links to it, so `codegraph_explore` and impact show the embedded interface as its base. Before, every embedded interface was dropped. An embedded field written through another package of your project, like `*refresh.Discovery` or `storage.Appender`, or with type arguments, like `List[int]`, now links too. One from the standard library or another module, like `io.Closer` or `sync.Mutex`, links to nothing rather than to a project type that shares its name, and a type-set constraint like `~int | ~float64` is not an embedding. An embedded type written without a package now always links to its own package's type: before, etcd's `Client`, which embeds its own `Lease` interface, was linked to a `Lease` struct in another package. Re-index Go projects after upgrading.
|
||||||
- In Go, working out which structs implement an interface now counts the methods of the interfaces it embeds, and the methods a struct gets from the types it embeds. Before, only each type's own methods counted: etcd's `AuthReadTx`, which embeds `UnsafeAuthReader` and adds `RLock` and `RUnlock`, was listed as implemented by every read-write lock in the project, an interface that only embeds others had no implementations at all, and a struct that gets its methods from an embedded base, like prometheus's service discoveries embedding `refresh.Discovery` or gin's `Engine` embedding `RouterGroup`, was missing. `codegraph_explore`, impact and the type hierarchy now list the right implementations, and a call through the interface reaches a method the embedding struct overrides, like gin's `Engine.Use`. Re-index Go projects after upgrading.
|
- In Go, working out which structs implement an interface now counts the methods of the interfaces it embeds, and the methods a struct gets from the types it embeds. Before, only each type's own methods counted: etcd's `AuthReadTx`, which embeds `UnsafeAuthReader` and adds `RLock` and `RUnlock`, was listed as implemented by every read-write lock in the project, an interface that only embeds others had no implementations at all, and a struct that gets its methods from an embedded base, like prometheus's service discoveries embedding `refresh.Discovery` or gin's `Engine` embedding `RouterGroup`, was missing. `codegraph_explore`, impact and the type hierarchy now list the right implementations, and a call through the interface reaches a method the embedding struct overrides, like gin's `Engine.Use`. Re-index Go projects after upgrading.
|
||||||
|
|||||||
@@ -175,7 +175,7 @@ describe.each(['js', 'ts'])('a class expression\'s name reaches it from inside a
|
|||||||
|
|
||||||
expect(edgesFrom(graph, `src/widgets.${ext}`)).toEqual([
|
expect(edgesFrom(graph, `src/widgets.${ext}`)).toEqual([
|
||||||
'calls Panel::PanelImpl::go -> Base::run [instance-method 0.9]',
|
'calls Panel::PanelImpl::go -> Base::run [instance-method 0.9]',
|
||||||
'calls Widget::extra -> Widget::run [exact-match 0.4]',
|
'calls Widget::extra -> Widget::run [exact-match 0.9]',
|
||||||
'calls Widget::run -> Base::run [instance-method 0.9]',
|
'calls Widget::run -> Base::run [instance-method 0.9]',
|
||||||
`calls src/widgets.${ext} -> register [import 0.9]`,
|
`calls src/widgets.${ext} -> register [import 0.9]`,
|
||||||
'extends Panel::PanelImpl -> Base [import 0.9]',
|
'extends Panel::PanelImpl -> Base [import 0.9]',
|
||||||
|
|||||||
@@ -0,0 +1,123 @@
|
|||||||
|
/**
|
||||||
|
* Confidence of an exact-name match the call site's own file decided.
|
||||||
|
*
|
||||||
|
* With several same-named candidates, the name-matcher ranks the one in the
|
||||||
|
* call site's file first, then scored the winner by DIRECTORY proximity — the
|
||||||
|
* number of leading folders its path shares with the caller's. A same-file
|
||||||
|
* winner shares every folder, but a shallow file has few to share, so
|
||||||
|
* `scripts/x.py` calling its own `main` got 0.4, the score of a guess across
|
||||||
|
* unrelated modules. A lone same-file definition now scores like a lone
|
||||||
|
* candidate; the matcher's pick itself never changes, and a pick the file
|
||||||
|
* does not settle (two same-file candidates, a sibling class's method, the
|
||||||
|
* caller itself) keeps the proximity score.
|
||||||
|
*/
|
||||||
|
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||||
|
import * as fs from 'node:fs';
|
||||||
|
import * as path from 'node:path';
|
||||||
|
import * as os from 'node:os';
|
||||||
|
import { CodeGraph } from '../src';
|
||||||
|
|
||||||
|
describe('same-file exact-match confidence', () => {
|
||||||
|
let dir: string;
|
||||||
|
beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'same-file-conf-')); });
|
||||||
|
afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); });
|
||||||
|
|
||||||
|
const write = (rel: string, body: string) => {
|
||||||
|
const p = path.join(dir, rel);
|
||||||
|
fs.mkdirSync(path.dirname(p), { recursive: true });
|
||||||
|
fs.writeFileSync(p, body);
|
||||||
|
};
|
||||||
|
|
||||||
|
type CallEdge = { src: string; tgt: string; tgtQn: string; tgtFile: string; confidence: number };
|
||||||
|
|
||||||
|
const calls = async (kind = 'calls'): Promise<CallEdge[]> => {
|
||||||
|
const cg = await CodeGraph.init(dir, { silent: true });
|
||||||
|
await cg.indexAll();
|
||||||
|
const db = (cg as any).db.db;
|
||||||
|
const rows: CallEdge[] = db
|
||||||
|
.prepare(
|
||||||
|
`SELECT s.name src, t.name tgt, t.qualified_name tgtQn, t.file_path tgtFile,
|
||||||
|
json_extract(e.metadata, '$.confidence') confidence
|
||||||
|
FROM edges e
|
||||||
|
JOIN nodes s ON s.id = e.source
|
||||||
|
JOIN nodes t ON t.id = e.target
|
||||||
|
WHERE e.kind = ?`
|
||||||
|
)
|
||||||
|
.all(kind);
|
||||||
|
cg.close?.();
|
||||||
|
return rows;
|
||||||
|
};
|
||||||
|
|
||||||
|
it('scores a shallow-path call to its own file as a strong match', async () => {
|
||||||
|
write('x.py', 'def helper():\n return 1\n\ndef main():\n return helper()\n');
|
||||||
|
write('pkg/other.py', 'def helper():\n return 2\n');
|
||||||
|
const edge = (await calls()).find((e) => e.src === 'main' && e.tgt === 'helper');
|
||||||
|
expect(edge?.tgtFile).toBe('x.py');
|
||||||
|
expect(edge?.confidence).toBe(0.9);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps the same-file target when a sibling file defines the name too', async () => {
|
||||||
|
write('a/b/c/x.py', 'def helper():\n return 1\n\ndef main():\n return helper()\n');
|
||||||
|
write('a/b/c/y.py', 'def helper():\n return 2\n');
|
||||||
|
const edge = (await calls()).find((e) => e.src === 'main' && e.tgt === 'helper');
|
||||||
|
expect(edge?.tgtFile).toBe('a/b/c/x.py');
|
||||||
|
expect(edge?.confidence).toBe(0.9);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves the proximity score when the file holds several candidates', async () => {
|
||||||
|
// Both branches of an `#ifdef` define `helper`: line distance picks one.
|
||||||
|
write(
|
||||||
|
'x.c',
|
||||||
|
'#ifdef FAST\nstatic int helper(void) { return 1; }\n#else\nstatic int helper(void) { return 2; }\n#endif\n' +
|
||||||
|
'int main(void) { return helper(); }\n'
|
||||||
|
);
|
||||||
|
write('lib/other.c', 'int helper(void) { return 3; }\n');
|
||||||
|
const edge = (await calls()).find((e) => e.src === 'main' && e.tgt === 'helper');
|
||||||
|
expect(edge?.tgtFile).toBe('x.c');
|
||||||
|
expect(edge?.confidence).toBe(0.4);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves the proximity score when the match is the caller itself', async () => {
|
||||||
|
// Recursion, or a receiver the call lost (`json.dumps` inside `dumps`).
|
||||||
|
write('x.py', 'def walk(n):\n return walk(n - 1)\n');
|
||||||
|
write('pkg/other.py', 'def walk(n):\n return 2\n');
|
||||||
|
const edge = (await calls()).find((e) => e.src === 'walk' && e.tgt === 'walk');
|
||||||
|
expect(edge?.tgtFile).toBe('x.py');
|
||||||
|
expect(edge?.confidence).toBe(0.4);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves the proximity score for a sibling class's method", async () => {
|
||||||
|
// The other `run` is one the file includes: C++ sees no other file's (#2684).
|
||||||
|
write('x.cpp', '#include "pkg/other.h"\nstruct A {\n int run() { return 1; }\n};\nint go() { return run(); }\n');
|
||||||
|
write('pkg/other.h', 'inline int run() { return 2; }\n');
|
||||||
|
const edge = (await calls()).find((e) => e.src === 'go' && e.tgt === 'run');
|
||||||
|
expect(edge?.tgtQn).toBe('A::run');
|
||||||
|
expect(edge?.confidence).toBe(0.4);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("outranks a framework's naming guess that sends the name to another file", async () => {
|
||||||
|
// ASP.NET's naming rule takes `ToastService` for the class in Services/,
|
||||||
|
// but the component reads its own property named after that class, as
|
||||||
|
// eShopOnWeb's Blazor ToastComponent does (C#'s "Color Color").
|
||||||
|
write('Startup.cs', 'namespace App { public class Startup { } }\n');
|
||||||
|
write('Services/ToastService.cs', 'namespace App.Services {\n public class ToastService { public event System.Action OnShow; }\n}\n');
|
||||||
|
write(
|
||||||
|
'Helpers/ToastComponent.cs',
|
||||||
|
'using App.Services;\nnamespace App.Helpers {\n public class ToastComponent {\n ToastService ToastService { get; set; }\n' +
|
||||||
|
' void OnInitialized() { ToastService.OnShow += Show; }\n void Show() { }\n }\n}\n'
|
||||||
|
);
|
||||||
|
const edge = (await calls('references')).find((e) => e.src === 'OnInitialized' && e.tgt === 'ToastService');
|
||||||
|
expect(edge?.tgtQn).toBe('App.Helpers::ToastComponent::ToastService');
|
||||||
|
expect(edge?.confidence).toBe(0.9);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not raise a sibling type's member found by its bare name", async () => {
|
||||||
|
// `Dns()` in one class is not the `Dns` entry of the enum beside it
|
||||||
|
// (okhttp's InterceptorOverridesTest): no more settled by the file than
|
||||||
|
// a sibling class's method.
|
||||||
|
write('src/app/Overrides.kt', 'package app\n\nenum class Param { Dns }\n\nclass Override {\n val value = Dns()\n}\n');
|
||||||
|
write('src/app/Dns.kt', 'package app\n\nclass Dns\n');
|
||||||
|
const raised = (await calls()).filter((e) => e.tgtQn === 'app::Param::Dns' && e.confidence >= 0.9);
|
||||||
|
expect(raised).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -12049,7 +12049,9 @@ export function matchByExactName(
|
|||||||
if (bestMatch && isCrossFileReachable(bestMatch, ref, context)) {
|
if (bestMatch && isCrossFileReachable(bestMatch, ref, context)) {
|
||||||
// Lower confidence when the match is from a distant/unrelated module
|
// Lower confidence when the match is from a distant/unrelated module
|
||||||
const proximity = computePathProximity(ref.filePath, bestMatch.filePath);
|
const proximity = computePathProximity(ref.filePath, bestMatch.filePath);
|
||||||
const confidence = proximity >= 30 ? 0.7 : 0.4;
|
const confidence = isDecisiveSameFileMatch(bestMatch, ref, candidates, context)
|
||||||
|
? 0.9
|
||||||
|
: proximity >= 30 ? 0.7 : 0.4;
|
||||||
return {
|
return {
|
||||||
original: ref,
|
original: ref,
|
||||||
targetNodeId: bestMatch.id,
|
targetNodeId: bestMatch.id,
|
||||||
@@ -26730,6 +26732,55 @@ function computePathProximity(filePath1: string, filePath2: string): number {
|
|||||||
return pathProximityFromDirs(dir1, filePath2);
|
return pathProximityFromDirs(dir1, filePath2);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether `findBestMatch` picked `best` because it is THE definition in the
|
||||||
|
* call site's own file, so the edge deserves a same-file confidence rather than
|
||||||
|
* the directory-proximity one. Proximity measures shared folders, so a
|
||||||
|
* same-file match in a shallow path (`scripts/x.py` → its own `main`) scored
|
||||||
|
* 0.4 like a guess across unrelated modules. Decisive only when:
|
||||||
|
* - `best` is the ONLY same-file candidate — several (two classes' `run` in
|
||||||
|
* one file) are separated by line distance, which is not evidence; and
|
||||||
|
* - a member of a type, module or namespace belongs to one the caller is
|
||||||
|
* inside — a bare call does not reach a sibling class's method just
|
||||||
|
* because it is written in that file, nor is okhttp's `Dns { … }` in one
|
||||||
|
* nested object the `Dns` entry of the enum beside it. A method's owner is
|
||||||
|
* always such a scope; another kind's is judged when the file declares it
|
||||||
|
* as anything but a function, whose nested definitions lexical
|
||||||
|
* reachability already scoped; and
|
||||||
|
* - `best` is not the caller itself — `super.onCreate()`,
|
||||||
|
* `parent::__construct()` and `json.dumps` inside `dumps` all arrive here
|
||||||
|
* without their receiver, and recursion cannot be told apart from them.
|
||||||
|
* 0.9 matches the unique exact-match above: the same-file bonus in
|
||||||
|
* findBestMatch outweighs any directory proximity, so a lone same-file
|
||||||
|
* definition is as settled as a lone candidate. The resolver keeps the most
|
||||||
|
* confident strategy, so this also outranks a sub-0.9 framework guess that
|
||||||
|
* sent the name to another file (ASP.NET's naming rule took eShopOnWeb's
|
||||||
|
* `ToastService.OnShow` for the `ToastService` class, not the component's
|
||||||
|
* property of that name) — the file's own definition is the one the name
|
||||||
|
* means.
|
||||||
|
*/
|
||||||
|
function isDecisiveSameFileMatch(
|
||||||
|
best: Node,
|
||||||
|
ref: UnresolvedRef,
|
||||||
|
candidates: Node[],
|
||||||
|
context: ResolutionContext
|
||||||
|
): boolean {
|
||||||
|
if (best.filePath !== ref.filePath || best.id === ref.fromNodeId) return false;
|
||||||
|
if (candidates.some((c) => c !== best && c.filePath === ref.filePath)) return false;
|
||||||
|
const sep = best.qualifiedName.lastIndexOf('::');
|
||||||
|
if (sep <= 0) return best.kind !== 'method';
|
||||||
|
const owner = best.qualifiedName.slice(0, sep);
|
||||||
|
if (best.kind !== 'method') {
|
||||||
|
const cut = owner.lastIndexOf('::');
|
||||||
|
const ownerName = cut >= 0 ? owner.slice(cut + 2) : owner;
|
||||||
|
const declared = (context.getNodesInFileNamed?.(best.filePath, ownerName) ?? context.getNodesInFile(best.filePath))
|
||||||
|
.filter((n) => n.qualifiedName === owner);
|
||||||
|
if (declared.every((n) => n.kind === 'function' || n.kind === 'method')) return true;
|
||||||
|
}
|
||||||
|
const from = context.getNodeById?.(ref.fromNodeId);
|
||||||
|
return !!from && (from.qualifiedName === owner || from.qualifiedName.startsWith(`${owner}::`));
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Find the best matching node when there are multiple candidates
|
* Find the best matching node when there are multiple candidates
|
||||||
*/
|
*/
|
||||||
|
|||||||
Reference in New Issue
Block a user