diff --git a/CHANGELOG.md b/CHANGELOG.md index 16d4ed8d3..1ac3d601f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Dart, calling a parameter or a local variable, like `create(ref, arg)` inside `overrideWith(… create)`, a closure's parameter, a loop variable or a test's mock `listener(0, 1)`, no longer links the call to an unrelated function or constant of the same name in another file, or to a method the class inherits: a Bloc event handler's `emit(…)` is the handler's own `emit` parameter, not `Bloc.emit`. So `codegraph callers` and impact no longer list code that never calls them, while calls the parameter or variable doesn't cover, and calls to a local function, link as before. Re-index Dart projects after upgrading. - In Dart, `const` constructors and redirecting factories, like flutter_bloc's `const BlocProvider.value(…)` or freezed's `const factory LoginState.loading() = _Loading;`, are now indexed, so a call such as `BlocProvider.value(…)` or `BlocProvider.value(…)` links to that constructor instead of another class's member with the same name. A call through a class name now reaches only that class's own constructors and static members, never a look-alike class, a parent class, an instance getter of the same name, or a project method that merely shares its name with an SDK call like `Uri.parse(…)`, and a constructor is no longer linked from a bare name, the end of a call chain or an annotation like `@internal`. Re-index Dart projects after upgrading. - In Dart, a call that passes a type argument and arguments, like `ref.read(repoProvider)`, `ref.watch(counterProvider)`, `Provider((ref) => …)`, `BlocProvider.of(context)` or `RepositoryProvider(create: …)`, now links to the method or class it calls, and the type it passes counts as used there. Depending on the code around it, the parser often read such a call as two comparisons, so it linked nothing, and Riverpod providers, repository reads and bloc lookups were missing from `callers`, impact and `codegraph_explore`. A real comparison like `a < b` is still left alone. Re-index Dart projects after upgrading. +- In Dart, a constructor or member written without a body, like flutter_bloc's `const BlocProvider.value(…)`, `Foo._();`, `Point.origin() : x = 0, y = 0;` or an abstract or `external` method, now keeps the `///` documentation and the annotations written above it, the way one with a body already did. Before, that documentation was lost, so searching for words in it didn't find the symbol, and an annotation like `@visibleForTesting` or `@protected` on it wasn't recorded. Re-index Dart projects after upgrading. ## [1.6.2] - 2026-10-03 diff --git a/__tests__/dart-bodiless-member-docs.test.ts b/__tests__/dart-bodiless-member-docs.test.ts new file mode 100644 index 000000000..13b875709 --- /dev/null +++ b/__tests__/dart-bodiless-member-docs.test.ts @@ -0,0 +1,134 @@ +/** + * A Dart member with no body — a constructor like `Foo._();` or `Foo.parts() + * : id = 0;`, an abstract `void m();`, an `external` one — parses as a + * `declaration` wrapping its signature, and the member's `///` dartdoc and + * `@annotation`s sit before that wrapper, not before the signature. + * + * Both extractors looked for them before the signature, inside the wrapper, + * where there is nothing. Such a member lost its documentation, and + * `@visibleForTesting Foo._();` emitted no `decorates` reference. + * (flutter_bloc's `const BlocProvider.value(…)`, with its long dartdoc, is one + * of these constructors.) + * + * Runs against the native kernel (when built) and the wasm extractor, which + * must agree. + */ +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars'; +import { tryKernelExtract, resetKernelForTests } from '../src/extraction/kernel'; +import type { ExtractionResult } from '../src/types'; + +const KERNEL_PATH = path.join( + __dirname, + '..', + 'codegraph-kernel', + 'prebuilds', + `${process.platform}-${process.arch}`, + 'codegraph-kernel.node' +); +const kernelAvailable = fs.existsSync(KERNEL_PATH) || process.env.CODEGRAPH_KERNEL_EXPECT === '1'; + +const SOURCE = `abstract class Repository { + /// Loads the user with [id]. + Future load(String id); + + @visibleForTesting + void reset(); + + /// Calls into the platform. + external int platformVersion(); +} + +class User { + /// Creates a user from its parts. + User.parts(this.name) : id = 0; + + @visibleForTesting + User.test(this.name) : id = 1; + + @Deprecated('use parts') + @internal + User.old(this.name) : id = 2; + + /// The user's name. + @deprecated + final String name; + User.afterField() : name = '', id = 3; + + final int id; +} +`; + +const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const; + +describe('Dart members with no body keep their dartdoc and annotations', () => { + let savedEnv: Record = {}; + + beforeAll(async () => { + await initGrammars(); + await loadGrammarsForLanguages(['dart']); + }); + + beforeEach(() => { + savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + resetKernelForTests(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + resetKernelForTests(); + }); + + function extract(backend: 'kernel' | 'wasm', source: string): ExtractionResult { + if (backend === 'wasm') { + process.env.CODEGRAPH_KERNEL = '0'; + return extractFromSource('lib/user.dart', source, 'dart'); + } + delete process.env.CODEGRAPH_KERNEL; + process.env.CODEGRAPH_KERNEL_LANGS = 'all'; + const result = tryKernelExtract('lib/user.dart', source, 'dart'); + expect(result, 'kernel extraction').not.toBeNull(); + return result!; + } + + const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const); + + for (const crlf of [false, true]) { + it.each(backends)(`%s${crlf ? ' (CRLF)' : ''}`, (backend) => { + const result = extract(backend, crlf ? SOURCE.replace(/\n/g, '\r\n') : SOURCE); + const node = (qualifiedName: string) => { + const found = result.nodes.find((n) => n.qualifiedName === qualifiedName); + expect(found, qualifiedName).toBeDefined(); + return found!; + }; + const decorators = (qualifiedName: string): string[] => { + const id = node(qualifiedName).id; + return result.unresolvedReferences + .filter((r) => r.referenceKind === 'decorates' && r.fromNodeId === id) + .map((r) => r.referenceName) + .sort(); + }; + + // The dartdoc before the declaration is the member's. + expect(node('Repository::load').docstring).toBe('Loads the user with [id].'); + expect(node('Repository::platformVersion').docstring).toBe('Calls into the platform.'); + expect(node('User::parts').docstring).toBe('Creates a user from its parts.'); + + // So are the annotations, stacked ones included. + expect(decorators('Repository::reset')).toEqual(['visibleForTesting']); + expect(decorators('User::test')).toEqual(['visibleForTesting']); + expect(decorators('User::old')).toEqual(['Deprecated', 'internal']); + + // A field's dartdoc and annotations stay with the field, which mints no + // node, rather than moving on to the constructor after it. + expect(node('User::afterField').docstring).toBeUndefined(); + expect(decorators('User::afterField')).toEqual([]); + }); + } +}); diff --git a/__tests__/fixtures/kernel-parity/TortureDeclarationDocs.dart b/__tests__/fixtures/kernel-parity/TortureDeclarationDocs.dart new file mode 100644 index 000000000..b93a0bdd5 --- /dev/null +++ b/__tests__/fixtures/kernel-parity/TortureDeclarationDocs.dart @@ -0,0 +1,112 @@ +// Members with no body are a `declaration` wrapping their signature; their +// dartdoc and annotations sit before the wrapper. + +abstract class Store { + /// Abstract method doc. + Future save(Object value); + + /// Doc above an annotation. + @protected + void flush(); + + @mustCallSuper + @Deprecated('use flush') + void sync(); + + /// Abstract getter doc (no node). + int get size; + + /// Abstract setter doc (no node). + set size(int v); + + /// Abstract operator doc (no node). + Store operator +(Store other); + + /** Block dartdoc on an external member. */ + external int nativeCount(); + + /// Static external doc. + external static Store open(); + + // Plain comment on an abstract member. + bool isEmpty(); +} + +class Point { + /// Unnamed constructor doc (no node). + Point(this.x, this.y); + + /// Named constructor doc. + /// Second line. + Point.origin() : x = 0, y = 0; + + /// Redirecting constructor doc. + Point.onX(int x) : this(x, 0); + + @visibleForTesting + Point.test(this.x, this.y); + + /// Before annotation. + @Deprecated('use origin') + /// After annotation. + Point.zero() : x = 0, y = 0; + + /* é */ @visibleForTesting Point.inline(this.x) : y = 0; + + /// Field doc. + @observable + final int x; + Point.afterField(this.x, this.y); + + @first + Point.prev(this.x, this.y); + Point.next(this.x, this.y); + + final int y; + + /// Const constructor doc. + @literal + const Point.c(this.x, this.y); + + /// Redirecting factory doc. + const factory Point.r(int x, int y) = Point; + + /// Bodied named constructor doc. + @visibleForTesting + Point.bodied(this.x, this.y) { + validate(); + } + + void validate() {} +} + +mixin Walker { + /// Mixin abstract doc. + + void walk(); +} + +extension Ext on String { + /// Extension external doc. + @pragma('vm:prefer-inline') + external String shout(); +} + +extension type Meters(double value) { + /// Extension type external doc. + external double km(); +} + +enum Mode { + on, + off; + + /// Enum const constructor doc (no node). + const Mode(); + + /// Enum external doc. + external bool get isOn; + + /// Enum external method doc. + external void toggle(); +} diff --git a/__tests__/kernel-dart-parity.test.ts b/__tests__/kernel-dart-parity.test.ts index c6b248775..234051ed7 100644 --- a/__tests__/kernel-dart-parity.test.ts +++ b/__tests__/kernel-dart-parity.test.ts @@ -28,8 +28,10 @@ * constructors and redirecting factories as methods with the unnamed ones * skipped, every redirect-target form, and constructors called with type * arguments, as statements and as expressions; TortureGenericCalls.dart: - * generic calls parsed as two comparisons, recovered as calls) and their CRLF - * variants + * generic calls parsed as two comparisons, recovered as calls; + * TortureDeclarationDocs.dart: the dartdoc and annotations of a + * `declaration`-wrapped member with no body, read from before the wrapper) + * and their CRLF variants * (derived in-memory — #1329), plus defer and generated-file pins. * * The full-repo sweeps live in scripts/kernel-parity.mjs (shelf/bloc/flutter @@ -135,6 +137,10 @@ describe.skipIf(!kernelBuilt)('kernel Dart extraction parity', () => { // as `(ref.read < Repo) > (p)`): every callee, type-argument and argument // shape, in bodies and initializers, and the comparisons that look alike. ['TortureGenericCalls.dart', 10], + // Members with no body (`declaration`-wrapped constructors, abstract and + // external members) take their dartdoc and annotations from before the + // wrapper; a field's stay with the field. + ['TortureDeclarationDocs.dart', 20], ] as const; for (const [file, minNodes] of FIXTURES) { diff --git a/codegraph-kernel/src/dart.rs b/codegraph-kernel/src/dart.rs index 116414fc6..002e0a2f5 100644 --- a/codegraph-kernel/src/dart.rs +++ b/codegraph-kernel/src/dart.rs @@ -582,6 +582,29 @@ impl<'t> Walker<'t> { false } + /// getDeclarationWrapper (dart.ts) — a member with no body (`Foo._();`, + /// `const Foo.c();`, an abstract `void m();`) is a `declaration` wrapping + /// its signature, and the member's `///` dartdoc and `@annotation`s come + /// before the wrapper. A signature that opens the declaration takes both + /// from there. + fn declaration_wrapper(&self, node: Node<'t>) -> Option> { + let parent = node.parent()?; + if parent.kind() != "declaration" { + return None; + } + if parent.named_child(0)? == node { + Some(parent) + } else { + None + } + } + + /// docstringFor (tree-sitter.ts) — the preceding comment run, looked up + /// from the declaration wrapper when there is one. + fn docstring_of(&self, node: Node<'t>) -> Option { + preceding_docstring(self.declaration_wrapper(node).unwrap_or(node), self.src) + } + /// resolveBody (dart.ts:158-171). fn resolve_body(&self, node: Node<'t>) -> Option> { if matches!(node.kind(), "function_signature" | "method_signature") { @@ -804,7 +827,7 @@ impl<'t> Walker<'t> { } return; } - let docstring = preceding_docstring(node, self.src); + let docstring = self.docstring_of(node); let signature = self.signature_of(node); let visibility = self.visibility_of(node); let is_async = self.is_async_of(node); @@ -851,7 +874,7 @@ impl<'t> Walker<'t> { } return; } - let docstring = preceding_docstring(node, self.src); + let docstring = self.docstring_of(node); let signature = self.signature_of(node); let visibility = self.visibility_of(node); let is_async = self.is_async_of(node); @@ -893,7 +916,7 @@ impl<'t> Walker<'t> { // fallback finds the ON type's type_identifier — a class named after // the extended type (preserved). let name = self.extract_name(node); - let docstring = preceding_docstring(node, self.src); + let docstring = self.docstring_of(node); let visibility = self.visibility_of(node); let row = self.create_node( "class", @@ -924,7 +947,7 @@ impl<'t> Walker<'t> { None => return, }; let name = self.extract_name(node); - let docstring = preceding_docstring(node, self.src); + let docstring = self.docstring_of(node); let visibility = self.visibility_of(node); let row = self.create_node( "enum", @@ -966,7 +989,7 @@ impl<'t> Walker<'t> { if name == "" { return false; } - let docstring = preceding_docstring(node, self.src); + let docstring = self.docstring_of(node); // `value` field is null (type_alias has no fields) → no refs from // the aliased type; returns false → children re-visited. self.create_node("type_alias", &name, node, Extra { docstring, ..Default::default() }); @@ -1397,8 +1420,10 @@ impl<'t> Walker<'t> { } // Scan 2: preceding siblings, backward, stop at the first // non-annotation — stacked annotations emit in REVERSE source order. - if let Some(parent) = decl.parent() { - let decl_start = decl.start_byte(); + // A `declaration`-wrapped member's annotations precede the wrapper. + let anchor = self.declaration_wrapper(decl).unwrap_or(decl); + if let Some(parent) = anchor.parent() { + let decl_start = anchor.start_byte(); let mut decl_idx: Option = None; for i in 0..parent.named_child_count() { if let Some(sib) = parent.named_child(i) { diff --git a/docs/design/dart-kernel-port-checklist.md b/docs/design/dart-kernel-port-checklist.md index 7f243c1ab..f9fbd71f6 100644 --- a/docs/design/dart-kernel-port-checklist.md +++ b/docs/design/dart-kernel-port-checklist.md @@ -246,7 +246,11 @@ Grammar-shape facts the config leans on (all probed, `mini-cst.txt` / constructor's name is a `type_identifier` without type arguments and an `identifier` after them). Until 2026-10-06 neither was in methodTypes (no node, no refs); now both mint methods exactly like the bare - `constructor_signature` (§isMisparsedFunction, §resolveName). Fields are `declaration > + `constructor_signature` (§isMisparsedFunction, §resolveName). Abstract and + `external` members are `declaration > function_signature` too (an anon + `external`/`static` keyword may precede the signature). In every one of + these the member's dartdoc and annotations are siblings of the + `declaration`, not of the signature — §getDeclarationWrapper. Fields are `declaration > (static)? (final|const|type|var) > initialized_identifier_list | static_final_declaration_list` (§Constants). - **Statements/expressions:** calls have NO call node — a postfix chain is @@ -327,6 +331,16 @@ Hooks PRESENT (port each exactly — anchors into languages/dart.ts): **LIVE and load-bearing for dart**: the comment there names Dart; a method/function node's endLine extends to the sibling body's end, e.g. `named` L123-125 spans its body). +- **getDeclarationWrapper** — node's parent is a `declaration` AND node is + that declaration's FIRST named child (`firstNamedChild.equals(node)`; + kernel `named_child(0) == node` — an anon `external`/`static` keyword + before the signature doesn't count) → the `declaration`, else undefined. + Read by tree-sitter.ts's `docstringFor` (every docstring lookup, so the + kernel's `docstring_of` wraps all five `preceding_docstring` calls) and + by extractDecoratorsFor's scan #2 (§Decorators); scan #1 and + `getBodyDocstring` keep the node itself. Only Dart defines it — never add + `declaration` to DOCSTRING_WRAPPER_TYPES / docstring.rs `is_wrapper`, + which C/C++ (whose `declaration` wraps declarators) share. - **getReturnType = extractDartReturnType (:80-92)** — ctor = dartCtorInfo (§below): a validated ctor returns the CLASS name (named ctors, factories → ret = enclosing class). Else sig = dartInnerSignature (:9 — @@ -846,12 +860,19 @@ enum emits nothing). Mechanics for dart: - For a method: the previous member's function_body (or any declaration) breaks the chain correctly. The annotation-BETWEEN-doc-and-decl also breaks the DOCSTRING chain (§Docstrings). -- Bodiless ctors (declaration-wrapped): extractMethod runs on - constructor_signature whose PARENT is the `declaration` node — the - backward scan runs over declaration's children (constructor_signature is - namedChild(0) → declIdx 0 → no siblings scanned) → an annotation before - the declaration attaches to NOTHING. Annotated fields likewise emit - nothing (no extractor runs). +- Members with no body (declaration-wrapped ctors, const ctors, redirecting + factories, abstract and `external` members): the signature's PARENT is the + `declaration`, so until 2026-10-06 the backward scan ran over the + declaration's own children (the signature is namedChild(0) → declIdx 0 → + nothing scanned) and an annotation before the declaration attached to + NOTHING (`@visibleForTesting Foo._();`, `@protected void m();`). Now scan + #2 starts at getDeclarationWrapper's `declaration`: same backward walk, + same reverse order, and a field's `declaration` in between still stops + it. Annotated fields themselves emit nothing (no extractor runs). + Validation (bloc @b9be1e2 / riverpod @4ba1be2): +10 / +186 `decorates` + refs, all SDK or package:meta names (`useResult`, `override`, `internal`, + `visibleForTesting`, …), so no edges; before #2380, riverpod's 23 new + `@internal` refs on `X.internal(…)` ctors resolved to the ctor itself. ### Docstrings (tree-sitter-helpers.ts:95-127) — dartdoc is KEPT, both forms @@ -865,7 +886,11 @@ plain `//` comments all become docstrings and accumulate together** (pinned: strip + `^\/\/[/!]?\s?` + `^\s*\*\s?` gm strips fire — **all `gm` strips ride `js_multiline_strip` in docstring.rs (#1329 CRLF semantics) — call the shared code, port nothing**. DOCSTRING_WRAPPER_TYPES contains no dart kinds -→ no anchor climbing. **An `annotation` between the comment run and the +→ no anchor climbing; instead `docstringFor` starts from +getDeclarationWrapper's `declaration` for a member with no body (until +2026-10-06 those looked inside the wrapper and found nothing, e.g. +`const BlocProvider.value(…)`'s dartdoc; validation added 32 docstrings on +bloc and 126 on riverpod, none changed or removed). **An `annotation` between the comment run and the declaration BREAKS the chain** (pinned: `/// Broken by annotation.` `@deprecated` `void annotated()` → doc undefined — the dominant real-world loss since `@override` is ubiquitous). Docstrings attach to: functions, diff --git a/src/extraction/languages/dart.ts b/src/extraction/languages/dart.ts index 788d3392a..c12a02a56 100644 --- a/src/extraction/languages/dart.ts +++ b/src/extraction/languages/dart.ts @@ -497,6 +497,15 @@ export const dartExtractor: LanguageExtractor = { } return false; }, + // A member with no body — a constructor like `Foo._();` or `const Foo.c();`, + // an abstract `void m();` — is a `declaration` wrapping its signature, and + // the member's `///` dartdoc and `@annotation`s come before the wrapper. A + // signature that opens the declaration takes both from there. + getDeclarationWrapper: (node) => { + const parent = node.parent; + if (parent?.type !== 'declaration') return undefined; + return parent.firstNamedChild?.equals(node) ? parent : undefined; + }, resolveBody: (node, bodyField) => { // Dart: function_body is a next sibling of function_signature/method_signature if (node.type === 'function_signature' || node.type === 'method_signature') { diff --git a/src/extraction/tree-sitter-types.ts b/src/extraction/tree-sitter-types.ts index 941e6b36c..d82a66b3c 100644 --- a/src/extraction/tree-sitter-types.ts +++ b/src/extraction/tree-sitter-types.ts @@ -166,6 +166,14 @@ export interface LanguageExtractor { * their own variants. Returns cleaned prose, or undefined when there is none. */ getBodyDocstring?: (node: SyntaxNode, source: string) => string | undefined; + /** + * The node that wraps `node` and stands in its place among the declarations + * around it, when the grammar adds one. The doc comment and annotations + * written before the declaration precede that wrapper, so both are looked up + * from it. Dart wraps a member with no body (`Foo._();`, an abstract + * `void m();`) in a `declaration` node. Returns undefined when there is none. + */ + getDeclarationWrapper?: (node: SyntaxNode) => SyntaxNode | undefined; /** Extract visibility from node */ getVisibility?: (node: SyntaxNode) => 'public' | 'private' | 'protected' | 'internal' | undefined; /** Check if node is exported */ diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 7a4ea76ad..bf8f5f35a 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -562,7 +562,8 @@ export class TreeSitterExtractor { * same symbol, and the column holds free text (#1905). */ private docstringFor(node: SyntaxNode): string | undefined { - const preceding = getPrecedingDocstring(node, this.source); + const anchor = this.extractor?.getDeclarationWrapper?.(node) ?? node; + const preceding = getPrecedingDocstring(anchor, this.source); const body = this.extractor?.getBodyDocstring?.(node, this.source); if (preceding && body) return `${preceding}\n\n${body}`; return body || preceding; @@ -6201,9 +6202,14 @@ export class TreeSitterExtractor { // wrapper objects from `parent`/`namedChild` navigation, so // `sibling === declNode` is unreliable — `startIndex` does // the matching instead. - const parent = declNode.parent; + // + // A grammar that wraps the declaration (Dart's `declaration` + // around a member with no body) puts the annotations before the + // wrapper, so the scan starts there. + const anchor = this.extractor?.getDeclarationWrapper?.(declNode) ?? declNode; + const parent = anchor.parent; if (parent) { - const declStart = declNode.startIndex; + const declStart = anchor.startIndex; let declIdx = -1; for (let i = 0; i < parent.namedChildCount; i++) { const sibling = parent.namedChild(i);