diff --git a/CHANGELOG.md b/CHANGELOG.md index f9421824c..4364e23b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- In Dart, a call written without a receiver now reaches only a method of the class it's written in, of a class it extends, mixes in or implements, or of an extension on one of those types, and picks the nearest one. On riverpod, package:test's `test(…)` in the test files used to land on a container class's `test` method over 1,100 times, shelf's and getx's `expect(…)` on unrelated classes' `expect`, and bloc's `emit(…)` in a Cubit on the wrong base class. Classes declared as `class A = B with C;` are now indexed under their name, with what they extend and mix in. Re-index after upgrading. - In R, a variable assigned at the top of a test file is no longer linked from the package or from other tests, because testthat runs each test file in its own environment. On ggplot2, a test's `c <- ggplot(…)` used to collect 2,455 of the package's `c(…)` calls. Variables in `helper-*.R` and `setup-*.R`, which testthat loads for every test, are still shared. - In Lua and Luau, a `local` variable or function is no longer linked from other files, since it's only visible in the file that declares it. On kong, a spec helper's `local it = it` used to collect every other spec file's `it(…)` (over 4,000 links), and one module's `local ipairs = ipairs` took the whole project's `ipairs(…)`. Global functions are still linked across files. - PHP calls on `$this`, `self`, `static` and `parent` now reach the method of the class they're written in, a class it extends, or a trait any of them uses, with parent classes and traits found through the file's `namespace` and `use` imports. On Drupal core, `$this->assertEquals()` (PHPUnit's) used to link to an unrelated comparator class 8,832 times, `$this->assertSession()` to a JavaScript-test base class, and `$this->t()` to `Views::t`. They now reach `UiHelperTrait` and `StringTranslationTrait`, or nothing when the method lives in a package outside the repository. A call inside a trait, and a base class calling a method its subclass defines, keep their links. Re-index PHP projects after upgrading. diff --git a/__tests__/dart-bare-call-scope.test.ts b/__tests__/dart-bare-call-scope.test.ts new file mode 100644 index 000000000..9bddb5527 --- /dev/null +++ b/__tests__/dart-bare-call-scope.test.ts @@ -0,0 +1,162 @@ +/** + * A bare Dart call — `expect(…)`, `emit(…)`, `refresh()` — reaches a method + * only through the class it is written in: that class's own members, what it + * extends, mixes in or implements, or an extension on one of those types. + * + * Without the scope, riverpod's tests' `test(…)` (package:test's function) + * went to `ProviderContainer.test` 1,140 times, shelf's `expect(…)` to a test + * handler's `expect` method, getx's `expect(…)` to a header class's, and an + * extension on `List` calling `add(…)` to `HttpHeaders.add`. Of the in-scope + * members the nearest wins: bloc's `emit(…)` in a `Cubit` is `BlocBase.emit`, + * not the `Emittable` interface's. + */ +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-scope-')); + const files: Record = { + 'pubspec.yaml': 'name: app\n', + 'lib/handler.dart': `class TestHandler { + void expect(Object value) {} + void test(String name) {} +} + +abstract class HttpHeaders { + void add(String name, Object value); + void clear(); +} +`, + 'lib/bloc.dart': `abstract class Emittable { + void emit(S state); +} + +abstract class BlocBase implements Emittable { + @override + void emit(S state) {} +} + +abstract class Cubit extends BlocBase {} + +class CounterCubit extends Cubit { + void increment() => emit(1); +} +`, + 'lib/notifier.dart': `class Listenable {} + +mixin SingleMixin on Listenable { + void refresh() {} +} + +class ListNotifier extends Listenable {} + +class ListNotifierSingle = ListNotifier with SingleMixin; + +class RxList extends ListNotifierSingle { + void add(Object item) { + refresh(); + } +} +`, + 'lib/element.dart': `abstract class AnyNotifier {} + +extension NotifierX on AnyNotifier { + void requireElement() {} +} + +class Notifier< + // the state + S +> // + extends + AnyNotifier< + S + > { + void read() { + requireElement(); + } +} +`, + 'lib/list_ext.dart': `extension ListExtension on List { + void addIf(bool condition, E item) { + if (condition) add(item); + } +} +`, + 'lib/login.dart': `class LoginState { + LoginState withEmail(String e) => this; + LoginState withPassword(String p) => this; +} + +LoginState build() => LoginState().withEmail('a').withPassword('b'); +`, + 'test/handler_test.dart': `import 'package:test/test.dart'; + +void main() { + test('works', () { + expect(1, 1); + }); +} +`, + }; + 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 }); +}); + +/** `from -> Owner::name` for every call edge out of a file. */ +function callsFrom(file: string): string[] { + const ids = cg.getNodesInFile(file).map((n) => n.id); + return cg + .getOutgoingEdgesFrom(ids) + .filter((e) => e.kind === 'calls') + .map((e) => `${cg.getNode(e.source)!.name} -> ${cg.getNode(e.target)!.qualifiedName}`) + .sort(); +} + +describe('bare Dart calls stay in the class hierarchy', () => { + it('a test file’s `test(…)` and `expect(…)` never reach some class’s method', () => { + expect(callsFrom('test/handler_test.dart')).toEqual([]); + }); + + it('an inherited method is the nearest one: the implementation, not the interface', () => { + expect(callsFrom('lib/bloc.dart')).toEqual(['increment -> BlocBase::emit']); + }); + + it('a mixin applied through `class A = B with M;` is in scope', () => { + expect(callsFrom('lib/notifier.dart')).toEqual(['add -> SingleMixin::refresh']); + const single = cg.getNodesByKind('class').find((n) => n.name === 'ListNotifierSingle'); + expect(single).toBeDefined(); + const supers = cg + .getOutgoingEdges(single!.id) + .filter((e) => e.kind === 'extends' || e.kind === 'implements') + .map((e) => `${e.kind}:${cg.getNode(e.target)!.name}`) + .sort(); + expect(supers).toEqual(['extends:ListNotifier', 'implements:SingleMixin']); + }); + + it('an extension on the class’s supertype is in scope, even behind a commented multi-line header', () => { + expect(callsFrom('lib/element.dart')).toEqual(['read -> NotifierX::requireElement']); + }); + + it('an extension on `List` calling `add(…)` means the list’s, not an unrelated class’s bodiless member', () => { + expect(callsFrom('lib/list_ext.dart')).toEqual([]); + }); + + it('the later links of a call chain keep their method', () => { + expect(callsFrom('lib/login.dart')).toContain('build -> LoginState::withPassword'); + }); +}); diff --git a/codegraph-kernel/src/dart.rs b/codegraph-kernel/src/dart.rs index 8182069b0..8faacf368 100644 --- a/codegraph-kernel/src/dart.rs +++ b/codegraph-kernel/src/dart.rs @@ -567,6 +567,18 @@ impl<'t> Walker<'t> { return ctor_name; } } + // `class A = B with C;` — the mixin application's own identifier. + if node.kind() == "class_definition" { + let mut cursor = node.walk(); + let application = node.named_children(&mut cursor).find(|c| c.kind() == "mixin_application_class"); + if let Some(application) = application { + let mut ac = application.walk(); + let id = application.named_children(&mut ac).find(|c| c.kind() == "identifier"); + if let Some(id) = id { + return self.text(id).to_string(); + } + } + } if let Some(name_node) = node.child_by_field_name("name") { return self.text(name_node).to_string(); } @@ -1181,7 +1193,34 @@ impl<'t> Walker<'t> { let mut cursor = node.walk(); let kids: Vec> = node.named_children(&mut cursor).collect(); for child in kids { - if child.kind() == "superclass" { + if child.kind() == "mixin_application_class" { + // `class A = B with M implements I;` — extends B, implements + // each mixin and interface, in source order. + let mut cc = child.walk(); + let application = child.named_children(&mut cc).find(|c| c.kind() == "mixin_application"); + let Some(application) = application else { continue }; + let mut ac = application.walk(); + let parts: Vec> = application.named_children(&mut ac).collect(); + for t in parts { + match t.kind() { + "type_identifier" => { + let name = self.text(t).to_string(); + self.push_ref_at(class_row, &name, "extends", t); + } + "mixins" | "interfaces" => { + let mut mc = t.walk(); + let targets: Vec> = t.named_children(&mut mc).collect(); + for m in targets { + if t.kind() == "interfaces" || m.kind() == "type_identifier" { + let name = self.text(m).to_string(); + self.push_ref_at(class_row, &name, "implements", m); + } + } + } + _ => {} + } + } + } else if child.kind() == "superclass" { // extends type + `with` mixins (implements) — dart branch. let mut cc = child.walk(); let targets: Vec> = child.named_children(&mut cc).collect(); diff --git a/src/extraction/languages/dart.ts b/src/extraction/languages/dart.ts index 5ba07a0f0..ce7567185 100644 --- a/src/extraction/languages/dart.ts +++ b/src/extraction/languages/dart.ts @@ -247,6 +247,13 @@ export const dartExtractor: LanguageExtractor = { return false; }, resolveName: (node) => { + // `class A = B with C;` — a mixin application — names its class inside + // the `mixin_application_class`, not in a `name` field. + if (node.type === 'class_definition') { + const application = node.namedChildren.find((c: SyntaxNode) => c.type === 'mixin_application_class'); + const id = application?.namedChildren.find((c: SyntaxNode) => c.type === 'identifier'); + if (id) return id.text; + } // Name a factory / named constructor by its constructor name — the 2nd // identifier (`create` in `factory Foo.create()`, `_` in `Foo._()`) — not // the class, so a call `Foo.create()` resolves to `Foo::create` (#750). The diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 6522bcfbd..80ebd224b 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -6141,6 +6141,25 @@ export class TreeSitterExtractor { const child = node.namedChild(i); if (!child) continue; + // Dart: `class A = B with M implements I;` keeps its supertypes in a + // `mixin_application` — the same shapes as a class body's clauses. + if (this.language === 'dart' && child.type === 'mixin_application_class') { + const application = child.namedChildren.find((c: SyntaxNode) => c.type === 'mixin_application'); + for (const t of application?.namedChildren ?? []) { + const targets = t.type === 'type_identifier' ? [t] : t.type === 'mixins' ? t.namedChildren.filter((m: SyntaxNode) => m.type === 'type_identifier') : t.type === 'interfaces' ? t.namedChildren : []; + for (const target of targets) { + this.unresolvedReferences.push({ + fromNodeId: classId, + referenceName: getNodeText(target, this.source), + referenceKind: t.type === 'type_identifier' ? 'extends' : 'implements', + line: target.startPosition.row + 1, + column: target.startPosition.column, + }); + } + } + continue; + } + if ( child.type === 'extends_clause' || child.type === 'superclass' || diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 5e5877274..30467d7af 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1271,6 +1271,166 @@ function javaStaticImportsOf(filePath: string, context: ResolutionContext): { ow return found; } +const DART_TYPE_KINDS: ReadonlySet = new Set(['class', 'interface', 'enum', 'mixin', 'extension', 'struct', 'trait']); +const DART_SUPERS = new WeakMap>(); + +/** + * Whether a bare Dart call can reach `method`: only a method of the class it + * is written in, or of what that class extends, mixes in or implements (an + * extension's `on` type included). riverpod's tests' `test(…)` — package:test's + * function — went to `ProviderContainer.test` 2,360 times; shelf's `expect(…)` + * to a test handler's `expect` method. Outside any class (`main`), no method. + */ +function isDartMethodInScope(method: Node, ref: UnresolvedRef, context: ResolutionContext): boolean { + return dartMemberDepth(method, ref, context) < Infinity; +} + +/** + * How many supertype steps separate the class a Dart call is written in from + * `method`'s owner: 0 for its own member, Infinity when the owner is not in + * its hierarchy at all. An extension's member is in scope when the extension + * is `on` a type of that hierarchy — a bare `requireElement()` inside a + * notifier is `this.requireElement()`, the `on AnyNotifier` extension's — and + * ranks after every real member, as Dart resolves it. + */ +function dartMemberDepth(method: Node, ref: UnresolvedRef, context: ResolutionContext): number { + const cut = method.qualifiedName.lastIndexOf('::'); + if (cut < 0) return 0; + const owner = method.qualifiedName.slice(0, cut).split('::').pop()!; + const hierarchy = dartHierarchyAt(ref, context); + const own = hierarchy.get(owner); + if (own !== undefined) return own; + const decl = context + .getNodesInFile(method.filePath) + .find((n) => n.name === owner && DART_TYPE_KINDS.has(n.kind) && n.startLine <= method.startLine && n.endLine >= method.endLine); + if (!decl) return Infinity; + const head = dartHeadOf(decl, context); + if (!head.extension) return Infinity; + const on = Math.min(...head.supers.map((t) => hierarchy.get(t) ?? Infinity)); + return on === Infinity ? Infinity : DART_EXTENSION_RANK + on; +} + +/** Past any real member's depth: an extension member only applies when no instance member does. */ +const DART_EXTENSION_RANK = 1000; +const DART_HIERARCHIES = new WeakMap>>(); + +/** Every type the classes around a Dart call site are, by supertype distance (at most 40). */ +function dartHierarchyAt(ref: UnresolvedRef, context: ResolutionContext): Map { + let memo = DART_HIERARCHIES.get(context); + if (!memo) { + memo = new WeakMap(); + DART_HIERARCHIES.set(context, memo); + } + const hit = memo.get(ref); + if (hit) return hit; + const depths = new Map(); + const queue: Array<[string, number]> = context + .getNodesInFile(ref.filePath) + .filter((n) => DART_TYPE_KINDS.has(n.kind) && n.startLine <= ref.line && n.endLine >= ref.line) + .map((n) => [n.name, 0]); + while (queue.length > 0 && depths.size < 40) { + const [name, depth] = queue.shift()!; + if (depths.has(name)) continue; + depths.set(name, depth); + for (const sup of dartSupertypesOf(name, context)) queue.push([sup, depth + 1]); + } + memo.set(ref, depths); + return depths; +} + +/** + * Of the in-scope members a bare Dart call could mean, the nearest: a + * subclass's override, or the class that implements what an interface only + * declares. bloc's `emit(…)` in a `Cubit` is `BlocBase.emit`, not the + * `Emittable` interface's. + */ +function nearestDartMembers(candidates: Node[], ref: UnresolvedRef, context: ResolutionContext): Node[] { + const members = candidates.filter(isDartMember); + if (members.length < 2) return candidates; + const depth = new Map(members.map((n) => [n.id, dartMemberDepth(n, ref, context)])); + const nearest = Math.min(...depth.values()); + return candidates.filter((n) => !depth.has(n.id) || depth.get(n.id) === nearest); +} + +/** A member of a Dart type — a method, or an abstract member written without a body (extracted as a `function` owned by the type). */ +function isDartMember(n: Node): boolean { + return n.kind === 'method' || (n.kind === 'function' && n.qualifiedName.includes('::')); +} + +/** + * Whether a bare Dart reference really is receiver-less at its call site. The + * extractor keeps one receiver level, so the later links of a chain — + * `LoginState().withEmail(e).withPassword(p)` — arrive as bare names; their + * line shows `.withPassword(` all the same. (A Dart ref's column sits just + * past the name; the name's start is found either way.) + */ +function isReceiverLessDartCall(ref: UnresolvedRef, context: ResolutionContext): boolean { + const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] ?? context.readFile(ref.filePath)?.split('\n')[ref.line - 1]; + if (line === undefined) return true; + const name = ref.referenceName; + let start = -1; + if (line.startsWith(name, ref.column)) start = ref.column; + else if (ref.column >= name.length && line.startsWith(name, ref.column - name.length)) start = ref.column - name.length; + else start = line.indexOf(name); + if (start < 0) return true; + return !/\.\s*$/.test(line.slice(0, start)); +} + +/** The simple names a Dart type's declarations extend, mix in, implement, or (an extension / mixin) sit `on`. */ +function dartSupertypesOf(typeName: string, context: ResolutionContext): string[] { + let memo = DART_SUPERS.get(context); + if (!memo) { + memo = new Map(); + DART_SUPERS.set(context, memo); + } + const hit = memo.get(typeName); + if (hit) return hit; + const names: string[] = []; + for (const decl of context.getNodesByName(typeName)) { + if (decl.language !== 'dart' || !DART_TYPE_KINDS.has(decl.kind)) continue; + names.push(...dartHeadOf(decl, context).supers); + } + memo.set(typeName, names); + return names; +} + +const DART_HEAD_WORDS: ReadonlySet = new Set(['extends', 'with', 'implements', 'on']); + +/** + * A Dart type declaration's head, read from source up to its body: the + * supertypes it names, and whether it is an `extension` (whose one supertype + * is the type it extends). Comments and type arguments are dropped first, so + * riverpod's `class $NotifierProviderElement< // … NotifierT extends + * $Notifier, ValueT > extends $ClassProviderElement<…> with + * ElementWithFuture<…>` reads as `extends $ClassProviderElement with + * ElementWithFuture`, and `class A = B with C;` as its mixin application. + */ +function dartHeadOf(decl: Node, context: ResolutionContext): { supers: string[]; extension: boolean } { + const lines = context.getFileLines?.(decl.filePath) ?? context.readFile(decl.filePath)?.split(/\r?\n/) ?? []; + const text = lines + .slice(decl.startLine - 1, Math.min(decl.endLine, decl.startLine + 40)) + .join('\n') + .replace(/\/\*[\s\S]*?\*\//g, ' ') + .replace(/\/\/[^\n]*/g, ' '); + let depth = 0; + let flat = ''; + for (const ch of text) { + if (ch === '<') depth++; + else if (ch === '>') depth = Math.max(0, depth - 1); + else if (depth === 0) { + if (ch === '{' || ch === ';') break; + flat += ch; + } + } + const keyword = /\b(class|mixin|extension|enum)\b/.exec(flat); + const head = keyword ? flat.slice(keyword.index) : flat; + const clause = /(?:\b(?:extends|with|implements|on)\b|=)([\s\S]*)$/.exec(head)?.[1] ?? ''; + return { + supers: [...clause.matchAll(/[A-Za-z_$][\w$]*/g)].map((m) => m[0]).filter((w) => !DART_HEAD_WORDS.has(w)), + extension: keyword?.[1] === 'extension' && !/^extension\s+type\b/.test(head), + }; +} + const PHP_TYPE_KINDS: ReadonlySet = new Set(['class', 'trait', 'interface', 'enum']); const PHP_SUPERS = new WeakMap>(); @@ -1698,9 +1858,11 @@ export function matchByExactName( const rustBare = ref.language === 'rust' && /^[A-Za-z_]\w*$/.test(ref.referenceName); const pythonShape = pythonCallShape(ref, context); const javaBare = ref.language === 'java' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName); + const dartBare = ref.language === 'dart' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName) && isReceiverLessDartCall(ref, context); const phpSelf = phpSelfReceiver(ref, context); - const candidates = sameName.filter((n) => + const filtered = sameName.filter((n) => !(javaBare && n.kind === 'method' && !isJavaMethodInScope(n, ref, context)) && + !(dartBare && isDartMember(n) && !isDartMethodInScope(n, ref, context)) && !(phpSelf && (n.kind !== 'method' || !isPhpMethodInScope(n, ref, phpSelf, context))) && !(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)) && !(rustBare && !isRustNameInScope(n, ref, context)) && @@ -1746,6 +1908,7 @@ export function matchByExactName( // file's symbol of that name, so a bare call has no cross-file candidate. !(bareJs && n.filePath !== ref.filePath && isLocallyBoundJsName(ref.referenceName, ref.filePath, context)) ); + const candidates = dartBare ? nearestDartMembers(filtered, ref, context) : filtered; if (candidates.length === 0) { return null; @@ -2810,6 +2973,8 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { LEXICAL_SCOPE_MEMO.delete(context); JAVA_SUPERS.delete(context); PHP_SUPERS.delete(context); + DART_SUPERS.delete(context); + DART_HIERARCHIES.delete(context); LUA_LOCALS.delete(context); PHP_FILE_SCOPES.delete(context); JAVA_STATIC_IMPORTS.delete(context); @@ -4763,6 +4928,7 @@ export function matchFuzzy( const rustBare = ref.language === 'rust' && /^[A-Za-z_]\w*$/.test(ref.referenceName); const pythonShape = pythonCallShape(ref, context); const javaBare = ref.language === 'java' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName); + const dartBare = ref.language === 'dart' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName) && isReceiverLessDartCall(ref, context); const phpSelf = phpSelfReceiver(ref, context); // Names are case-sensitive in every language but a handful: Rust's // `Bytes` is not the method `bytes`, Python's builtin `dir(…)` not a class @@ -4777,6 +4943,7 @@ export function matchFuzzy( !(!CASE_INSENSITIVE_LANGUAGES.has(ref.language) && n.name !== ref.referenceName) && !(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)) && !(javaBare && n.kind === 'method' && !isJavaMethodInScope(n, ref, context)) && + !(dartBare && isDartMember(n) && !isDartMethodInScope(n, ref, context)) && !(phpSelf && (n.kind !== 'method' || !isPhpMethodInScope(n, ref, phpSelf, context)))) .filter((n) => (ref.referenceKind !== 'references' && ref.referenceKind !== 'function_ref') || sameLanguageFamily(n.language, ref.language));