mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-26 04:19:33 +08:00
fix(dart): calls to a private name in another part of the same library link again (#2832)
Dart keeps a `_name` to its library: the library file and every file its `part` directives name. isVisibleAcrossFiles still listed Dart in PRIVATE_IS_FILE_LOCAL (added with PR #1745 for Kotlin, Java, C#, Swift, Scala and PHP), and both Dart extractors mark a `_` function, method or class `private`. So once the name-matching pipeline had picked a private candidate in another file, the pick was declined and the reference stayed unresolved. A library split into parts, generated `.g.dart` and `.freezed.dart` parts included, lost the calls and type uses between its files. Fix: - dartPrivateVisible (dart-libraries.ts) says whether a file belongs to the reference's library, read from `part` / `part of` alone. Unlike inSameDartLibrary it needs no pubspec.yaml, since a part names its library by a path or a library name. When the reference's library can't be found, the name stays file-local, as before. - isVisibleAcrossFiles takes Dart out of PRIVATE_IS_FILE_LOCAL and lets a private Dart candidate through when it is in the reference's library. - matchByExactName drops another library's private Dart candidates before the ranking, as it does for C#, PHP, Java and Go names the file can't see. Otherwise another library's namesake can rank first, be declined, and take the library's own declaration down with it. Without a pubspec nothing else filters a top-level `_name`, and a member's class hierarchy is matched by type name, which can reach another library's class of the same name (the SDK's wasm and VM `_ImmutableLinkedHashMapMixin`). Validation: - New __tests__/dart-part-privacy.test.ts, run with and without a pubspec: the issue's library plus a receiver call, a generated part's `_$UserFromJson` and `_$Counter`, another library that imports the one declaring the names, a namesake beside a part, a part named by library name, and a part whose library is missing. 6 of 12 fail on main's resolver, 12 pass. With the pre-ranking filter disabled, 1 fails (the namesake without a pubspec). - A/B vs main58eb57a3, each arm with its own dist and kernel; the kernel and wasm paths give the same changed-site sets. Re-run after the rebases, on5b47bc49(kernel) and8fdc8a1e(wasm), with the same changed sites. Edges added: getx (217 Dart files) 0, bloc (616) +49, riverpod (1,300) +542, localsend (393) +1, protobuf.dart (225) +143, apidash (954) +342, dart-lang/sdk sdk/lib (480, no pubspec) +207. Nothing removed or retargeted, and 1,064 edges changed only their confidence. Every newly resolved ref has its edge at the same site; nodes and files are unchanged. - Triage of all 1,284 added edges: 1,274 right. An independent part/part-of oracle confirms 1,195 (a top-level `_name` unique in its library, a member of the call site's own hierarchy, or a member of the receiver's declared type); 79 were checked by hand. 10 are wrong, all receiver-name guesses (instance-method 0.65-0.8) that pick one implementation of an abstract member, or the first of a tie: the guess public names already get, which the file-local rule had hidden. The 1,064 confidence changes are same-file targets that had another library's namesake beside them (exact-match 0.7 to 0.9). - gin and moshi controls byte-identical, kernel and wasm. Kernel and wasm full-index dumps byte-identical on every Dart repo. kernel-parity --lang dart: 0 diffs on all seven repos. - Index time, median of 3 interleaved runs on the kernel path, CPU with workers, main vs fix: riverpod 55.6 / 50.3 s, sdk/lib 59.4 / 59.6 s, apidash 29.5 / 30.8 s, bloc 18.6 / 18.3 s, protobuf.dart 14.7 / 14.3 s. Within noise. Fixes #2466 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
8fdc8a1efb
commit
f7820b584f
@@ -0,0 +1,201 @@
|
||||
/**
|
||||
* A Dart `_name` is private to its library — the library file and every file
|
||||
* its `part` directives name — not to the one file that declares it (#2466).
|
||||
* A library split into parts calls its own private helpers across those
|
||||
* files, and those calls were dropped as if each file were a library of its
|
||||
* own. A file of another library still never reaches them, importing the
|
||||
* library or not. Whether a pubspec.yaml sits above the files makes no
|
||||
* difference: a part names its library by a path or by a library name.
|
||||
*/
|
||||
import { describe, it, expect, afterAll, beforeAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
|
||||
function writeTree(root: string, 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);
|
||||
}
|
||||
}
|
||||
|
||||
/** `<source qualified name> -> <target file>:<target qualified name>` for every `calls` edge out of `files`. */
|
||||
function callEdges(cg: CodeGraph, files: string[]): string[] {
|
||||
return edgesOf(cg, files, ['calls']).map((e) => e.slice(e.indexOf(' ') + 1));
|
||||
}
|
||||
|
||||
/** `<kind> <source qualified name> -> <target file>:<target qualified name>` for every edge of `kinds` out of `files`. */
|
||||
function edgesOf(cg: CodeGraph, files: string[], kinds: string[]): string[] {
|
||||
const ids = files.flatMap((f) => cg.getNodesInFile(f).map((n) => n.id));
|
||||
return cg.getOutgoingEdgesFrom(ids)
|
||||
.filter((e) => kinds.includes(e.kind))
|
||||
.map((e) => {
|
||||
const t = cg.getNode(e.target)!;
|
||||
return `${e.kind} ${cg.getNode(e.source)!.qualifiedName} -> ${t.filePath}:${t.qualifiedName}`;
|
||||
})
|
||||
.sort();
|
||||
}
|
||||
|
||||
/**
|
||||
* The issue's library (a library file and three parts, one of them in a
|
||||
* subdirectory), a library with a generated part, and a library that uses both.
|
||||
*/
|
||||
const GREETER: Record<string, string> = {
|
||||
'lib/greeter.dart': `part 'greeter_format.dart';
|
||||
part 'greeter_shout.dart';
|
||||
part 'src/_greeter_whisper.dart';
|
||||
|
||||
class Greeter with _Formatting, _Shouting, _Whispering {
|
||||
String greet(String name) => _format(name); // library file -> private member in a part
|
||||
String hello() => _local(); // same-file control
|
||||
String _local() => 'hi';
|
||||
}
|
||||
|
||||
void _trace(String s) => print(s);
|
||||
`,
|
||||
'lib/greeter_format.dart': `part of 'greeter.dart';
|
||||
|
||||
mixin _Formatting {
|
||||
String _format(String name) {
|
||||
_trace(name); // part -> private top-level function in the library file
|
||||
return 'Hello, $name';
|
||||
}
|
||||
}
|
||||
`,
|
||||
'lib/greeter_shout.dart': `part of 'greeter.dart';
|
||||
|
||||
mixin _Shouting on _Formatting {
|
||||
String shout(String name) => _format(name).toUpperCase(); // part -> sibling part
|
||||
String echo(Greeter g) => g._local(); // part -> private member, through a receiver
|
||||
}
|
||||
`,
|
||||
'lib/src/_greeter_whisper.dart': `part of '../greeter.dart';
|
||||
|
||||
mixin _Whispering on _Formatting {
|
||||
String whisper(String name) => _format(name).toLowerCase(); // part in a subdirectory -> sibling part
|
||||
void log(String s) => _trace(s); // -> the library file's, not the namesake beside this part
|
||||
}
|
||||
`,
|
||||
// What code generators write into a part: json_serializable's `_$UserFromJson`,
|
||||
// and a base class like riverpod_generator's `_$Counter`.
|
||||
'lib/models/user.dart': `part 'user.g.dart';
|
||||
|
||||
class User {
|
||||
final String name;
|
||||
User(this.name);
|
||||
factory User.fromJson(Map<String, dynamic> json) => _$UserFromJson(json);
|
||||
}
|
||||
|
||||
class Counter extends _$Counter {}
|
||||
`,
|
||||
'lib/models/user.g.dart': `part of 'user.dart';
|
||||
|
||||
User _$UserFromJson(Map<String, dynamic> json) => User(json['name'] as String);
|
||||
|
||||
abstract class _$Counter {}
|
||||
`,
|
||||
// Another library: the greeter library's privates are not its to name, even through an import.
|
||||
'lib/other.dart': `import 'greeter.dart';
|
||||
import 'models/user.dart';
|
||||
|
||||
void callsOther(Greeter g, Object o) {
|
||||
_trace('x');
|
||||
g._local();
|
||||
if (o is _$Counter) _$UserFromJson({});
|
||||
}
|
||||
`,
|
||||
};
|
||||
|
||||
const GREETER_FILES = ['lib/greeter.dart', 'lib/greeter_format.dart', 'lib/greeter_shout.dart', 'lib/src/_greeter_whisper.dart'];
|
||||
|
||||
const GREETER_EDGES = [
|
||||
'Greeter::greet -> lib/greeter_format.dart:_Formatting::_format',
|
||||
'Greeter::hello -> lib/greeter.dart:Greeter::_local',
|
||||
'_Formatting::_format -> lib/greeter.dart:_trace',
|
||||
'_Shouting::echo -> lib/greeter.dart:Greeter::_local',
|
||||
'_Shouting::shout -> lib/greeter_format.dart:_Formatting::_format',
|
||||
'_Whispering::log -> lib/greeter.dart:_trace',
|
||||
'_Whispering::whisper -> lib/greeter_format.dart:_Formatting::_format',
|
||||
];
|
||||
|
||||
for (const pubspec of [true, false]) {
|
||||
describe(`Dart private names across a library's parts (${pubspec ? 'with' : 'without'} a pubspec.yaml)`, () => {
|
||||
let root = '';
|
||||
let cg: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-dart-part-private-'));
|
||||
writeTree(root, {
|
||||
...(pubspec ? { 'pubspec.yaml': 'name: greeter\n' } : {}),
|
||||
...GREETER,
|
||||
// A library that keeps its own `_trace` and `_local`, beside the
|
||||
// greeter's part in lib/src/ and so nearer it than the greeter's own.
|
||||
'lib/src/farewell.dart': `void _trace(String s) => print(s);
|
||||
|
||||
class Farewell {
|
||||
String _local() => 'bye';
|
||||
String bye() => _local();
|
||||
}
|
||||
|
||||
void wave() => _trace('wave');
|
||||
`,
|
||||
// A library whose part is named by the library's name.
|
||||
'lib/named.dart': `library app.named;
|
||||
|
||||
part 'named_part.dart';
|
||||
|
||||
int _seed() => 6;
|
||||
`,
|
||||
'lib/named_part.dart': `part of app.named;
|
||||
|
||||
int usesSeed() => _seed();
|
||||
`,
|
||||
// A part whose library the project doesn't have: its own file is all it can see.
|
||||
'lib/stray_part.dart': `part of 'missing.dart';
|
||||
|
||||
int usesStray() => _seed();
|
||||
`,
|
||||
});
|
||||
cg = await CodeGraph.init(root, { index: true });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cg?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('links a call to a private member or function declared in another file of the library', () => {
|
||||
expect(callEdges(cg, GREETER_FILES)).toEqual(GREETER_EDGES);
|
||||
});
|
||||
|
||||
it('links the private function and class a generated part declares', () => {
|
||||
expect(edgesOf(cg, ['lib/models/user.dart'], ['calls', 'extends'])).toEqual([
|
||||
'calls User::fromJson -> lib/models/user.g.dart:_$UserFromJson',
|
||||
'extends Counter -> lib/models/user.g.dart:_$Counter',
|
||||
]);
|
||||
});
|
||||
|
||||
it("never links another library's private names, even when the file imports that library", () => {
|
||||
// Only the public `Greeter` the parameter names.
|
||||
expect(edgesOf(cg, ['lib/other.dart'], ['calls', 'references', 'instantiates'])).toEqual([
|
||||
'references callsOther -> lib/greeter.dart:Greeter',
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps another library's private namesakes to that library", () => {
|
||||
expect(callEdges(cg, ['lib/src/farewell.dart'])).toEqual([
|
||||
'Farewell::bye -> lib/src/farewell.dart:Farewell::_local',
|
||||
'wave -> lib/src/farewell.dart:_trace',
|
||||
]);
|
||||
});
|
||||
|
||||
it('finds the library a part names by library name', () => {
|
||||
expect(callEdges(cg, ['lib/named_part.dart'])).toEqual(['usesSeed -> lib/named.dart:_seed']);
|
||||
});
|
||||
|
||||
it("links nothing from a part whose library isn't in the project", () => {
|
||||
expect(callEdges(cg, ['lib/stray_part.dart'])).toEqual([]);
|
||||
});
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user