From f1c8540a3efb57c3a2cd15790928790db191ee5c Mon Sep 17 00:00:00 2001 From: Alexandru Mariuti Date: Wed, 26 Aug 2026 17:36:37 +0700 Subject: [PATCH 1/2] =?UTF-8?q?chore:=20repo=20hygiene=20=E2=80=94=20regen?= =?UTF-8?q?erable=20fixtures,=20accepted=20toolchain=20migrations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - example/lib/main.dart: commit the regenerated output; the const drift predated this branch (source lost the const in dev.1, the committed lib was stale) and every rebuild has been surfacing it. The generated call sites legitimately lack const (the const-call-site rewriter is per-file and cannot see sibling-file lowered classes), so example's analysis_options now suppresses prefer_const_constructors. - integration_tests cross_file_env: relative same-package imports (the generator rejects package: self-imports) and a non-const List initializer (const literals are rejected for collection signals) — the fixture was unregenerable: build_runner deleted its two tracked lib/ outputs on every run, masking the second blocker. lib/ keeps always_use_package_imports off (it mirrors source's relative style; test/ deliberately keeps package: imports). - example/examples analysis_options: accept the Flutter-tooling exclude-block auto-migration so pub get stops churning the files. --- example/analysis_options.yaml | 15 +++++++++++++++ example/lib/main.dart | 2 +- examples/chat/analysis_options.yaml | 8 ++++++++ examples/todos/analysis_options.yaml | 8 ++++++++ examples/weather/analysis_options.yaml | 8 ++++++++ packages/integration_tests/analysis_options.yaml | 9 +++++++++ .../lib/cross_file_env/counter_controller.dart | 2 +- .../lib/cross_file_env/counter_display.dart | 2 +- .../source/cross_file_env/counter_controller.dart | 2 +- .../source/cross_file_env/counter_display.dart | 11 ++++++----- .../test/cross_file_environment_test.dart | 10 +++++----- 11 files changed, 63 insertions(+), 14 deletions(-) diff --git a/example/analysis_options.yaml b/example/analysis_options.yaml index a395964..78c6a1b 100644 --- a/example/analysis_options.yaml +++ b/example/analysis_options.yaml @@ -7,6 +7,21 @@ analyzer: avoid_print: ignore unnecessary_statements: ignore always_use_package_imports: ignore + # The generator's const-call-site rewriter is per-file: it can't know + # that a sibling file's class became const-eligible after lowering + # (e.g. `CounterPage` gains a const constructor once its `@SolidState` + # fields move to a generated State class), and `source/` can't carry + # the `const` pre-lowering since the class isn't const-eligible there. + # Generated call sites legitimately lack `const` in this case. + prefer_const_constructors: ignore + exclude: + - build/** + - android/** + - ios/** + - web/** + - windows/** + - macos/** + - linux/** linter: rules: public_member_api_docs: false diff --git a/example/lib/main.dart b/example/lib/main.dart index 812235a..63d27c8 100644 --- a/example/lib/main.dart +++ b/example/lib/main.dart @@ -3,5 +3,5 @@ import 'package:flutter/material.dart'; import 'counter.dart'; void main() { - runApp(const MaterialApp(home: CounterPage())); + runApp(MaterialApp(home: CounterPage())); } diff --git a/examples/chat/analysis_options.yaml b/examples/chat/analysis_options.yaml index 6f43866..a95c70d 100644 --- a/examples/chat/analysis_options.yaml +++ b/examples/chat/analysis_options.yaml @@ -15,6 +15,14 @@ analyzer: use_setters_to_change_properties: ignore deprecated_member_use: ignore avoid_equals_and_hash_code_on_mutable_classes: ignore + exclude: + - build/** + - android/** + - ios/** + - web/** + - windows/** + - macos/** + - linux/** linter: rules: public_member_api_docs: false diff --git a/examples/todos/analysis_options.yaml b/examples/todos/analysis_options.yaml index 4698185..4008deb 100644 --- a/examples/todos/analysis_options.yaml +++ b/examples/todos/analysis_options.yaml @@ -30,6 +30,14 @@ analyzer: # `Foo` from the generic call but `very_good_analysis` treats this as # non-obvious. The type IS clear from the receiver; suppress here. specify_nonobvious_property_types: ignore + exclude: + - build/** + - android/** + - ios/** + - web/** + - windows/** + - macos/** + - linux/** linter: rules: public_member_api_docs: false diff --git a/examples/weather/analysis_options.yaml b/examples/weather/analysis_options.yaml index 6f43866..a95c70d 100644 --- a/examples/weather/analysis_options.yaml +++ b/examples/weather/analysis_options.yaml @@ -15,6 +15,14 @@ analyzer: use_setters_to_change_properties: ignore deprecated_member_use: ignore avoid_equals_and_hash_code_on_mutable_classes: ignore + exclude: + - build/** + - android/** + - ios/** + - web/** + - windows/** + - macos/** + - linux/** linter: rules: public_member_api_docs: false diff --git a/packages/integration_tests/analysis_options.yaml b/packages/integration_tests/analysis_options.yaml index a15083c..fd9e2a5 100644 --- a/packages/integration_tests/analysis_options.yaml +++ b/packages/integration_tests/analysis_options.yaml @@ -27,3 +27,12 @@ analyzer: linter: rules: public_member_api_docs: false + # `source/` uses relative imports for same-package siblings, and the + # generator mirrors those paths 1:1 into `lib/`. Turn off + # `very_good_analysis`'s default `always_use_package_imports` so the + # generated `lib/` imports stay relative. Unlike `example/`, don't + # enable `prefer_relative_imports` as a tripwire here: `test/` + # deliberately imports the generated code via `package:integration_tests/...` + # (it exercises the public, generated `lib/` API, not a same-package + # sibling), so the rule would flag those intentional imports. + always_use_package_imports: false diff --git a/packages/integration_tests/lib/cross_file_env/counter_controller.dart b/packages/integration_tests/lib/cross_file_env/counter_controller.dart index fe7c80a..c2a9f65 100644 --- a/packages/integration_tests/lib/cross_file_env/counter_controller.dart +++ b/packages/integration_tests/lib/cross_file_env/counter_controller.dart @@ -4,7 +4,7 @@ import 'package:solid_annotations/solid_annotations.dart'; class Counter implements Disposable { final value = Signal(0, name: 'value'); - final history = ListSignal(const [], name: 'history'); + final history = ListSignal([], name: 'history'); @override void dispose() { diff --git a/packages/integration_tests/lib/cross_file_env/counter_display.dart b/packages/integration_tests/lib/cross_file_env/counter_display.dart index cb7e82a..6027949 100644 --- a/packages/integration_tests/lib/cross_file_env/counter_display.dart +++ b/packages/integration_tests/lib/cross_file_env/counter_display.dart @@ -1,7 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_solidart/flutter_solidart.dart'; -import 'package:integration_tests/cross_file_env/counter_controller.dart'; import 'package:provider/provider.dart'; +import 'counter_controller.dart'; class CounterDisplay extends StatefulWidget { const CounterDisplay({super.key}); diff --git a/packages/integration_tests/source/cross_file_env/counter_controller.dart b/packages/integration_tests/source/cross_file_env/counter_controller.dart index 1072f67..9584b3c 100644 --- a/packages/integration_tests/source/cross_file_env/counter_controller.dart +++ b/packages/integration_tests/source/cross_file_env/counter_controller.dart @@ -11,7 +11,7 @@ class Counter { int value = 0; @SolidState() - List history = const []; + List history = []; // Empty stub — the source-layer analyzer needs to resolve `c.dispose()` in // the `.environment(... dispose: (_, c) => c.dispose())` callback. diff --git a/packages/integration_tests/source/cross_file_env/counter_display.dart b/packages/integration_tests/source/cross_file_env/counter_display.dart index 67a456b..3e09c7b 100644 --- a/packages/integration_tests/source/cross_file_env/counter_display.dart +++ b/packages/integration_tests/source/cross_file_env/counter_display.dart @@ -1,13 +1,14 @@ // Cross-file env-injection consumer — imports `Counter` from a sibling -// source file via `package:integration_tests/cross_file_env/...`. The -// generator's resolver pass redirects same-package `lib/` URIs back to -// `source/` so the `@SolidState` annotations are visible to the rewriter -// even though the import resolves to `lib/cross_file_env/...` post-build. +// source file via a relative URI. The generator's resolver pass redirects +// relative source imports to their `lib/` counterparts so the +// `@SolidState` annotations are visible to the rewriter even though the +// import resolves to `lib/cross_file_env/...` post-build. import 'package:flutter/material.dart'; -import 'package:integration_tests/cross_file_env/counter_controller.dart'; import 'package:solid_annotations/solid_annotations.dart'; +import 'counter_controller.dart'; + class CounterDisplay extends StatelessWidget { CounterDisplay({super.key}); diff --git a/packages/integration_tests/test/cross_file_environment_test.dart b/packages/integration_tests/test/cross_file_environment_test.dart index b3f13b5..2538408 100644 --- a/packages/integration_tests/test/cross_file_environment_test.dart +++ b/packages/integration_tests/test/cross_file_environment_test.dart @@ -1,11 +1,11 @@ // Runtime fence for the cross-file `@SolidEnvironment` chain rewrite. // `Counter` is declared in `source/cross_file_env/counter_controller.dart` // and consumed by `CounterDisplay` in `source/cross_file_env/counter_display.dart` -// via `package:integration_tests/cross_file_env/...`. The two files are -// generated independently — the generator's resolver pass redirects the -// same-package `lib/` URI back to `source/` so the rewriter sees the -// pre-transformation `@SolidState` annotations even though Dart resolves -// the import to `lib/`. +// via a relative URI. The two files are generated independently — the +// generator's resolver pass redirects relative source imports to their +// `lib/` counterparts so the rewriter sees the pre-transformation +// `@SolidState` annotations even though Dart resolves the import to +// `lib/`. // // This test asserts that the SignalBuilder placements emitted in // `counter_display.dart` actually subscribe to the live ListSignal/Signal From e7504db5e25910be8a613e969540da27be74184b Mon Sep 17 00:00:00 2001 From: Alexandru Mariuti Date: Wed, 26 Aug 2026 18:14:15 +0700 Subject: [PATCH 2/2] fix(solid_generator): seed bare super.x constructor params syntactically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #108. An annotation-free pure consumer whose only link to a @SolidState class is a bare super.x parameter carried no type text for the probe gate to seed, so the file passed through verbatim and its cross-file reads stayed silently un-lowered. The seeding walk now resolves bare super formals syntactically: the extends-clause name is located in the current unit or via the import walk (alias/prefix-aware, same-package imports only, local declarations shadow), the targeted super constructor is matched (named or unnamed; positional matching among super formals is unambiguous — Dart rejects the mixes that could confuse it), untyped this.x matches fall back to the superclass field's type, super-formal chains recurse with a depth bound, and generic type parameters map by index to the extends-clause type arguments. Ambiguity over-seeds candidate names instead of guessing. The registry walk gains a one-hop extension into the located superclass file's own imports — gated by the consuming file's local declarations (review caught the ungated version rewriting a local plain class) — since the consumer typically imports only its base file. Syntactic seeding is skipped when resolved-path resolution already succeeded, so ubiquitous {super.key} params never trigger file reads. Five golden fixture families cover the issue shape (de-rigged: the consumer imports only its base file), named targets and named params, two-hop chains, generic bases, the local-shadow decoy, and a no-@SolidState negative proving the fast path survives. Validated against a real downstream app via path overrides before release: byte-identical output on the app itself plus a probe of the exact issue shape lowering correctly. --- SPEC.md | 6 +- packages/solid_generator/CHANGELOG.md | 4 + packages/solid_generator/lib/builder.dart | 722 ++++++++++++++++-- packages/solid_generator/pubspec.yaml | 2 +- .../auth_repository.dart | 11 + .../base_repository.dart | 9 + .../leaf_repository.dart | 20 + .../mid_repository.dart | 11 + .../base_repository.dart | 11 + .../consumer.dart | 20 + .../plain_repository.dart | 7 + .../auth_repository.dart | 14 + .../base_repository.dart | 23 + .../customers_repository.dart | 49 ++ .../auth_repository.dart | 8 + .../base_repository.dart | 10 + .../consumer.dart | 21 + .../base.dart | 11 + .../consumer.dart | 48 ++ .../foreign_foo.dart | 11 + .../auth_repository.g.dart | 11 + .../base_repository.g.dart | 9 + .../leaf_repository.g.dart | 20 + .../mid_repository.g.dart | 11 + .../base_repository.g.dart | 11 + .../consumer.g.dart | 20 + .../plain_repository.g.dart | 7 + .../auth_repository.g.dart | 11 + .../base_repository.g.dart | 23 + .../customers_repository.g.dart | 49 ++ .../auth_repository.g.dart | 11 + .../base_repository.g.dart | 10 + .../consumer.g.dart | 21 + .../base.g.dart | 11 + .../consumer.g.dart | 48 ++ .../foreign_foo.g.dart | 11 + .../test/integration/golden_helpers.dart | 5 + 37 files changed, 1225 insertions(+), 82 deletions(-) create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/auth_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/base_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/leaf_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/mid_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/base_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/consumer.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/plain_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/auth_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/base_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/customers_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/auth_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/base_repository.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/consumer.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/base.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/consumer.dart create mode 100644 packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/foreign_foo.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/auth_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/base_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/leaf_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/mid_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/base_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/consumer.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/plain_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/auth_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/base_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/customers_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/auth_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/base_repository.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/consumer.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/base.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/consumer.g.dart create mode 100644 packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/foreign_foo.g.dart diff --git a/SPEC.md b/SPEC.md index b40e5e3..2a69a24 100644 --- a/SPEC.md +++ b/SPEC.md @@ -30,7 +30,7 @@ Rules: - **Input path**: any `.dart` file under `source/` at any depth. - **Output path**: same relative path under `lib/`. No suffix change. `source/foo/bar.dart` becomes `lib/foo/bar.dart`. -- **Transformation vs verbatim copy.** Solid reads every `.dart` file under `source/`. A file is transformed if it contains at least one `@Solid*` annotation (`@SolidState`, `@SolidEffect`, `@SolidQuery`, `@SolidEnvironment`), OR if a class it declares reaches a `@SolidState`-bearing class declared in another source file purely through a plain constructor-injected field or instance field (a "pure consumer") — in that narrower case only the `.value` reads reaching the injected class are lowered; the consuming class gains no `Disposable`/`dispose()` synthesis, since it owns no reactive member of its own to dispose. Otherwise the file is copied verbatim to the mirrored path under `lib/`. Non-`.dart` files (assets, configs, etc.) are always copied verbatim. The key is annotation presence or a resolvable cross-file `@SolidState` reference, not file extension. A `StatelessWidget`/`State` reached the same way is a pure consumer too: its `build()` method gets the same `.value` lowering plus a `SignalBuilder` wrap around the tracked subtree — the same wrap-placement rule (Section 7) an `@SolidEnvironment`-consuming widget's `build()` already gets — with no widget-class Stateless→Stateful lift, since a plain constructor-injected field needs no `BuildContext`. Static-field-mediated DI (`static final AuthRepository instance = …;`, consumed elsewhere as `Holder.instance.session`) is recognized as well: a class reached only through a static field or singleton holder enters the registry the same as one reached through an instance field. When a file contains BOTH a pure-consumer widget and a pure-consumer plain class (e.g. a `StatelessWidget` alongside a router-guard class that reads a static-holder receiver like `Holder.instance.session`), both are lowered correctly against the same fully-resolved read of the file — the widget's `.value`-plus-`SignalBuilder` treatment and the plain class's `.value` lowering do not interfere with each other, including when the plain class's reactive read is a shape (a static-holder receiver, a for-in loop variable, `.first`) that only resolves through the type-resolved analyzer pass. If a pure-consumer widget's `SignalBuilder` wrap is the file's only reference to `flutter_solidart`, and the file already imports `flutter_solidart` with a `show`/`hide` combinator that doesn't expose `SignalBuilder` (e.g. a pre-existing `show Signal` for an unrelated hand-rolled signal), the generator repairs that import's combinators in place rather than leaving `SignalBuilder` unresolvable. One narrower, related gap remains: a pure consumer whose ONLY link to the injected class is a bare-shorthand `super.x` constructor parameter (no type written at that position) is not recognized, because detecting a pure consumer at all requires a syntactic (unresolved) pre-parse, and a bare `super.x`'s type is not present in the source text for that pass to find. A file that is transformed for some OTHER reason (it has its own `@Solid*` annotation, or a provider call site) does not have this limitation — such a file is already committed to the full, type-resolved lowering pass, which recognizes a bare `super.x`'s type the normal way. +- **Transformation vs verbatim copy.** Solid reads every `.dart` file under `source/`. A file is transformed if it contains at least one `@Solid*` annotation (`@SolidState`, `@SolidEffect`, `@SolidQuery`, `@SolidEnvironment`), OR if a class it declares reaches a `@SolidState`-bearing class declared in another source file purely through a plain constructor-injected field or instance field (a "pure consumer") — in that narrower case only the `.value` reads reaching the injected class are lowered; the consuming class gains no `Disposable`/`dispose()` synthesis, since it owns no reactive member of its own to dispose. Otherwise the file is copied verbatim to the mirrored path under `lib/`. Non-`.dart` files (assets, configs, etc.) are always copied verbatim. The key is annotation presence or a resolvable cross-file `@SolidState` reference, not file extension. A `StatelessWidget`/`State` reached the same way is a pure consumer too: its `build()` method gets the same `.value` lowering plus a `SignalBuilder` wrap around the tracked subtree — the same wrap-placement rule (Section 7) an `@SolidEnvironment`-consuming widget's `build()` already gets — with no widget-class Stateless→Stateful lift, since a plain constructor-injected field needs no `BuildContext`. Static-field-mediated DI (`static final AuthRepository instance = …;`, consumed elsewhere as `Holder.instance.session`) is recognized as well: a class reached only through a static field or singleton holder enters the registry the same as one reached through an instance field. When a file contains BOTH a pure-consumer widget and a pure-consumer plain class (e.g. a `StatelessWidget` alongside a router-guard class that reads a static-holder receiver like `Holder.instance.session`), both are lowered correctly against the same fully-resolved read of the file — the widget's `.value`-plus-`SignalBuilder` treatment and the plain class's `.value` lowering do not interfere with each other, including when the plain class's reactive read is a shape (a static-holder receiver, a for-in loop variable, `.first`) that only resolves through the type-resolved analyzer pass. If a pure-consumer widget's `SignalBuilder` wrap is the file's only reference to `flutter_solidart`, and the file already imports `flutter_solidart` with a `show`/`hide` combinator that doesn't expose `SignalBuilder` (e.g. a pre-existing `show Signal` for an unrelated hand-rolled signal), the generator repairs that import's combinators in place rather than leaving `SignalBuilder` unresolvable. A pure consumer whose ONLY link to the injected class is a bare-shorthand `super.x` constructor parameter (no type written at that position) is recognized too: detecting a pure consumer at all requires a syntactic (unresolved) pre-parse, and a bare `super.x`'s type is not present in the source text at that position — but the class's `extends` clause names the superclass, and the superclass declaration (found locally, or via the same import walk this pre-parse already does — same-package imports only, alias-aware: an unprefixed `extends Base` skips any `import ... as x;` directive, a prefixed `extends x.Base` considers only the import carrying that exact prefix) still carries the type syntactically, either on the matching super-constructor parameter or, for the common `Base(this.x)` shape, on the base class's own same-named instance field. The generator locates the super-constructor the subclass's initializer list targets (the unnamed constructor by default, or a named one via `super.named(...)`) and matches the bare parameter against it — positionally among the constructor's other `super.` parameters, or by name — falling back to the field declaration when the matched parameter is itself an untyped `this.x`. When the matched parameter is ITSELF a bare `super.x` (a chain of plain relaying subclasses, e.g. `GrandChild(super.x) extends Child(super.x) extends Base(this.x)`), the search recurses up the chain, up to five superclasses deep, before giving up. When that match can't be pinned down confidently, every candidate type name from the located superclass (every constructor parameter, every instance field) is seeded instead of guessing wrong or skipping silently — over-seeding a name is harmless, since the registry only fills from classes that actually carry `@SolidState` members. Locating the injected class's OWN declaration, once its name is known, follows the SAME file the superclass search already reached, plus exactly one more hop through that file's own imports (never a general transitive walk) — enough to cover the common shape where a consumer imports only its immediate superclass's file, and that file in turn imports the class the superclass's constructor names. A class reachable only through a longer chain of imports beyond that one extra hop is a documented residual, not recognized. - **Both are committed to git.** Source is the review artifact for intent. Lib is the review artifact for correctness — every PR that changes `source/` must include the regenerated `lib/` diff so reviewers catch generator regressions. - **Solid emits no `.g.dart` files of its own.** Third-party generators (freezed, json_serializable, drift) may emit `.g.dart` or `.freezed.dart` files under `source/`; Solid copies those verbatim to the mirrored path under `lib/`. - **The example app's `main.dart`** lives in `lib/` (or `source/` if itself annotated) and imports from `lib/` using normal Flutter imports (`import 'counter.dart';`). @@ -1087,9 +1087,9 @@ Rules: - **The builder's no-annotation fast path** (a file with zero `@Solid*`-annotated classes) hands the generator a fully-RESOLVED `CompilationUnit`. Clause 1 uses `InterfaceType.lookUpMethod`; clause 2 is checked BOTH via the name-based registry below AND, secondarily, via the resolved element's own annotation metadata. The registry is the primary cross-file mechanism: it walks the importing file's imports in order, and a same-named class with no `@SolidState` members no longer halts the search — the wanted type name stays outstanding until an actually-annotated match is found (or every import is exhausted), so a non-annotated decoy earlier in import order can't strand a real annotated class in a later import unrecognized. The secondary resolved-element-annotation check remains a belt-and-suspenders backstop for cases the registry doesn't reach; it is not the mechanism the design relies on. A resolved-but-unproven type is clause 3 (skip): resolution IS visibility. - **The main lowering path** (a file with at least one `@Solid*`-annotated class) re-parses the fully ASSEMBLED, already-lowered output text with NO resolver at all. Clause 1 is a same-file-only AST check (a Solid-lowered class's synthesized `dispose()` is already textually present in that reassembled text by the time this check runs). Clause 2 is name-based only, via the registry below. A type whose declaration lives in another file and ISN'T in the registry — e.g. a plain, dispose-less repository imported and provided but never itself consuming anything — cannot be proven plain on this path and falls to clause 4 (inject); `dispose: null` is required there even though the type genuinely has no `dispose()`. - Clause 2's registry is the SAME class-name → reactive-member-name map (`classRegistry`) the builder threads through the `.value` cross-class rewrite (§5.1) — populated same-file by a fast member-scan and cross-file by walking imports (the resolver pass documented under §3.6's env-field rule), extended to also seed from every type created at a `Provider(...)` / `.environment()` call site in the file (not just `@SolidEnvironment` field types), so a controller that's provided but never consumed via `@SolidEnvironment` still gets recognized. It is further extended to seed from the declared type name of every class's instance field and constructor parameter in the file — the plain constructor-injection DI shape (`CustomersRepository({required AuthRepository authRepository})`, `final AuthRepository _authRepository;`), which has no `@SolidEnvironment` field and creates nothing at a `Provider(...)` / `.environment()` call site, so neither of the other two rules ever seeds it (issue #104). A type name enters the registry ONLY when its declaration carries at least one `@SolidState` field or getter; a candidate name that matches nothing found is harmless. + Clause 2's registry is the SAME class-name → reactive-member-name map (`classRegistry`) the builder threads through the `.value` cross-class rewrite (§5.1) — populated same-file by a fast member-scan and cross-file by walking imports (the resolver pass documented under §3.6's env-field rule), extended to also seed from every type created at a `Provider(...)` / `.environment()` call site in the file (not just `@SolidEnvironment` field types), so a controller that's provided but never consumed via `@SolidEnvironment` still gets recognized. It is further extended to seed from the declared type name of every class's instance field and constructor parameter in the file — the plain constructor-injection DI shape (`CustomersRepository({required AuthRepository authRepository})`, `final AuthRepository _authRepository;`), which has no `@SolidEnvironment` field and creates nothing at a `Provider(...)` / `.environment()` call site, so neither of the other two rules ever seeds it (issue #104). A `super.` constructor parameter seeds the same way: an explicit-typed one (`Foo(AuthRepository super.repo)`) reads its own annotation like any other parameter, and a bare one (`Foo(super.repo)`) is resolved — on a resolved unit, from the analyzer's own resolution of the forwarded parameter; on an unresolved unit (the pure-consumer probe), by locating the superclass declaration and matching the targeted super-constructor's corresponding parameter, recursing up a chain of further bare `super.` relays where needed — or, for a `this.x` field-formal target, the base class's same-named instance field (§2; issue #108). The superclass-location search is same-package-only and alias-aware (§2), and never runs at all once the resolved path has already answered the question — both matter in practice because this same code path fires for `{super.key}` on every widget, and without them would otherwise chase `StatefulWidget`/`State` into `package:flutter` on every unresolved probe. Once the wanted name is known, finding ITS OWN declaration gets one extra hop beyond the import walk this paragraph describes — through the file that hosts the superclass's own imports — never a general transitive walk (§2). A type name enters the registry ONLY when its declaration carries at least one `@SolidState` field or getter; a candidate name that matches nothing found is harmless. - Two rules narrow the cross-file import walk to mirror Dart's own name resolution, closing collision windows the annotation-blind seeding above would otherwise open: a simple name that the CURRENT file itself declares as a class/enum/mixin/etc. is dropped from the wanted set before the import walk starts — a local top-level declaration always shadows a same-name import (no error, no ambiguity), so cross-file attribution for that name is provably wrong and the import walk never even looks for it; and each import's `show`/`hide` combinators are honored — an import that hides the wanted name, or carries a `show` list that doesn't include it, cannot be credited as that name's source and is skipped for that name (its other names are unaffected). Matching is otherwise by SIMPLE class name, not library identity — two distinct types sharing a name across two DIFFERENT imported libraries, neither of them local to the current file and neither excluded by a combinator, could in principle still collide; a residual risk of the name-based registry design, not resolved by the two rules above. + Two rules narrow the cross-file import walk to mirror Dart's own name resolution, closing collision windows the annotation-blind seeding above would otherwise open: a simple name that the CURRENT file itself declares as a class/enum/mixin/etc. is dropped from the wanted set before the import walk starts — a local top-level declaration always shadows a same-name import (no error, no ambiguity), so cross-file attribution for that name is provably wrong and the import walk never even looks for it; and each import's `show`/`hide` combinators are honored — an import that hides the wanted name, or carries a `show` list that doesn't include it, cannot be credited as that name's source and is skipped for that name (its other names are unaffected). Matching is otherwise by SIMPLE class name, not library identity — two distinct types sharing a name across two DIFFERENT imported libraries, neither of them local to the current file and neither excluded by a combinator, could in principle still collide; a residual risk of the name-based registry design, not resolved by the two rules above. The bare-`super.x` seeder's own ambiguity fallback (§2; issue #108) widens this collision surface a little further when it fires: unable to pin down a single matching parameter, it seeds EVERY field's and constructor parameter's type name from the located superclass, including names that would otherwise never have been proposed as candidates for the current file at all. The injected `provider.dispose()` always compiles for Solid-lowered types because Section 10 attaches `implements Disposable` and a synthesized `dispose()` to every annotated class. For non-Solid types, the user declares `void dispose()` (or a known disposable base) on the source class for clause 1 to fire; otherwise nothing is injected (clause 3) unless the type is cross-file and unproven (clause 4), and the user may always opt in or out by writing `dispose:` explicitly. diff --git a/packages/solid_generator/CHANGELOG.md b/packages/solid_generator/CHANGELOG.md index f78b0d7..c5a8278 100644 --- a/packages/solid_generator/CHANGELOG.md +++ b/packages/solid_generator/CHANGELOG.md @@ -1,3 +1,7 @@ +## 3.0.0-dev.5 + +- **FIX**: The dev.4 residual gap — a PURE CONSUMER whose ONLY link to a cross-file `@SolidState`-bearing class is a bare `super.x` constructor parameter (no type written at that position) — is now closed (#108), including issue #108's own primary example: a consumer that imports only its immediate superclass's file, never the file that actually declares the injected class. Detecting a pure consumer at all still requires the no-annotation fast path's UNRESOLVED syntactic probe, and a bare `super.x` still carries no type text at that position, but `_populateCrossFileTypes`'s seeding loop now resolves it from the AST alone: it reads the subclass's own `extends` clause for the superclass's simple name, locates that class's declaration (checking the current unit's own top-level declarations first — skipped when the `extends` clause is import-prefixed, since a prefixed reference can never resolve to a local declaration — then walking imports with the same shadowing/combinator/skip-set discipline the rest of the function already applies, PLUS two more: same-package only, since this package's `@SolidState` classes can never live in a cross-package file — critical in practice, since this same code path fires for `{super.key}` on every widget, and without the guard would otherwise chase `StatefulWidget`/`State` into `package:flutter`; and alias-aware, since an unprefixed name can only come from an unprefixed import and a prefixed name only from the import carrying that exact prefix — `_importExposesName`'s show/hide check alone can't tell a same-named decoy behind a DIFFERENT prefix from the real, unprefixed source), finds the super-constructor the subclass's initializer list targets (`super.named(...)` for a named constructor, else the implicit call to the unnamed one), and matches the bare parameter against that constructor's corresponding parameter — positionally among its other `super.` siblings (never ambiguous for code that compiles, since Dart itself rejects mixing a positional `super.x` with an explicit positional argument in the same invocation) or by name. When the matched parameter is itself an untyped `this.x` field formal, the same-named instance field's declared type is used instead; when it is ITSELF a further bare `super.x` (a chain of plain relaying subclasses forwarding the same parameter), the search recurses up the inheritance chain, up to five superclasses deep, before giving up. The syntactic walk is skipped entirely once the RESOLVED fallback (`_seedFromResolvedSuperFormal`, dev.3) already answered the question — running the full import-and-parse walk on top of a successful resolution was pure waste, and, unconditional, was the other half of the `{super.key}`-on-every-widget cost. Once the wanted class's simple name is known, locating ITS OWN declaration gets exactly one further hop beyond the import walk just described — through the file that hosts the superclass's own imports, same-package-only, never a general transitive walk: this is what closes issue #108's own primary example, where the injected class is declared in a file the pure consumer never imports at all, only its superclass's file does. A class reachable only through a LONGER chain of imports beyond that one extra hop remains a documented residual, not recognized. When the matched parameter's declared type is itself one of the superclass's OWN type parameters (`class Base { Base(this.repo); final T repo; }`), the wanted name is mapped BY INDEX to the subclass's own `extends Base` type argument, recursing through nested generics, instead of seeding the placeholder name verbatim — a bare type-parameter reference is a syntactically ordinary type name, so without this mapping it silently seeded a name (`T`) that matches no class anywhere and the real dependency (`AuthRepository`) was never sought at all (fix review addendum finding 2). When the superclass can't be located, the targeted constructor can't be pinned down, the matched parameter resolves to no usable type, or the type-parameter mapping has no type argument to resolve against, every candidate name from the located superclass (every constructor parameter, every instance field) is seeded rather than guessing wrong or skipping silently — over-seeding a name is harmless, since the registry only fills from classes that actually carry `@SolidState` members, though it does further widen the pre-existing same-simple-name registry-collision surface (§4.9) by proposing names that would otherwise never have been candidates for the file at all. The fix lives in two shared helpers, `_seedFromSyntacticSuperFormal` (the chain search) and `_populateCrossFileTypesOneHop` (the one-hop class-location extension, sharing its registration rule with the main cross-file walk via `_registerWantedClassesFrom`), both reached from `_populateCrossFileTypes`'s single seeding loop, so the probe and the main pipeline pick them up identically. + ## 3.0.0-dev.4 - **FIX**: The builder's fast bailout for files with no `@Solid*` annotation and no `Provider(...)`/`.environment()` call site now runs the same cheap cross-file registry seeding `_populateCrossFileTypes` already implements (#104/#105) against the file's UNRESOLVED parsed unit before giving up. A PURE CONSUMER — a file that holds a `@SolidState`-bearing class through plain constructor injection or an instance field, but carries no `@Solid*` annotation and no provider call site of its own — previously matched neither hint text, so the file was never even parsed and the cross-file seeding never ran for it: every read through the injected field (`_authRepository.session`) stayed silently un-lowered. Reproduced in a real app as a generated authentication bypass, where `dart fix`'s `dead_code`/`unnecessary_null_comparison` passes collapsed a router guard's null check because the un-lowered field looked always-non-null (#106). Honest cost accounting: a hint-free file now always pays the syntactic parse (previously skipped entirely for such files); only a file with no custom-typed fields/params at all (just core-SDK/self-declared types) avoids the import walk and any resolver call, and that narrower path is what stays zero-cost — not "no work at all". A file whose field/param DOES name an externally-declared type pays a bounded import walk (bounded by the file's own import count), and a same-named-but-non-reactive match along the way (the `cross_file_constructor_injected_no_state` shape) does not stop that walk early — the probe's registries are threaded into the pipeline's own cross-file pass so the walk still only runs once per file. Lowering itself is applied by a new, narrower pass (`collectPureConsumerCrossFileEdits`) that only rewrites `.value` reads on plain-class members — it does not add `implements Disposable` or synthesize `dispose()`, since a pure consumer owns no reactive member of its own to dispose. The builder runs this lowering BEFORE the `Provider`/`.environment` auto-dispose pass, not after: lowering depends on `value_rewriter.dart`'s tier-1 `Expression.staticType` resolution to type receivers the AST-only tiers can't (e.g. a for-in loop variable), so it needs the still-valid resolved unit the dispose pass's text edits would otherwise invalidate; the dispose pass, by contrast, degrades gracefully to its AST-only tiers on a re-parsed, unresolved unit. Both shapes originally named here as residual gaps are now closed. A pure-consumer `StatelessWidget`/`State` gets the same `.value` lowering plus reactive rebuild through a sibling pass, `collectPureConsumerWidgetEdits`: it reuses `rewriteBuildMethod` verbatim on the class's `build()` method — the exact SignalBuilder-wrap-placement machinery an `@SolidEnvironment`-consuming widget's `build()` already goes through — with no widget-class Stateless→Stateful lift, since a plain constructor-injected field needs no `BuildContext` (unlike `@SolidEnvironment`, which needs `context.read()`). The plain-class pass and the widget pass are unified by `lowerPureConsumers`: both collect their edits against the SAME pristine source text and resolved unit, and the two edit lists are merged and applied together in a single text transformation — safe because the two passes visit disjoint class-kind sets (`plainClass` vs `statelessWidget`/`stateClass`), so their edit ranges never overlap. (An earlier draft of this fix ran the two passes sequentially, re-parsing the widget pass's OUTPUT for the plain-class pass whenever the widget pass had edited anything — which silently dropped resolved-unit-dependent receiver resolution, e.g. a static-holder read, for a plain-class guard sharing a file with a widget pure consumer; fixed before release by collecting both edit sets up front instead of reordering.) Static-field-mediated DI (`static final AuthRepository instance = …;`, consumed elsewhere as `Holder.instance.session`) is also now recognized: the `_populateCrossFileTypes` DI-seeding loop's `if (member.isStatic) continue;` guard carried no stated rationale anywhere in the #104/#105 history — it simply mirrored `@SolidState`'s own unrelated instance-only restriction — and is removed. Receiver resolution needed no new code at all: `Holder.instance.session` parses as a `PropertyAccess` whose target (`Holder.instance`) already resolves through the existing tier-1 `Expression.staticType` check to `AuthRepository` once the type is seeded, verified empirically against `package:analyzer` directly. The `flutter_solidart` import needed for a widget pure consumer's `SignalBuilder` wrap is spliced in — or, if the file already imports `flutter_solidart` with a `show`/`hide` combinator that doesn't expose `SignalBuilder` (e.g. a pre-existing `show Signal` for an unrelated hand-rolled signal), REPAIRED in place — based on the lowering pass's own report of whether it emitted a wrap, not a substring scan for `SignalBuilder(` in the assembled output (which both risked a false positive on a preserved source comment and couldn't distinguish an import that exposes the name from one that doesn't). diff --git a/packages/solid_generator/lib/builder.dart b/packages/solid_generator/lib/builder.dart index f291695..3bb7ab9 100644 --- a/packages/solid_generator/lib/builder.dart +++ b/packages/solid_generator/lib/builder.dart @@ -1124,6 +1124,11 @@ Future _populateCrossFileTypes( // present in [classRegistry] are skipped (the same-file pass is the // source of truth there). final wantedTypes = {}; + // Computed up front (issue #108 fix review finding 1): a name [unit] + // itself declares always shadows a same-name import, no matter how many + // hops away the import lives — see the `wantedTypes.removeAll(...)` call + // below, and [_populateCrossFileTypesOneHop]'s use of this same set. + final declaredInUnit = _collectDeclaredTypeNames(unit); for (final decl in unit.declarations) { if (decl is! ClassDeclaration) continue; for (final member in decl.members) { @@ -1202,17 +1207,71 @@ Future _populateCrossFileTypes( // `package:analyzer`). `_seedFromResolvedSuperFormal` is the // fallback for exactly that case. // - // This resolved-only fallback is MOOT for a pure consumer whose + // The resolved fallback above is MOOT for a pure consumer whose // ONLY link to the cross-file class is a bare `super.x`: such a // file has no `@Solid*` annotation and no provider hint, so it // takes the no-annotation fast path's UNRESOLVED syntactic // probe first (`parsed.unit`, no `declaredFragment` available - // either) — an empty probe result short-circuits to a verbatim - // copy before this resolved call ever runs. The fallback only - // helps a file that reaches this function for another reason. + // either) — the resolved call above is a no-op there. Issue + // #108 closes exactly that gap with a purely syntactic + // counterpart, [_seedFromSyntacticSuperFormal]: it walks + // [decl]'s own `extends` clause to find the superclass + // declaration (locally or via this same import-walk + // discipline, same-package-only — issue #108 fix review + // finding 2), matches [inner] against the super-constructor + // the enclosing constructor's initializer list targets, and + // seeds the matched parameter's (or, for a `this.x` field + // formal, the same-named instance field's) declared type. When + // the located superclass lives in another file entirely (the + // common case: a pure consumer's own file rarely declares its + // own base class), that file's declaration may itself only + // reference the wanted type through ONE of ITS OWN imports — + // [_populateCrossFileTypesOneHop] below is that one purposeful + // extra hop (issue #108 fix review finding 1), not a general + // transitive walk. paramType = inner.type; if (paramType == null) { - _seedFromResolvedSuperFormal(inner, wantedTypes, classRegistry); + // Finding 2 also cuts a real perf corner here: skip the + // syntactic walk entirely once the resolved path already + // found this parameter's type. The syntactic walk exists + // ONLY to cover the unresolved fast path where the resolved + // call is a guaranteed no-op (see above); running it anyway + // after a successful resolution is a wasted import-and-parse + // walk for a name already seeded — and, left unconditional, + // this walk fires for EVERY bare `super.` parameter on the + // RESOLVED path too, including the ubiquitous `{super.key}` + // on a Flutter widget, where it would otherwise chase + // `StatefulWidget`/`State` into `package:flutter` itself. + final resolvedHere = _seedFromResolvedSuperFormal( + inner, + wantedTypes, + classRegistry, + ); + if (!resolvedHere) { + final hop = await _seedFromSyntacticSuperFormal( + inner, + decl, + member, + unit, + step.inputId, + step, + wantedTypes, + classRegistry, + ); + if (hop != null) { + await _populateCrossFileTypesOneHop( + hop.$1, + hop.$2, + step, + wantedTypes, + declaredInUnit, + classRegistry, + classCollectionFields, + classFieldTypes, + crossClassFieldTypeOriginUris, + ); + } + } } } else { paramType = null; @@ -1232,7 +1291,7 @@ Future _populateCrossFileTypes( // simple-name shadowing collision: a local plain `class Address` plus an // unrelated imported `@SolidState`-annotated `class Address` elsewhere // must not attribute the import's reactive fields to the local class). - wantedTypes.removeAll(_collectDeclaredTypeNames(unit)); + wantedTypes.removeAll(declaredInUnit); if (wantedTypes.isEmpty) return; for (final directive in unit.directives.whereType()) { @@ -1259,79 +1318,223 @@ Future _populateCrossFileTypes( } on Object { continue; } - for (final decl in imported.declarations) { - if (decl is! ClassDeclaration) continue; - final className = decl.name.lexeme; - if (!wantedTypes.contains(className)) continue; - if (!_importExposesName(directive, className)) continue; - final scalarNames = {}; - final collectionNames = {}; - final fieldTypeTexts = {}; - for (final member in decl.members) { - if (member is FieldDeclaration) { - if (!hasAnnotation(solidStateName, member.metadata)) continue; - final variable = member.fields.variables.first; - final fieldName = variable.name.lexeme; - scalarNames.add(fieldName); - fieldTypeTexts[fieldName] = member.fields.type?.toSource() ?? ''; - // Mirror the collection-detection rule in signal_emitter.dart so - // the cross-file collection set agrees with the same-file one: - // collection signals are emitted for any non-nullable `List` - // / `Set` / `Map` field — `late` does not exclude. - final type = member.fields.type; - if (type == null) continue; - if (type.question != null) continue; - if (parseCollectionTypeText(type.toSource()) != null) { - collectionNames.add(fieldName); - } - } else if (member is MethodDeclaration && member.isGetter) { - if (!hasAnnotation(solidStateName, member.metadata)) continue; - scalarNames.add(member.name.lexeme); - fieldTypeTexts[member.name.lexeme] = - member.returnType?.toSource() ?? ''; + _registerWantedClassesFrom( + imported, + assetId, + directive, + step, + wantedTypes, + classRegistry, + classCollectionFields, + classFieldTypes, + crossClassFieldTypeOriginUris, + ); + } +} + +/// Scans [imported]'s top-level class declarations for names still present +/// in [wantedTypes], registering every match that carries at least one +/// `@SolidState` field or getter into [classRegistry] (and +/// [classCollectionFields] / [classFieldTypes] / +/// [crossClassFieldTypeOriginUris] alongside it). [directive] supplies the +/// `show`/`hide` combinators that gate whether [imported] may be credited +/// as [wantedTypes]'s source; [importedAssetId] anchors the field-type- +/// origin URI resolution below to [imported]'s own location, not the +/// original file being built. +/// +/// Shared by [_populateCrossFileTypes]'s own directive walk and its one-hop +/// extension ([_populateCrossFileTypesOneHop], issue #108 fix review +/// finding 1) so both apply IDENTICAL registration rules — including the +/// "don't stop at the first same-named-but-unannotated decoy" discipline: a +/// match with zero `@SolidState` members does NOT remove the name from +/// [wantedTypes], so a caller that walks more than one import for the same +/// name keeps looking. +void _registerWantedClassesFrom( + CompilationUnit imported, + AssetId importedAssetId, + ImportDirective directive, + BuildStep step, + Set wantedTypes, + Map> classRegistry, + Map> classCollectionFields, + Map> classFieldTypes, + Map> crossClassFieldTypeOriginUris, +) { + for (final decl in imported.declarations) { + if (decl is! ClassDeclaration) continue; + final className = decl.name.lexeme; + if (!wantedTypes.contains(className)) continue; + if (!_importExposesName(directive, className)) continue; + final scalarNames = {}; + final collectionNames = {}; + final fieldTypeTexts = {}; + for (final member in decl.members) { + if (member is FieldDeclaration) { + if (!hasAnnotation(solidStateName, member.metadata)) continue; + final variable = member.fields.variables.first; + final fieldName = variable.name.lexeme; + scalarNames.add(fieldName); + fieldTypeTexts[fieldName] = member.fields.type?.toSource() ?? ''; + // Mirror the collection-detection rule in signal_emitter.dart so + // the cross-file collection set agrees with the same-file one: + // collection signals are emitted for any non-nullable `List` + // / `Set` / `Map` field — `late` does not exclude. + final type = member.fields.type; + if (type == null) continue; + if (type.question != null) continue; + if (parseCollectionTypeText(type.toSource()) != null) { + collectionNames.add(fieldName); } + } else if (member is MethodDeclaration && member.isGetter) { + if (!hasAnnotation(solidStateName, member.metadata)) continue; + scalarNames.add(member.name.lexeme); + fieldTypeTexts[member.name.lexeme] = + member.returnType?.toSource() ?? ''; } - if (scalarNames.isNotEmpty) { - classRegistry[className] = scalarNames; - if (collectionNames.isNotEmpty) { - classCollectionFields[className] = collectionNames; - } - if (fieldTypeTexts.isNotEmpty) { - classFieldTypes[className] = fieldTypeTexts; - } - // For each `@SolidState` field whose declared type is NOT declared - // inside the same class file, capture the file's same-package import - // URIs as candidate origins. The consumer's lib output will inject - // these so the synthesized Record-Computed `Computed<(…, T, …)>` - // resolves at lib-time even when the consumer's source never - // textually references `T`. - final declaredHere = _collectDeclaredTypeNames(imported); - final localCandidateUris = {}; - for (final directive - in imported.directives.whereType()) { - final uri = directive.uri.stringValue; - if (uri == null) continue; - final importedAsset = _resolveImportToSourceAsset(uri, assetId); - if (importedAsset == null) continue; - if (importedAsset.package != step.inputId.package) continue; - if (!importedAsset.path.startsWith('source/')) continue; - localCandidateUris.add( - _sourceToLibAsset(importedAsset).uri.toString(), + } + if (scalarNames.isNotEmpty) { + classRegistry[className] = scalarNames; + if (collectionNames.isNotEmpty) { + classCollectionFields[className] = collectionNames; + } + if (fieldTypeTexts.isNotEmpty) { + classFieldTypes[className] = fieldTypeTexts; + } + // For each `@SolidState` field whose declared type is NOT declared + // inside the same class file, capture the file's same-package import + // URIs as candidate origins. The consumer's lib output will inject + // these so the synthesized Record-Computed `Computed<(…, T, …)>` + // resolves at lib-time even when the consumer's source never + // textually references `T`. + final declaredHere = _collectDeclaredTypeNames(imported); + final localCandidateUris = {}; + for (final innerDirective + in imported.directives.whereType()) { + final innerUri = innerDirective.uri.stringValue; + if (innerUri == null) continue; + final importedAsset = _resolveImportToSourceAsset( + innerUri, + importedAssetId, + ); + if (importedAsset == null) continue; + if (importedAsset.package != step.inputId.package) continue; + if (!importedAsset.path.startsWith('source/')) continue; + localCandidateUris.add( + _sourceToLibAsset(importedAsset).uri.toString(), + ); + } + if (localCandidateUris.isNotEmpty) { + for (final entry in fieldTypeTexts.entries) { + final typeText = entry.value; + if (typeText.isEmpty) continue; + if (declaredHere.contains(typeText)) continue; + (crossClassFieldTypeOriginUris[typeText] ??= {}).addAll( + localCandidateUris, ); } - if (localCandidateUris.isNotEmpty) { - for (final entry in fieldTypeTexts.entries) { - final typeText = entry.value; - if (typeText.isEmpty) continue; - if (declaredHere.contains(typeText)) continue; - (crossClassFieldTypeOriginUris[typeText] ??= {}).addAll( - localCandidateUris, - ); - } - } - wantedTypes.remove(className); } + wantedTypes.remove(className); + } + } +} + +/// One-hop extension of [_populateCrossFileTypes]'s cross-file registry +/// walk (issue #108 fix review finding 1): a name +/// [_seedFromSyntacticSuperFormal] derives from a superclass declared in +/// ANOTHER file than the one being scanned can only be declared in one of +/// TWO places — [hostUnit] itself (already covered by the main walk above, +/// since [hostUnit] is one of the scanned file's own imports and the main +/// walk visits it directly) or one of [hostUnit]'s OWN imports, a file the +/// scanned file never directly imports and the main walk therefore never +/// reaches. This function is that one additional hop: same registration +/// rule as the main walk ([_registerWantedClassesFrom]), anchored at +/// [hostUnit] / [hostAssetId] instead of the original file. Deliberately +/// NOT a general transitive walk — this function never calls itself, and +/// its only caller passes the immediate superclass's own host file, never +/// anything further removed. +/// +/// Same-package guard (issue #108 fix review finding 2): a cross-package +/// import (e.g. `package:flutter/...`) can never host this package's +/// `@SolidState` classes, so it's skipped before any read or parse. The +/// pre-existing main walk above carries no such restriction (out of scope +/// for this fix — it seeds from names written directly in the scanned +/// file, never from a syntactically-derived superclass chase), but this +/// one-hop extension exists ONLY to serve the superclass seeder, which +/// itself only ever locates a same-package superclass in the first place. +/// +/// Consuming-unit shadow guard (issue #108 fix review addendum, finding 1): +/// [declaredInConsumingUnit] is the ORIGINAL scanned file's own declared +/// type names — computed once in [_populateCrossFileTypes] and threaded +/// through unchanged. Filtering [wantedTypes] against [hostUnit]'s own +/// declared names (below) is NOT enough on its own: this function runs +/// *during* [_populateCrossFileTypes]'s per-declaration seeding loop, +/// strictly BEFORE that function's own `wantedTypes.removeAll( +/// declaredInUnit)` shadow filter runs at the end of the loop. Without this +/// extra guard, a name the ORIGINAL consuming file declares locally (e.g. +/// its own plain `class Foo`) could still be registered into +/// [classRegistry] from a `@SolidState`-bearing same-named `Foo` reached +/// through [hostUnit]'s imports — the registration is irreversible even +/// though the name is removed from `wantedTypes` moments later, because +/// [classRegistry] is a separate map already written by then. Filtering +/// here, before [_registerWantedClassesFrom] ever runs, closes that +/// ordering hole. +Future _populateCrossFileTypesOneHop( + CompilationUnit hostUnit, + AssetId hostAssetId, + BuildStep step, + Set wantedTypes, + Set declaredInConsumingUnit, + Map> classRegistry, + Map> classCollectionFields, + Map> classFieldTypes, + Map> crossClassFieldTypeOriginUris, +) async { + // Same local-shadowing discipline the main walk applies to the original + // file, applied here relative to [hostUnit]: a name [hostUnit] declares + // itself can never be the wanted type's source via one of ITS imports — + // and in practice this is always already a no-op, since a name declared + // directly in [hostUnit] would already have been found by the main + // walk's own scan of [hostUnit]'s declarations before this function is + // ever reached. Kept for defense and to make the discipline explicit. + wantedTypes + ..removeAll(_collectDeclaredTypeNames(hostUnit)) + // The guard this function actually exists to add — see the doc + // comment above. + ..removeAll(declaredInConsumingUnit); + if (wantedTypes.isEmpty) return; + + for (final directive in hostUnit.directives.whereType()) { + if (wantedTypes.isEmpty) break; + final uri = directive.uri.stringValue; + if (uri == null || uri.startsWith('dart:')) continue; + final assetId = _resolveImportToSourceAsset(uri, hostAssetId); + if (assetId == null) continue; + if (assetId.package != step.inputId.package) continue; + if (!assetId.path.endsWith('.dart')) continue; + bool exists; + try { + exists = await step.canRead(assetId); + } on Object { + continue; } + if (!exists) continue; + final CompilationUnit imported; + try { + imported = await step.resolver.compilationUnitFor(assetId); + } on Object { + continue; + } + _registerWantedClassesFrom( + imported, + assetId, + directive, + step, + wantedTypes, + classRegistry, + classCollectionFields, + classFieldTypes, + crossClassFieldTypeOriginUris, + ); } } @@ -1653,18 +1856,381 @@ String _ensureFlutterSolidartImport(String text, CompilationUnit unit) { /// the resolved type isn't an [InterfaceType] (e.g. a dynamic/unresolved /// forward) — the caller already has nothing to add in that case, same as /// today's syntactic-only behavior. -void _seedFromResolvedSuperFormal( +/// +/// Returns whether [param] was resolved at all (an [InterfaceType] was +/// found), regardless of whether the type text turned out to be a core-SDK +/// name or already in the registry — the caller (issue #108 fix review +/// finding 2) uses this to skip [_seedFromSyntacticSuperFormal] entirely +/// once this resolved path has already answered the question; running the +/// syntactic import-and-parse walk anyway on a fully-resolved unit is pure +/// waste. +bool _seedFromResolvedSuperFormal( SuperFormalParameter param, Set wantedTypes, Map> classRegistry, ) { final type = param.declaredFragment?.element.type; - if (type is! InterfaceType) return; + if (type is! InterfaceType) return false; final typeText = type.element.name; - if (typeText == null) return; - if (_coreSdkTypeNames.contains(typeText)) return; - if (classRegistry.containsKey(typeText)) return; + if (typeText == null) return false; + if (_coreSdkTypeNames.contains(typeText)) return true; + if (classRegistry.containsKey(typeText)) return true; wantedTypes.add(typeText); + return true; +} + +/// Depth bound for [_seedFromSyntacticSuperFormal]'s chain recursion (issue +/// #108 fix review addendum finding B): a bare super-formal parameter can +/// itself forward through another bare super-formal parameter one or more +/// superclasses further up (`GrandChild(super.x) extends Child(super.x) +/// extends Base(this.x)`). Five hops is generous for any real inheritance +/// chain while still bounding the walk — same rationale as any other +/// bounded search in this file, never expected to bite in practice. +const int _kMaxSuperFormalChainDepth = 5; + +/// Syntactic counterpart to [_seedFromResolvedSuperFormal] for a bare +/// `super.x` in a file that never reaches a resolved unit — the PURE +/// CONSUMER shape issue #108 reports: a file with no `@Solid*` annotation +/// and no provider hint takes the no-annotation fast path's UNRESOLVED +/// probe, where `param.declaredFragment` is never populated. +/// +/// Resolves [param]'s type from the AST alone: reads [subclass]'s own +/// `extends` clause for the superclass's simple name, locates that class's +/// declaration — checking [unit]'s own top-level declarations first (a +/// local declaration always shadows a same-name import, mirroring the +/// shadowing rule [_populateCrossFileTypes] itself applies to `wantedTypes` +/// — skipped entirely when the `extends` clause is itself import-prefixed, +/// since a prefixed reference can never resolve to a local declaration) +/// and otherwise walking [unit]'s imports — alias-aware (issue #108 fix +/// review finding 3): an unprefixed `extends Base` is skipped past any +/// `import ... as x;` directive (such an import can never bring an +/// unprefixed name into scope), and a prefixed `extends x.Base` considers +/// ONLY the import whose own prefix is `x`; same-package-only (finding 2: +/// a cross-package file can never host this package's `@SolidState` +/// classes — critically, this is what keeps `{super.key}` on an ordinary +/// widget from ever chasing `StatefulWidget`/`State` into +/// `package:flutter`); and never commits to the first import that merely +/// LOOKS like it could expose the name — scanning continues past a +/// resolvable import whose target file doesn't actually declare the class, +/// exactly like [_populateCrossFileTypes]'s own main walk does for a +/// same-named decoy. +/// +/// Then finds the super-constructor [constructor]'s initializer list +/// targets (`super.named(...)` names a named constructor; no explicit +/// invocation at all means the implicit call to the unnamed constructor) +/// and matches [param] against that target's corresponding parameter: +/// positional by position among [constructor]'s OTHER positional `super.` +/// siblings (mirrors the language's own rule that only super-formal +/// parameters, in declaration order, populate the implicit super call — +/// Dart itself rejects mixing a positional `super.x` with an explicit +/// positional argument in the same invocation, so this position is never +/// ambiguous for code that compiles), named by matching name. When the +/// matched parameter is itself a `this.x` field formal with no type written +/// at that position either, falls back to the superclass's same-named +/// instance field's declared type. When the matched parameter is ITSELF a +/// bare `super.x` (the chain shape from finding B above), this function +/// recurses one level further up the inheritance chain — rooted at the +/// located superclass as the new subclass, the matched constructor as the +/// new constructor, and the matched parameter as the new target — up to +/// [_kMaxSuperFormalChainDepth] hops; beyond that bound it gives up the +/// chase and falls back to [_seedAllFromSuperclass] at the last class +/// reached, same as any other ambiguity below. +/// +/// On any other ambiguity — the superclass declaration can't be located, +/// the targeted constructor can't be pinned down, or the matched parameter +/// resolves to no usable [NamedType] and isn't a further super-formal — +/// delegates to [_seedAllFromSuperclass], which seeds every candidate name +/// from the located superclass instead of guessing or skipping silently. +/// +/// Returns the `(CompilationUnit, AssetId)` of whichever file the located +/// superclass (at whatever depth the resolution bottomed out on) is +/// declared in, but ONLY when that file differs from [unit]/[hostAssetId] +/// — i.e. exactly when the caller needs the one-hop extension +/// ([_populateCrossFileTypesOneHop]) to find the derived name's own +/// declaration, because [unit]'s own import walk (the one this function +/// runs to locate the superclass) never reaches that file directly. +/// Returns `null` when the superclass couldn't be located at all, or was +/// found locally in [unit] (no hop needed — [unit]'s own directives are +/// already covered by the caller's main walk). +Future<(CompilationUnit, AssetId)?> _seedFromSyntacticSuperFormal( + SuperFormalParameter param, + ClassDeclaration subclass, + ConstructorDeclaration constructor, + CompilationUnit unit, + AssetId hostAssetId, + BuildStep step, + Set wantedTypes, + Map> classRegistry, { + int depth = 0, +}) async { + if (depth > _kMaxSuperFormalChainDepth) return null; + + final superclass = subclass.extendsClause?.superclass; + final superclassName = superclass?.name.lexeme; + if (superclassName == null) return null; + final requiredPrefix = superclass?.importPrefix?.name.lexeme; + + ClassDeclaration? superDecl; + CompilationUnit? superDeclUnit; + AssetId? superDeclAssetId; + + // A prefixed reference (`extends x.Base`) can never resolve to a local, + // unprefixed top-level declaration — only the import-walk branch below + // can satisfy it. + if (requiredPrefix == null) { + superDecl = unit.declarations + .whereType() + .where((d) => d.name.lexeme == superclassName) + .firstOrNull; + } + + if (superDecl == null) { + for (final directive in unit.directives.whereType()) { + final uri = directive.uri.stringValue; + if (uri == null || uri.startsWith('dart:')) continue; + // Alias-aware filtering (finding 3): an unprefixed name can only come + // from an unprefixed import; a prefixed name only from the import + // carrying that exact prefix. `_importExposesName` alone can't tell + // these apart — it only inspects show/hide combinators, never the + // `as` clause — so a prefixed decoy import would otherwise pass its + // check for an unprefixed name it can never actually expose. + final directivePrefix = directive.prefix?.name; + if (requiredPrefix == null) { + if (directivePrefix != null) continue; + } else if (directivePrefix != requiredPrefix) { + continue; + } + if (!_importExposesName(directive, superclassName)) continue; + final assetId = _resolveImportToSourceAsset(uri, hostAssetId); + if (assetId == null || !assetId.path.endsWith('.dart')) continue; + // Same-package guard (finding 2): this package's `@SolidState` + // classes can never live in a cross-package file, so cross-package + // imports (`package:flutter/...`, any other dependency) are skipped + // before the read/parse cost is paid at all. + if (assetId.package != step.inputId.package) continue; + bool exists; + try { + exists = await step.canRead(assetId); + } on Object { + continue; + } + if (!exists) continue; + final CompilationUnit imported; + try { + imported = await step.resolver.compilationUnitFor(assetId); + } on Object { + continue; + } + // Keep scanning past a resolvable import whose target file simply + // doesn't declare the class — same "don't commit to the first URI + // that merely looks promising" discipline the main registry walk + // applies to a same-named-but-unannotated decoy (finding 3). + final match = imported.declarations + .whereType() + .where((d) => d.name.lexeme == superclassName) + .firstOrNull; + if (match == null) continue; + superDecl = match; + superDeclUnit = imported; + superDeclAssetId = assetId; + break; + } + } + if (superDecl == null) return null; + + // The file hosting whatever candidate this call ultimately seeds from — + // the located superclass's own file when it came from an import, else + // `null` (found in `unit` itself, no one-hop needed). + (CompilationUnit, AssetId)? hopInfo() => + superDeclUnit != null ? (superDeclUnit, superDeclAssetId!) : null; + + // The target super-constructor's name: `null` for the unnamed + // constructor, which is also the correct default when [constructor] has + // no explicit `super(...)`/`super.named(...)` invocation at all — Dart's + // implicit super call always targets the unnamed constructor. + String? targetName; + var sawSuperInvocation = false; + for (final initializer in constructor.initializers) { + if (initializer is SuperConstructorInvocation) { + sawSuperInvocation = true; + targetName = initializer.constructorName?.name; + } + } + final targetCtor = sawSuperInvocation + ? superDecl.members + .whereType() + .where((c) => c.name?.lexeme == targetName) + .firstOrNull + : superDecl.members + .whereType() + .where((c) => c.name == null) + .firstOrNull; + if (targetCtor == null) { + _seedAllFromSuperclass(superDecl, wantedTypes, classRegistry); + return hopInfo(); + } + + FormalParameter? matchedParam; + if (param.isNamed) { + matchedParam = targetCtor.parameters.parameters + .where((p) => p.isNamed && p.name?.lexeme == param.name.lexeme) + .firstOrNull; + } else { + final positionalSuperSiblings = constructor.parameters.parameters.where(( + p, + ) { + final inner = p is DefaultFormalParameter ? p.parameter : p; + return inner is SuperFormalParameter && inner.isPositional; + }).toList(); + final index = positionalSuperSiblings.indexWhere((p) { + final inner = p is DefaultFormalParameter ? p.parameter : p; + return identical(inner, param); + }); + final targetPositional = targetCtor.parameters.parameters + .where((p) => p.isPositional) + .toList(); + if (index >= 0 && index < targetPositional.length) { + matchedParam = targetPositional[index]; + } + } + if (matchedParam == null) { + _seedAllFromSuperclass(superDecl, wantedTypes, classRegistry); + return hopInfo(); + } + + final matchedInner = matchedParam is DefaultFormalParameter + ? matchedParam.parameter + : matchedParam; + + // Chain case (addendum finding B): the matched parameter is itself a bare + // super-formal, forwarding one level further up the inheritance chain + // (`GrandChild(super.x) extends Child(super.x) extends Base(this.x)`). + // Recurse rooted at the located superclass, bounded by + // [_kMaxSuperFormalChainDepth]; beyond the bound, fall back to + // over-seeding at the last class actually reached rather than silently + // dropping the chain. + if (matchedInner is SuperFormalParameter) { + if (depth + 1 > _kMaxSuperFormalChainDepth) { + _seedAllFromSuperclass(superDecl, wantedTypes, classRegistry); + return hopInfo(); + } + return _seedFromSyntacticSuperFormal( + matchedInner, + superDecl, + targetCtor, + superDeclUnit ?? unit, + superDeclAssetId ?? hostAssetId, + step, + wantedTypes, + classRegistry, + depth: depth + 1, + ); + } + + TypeAnnotation? matchedType; + if (matchedInner is SimpleFormalParameter) { + matchedType = matchedInner.type; + } else if (matchedInner is FieldFormalParameter) { + matchedType = + matchedInner.type ?? + _fieldTypeInClass(superDecl, matchedInner.name.lexeme); + } + if (matchedType is! NamedType) { + _seedAllFromSuperclass(superDecl, wantedTypes, classRegistry); + return hopInfo(); + } + // Generic superclass type-parameter mapping (issue #108 fix review + // addendum finding 2): `matchedType`'s name may itself be one of + // [superDecl]'s own declared type parameters (`class Base { + // Base(this.repo); final T repo; }`) rather than a real class name — a + // bare type-parameter reference is a perfectly ordinary [NamedType], so + // it passes the `matchedType is! NamedType` ambiguity gate above + // undetected, and seeding it verbatim would add the literal placeholder + // name (`"T"`) to `wantedTypes`, matching no class anywhere and silently + // dropping the real dependency. Map it BY INDEX to the type argument + // [subclass] itself supplies in its `extends Base` + // clause instead, then seed THAT (recursing through + // [_seedWantedTypeRecursive] handles further nesting, e.g. `extends + // Base>`). Falls back to [_seedAllFromSuperclass] + // — never a bare type-parameter name — when the extends clause supplies + // no type arguments at all, or fewer than [superDecl] declares. + final matchedTypeName = matchedType.name.lexeme; + final declaredTypeParams = superDecl.typeParameters?.typeParameters; + final typeParamIndex = declaredTypeParams?.indexWhere( + (p) => p.name.lexeme == matchedTypeName, + ); + if (typeParamIndex != null && typeParamIndex != -1) { + final typeArgs = superclass?.typeArguments?.arguments; + final resolvedArg = typeArgs != null && typeParamIndex < typeArgs.length + ? typeArgs[typeParamIndex] + : null; + if (resolvedArg is NamedType) { + _seedWantedTypeRecursive(resolvedArg, wantedTypes, classRegistry); + } else { + _seedAllFromSuperclass(superDecl, wantedTypes, classRegistry); + } + return hopInfo(); + } + _seedWantedTypeRecursive(matchedType, wantedTypes, classRegistry); + return hopInfo(); +} + +/// Ambiguity fallback for [_seedFromSyntacticSuperFormal]: seeds every +/// candidate type name from [superDecl] — every instance field's declared +/// type, plus every constructor parameter's declared type across every +/// constructor — rather than guessing a single (possibly wrong) match or +/// skipping silently. Mirrors the annotation-blind, over-seeding-is-harmless +/// contract the rest of [_populateCrossFileTypes]'s seeding already relies +/// on: a candidate name that matches no `@SolidState`-bearing class is +/// simply never consumed downstream. Widens the pre-existing same-simple- +/// name registry-collision surface a little further (§4.9's residual risk +/// of the name-based registry design): every field/param type name on +/// [superDecl] enters `wantedTypes`, including names that would otherwise +/// never have been proposed as candidates for this file at all. +void _seedAllFromSuperclass( + ClassDeclaration superDecl, + Set wantedTypes, + Map> classRegistry, +) { + for (final member in superDecl.members) { + if (member is FieldDeclaration) { + final type = member.fields.type; + if (type is NamedType) { + _seedWantedTypeRecursive(type, wantedTypes, classRegistry); + } + } else if (member is ConstructorDeclaration) { + for (final param in member.parameters.parameters) { + final inner = param is DefaultFormalParameter ? param.parameter : param; + TypeAnnotation? type; + if (inner is SimpleFormalParameter) { + type = inner.type; + } else if (inner is FieldFormalParameter) { + type = inner.type; + } + if (type is NamedType) { + _seedWantedTypeRecursive(type, wantedTypes, classRegistry); + } + } + } + } +} + +/// Returns the declared [TypeAnnotation] of [classDecl]'s instance field +/// named [fieldName], or `null` if no such field exists. Fallback for a +/// `this.x` field-formal constructor parameter written with no type of its +/// own — the common shape (`Foo(this.x); final AuthRepository x;`), where +/// the type lives on the field declaration, not the parameter. +TypeAnnotation? _fieldTypeInClass( + ClassDeclaration classDecl, + String fieldName, +) { + for (final member in classDecl.members) { + if (member is! FieldDeclaration) continue; + for (final variable in member.fields.variables) { + if (variable.name.lexeme == fieldName) return member.fields.type; + } + } + return null; } /// Translates a `source/` AssetId to its `lib/` sibling. The diff --git a/packages/solid_generator/pubspec.yaml b/packages/solid_generator/pubspec.yaml index 63b9dd4..a945046 100644 --- a/packages/solid_generator/pubspec.yaml +++ b/packages/solid_generator/pubspec.yaml @@ -1,6 +1,6 @@ name: solid_generator description: Solid source-to-lib code generator for Flutter reactive state. -version: 3.0.0-dev.4 +version: 3.0.0-dev.5 homepage: https://solid.mariuti.com repository: https://github.com/nank1ro/solid issue_tracker: https://github.com/nank1ro/solid/issues diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/auth_repository.dart new file mode 100644 index 0000000..9c54f66 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/auth_repository.dart @@ -0,0 +1,11 @@ +// Cross-file `@SolidState` host, reached only through a TWO-LEVEL chain of +// bare `super.x` constructor parameters (issue #108 fix review addendum +// finding B) — `leaf_repository.dart` forwards to `mid_repository.dart`, +// which itself forwards to `base_repository.dart`, which finally names +// this type on its own field declaration. +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/base_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/base_repository.dart new file mode 100644 index 0000000..afc03d6 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/base_repository.dart @@ -0,0 +1,9 @@ +// Bottom of the chain: owns `authRepository` through the ordinary `this.x` +// field-formal shape, same as `cross_file_super_param_bare_pure_consumer`. +import 'auth_repository.dart'; + +class BaseRepository { + BaseRepository(this.authRepository); + + final AuthRepository authRepository; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/leaf_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/leaf_repository.dart new file mode 100644 index 0000000..a3b5a42 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/leaf_repository.dart @@ -0,0 +1,20 @@ +// PURE CONSUMER at the top of a TWO-LEVEL bare-`super.x` chain (issue #108 +// fix review addendum finding B): `LeafRepository`'s bare parameter +// forwards to `MidRepository`'s OWN bare parameter (not a typed field or a +// `this.x` field-formal — `MidRepository` merely relays it one level +// further), which in turn forwards to `BaseRepository`'s `this.x` +// field-formal, which finally names `AuthRepository` on its field +// declaration. Locating `AuthRepository`'s own class declaration from +// there needs the fix review finding 1 one-hop extension too — this file +// imports only `mid_repository.dart`; `mid_repository.dart` imports only +// `base_repository.dart`; `AuthRepository` is declared in +// `auth_repository.dart`, which only `base_repository.dart` imports. No +// `@Solid*` annotation and no provider hint anywhere in this file, so it +// takes the no-annotation fast path's syntactic probe. +import 'mid_repository.dart'; + +class LeafRepository extends MidRepository { + LeafRepository(super.authRepository); + + bool hasSession() => authRepository.session != null; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/mid_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/mid_repository.dart new file mode 100644 index 0000000..169553e --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_chain/mid_repository.dart @@ -0,0 +1,11 @@ +// Middle of the chain: forwards `authRepository` through its OWN bare +// `super.x` parameter rather than re-declaring a typed field or a `this.x` +// field-formal — the shape issue #108 fix review addendum finding B +// targets. Resolving `LeafRepository`'s bare parameter (in +// `leaf_repository.dart`) requires recursing PAST this class's own +// constructor to `BaseRepository`'s field-formal parameter. +import 'base_repository.dart'; + +class MidRepository extends BaseRepository { + MidRepository(super.authRepository); +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/base_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/base_repository.dart new file mode 100644 index 0000000..06739e0 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/base_repository.dart @@ -0,0 +1,11 @@ +// Plain, unannotated base class owning the `plain` field through the +// `this.x` field-formal shape — same DI pattern as +// `cross_file_super_param_bare_pure_consumer`, but pointing at a +// non-`@SolidState` type (see `plain_repository.dart`). +import 'plain_repository.dart'; + +class BaseRepository { + BaseRepository(this.plain); + + final PlainRepository plain; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/consumer.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/consumer.dart new file mode 100644 index 0000000..cad7bed --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/consumer.dart @@ -0,0 +1,20 @@ +// Negative fixture for issue #108: `Consumer` has the exact bare-`super.x` +// PURE CONSUMER shape as `cross_file_super_param_bare_pure_consumer/ +// customers_repository.dart` — importing ONLY `base_repository.dart`, never +// `plain_repository.dart` directly — but `PlainRepository` carries no +// `@SolidState` members. The syntactic super-formal seeding proposes +// `PlainRepository` as a candidate (seeding is annotation-blind by design), +// and the fix review finding 1 one-hop extension genuinely reaches its +// declaration through `base_repository.dart`'s own import of +// `plain_repository.dart` (this file never imports it directly, so without +// that one hop the class would never be found at all) — but finds zero +// `@SolidState` members there, so the cross-file registry stays empty and +// this file must pass through the fast, verbatim-passthrough path exactly +// as before the fix. +import 'base_repository.dart'; + +class Consumer extends BaseRepository { + Consumer(super.plain); + + bool hasLabel() => plain.label != null; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/plain_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/plain_repository.dart new file mode 100644 index 0000000..33ba91d --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_no_state/plain_repository.dart @@ -0,0 +1,7 @@ +// Negative-fixture companion for `consumer.dart`: `PlainRepository` carries +// no `@Solid*` annotations at all — a genuinely solid-free type reached +// only through a bare `super.x` constructor parameter (issue #108). + +class PlainRepository { + String? label; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/auth_repository.dart new file mode 100644 index 0000000..27a123c --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/auth_repository.dart @@ -0,0 +1,14 @@ +// Cross-file `@SolidState` host for the bare-`super.` PURE CONSUMER +// regression (issue #108). `customers_repository.dart` reaches this class +// ONLY through a bare `super.x` constructor parameter, with no `@Solid*` +// annotation and no provider hint anywhere in that file — the narrowest +// pure-consumer shape the #106/#107 machinery still missed, because +// detecting a pure consumer at all requires an UNRESOLVED syntactic +// pre-parse, and a bare `super.x` carries no type text for that pass to +// find without the fix. +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/base_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/base_repository.dart new file mode 100644 index 0000000..4a5bf30 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/base_repository.dart @@ -0,0 +1,23 @@ +// Plain, unannotated base class owning the `authRepository` field through +// the `this.x` field-formal shape — the primary real-world DI pattern issue +// #108 targets: the field itself carries no type annotation at the +// parameter position (`this.authRepository`), only on the field +// declaration below it. Also declares a NAMED constructor forwarding the +// same field, exercising the named-super-constructor matching branch +// (`super.named(...)` in `customers_repository.dart`), and a second named +// constructor whose forwarded param is itself NAMED (`{required +// this.authRepository}`) — issue #108 fix review addendum finding 4's +// NAMED bare-super-formal branch, matched against +// `PartnersRepository({required super.authRepository})` in +// `customers_repository.dart`. +import 'auth_repository.dart'; + +class BaseRepository { + BaseRepository(this.authRepository); + + BaseRepository.named(this.authRepository); + + BaseRepository.namedParam({required this.authRepository}); + + final AuthRepository authRepository; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/customers_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/customers_repository.dart new file mode 100644 index 0000000..9019513 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_pure_consumer/customers_repository.dart @@ -0,0 +1,49 @@ +// PURE CONSUMER whose ONLY link to `AuthRepository` is a BARE `super.x` +// constructor parameter (issue #108) — no `@Solid*` annotation and no DI +// call site of any kind anywhere in this file, so it takes the +// no-annotation fast path's syntactic probe. Before the fix, a +// bare super-formal parameter carried no type text anywhere in the source +// for that unresolved probe to find, so the probe found nothing to seed +// and the file short-circuited to a verbatim copy — every read through +// `authRepository` stayed silently un-lowered (no `.value`). +// +// `CustomersRepository` covers the default (unnamed) super-constructor +// target, with a null-guard read and a `!`-chain read. `VendorsRepository` +// covers the NAMED super-constructor target (`super.named()`), matching +// the bare param against `BaseRepository.named`'s own field-formal +// parameter. `PartnersRepository` covers the NAMED bare-super-formal +// branch (`{required super.authRepository}`, issue #108 fix review +// addendum finding 4), matched against `BaseRepository.namedParam`'s own +// NAMED field-formal parameter. +// +// This file deliberately imports ONLY `base_repository.dart` — issue +// #108's fix review, finding 1: `AuthRepository` is never named in this +// file's text at all (the bare `super.authRepository` shorthand never +// spells it out), so the generator's cross-file registry walk can only +// find its declaration by following ONE MORE hop through +// `base_repository.dart`'s own import of `auth_repository.dart` — proving +// the fix actually closes issue #108's own primary example, a consumer +// that imports only the immediate superclass's file. +import 'base_repository.dart'; + +class CustomersRepository extends BaseRepository { + CustomersRepository(super.authRepository); + + bool hasSession() { + return authRepository.session != null; + } + + int? sessionLength() => authRepository.session!.length; +} + +class VendorsRepository extends BaseRepository { + VendorsRepository(super.authRepository) : super.named(); + + bool hasSession() => authRepository.session != null; +} + +class PartnersRepository extends BaseRepository { + PartnersRepository({required super.authRepository}) : super.namedParam(); + + bool hasSession() => authRepository.session != null; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/auth_repository.dart new file mode 100644 index 0000000..110b1e8 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/auth_repository.dart @@ -0,0 +1,8 @@ +// Cross-file `@SolidState` host for the generic-superclass regression +// (issue #108 fix review addendum finding 2). +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/base_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/base_repository.dart new file mode 100644 index 0000000..969a44b --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/base_repository.dart @@ -0,0 +1,10 @@ +// Generic base class whose field is typed with its OWN type parameter `T` +// (issue #108 fix review addendum finding 2) — `Base(this.repo); final T +// repo;`. No import of any concrete type is needed here: `T` only becomes a +// real class once a subclass supplies a concrete type argument in its own +// `extends` clause. +class Base { + Base(this.repo); + + final T repo; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/consumer.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/consumer.dart new file mode 100644 index 0000000..749646a --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_generic_base/consumer.dart @@ -0,0 +1,21 @@ +// PURE CONSUMER whose ONLY link to `AuthRepository` is a BARE `super.repo` +// constructor parameter forwarding into a GENERIC superclass (issue #108 +// fix review addendum finding 2): `Base`'s own field is typed with ITS +// OWN type parameter `T`, not a concrete class name (`Base(this.repo); +// final T repo;`). Naively seeding the matched field's declared type would +// seed the literal string `"T"` — a syntactically valid `NamedType`, so it +// slips past the `matchedType is! NamedType` ambiguity gate — instead of +// the REAL concrete type, `AuthRepository`; no class is ever named `T`, so +// the wanted-type set fills with a name that matches nothing and +// `repo.session` stays silently un-lowered. +// +// The fix maps `T` BY INDEX to `C`'s own `extends Base` +// type argument instead of seeding the placeholder name verbatim. +import 'auth_repository.dart'; +import 'base_repository.dart'; + +class C extends Base { + C(super.repo); + + bool hasSession() => repo.session != null; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/base.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/base.dart new file mode 100644 index 0000000..91b7b91 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/base.dart @@ -0,0 +1,11 @@ +// `Bar`'s superclass, reached one hop away from `consumer.dart` (which +// imports only this file, never `foreign_foo.dart` directly). Owns `thing` +// through the ordinary `this.x` field-formal shape, typed with THIS file's +// own imported `Foo` — the FOREIGN, `@SolidState`-bearing one. +import 'foreign_foo.dart'; + +class Base { + Base(this.thing); + + final Foo thing; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/consumer.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/consumer.dart new file mode 100644 index 0000000..51ab04a --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/consumer.dart @@ -0,0 +1,48 @@ +// PURE CONSUMER regression fixture (issue #108 fix review, finding 1): +// this file declares its OWN plain `class Foo` (zero `@SolidState` +// members) — held by `Holder` through ordinary constructor injection — and +// ALSO extends `Base` (declared in `base.dart`) through a bare `super.x` +// constructor parameter. `Base` itself is reached directly (main walk), but +// `Base`'s OWN field `thing` is typed with a DIFFERENT, `@SolidState`- +// bearing `Foo` declared in `foreign_foo.dart` — a file this file never +// imports at all; only `base.dart` does. Locating that declaration needs +// the fix review finding 1 one-hop extension. +// +// Before the fix: the one-hop walk registered the FOREIGN `Foo`'s reactive +// `label` field into `classRegistry['Foo']` before this file's own local +// `Foo` had a chance to shadow it, so `Holder.f.label` — a read against the +// LOCAL plain class — was wrongly rewritten to `f.label.value` (BLOCKER: +// non-compiling output, `.value` on a plain `String?` field). +// +// After the fix: the one-hop registration is gated against THIS file's own +// declared type names, same discipline as the main walk (issue #104) — so +// `classRegistry` never gains a `'Foo'` entry at all once this file +// declares its own local `Foo`, name-based collision with no library +// qualification being the design's documented residual risk (SPEC §4.9). +// `f.label` correctly stays un-rewritten (the BLOCKER is closed); `Bar`'s +// `thing.label` — genuinely reactive through the foreign `Foo` — ALSO stays +// un-rewritten as the accepted, conservative cost of a safe fix: this file +// already has a local `Foo` claim on that simple name, so the registry +// cannot safely attribute `label` to `thing` either, exactly the same +// trade-off `cross_file_local_shadowing_decoy` (issue #104) already +// accepts. Favoring "no wrong rewrite" over "no missed rewrite" is +// consistent with every other ambiguity-handling rule in this file. +import 'base.dart'; + +class Foo { + String? label; +} + +class Holder { + Holder(this.f); + + final Foo f; + + String? readLabel() => f.label; +} + +class Bar extends Base { + Bar(super.thing); + + String? readThing() => thing.label; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/foreign_foo.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/foreign_foo.dart new file mode 100644 index 0000000..176c36a --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_one_hop_local_shadow/foreign_foo.dart @@ -0,0 +1,11 @@ +// The FOREIGN, `@SolidState`-bearing `Foo` — deliberately sharing its +// simple name with `consumer.dart`'s own LOCAL plain `Foo` (issue #108 fix +// review finding 1). Only reachable from `consumer.dart` through the +// one-hop extension: `consumer.dart` imports `base.dart`, and `base.dart` +// imports THIS file. +import 'package:solid_annotations/solid_annotations.dart'; + +class Foo { + @SolidState() + String? label; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/auth_repository.g.dart @@ -0,0 +1,11 @@ +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository implements Disposable { + final session = Signal(null, name: 'session'); + + @override + void dispose() { + session.dispose(); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/base_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/base_repository.g.dart new file mode 100644 index 0000000..afc03d6 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/base_repository.g.dart @@ -0,0 +1,9 @@ +// Bottom of the chain: owns `authRepository` through the ordinary `this.x` +// field-formal shape, same as `cross_file_super_param_bare_pure_consumer`. +import 'auth_repository.dart'; + +class BaseRepository { + BaseRepository(this.authRepository); + + final AuthRepository authRepository; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/leaf_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/leaf_repository.g.dart new file mode 100644 index 0000000..74ff5db --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/leaf_repository.g.dart @@ -0,0 +1,20 @@ +// PURE CONSUMER at the top of a TWO-LEVEL bare-`super.x` chain (issue #108 +// fix review addendum finding B): `LeafRepository`'s bare parameter +// forwards to `MidRepository`'s OWN bare parameter (not a typed field or a +// `this.x` field-formal — `MidRepository` merely relays it one level +// further), which in turn forwards to `BaseRepository`'s `this.x` +// field-formal, which finally names `AuthRepository` on its field +// declaration. Locating `AuthRepository`'s own class declaration from +// there needs the fix review finding 1 one-hop extension too — this file +// imports only `mid_repository.dart`; `mid_repository.dart` imports only +// `base_repository.dart`; `AuthRepository` is declared in +// `auth_repository.dart`, which only `base_repository.dart` imports. No +// `@Solid*` annotation and no provider hint anywhere in this file, so it +// takes the no-annotation fast path's syntactic probe. +import 'mid_repository.dart'; + +class LeafRepository extends MidRepository { + LeafRepository(super.authRepository); + + bool hasSession() => authRepository.session.value != null; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/mid_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/mid_repository.g.dart new file mode 100644 index 0000000..169553e --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_chain/mid_repository.g.dart @@ -0,0 +1,11 @@ +// Middle of the chain: forwards `authRepository` through its OWN bare +// `super.x` parameter rather than re-declaring a typed field or a `this.x` +// field-formal — the shape issue #108 fix review addendum finding B +// targets. Resolving `LeafRepository`'s bare parameter (in +// `leaf_repository.dart`) requires recursing PAST this class's own +// constructor to `BaseRepository`'s field-formal parameter. +import 'base_repository.dart'; + +class MidRepository extends BaseRepository { + MidRepository(super.authRepository); +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/base_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/base_repository.g.dart new file mode 100644 index 0000000..06739e0 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/base_repository.g.dart @@ -0,0 +1,11 @@ +// Plain, unannotated base class owning the `plain` field through the +// `this.x` field-formal shape — same DI pattern as +// `cross_file_super_param_bare_pure_consumer`, but pointing at a +// non-`@SolidState` type (see `plain_repository.dart`). +import 'plain_repository.dart'; + +class BaseRepository { + BaseRepository(this.plain); + + final PlainRepository plain; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/consumer.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/consumer.g.dart new file mode 100644 index 0000000..cad7bed --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/consumer.g.dart @@ -0,0 +1,20 @@ +// Negative fixture for issue #108: `Consumer` has the exact bare-`super.x` +// PURE CONSUMER shape as `cross_file_super_param_bare_pure_consumer/ +// customers_repository.dart` — importing ONLY `base_repository.dart`, never +// `plain_repository.dart` directly — but `PlainRepository` carries no +// `@SolidState` members. The syntactic super-formal seeding proposes +// `PlainRepository` as a candidate (seeding is annotation-blind by design), +// and the fix review finding 1 one-hop extension genuinely reaches its +// declaration through `base_repository.dart`'s own import of +// `plain_repository.dart` (this file never imports it directly, so without +// that one hop the class would never be found at all) — but finds zero +// `@SolidState` members there, so the cross-file registry stays empty and +// this file must pass through the fast, verbatim-passthrough path exactly +// as before the fix. +import 'base_repository.dart'; + +class Consumer extends BaseRepository { + Consumer(super.plain); + + bool hasLabel() => plain.label != null; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/plain_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/plain_repository.g.dart new file mode 100644 index 0000000..33ba91d --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_no_state/plain_repository.g.dart @@ -0,0 +1,7 @@ +// Negative-fixture companion for `consumer.dart`: `PlainRepository` carries +// no `@Solid*` annotations at all — a genuinely solid-free type reached +// only through a bare `super.x` constructor parameter (issue #108). + +class PlainRepository { + String? label; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/auth_repository.g.dart @@ -0,0 +1,11 @@ +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository implements Disposable { + final session = Signal(null, name: 'session'); + + @override + void dispose() { + session.dispose(); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/base_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/base_repository.g.dart new file mode 100644 index 0000000..4a5bf30 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/base_repository.g.dart @@ -0,0 +1,23 @@ +// Plain, unannotated base class owning the `authRepository` field through +// the `this.x` field-formal shape — the primary real-world DI pattern issue +// #108 targets: the field itself carries no type annotation at the +// parameter position (`this.authRepository`), only on the field +// declaration below it. Also declares a NAMED constructor forwarding the +// same field, exercising the named-super-constructor matching branch +// (`super.named(...)` in `customers_repository.dart`), and a second named +// constructor whose forwarded param is itself NAMED (`{required +// this.authRepository}`) — issue #108 fix review addendum finding 4's +// NAMED bare-super-formal branch, matched against +// `PartnersRepository({required super.authRepository})` in +// `customers_repository.dart`. +import 'auth_repository.dart'; + +class BaseRepository { + BaseRepository(this.authRepository); + + BaseRepository.named(this.authRepository); + + BaseRepository.namedParam({required this.authRepository}); + + final AuthRepository authRepository; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/customers_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/customers_repository.g.dart new file mode 100644 index 0000000..1f0c71d --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_pure_consumer/customers_repository.g.dart @@ -0,0 +1,49 @@ +// PURE CONSUMER whose ONLY link to `AuthRepository` is a BARE `super.x` +// constructor parameter (issue #108) — no `@Solid*` annotation and no DI +// call site of any kind anywhere in this file, so it takes the +// no-annotation fast path's syntactic probe. Before the fix, a +// bare super-formal parameter carried no type text anywhere in the source +// for that unresolved probe to find, so the probe found nothing to seed +// and the file short-circuited to a verbatim copy — every read through +// `authRepository` stayed silently un-lowered (no `.value`). +// +// `CustomersRepository` covers the default (unnamed) super-constructor +// target, with a null-guard read and a `!`-chain read. `VendorsRepository` +// covers the NAMED super-constructor target (`super.named()`), matching +// the bare param against `BaseRepository.named`'s own field-formal +// parameter. `PartnersRepository` covers the NAMED bare-super-formal +// branch (`{required super.authRepository}`, issue #108 fix review +// addendum finding 4), matched against `BaseRepository.namedParam`'s own +// NAMED field-formal parameter. +// +// This file deliberately imports ONLY `base_repository.dart` — issue +// #108's fix review, finding 1: `AuthRepository` is never named in this +// file's text at all (the bare `super.authRepository` shorthand never +// spells it out), so the generator's cross-file registry walk can only +// find its declaration by following ONE MORE hop through +// `base_repository.dart`'s own import of `auth_repository.dart` — proving +// the fix actually closes issue #108's own primary example, a consumer +// that imports only the immediate superclass's file. +import 'base_repository.dart'; + +class CustomersRepository extends BaseRepository { + CustomersRepository(super.authRepository); + + bool hasSession() { + return authRepository.session.value != null; + } + + int? sessionLength() => authRepository.session.value!.length; +} + +class VendorsRepository extends BaseRepository { + VendorsRepository(super.authRepository) : super.named(); + + bool hasSession() => authRepository.session.value != null; +} + +class PartnersRepository extends BaseRepository { + PartnersRepository({required super.authRepository}) : super.namedParam(); + + bool hasSession() => authRepository.session.value != null; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/auth_repository.g.dart @@ -0,0 +1,11 @@ +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository implements Disposable { + final session = Signal(null, name: 'session'); + + @override + void dispose() { + session.dispose(); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/base_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/base_repository.g.dart new file mode 100644 index 0000000..969a44b --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/base_repository.g.dart @@ -0,0 +1,10 @@ +// Generic base class whose field is typed with its OWN type parameter `T` +// (issue #108 fix review addendum finding 2) — `Base(this.repo); final T +// repo;`. No import of any concrete type is needed here: `T` only becomes a +// real class once a subclass supplies a concrete type argument in its own +// `extends` clause. +class Base { + Base(this.repo); + + final T repo; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/consumer.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/consumer.g.dart new file mode 100644 index 0000000..645c68a --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_generic_base/consumer.g.dart @@ -0,0 +1,21 @@ +// PURE CONSUMER whose ONLY link to `AuthRepository` is a BARE `super.repo` +// constructor parameter forwarding into a GENERIC superclass (issue #108 +// fix review addendum finding 2): `Base`'s own field is typed with ITS +// OWN type parameter `T`, not a concrete class name (`Base(this.repo); +// final T repo;`). Naively seeding the matched field's declared type would +// seed the literal string `"T"` — a syntactically valid `NamedType`, so it +// slips past the `matchedType is! NamedType` ambiguity gate — instead of +// the REAL concrete type, `AuthRepository`; no class is ever named `T`, so +// the wanted-type set fills with a name that matches nothing and +// `repo.session` stays silently un-lowered. +// +// The fix maps `T` BY INDEX to `C`'s own `extends Base` +// type argument instead of seeding the placeholder name verbatim. +import 'auth_repository.dart'; +import 'base_repository.dart'; + +class C extends Base { + C(super.repo); + + bool hasSession() => repo.session.value != null; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/base.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/base.g.dart new file mode 100644 index 0000000..91b7b91 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/base.g.dart @@ -0,0 +1,11 @@ +// `Bar`'s superclass, reached one hop away from `consumer.dart` (which +// imports only this file, never `foreign_foo.dart` directly). Owns `thing` +// through the ordinary `this.x` field-formal shape, typed with THIS file's +// own imported `Foo` — the FOREIGN, `@SolidState`-bearing one. +import 'foreign_foo.dart'; + +class Base { + Base(this.thing); + + final Foo thing; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/consumer.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/consumer.g.dart new file mode 100644 index 0000000..51ab04a --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/consumer.g.dart @@ -0,0 +1,48 @@ +// PURE CONSUMER regression fixture (issue #108 fix review, finding 1): +// this file declares its OWN plain `class Foo` (zero `@SolidState` +// members) — held by `Holder` through ordinary constructor injection — and +// ALSO extends `Base` (declared in `base.dart`) through a bare `super.x` +// constructor parameter. `Base` itself is reached directly (main walk), but +// `Base`'s OWN field `thing` is typed with a DIFFERENT, `@SolidState`- +// bearing `Foo` declared in `foreign_foo.dart` — a file this file never +// imports at all; only `base.dart` does. Locating that declaration needs +// the fix review finding 1 one-hop extension. +// +// Before the fix: the one-hop walk registered the FOREIGN `Foo`'s reactive +// `label` field into `classRegistry['Foo']` before this file's own local +// `Foo` had a chance to shadow it, so `Holder.f.label` — a read against the +// LOCAL plain class — was wrongly rewritten to `f.label.value` (BLOCKER: +// non-compiling output, `.value` on a plain `String?` field). +// +// After the fix: the one-hop registration is gated against THIS file's own +// declared type names, same discipline as the main walk (issue #104) — so +// `classRegistry` never gains a `'Foo'` entry at all once this file +// declares its own local `Foo`, name-based collision with no library +// qualification being the design's documented residual risk (SPEC §4.9). +// `f.label` correctly stays un-rewritten (the BLOCKER is closed); `Bar`'s +// `thing.label` — genuinely reactive through the foreign `Foo` — ALSO stays +// un-rewritten as the accepted, conservative cost of a safe fix: this file +// already has a local `Foo` claim on that simple name, so the registry +// cannot safely attribute `label` to `thing` either, exactly the same +// trade-off `cross_file_local_shadowing_decoy` (issue #104) already +// accepts. Favoring "no wrong rewrite" over "no missed rewrite" is +// consistent with every other ambiguity-handling rule in this file. +import 'base.dart'; + +class Foo { + String? label; +} + +class Holder { + Holder(this.f); + + final Foo f; + + String? readLabel() => f.label; +} + +class Bar extends Base { + Bar(super.thing); + + String? readThing() => thing.label; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/foreign_foo.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/foreign_foo.g.dart new file mode 100644 index 0000000..1cb90e6 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_one_hop_local_shadow/foreign_foo.g.dart @@ -0,0 +1,11 @@ +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'package:solid_annotations/solid_annotations.dart'; + +class Foo implements Disposable { + final label = Signal(null, name: 'label'); + + @override + void dispose() { + label.dispose(); + } +} diff --git a/packages/solid_generator/test/integration/golden_helpers.dart b/packages/solid_generator/test/integration/golden_helpers.dart index 8d45c6e..70a36f4 100644 --- a/packages/solid_generator/test/integration/golden_helpers.dart +++ b/packages/solid_generator/test/integration/golden_helpers.dart @@ -155,6 +155,11 @@ const List goldenNames = [ 'cross_file_pure_consumer_widget', 'cross_file_pure_consumer_widget_and_static', 'cross_file_pure_consumer_widget_show_restricted_import', + 'cross_file_super_param_bare_pure_consumer', + 'cross_file_super_param_bare_no_state', + 'cross_file_super_param_bare_chain', + 'cross_file_super_param_one_hop_local_shadow', + 'cross_file_super_param_generic_base', ]; /// Memoized golden directory resolution. Resolved relative to the package