mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(dart): an unnamed extension is not its on-type and applies only in its library (#2237)
The extractor names `extension on X` after X; such a node is never a target, and its members are out of reach from other files (Dart scopes unnamed extensions to their library) — before ranking in Strategy 3 and as a veto. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- A Dart `extension on X { … }` with no name is no longer mistaken for the type `X`, and its members are reachable only inside the file that declares it, as in Dart. Before, bloc's linter linked analyzer's `Token` and Dart's `Uri` to small extension blocks, and every flutter_test `find.text(…)` call linked to a `text` getter on another example's `TaskStatus`.
|
||||
- In Java, a bare type name now resolves only to a type in reach. That is a type in the same package, one imported by name or with `.*`, or a nested type used from inside its owner, from a subclass (anonymous classes included), or written through its owner as in `Outer.Inner`. A constructor no longer stands in for its class, and an enum constant is reached by bare name only inside its enum, in a `case` label, or through a static import. Before, retrofit's `new Retrofit.Builder()` linked to `RequestFactory.Builder`, halo's micrometer `Counter` to its own `Counter` class, and jsoup's `java.lang.Class<?>` to a nested selector class.
|
||||
- In PHP, an unqualified class name now resolves to the current namespace's class or the one a `use` imports, as PHP does. If the imported class comes from a library, it resolves to nothing in the project. Before, koel's form requests linked `extends Request` to whichever `Request` class was indexed first, and BookStack's controllers linked `Request $request` (Laravel's, by import) to BookStack's own `Request` subclass.
|
||||
- A call in C code is no longer linked to a C++ class's method, since C has no methods and can't call one. Before, redis's POSIX `read(fd, buf, n)` calls and hiredis's `c->funcs->read(…)` function pointer pointed at a Qt adapter's `read` method.
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
/**
|
||||
* A Dart `extension on X { … }` has no name — so `X` is never it — and
|
||||
* applies only in its own library. bloc_lint's `extension on Token` took 79
|
||||
* of analyzer's `Token` references, and a concurrency visualizer's
|
||||
* `extension on TaskStatus { String get text }` took 133 flutter_test
|
||||
* `find.text(…)` calls from other packages.
|
||||
*/
|
||||
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';
|
||||
|
||||
let root = '';
|
||||
let cg: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-dart-ext-'));
|
||||
const files: Record<string, string> = {
|
||||
'pubspec.yaml': 'name: bloc_lint\n',
|
||||
'lib/src/rules/avoid_build_context_extensions.dart': `import 'package:analyzer/dart/ast/token.dart';
|
||||
|
||||
extension on Token {
|
||||
bool get isBlocType => lexeme.endsWith('Bloc');
|
||||
}
|
||||
`,
|
||||
'lib/src/linter.dart': `import 'package:analyzer/dart/ast/token.dart';
|
||||
|
||||
class Linter {
|
||||
void visit(Token beginToken) {}
|
||||
}
|
||||
`,
|
||||
'examples/visualizer/lib/timeline_page.dart': `enum TaskStatus { queued, done }
|
||||
|
||||
extension on TaskStatus {
|
||||
String get text => 'x';
|
||||
}
|
||||
|
||||
String label(TaskStatus status) => status.text;
|
||||
`,
|
||||
'examples/counter/test/counter_page_test.dart': `import 'package:flutter_test/flutter_test.dart';
|
||||
|
||||
void main() {
|
||||
testWidgets('shows', (tester) async {
|
||||
expect(find.text('0'), findsOneWidget);
|
||||
});
|
||||
}
|
||||
`,
|
||||
};
|
||||
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);
|
||||
}
|
||||
cg = await CodeGraph.init(root, { index: true });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cg?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const targetsFrom = (file: string) => {
|
||||
const ids = cg.getNodesInFile(file).map((n) => n.id);
|
||||
return cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind !== 'contains').map((e) => cg.getNode(e.target)!).map((t) => `${t.filePath}:${t.qualifiedName}`);
|
||||
};
|
||||
|
||||
describe('unnamed Dart extensions', () => {
|
||||
it('are never the type they extend', () => {
|
||||
expect(targetsFrom('lib/src/linter.dart')).not.toContain('lib/src/rules/avoid_build_context_extensions.dart:Token');
|
||||
});
|
||||
|
||||
it('apply only in their own library', () => {
|
||||
expect(targetsFrom('examples/counter/test/counter_page_test.dart').filter((t) => t.includes('TaskStatus'))).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -1615,6 +1615,12 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex
|
||||
if (!isPhpClassVisible(candidate, ref, context)) return false;
|
||||
// A bare Java type name is its package's, an import's, or a nested type in reach.
|
||||
if (!isJavaTypeVisible(candidate, ref, context)) return false;
|
||||
// A Dart `extension on Token { … }` has no name: `Token` is analyzer's type,
|
||||
// not bloc_lint's extension block (84 refs went there).
|
||||
if (dartExtensionDecl(candidate, context)?.named === false) return false;
|
||||
// And it applies only in its own library: flutter_test's `find.text(…)` is
|
||||
// no other file's `extension on TaskStatus { String get text }`.
|
||||
if (candidate.filePath !== ref.filePath && isDartUnnamedExtensionMember(candidate, context)) return false;
|
||||
// A Scala package object's member is in scope in its package and those under
|
||||
// it, or through an import: cats.laws' `Eq` is the `cats` package object's
|
||||
// alias, not the `algebra` one's (752 refs went there).
|
||||
@@ -2576,6 +2582,25 @@ function receiverNamesOwner(receiver: string, method: Node, context: ResolutionC
|
||||
return sharesReceiverWord(receiver, method);
|
||||
}
|
||||
|
||||
/** The Dart `extension` declaration a class node stands for, read from its line: `named` false for `extension on X`. */
|
||||
function dartExtensionDecl(n: Node, context: ResolutionContext): { named: boolean } | null {
|
||||
if (n.language !== 'dart' || n.kind !== 'class') return null;
|
||||
const line = (context.getFileLines?.(n.filePath) ?? context.readFile(n.filePath)?.split(/\r?\n/) ?? [])[n.startLine - 1] ?? '';
|
||||
if (!/^\s*extension\b(?!\s+type\b)/.test(line)) return null;
|
||||
return { named: !/^\s*extension\s+on\b/.test(line) };
|
||||
}
|
||||
|
||||
/** Whether a Dart method belongs to an unnamed `extension on X`, visible only in its own library. */
|
||||
function isDartUnnamedExtensionMember(method: Node, context: ResolutionContext): boolean {
|
||||
if (method.language !== 'dart' || method.kind !== 'method') return false;
|
||||
const cut = method.qualifiedName.lastIndexOf('::');
|
||||
if (cut <= 0) return false;
|
||||
const ownerQn = method.qualifiedName.slice(0, cut);
|
||||
const owner = context.getNodesInFile(method.filePath).find((n) => n.qualifiedName === ownerQn && n.kind === 'class' &&
|
||||
n.startLine <= method.startLine && n.endLine >= method.startLine);
|
||||
return owner !== undefined && dartExtensionDecl(owner, context)?.named === false;
|
||||
}
|
||||
|
||||
const CSHARP_ALIASES = new WeakMap<ResolutionContext, Map<string, Map<string, string>>>();
|
||||
|
||||
/**
|
||||
@@ -4972,6 +4997,7 @@ export function matchByExactName(
|
||||
// Likewise a bare Java type name: retrofit's tests' `new Builder()` is not
|
||||
// a wire converter test's nested `CrashingPhone.Builder`.
|
||||
isJavaTypeVisible(n, ref, context) &&
|
||||
dartExtensionDecl(n, context)?.named !== false &&
|
||||
// A Scala package object's member, only where it is in scope — ahead of
|
||||
// the ranking, so cats.laws' `Eq` can be the `cats` package object's.
|
||||
!(ref.language === 'scala' && n.language === 'scala' && n.filePath !== ref.filePath &&
|
||||
@@ -7526,6 +7552,14 @@ export function matchMethodCall(
|
||||
narrowed ||= kept.length !== targetMethods.length;
|
||||
targetMethods = kept;
|
||||
}
|
||||
// Another library's unnamed Dart extension does not apply here: bloc's
|
||||
// `tester.pumpApp(…)` is the imported `PumpApp`, not flutter_counter's
|
||||
// `extension on WidgetTester`.
|
||||
{
|
||||
const kept = targetMethods.filter((m) => m.filePath === ref.filePath || !isDartUnnamedExtensionMember(m, context));
|
||||
narrowed ||= kept.length !== targetMethods.length;
|
||||
targetMethods = kept;
|
||||
}
|
||||
// Production code never calls into a test suite: a guess from
|
||||
// rest_framework/renderers.py's `view.reverse_action(…)` is not a test's
|
||||
// `DummyView`. The test's methods were never in the running.
|
||||
|
||||
Reference in New Issue
Block a user