From f7820b584fc901bbce31763ec0ae1dbcff3cac5f Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 23 Oct 2026 07:18:05 +0000 Subject: [PATCH] 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 main 58eb57a3, each arm with its own dist and kernel; the kernel and wasm paths give the same changed-site sets. Re-run after the rebases, on 5b47bc49 (kernel) and 8fdc8a1e (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 --- CHANGELOG.md | 1 + __tests__/dart-part-privacy.test.ts | 201 ++++++++++++++++++++++++++++ src/resolution/dart-libraries.ts | 15 +++ src/resolution/name-matcher.ts | 35 ++++- 4 files changed, 248 insertions(+), 4 deletions(-) create mode 100644 __tests__/dart-part-privacy.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ec68ad3..d4189422 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Dart, a call, a constructor call or a type written without a receiver now links only to a declaration the file can see: one in its own library (the file and its `part` files) or one exported by a library it imports, with `export` chains, `show`, `hide` and `as` prefixes followed the way Dart follows them. So a generated `.g.dart` file reaches its own library's function or type instead of a same-named one in a neighbouring file, a call to a package function such as `fakeAsync`, `runApp` or `unawaited` no longer links to an unrelated project function of that name, and a bare call is never taken for another type's enum value or static constant. Re-index Dart projects after upgrading. - In Dart, an `import` or `export` now links to the file it names: `package:my_app/src/cart.dart` to that package's `lib/src/cart.dart`, and a relative path to the file it points to from the importing file. Before, only the file name at the end was matched, so `import 'package:flutter/widgets.dart'` linked to whichever project file was called `widgets.dart`, and most imports of your own files didn't link to the file at all, which threw off `codegraph affected` and the files listed as using a file. An import of a Dart SDK library or of a package from outside your project now links nothing, and when packages in one repository share a name, the one the importing app depends on by `path:` is used. Re-index Dart projects after upgrading. - In Dart, a `part` directive, like `part 'user.g.dart';` or `part 'src/widgets.dart';`, now links a library to the part's file the way an import does. A library and its parts are one library, so a change to a part, whether a generated `.g.dart` or `.freezed.dart` file or one you wrote, now reaches the library and the code that imports it in `codegraph affected` and in the files listed as using the part. Before, the two files were connected only when a call or a type happened to cross between them, which a library made only of `part` lines never has. A part your project doesn't have, like a generated file that isn't committed, links nothing. Re-index Dart projects after upgrading. +- In Dart, a private name declared in another file of the same library, the library file or one of its `part` files, now links again, as Dart shares such names across the whole library: a call to a `_format()` helper, a `_Foo()` constructor call, or a class extending Riverpod's generated `_$Counter`. Before, a library split into parts, including the code generated into `.g.dart` and `.freezed.dart` files like json_serializable's `_$UserFromJson(json)`, lost the calls and type uses between its files from `callers` and impact. A private name of another library is still never linked. Re-index Dart projects after upgrading. Thanks @libin1528 for the report and @maxmilian. (#2466) - In Dart, an annotation now links to the constant or constructor it names, read the way it is written: `@riverpod` reaches Riverpod's `const riverpod = Riverpod();`, `@Riverpod(keepAlive: true)` the `Riverpod` class, an annotation written through a class, like `@SomeClass.value` or `@SomeClass.named(…)`, that class's own constant, enum value or constructor, and `@meta.immutable` only what the `meta` import brings in. Before, an annotation was matched like a function decorator, so a method, getter or function that only shared its name won: every `@riverpod` provider in Riverpod's own repository was linked to an unrelated getter called `riverpod`, and `callers` and impact for the real annotation missed them. Re-index Dart projects after upgrading. - In Dart, an annotation is no longer lost when a comment sits between it and the declaration, like `@override` followed by `// ignore: must_call_super`, a `///` comment written below the annotations, or `@riverpod` followed by an `// expect_lint: …` line. Before, the comment hid the annotation, so a provider written that way was missing from the annotation's `callers` and impact. Re-index Dart projects after upgrading. - In Dart, a call written through an import prefix, like `http.get(…)` after `import 'package:http/http.dart' as http;`, or `utils.Listener()` and `kit.Box.create()` through a prefix of your own, now links to what the library imported with that prefix exports, following its `export` chains, `show` and `hide`, and to nothing when that library is a package outside your project. Before, the prefix was taken for a variable, so `http.get(…)` could link to an unrelated `Http` class's `get` method, and calls made through a prefix into your own libraries were missing from `callers` and impact. Re-index Dart projects after upgrading. diff --git a/__tests__/dart-part-privacy.test.ts b/__tests__/dart-part-privacy.test.ts new file mode 100644 index 00000000..4da99b6d --- /dev/null +++ b/__tests__/dart-part-privacy.test.ts @@ -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): 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); + } +} + +/** ` -> :` 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)); +} + +/** ` -> :` 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 = { + '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 json) => _$UserFromJson(json); +} + +class Counter extends _$Counter {} +`, + 'lib/models/user.g.dart': `part of 'user.dart'; + +User _$UserFromJson(Map 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([]); + }); + }); +} diff --git a/src/resolution/dart-libraries.ts b/src/resolution/dart-libraries.ts index 23018998..edc66910 100644 --- a/src/resolution/dart-libraries.ts +++ b/src/resolution/dart-libraries.ts @@ -794,3 +794,18 @@ export function inSameDartLibrary(fromFile: string, declFile: string, context: R const scope = scopeOf(fromFile, context, memoFor(context)); return scope !== null && scope.own.has(declFile); } + +/** + * Whether a private `_name` declared in `declFile` can be named in `fromFile`: + * both are files of one library, the library file and its parts. Dart keeps a + * `_name` to its library, not to its file, so a library split into `part`s + * calls its own private helpers across them. Unlike inSameDartLibrary this + * needs no pubspec.yaml above the files: a part names its library by a path + * or a library name. False when the library of `fromFile` is unknown. + */ +export function dartPrivateVisible(fromFile: string, declFile: string, context: ResolutionContext): boolean { + if (fromFile === declFile) return true; + const memo = memoFor(context); + const library = libraryOf(fromFile, context, memo); + return library !== null && membersOf(library, context, memo).has(declFile); +} diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 25a697a1..55fbe560 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -12,7 +12,7 @@ import { UnresolvedRef, ResolvedRef, ResolutionContext, ImportMapping, isSuperty import { blankStringContents, stripCommentsForRegex } from './strip-comments'; import { JS_ARRAY_METHODS, JS_BUILT_INS, JS_BUILTIN_METHODS, TS_PRIMITIVE_TYPES } from './js-builtins'; import { SWIFT_TYPE_PATH_CALL, resolveSwiftTypePathCall } from './swift-type-visibility'; -import { dartImportPrefixes, dartLibrarySees, dartPrefixSees, inSameDartLibrary } from './dart-libraries'; +import { dartImportPrefixes, dartLibrarySees, dartPrefixSees, dartPrivateVisible, inSameDartLibrary } from './dart-libraries'; import { isDartLocallyBound } from './dart-local-scope'; import { breakVbTie, isVbMemberInScope, isVbNestedTypeInScope, isVbTypeQualifiedBy, matchVbTypedCall, preferVbProject, sameVbProject } from './vbnet-receivers'; import { cppAliasedClass, cppAliasedTypeName, cppAliasedTypeWritten, cppDeclaresLocalAlias, cppParentScope, cppScopesWithin, cppTemplateArguments, cppTemplateParameters, cppTypeNamesDeclared, cppTypeSegments, isCppPointerType, resolveCppAliasedType, stripCppTemplateArguments, type CppAliasedType } from './cpp-type-aliases'; @@ -2150,9 +2150,10 @@ function luaModuleFile(name: string, decl: Node, context: ResolutionContext): st /** * Languages in which `visibility: 'private'` on a definition means no other * FILE can name it: a Kotlin `private fun` is file- or class-local, and the - * same holds for Java, C#, Swift, Scala, Dart and PHP members. + * same holds for Java, C#, Swift, Scala and PHP members. (A Dart `_name` is + * its library's, which can span several files: see isVisibleAcrossFiles.) */ -const PRIVATE_IS_FILE_LOCAL = new Set(['kotlin', 'java', 'csharp', 'swift', 'scala', 'dart', 'php']); +const PRIVATE_IS_FILE_LOCAL = new Set(['kotlin', 'java', 'csharp', 'swift', 'scala', 'php']); /** Per-context memo: node id → "this C/C++ function is declared `static`". */ const C_STATIC_MEMO = new WeakMap>(); @@ -3404,9 +3405,12 @@ function isGoNameVisible(candidate: Node, ref: UnresolvedRef, context: Resolutio * - **Objective-C**: a struct, union, enum, typedef or global a `.m` / `.mm` * source file defines is local to that file, as in C; other files reach a * global through its header's `extern` declaration. A class is not. - * - **Kotlin, Java, C#, Swift, Scala, Dart, PHP**: `private` is class- or + * - **Kotlin, Java, C#, Swift, Scala, PHP**: `private` is class- or * file-local. An Android `editor.apply()` resolved onto an unrelated class's * `private fun apply`. + * - **Dart**: a `_name` is private to its library — the library file and the + * files its `part` directives name — so another file of the library names + * it, and a file of another library never does (#2466). * - **Go**: an unexported (lowercase) identifier is package-local, and a * package is a directory. Judged by the name's case: the extractor's * `isExported` is unset for every Go method. @@ -3496,6 +3500,13 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex const owner = rustModuleDir(candidate.filePath); return ref.filePath.startsWith(owner + '/'); } + // A Dart `_name` is its library's, which spans the library file and its + // parts: riverpod_generator's `class Counter extends _$Counter` names the + // class it wrote into `main.g.dart`, and protobuf.dart's GeneratedMessage + // calls `_fieldSet._clear()` on the FieldSet another part declares. + if (lang === 'dart') { + return candidate.visibility !== 'private' || (ref.language === 'dart' && dartPrivateVisible(ref.filePath, candidate.filePath, context)); + } if (PRIVATE_IS_FILE_LOCAL.has(lang)) return candidate.visibility !== 'private'; // An R test file runs in an environment of its own (testthat): its top-level // `c <- ggplot(…)` is not what the package's 2,843 `c(…)` calls mean. The @@ -4364,6 +4375,21 @@ function isDartTopLevelVisible(candidate: Node, ref: UnresolvedRef, context: Res : dartPrefixSees(ref.filePath, prefix, candidate.filePath, candidate.name, context); } +/** + * Whether `n` is a Dart `_name` that another library declares, which nothing + * written in the reference's library can mean (#2466). It leaves the + * candidates before one is picked, so the library's own declaration wins + * where another library's namesake would have ranked first and then been + * declined: the Dart SDK's wasm `_ImmutableLinkedHashMapMixin` calls the + * `_createIndex` of the `_MapCreateIndexMixin` it is `on`, not the one the + * VM's mixin of the same name declares. isVisibleAcrossFiles applies the + * same rule to the pipeline's pick. + */ +function isDartPrivateOfAnotherLibrary(n: Node, ref: UnresolvedRef, context: ResolutionContext): boolean { + return n.language === 'dart' && n.visibility === 'private' && n.filePath !== ref.filePath && + !dartPrivateVisible(ref.filePath, n.filePath, context); +} + /** * The import prefix a Dart type name is written through — the `p` of * `p.Report`, `p.Color.red` — or null. A type reference's column is the @@ -11844,6 +11870,7 @@ export function matchByExactName( !(ref.language === 'dart' && isDartConstructor(n, context)) && !(dartBare && isDartTypeValue(n) && !isDartMethodInScope(n, ref, context)) && !((dartBare || dartTypeRef) && !isDartTopLevelVisible(n, ref, context, dartPrefix)) && + !(ref.language === 'dart' && isDartPrivateOfAnotherLibrary(n, ref, context)) && !(phpSelf && (n.kind !== 'method' || !isPhpMethodInScope(n, ref, phpSelf, context))) && !(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)) && !(rustBare && !isRustNameInScope(n, ref, context)) &&