diff --git a/SPEC.md b/SPEC.md index 5017ffa..b40e5e3 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/`. If a file contains at least one `@Solid*` annotation (`@SolidState`, `@SolidEffect`, `@SolidQuery`, `@SolidEnvironment`), Solid transforms it. 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, not file extension. +- **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. - **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';`). diff --git a/packages/solid_generator/CHANGELOG.md b/packages/solid_generator/CHANGELOG.md index b80eac1..f78b0d7 100644 --- a/packages/solid_generator/CHANGELOG.md +++ b/packages/solid_generator/CHANGELOG.md @@ -1,9 +1,15 @@ +## 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). + +Two narrower, adjacent gaps close partially in the same pass. The bare-`super.x` gap from dev.3 (below) is now closed on the RESOLVED path only: a file that already enters the main lowering pipeline for another reason (its own `@Solid*` annotation, or a `Provider`/`.environment()` call site) consults `SuperFormalParameter.declaredFragment.element.type` when the AST carries no explicit type annotation, seeding `wantedTypes` the same as if the type had been written out — verified empirically that the resolved element's type IS populated even without a source-level annotation. This is moot for a PURE consumer whose only link to the cross-file class is a bare `super.x`: such a file carries no annotation and no provider hint, so it takes the no-annotation fast path's UNRESOLVED syntactic probe first, finds nothing to seed, and short-circuits to a verbatim copy before any resolved unit is ever requested — that narrower shape remains a known, accepted gap. Separately, `value_rewriter.dart`'s cross-class receiver resolution gained a fourth fallback tier: when an instance field is declared with no type annotation at all (`final _service;` — infers `dynamic`, since a same-named constructor field-formal parameter's explicit type does NOT propagate back onto the field's inferred type, confirmed empirically against a real resolved unit), the resolver now falls back to a matching `FieldFormalParameter`'s explicit type, or an initializer-list-assigned `SimpleFormalParameter`'s explicit type; multiple constructors supplying conflicting types bail to unresolved rather than guess. + ## 3.0.0-dev.3 - **FIX**: The cross-file class registry (`_populateCrossFileTypes`) now seeds `wantedTypes` from the declared type names of every class's instance fields and constructor parameters, not just `@SolidEnvironment` field types and `Provider(...)`/`.environment()` call sites. A file that only *constructor-receives* a `@SolidState`-bearing class — the plain DI shape, e.g. `CustomersRepository({required AuthRepository authRepository})` storing `final AuthRepository _authRepository;`, with no `@SolidEnvironment` field and no same-file `.environment()`/`Provider()` call site — previously left `classRegistry` empty for that type, so cross-class `.value` reads through it (`_authRepository.session`) were silently un-lowered: no compile error, just an always-non-null `Signal` object that `dart fix`'s `unnecessary_null_comparison` could collapse into dead code (#104). - **FIX**: The seeding above (and, defensively, every other seeding path into `wantedTypes`) now mirrors Dart's own name-resolution rules instead of blindly matching on simple class name. A simple name that the CURRENT file itself declares as a class/enum/mixin is dropped from the wanted set before the cross-file import walk starts — a local top-level declaration always shadows a same-name import, so attributing an unrelated imported class's reactive members to it was provably wrong (e.g. a file with its own plain `class Address` plus an unrelated, unconnected `@SolidState`-annotated `class Address` imported from elsewhere previously emitted a non-compiling `address.line1.value` against the local class). Each import's `show`/`hide` combinators are also now honored: an import that hides the wanted name, or `show`s a list that excludes it, can no longer be credited as that name's source, closing a second, narrower collision window. - **FIX**: Constructor parameters using the explicit-typed `super.` shorthand (`Foo(AuthRepository super.repo)`) now seed `wantedTypes` the same as a plain or field-formal parameter. The far more common *bare* `super.repo` (no type written) is a known, accepted gap: the type isn't present in the source at that position at all — recovering it would require resolving `repo` against the superclass's matching field/parameter, which this syntactic AST walk does not do — so a bare-shorthand `super.` parameter still isn't a seeding source. -- **FIX**: A field or constructor parameter declared with a generic collection type (`final List repos;`) now seeds `wantedTypes` from every type argument, at every nesting level, not just the outer container name (`List`). This lets the existing resolved-static-type rewrite tiers recognize collection-derived receivers whose element type is Solid-lowered — covered by a golden fixture for a `for (final r in repos) { r.field }` loop variable; a `repos.first.field` receiver was verified manually to rewrite the same way (same resolved-type mechanism) but has no persisted fixture, and deeper nesting (`Map>` and beyond) is mechanically identical but likewise untested. Container names themselves (`List`, `Map`, `String`, and other `dart:core`/`dart:async` SDK types) are now filtered out of every seed in the new constructor-injection/field loop before being added, which also fixes a performance regression where annotation-blind seeding of primitive types on nearly every field defeated the `wantedTypes.isEmpty` fast-path. +- **FIX**: A field or constructor parameter declared with a generic collection type (`final List repos;`) now seeds `wantedTypes` from every type argument, at every nesting level, not just the outer container name (`List`). This lets the existing resolved-static-type rewrite tiers recognize collection-derived receivers whose element type is Solid-lowered — covered by golden fixtures for a `for (final r in repos) { r.field }` loop variable and a `repos.first.field` receiver (same resolved-type mechanism); deeper nesting (`Map>` and beyond) is mechanically identical but likewise untested. Container names themselves (`List`, `Map`, `String`, and other `dart:core`/`dart:async` SDK types) are now filtered out of every seed in the new constructor-injection/field loop before being added, which also fixes a performance regression where annotation-blind seeding of primitive types on nearly every field defeated the `wantedTypes.isEmpty` fast-path. ## 3.0.0-dev.2 diff --git a/packages/solid_generator/lib/builder.dart b/packages/solid_generator/lib/builder.dart index d66878f..f291695 100644 --- a/packages/solid_generator/lib/builder.dart +++ b/packages/solid_generator/lib/builder.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'package:analyzer/dart/analysis/features.dart'; import 'package:analyzer/dart/analysis/utilities.dart'; import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/type.dart'; import 'package:build/build.dart'; import 'package:dart_style/dart_style.dart'; import 'package:glob/glob.dart'; @@ -11,6 +12,7 @@ import 'package:solid_generator/src/annotation_reader.dart'; import 'package:solid_generator/src/ast_compat.dart'; import 'package:solid_generator/src/class_kind.dart'; import 'package:solid_generator/src/const_call_site_rewriter.dart'; +import 'package:solid_generator/src/cross_file_consumer_rewriter.dart'; import 'package:solid_generator/src/effect_model.dart'; import 'package:solid_generator/src/empty_dir_pruner.dart'; import 'package:solid_generator/src/environment_model.dart'; @@ -32,9 +34,13 @@ import 'package:solid_generator/src/transformation_error.dart'; Builder solidBuilder(BuilderOptions options) => _SolidBuilder(); /// Substring that must appear in any source file carrying a Solid annotation. -/// A file without this substring cannot possibly need transformation and is -/// skipped before `parseString` — the hot-path short-circuit for the typical -/// unannotated file. +/// A file without this substring cannot need transformation via an +/// annotation of its own. It does NOT skip `parseString`, though: the file +/// may still need the `Provider`/`.environment` auto-dispose pass, or (since +/// #106) the cross-file pure-consumer probe below, either of which needs a +/// parsed unit to decide. The true zero-parse short-circuit this hint used +/// to gate no longer exists; see `build()`'s fast-path comment for what is +/// (and isn't) still cheap for the typical unannotated file. /// /// `solid_annotations` (the package name) is the chosen hint because every /// file that uses any `@Solid*` annotation must import this package — both @@ -128,25 +134,86 @@ class _SolidBuilder implements Builder { buildStep.inputId.path, ); - // Files without any @Solid* annotation pass through verbatim — UNLESS they - // contain a `Provider(...)` or `.environment()` call site, which the - // auto-dispose pass must visit. A `source.contains` check is a cheap - // pre-parse guard — if neither marker is present the file cannot need - // transformation. + // Files without any @Solid* annotation pass through verbatim — UNLESS + // they contain a `Provider(...)` or `.environment()` call site (the + // auto-dispose pass must visit those), OR they are a PURE CONSUMER — a + // file that holds a cross-file `@SolidState`-bearing class through plain + // constructor injection / an instance field, with no `@Solid*` + // annotation and no provider call site of its own (issue #106). A + // `source.contains` check is a cheap pre-parse guard for the first two + // conditions; the third needs the cheap SYNTACTIC (unresolved) parse and + // the #105 cross-file registry seeding below to decide. final hasSolidAnnotation = source.contains(_solidAnnotationHint); final hasProviderHint = source.contains(_providerCallHint) || source.contains(_environmentCallHint); - if (!hasSolidAnnotation && !hasProviderHint) { - await buildStep.writeAsString(outputId, source); - return; - } final parsed = parseString( content: source, featureSet: FeatureSet.latestLanguageVersion(), throwIfDiagnostics: false, ); + + // Populated only when the fast-path probe below actually runs AND finds + // something — i.e. only on the `!hasSolidAnnotation && !hasProviderHint` + // path, and only past the early return. When non-null, these are the + // FULLY POPULATED results of a cross-file import walk this function + // already paid for; the pipeline below (`_populateCrossFileTypes` at its + // second call site) reuses them instead of walking the same imports + // again — see the merge just before that call. + Map>? probedCrossFileRegistry; + Map>? probedCrossFileCollections; + Map>? probedCrossFileFieldTypes; + Map>? probedCrossFileOriginUris; + + if (!hasSolidAnnotation && !hasProviderHint) { + // Cheap syntactic pre-check (#106): seed candidate cross-file wanted + // types from this file's instance fields / constructor params — the + // same rule `_populateCrossFileTypes` applies below for annotated + // files — against the UNRESOLVED parsed unit. No + // `buildStep.resolver.libraryFor` semantic resolution runs here; that + // is the expensive step this fast path exists to avoid. + // + // Honest cost accounting (this fast path's guarantee is NARROWER than + // it used to be — see [_solidAnnotationHint]'s doc comment): `parsed` + // above is now computed for EVERY hint-free file, annotated or not — + // this branch could not exist without a parsed unit to probe. What + // remains zero-cost is `_populateCrossFileTypes` itself: it exits + // before touching any import once its wanted-type set is empty, so a + // file with no custom-typed fields/params at all (only core-SDK / + // self-declared types, per `_coreSdkTypeNames`) pays only the parse + // plus this one field/param scan — no import walk, no resolver call. + // A file whose field/param DOES name an externally-declared type pays + // a BOUNDED import walk below — bounded by this file's own import + // count, not by anything global — and that walk does NOT stop early + // just because one import's same-named class turns out non-reactive: + // `wantedTypes` only drops a name once a `@SolidState`-bearing match + // is found (see the `cross_file_constructor_injected_no_state` + // shape), so a non-reactive same-named class in an early import does + // not short-circuit the scan of later imports for that name. + final probeRegistry = >{}; + final probeCollections = >{}; + final probeFieldTypes = >{}; + final probeOriginUris = >{}; + await _populateCrossFileTypes( + parsed.unit, + buildStep, + probeRegistry, + probeCollections, + probeFieldTypes, + probeOriginUris, + const {}, + ); + if (probeRegistry.isEmpty) { + await buildStep.writeAsString(outputId, source); + return; + } + probedCrossFileRegistry = probeRegistry; + probedCrossFileCollections = probeCollections; + probedCrossFileFieldTypes = probeFieldTypes; + probedCrossFileOriginUris = probeOriginUris; + } + for (final diagnostic in parsed.errors) { log.warning( '${buildStep.inputId}: ${diagnostic.message} ' @@ -229,15 +296,34 @@ class _SolidBuilder implements Builder { // branch below relies on that entry to recognize the type as // Solid-lowered) — same contract as the same-file pass, but for types // declared in other source files. - await _populateCrossFileTypes( - unit, - buildStep, - sameFileRegistry, - sameFileCollections, - sameFileFieldTypes, - crossClassFieldTypeOriginUris, - hasProviderHint ? collectProviderCreatedTypeNames(unit) : const {}, - ); + // + // Skipped when the fast-path probe above already ran this exact walk + // (only possible when `!hasSolidAnnotation && !hasProviderHint`, in + // which case `extraWantedTypes` below is `const {}` either way, and + // `sameFileRegistry`/etc are still empty from the same-file prescan + // above — `hasSolidAnnotation` being false rules out any same-file + // `@SolidState` class). `_populateCrossFileTypes` never reads + // `Expression.staticType` — it is a purely syntactic AST + import scan — + // so re-running it against the resolved [unit] here would produce + // byte-identical results to the probe's run against the unresolved + // `parsed.unit`; merging the probe's output is exactly equivalent to + // (and cheaper than) calling it again. + if (probedCrossFileRegistry != null) { + sameFileRegistry.addAll(probedCrossFileRegistry); + sameFileCollections.addAll(probedCrossFileCollections!); + sameFileFieldTypes.addAll(probedCrossFileFieldTypes!); + crossClassFieldTypeOriginUris.addAll(probedCrossFileOriginUris!); + } else { + await _populateCrossFileTypes( + unit, + buildStep, + sameFileRegistry, + sameFileCollections, + sameFileFieldTypes, + crossClassFieldTypeOriginUris, + hasProviderHint ? collectProviderCreatedTypeNames(unit) : const {}, + ); + } final annotatedClasses = _collectAnnotatedClasses( unit, @@ -246,23 +332,79 @@ class _SolidBuilder implements Builder { sameFileCollections, ); if (annotatedClasses.every((c) => c.hasNoAnnotations)) { - // No reactive annotations resolved. The file may still contain a - // `Provider(...)` or `.environment()` call site that the auto-dispose - // pass must visit; otherwise pass through verbatim. + // No reactive annotations resolved. The file may still need: + // (a) `SignalBuilder`-wrap placement + `.value` lowering on a PURE + // CONSUMER `StatelessWidget`/`State` (issue #106 residual gap + // survey, GAP 1), + // (b) `.value` lowering on a PURE CONSUMER plain class — one that + // reaches a cross-file `@SolidState`-bearing class only through + // constructor injection / an instance field, with no `@Solid*` + // annotation of its own (issue #106; `sameFileRegistry` can be + // non-empty here purely from the cross-file seeding above, even + // though this file owns zero reactive members), and/or + // (c) dispose-call-site injection at a `Provider(...)` / + // `.environment()` call site (pre-existing). + // None of the three restructures a class's `implements` clause or + // synthesizes `dispose()` — those only belong to a class that OWNS at + // least one reactive member (handled by `_renderOutput` below, for + // files where SOME class does). + // + // (a) and (b) are collected and applied TOGETHER by + // [lowerPureConsumers] against the pristine [source] and the + // still-resolved [unit] — see that function's doc comment for why: an + // earlier version ran them as two sequential text-mutating passes, + // which forced the second pass onto an unresolved re-parse whenever + // the first pass edited anything, silently degrading any tier-1-ONLY + // receiver resolution (a static-holder read, a for-in loop variable, + // `.first`) belonging to a DIFFERENT class in the same file. Merging + // both edit sets up front removes the ordering dependency entirely. + // + // (c) runs after, on the ALREADY-MERGED text: + // `addProviderDisposeAtCallSites` is explicitly designed to tolerate + // an unresolved re-parse — its + // `_createdTypeHasDispose` four-tier rule falls back to same-file AST + // tiers (2/3) or the inject-by-default tier 4 whenever `.staticType` + // isn't populated — exactly what happens on the main annotated path in + // `_renderOutput`, which invokes this same function on `combined`, a + // freshly re-assembled text string with no `unit` argument at all. + // Running dispose-injection BEFORE the pure-consumer lowering (an + // earlier order) would cost those passes their only source of + // loop-variable / other non-parameter-non-field receiver resolution + // whenever the dispose pass actually edited the text — reproduced by + // the `cross_file_pure_consumer_with_provider` golden fixture. + final lowered = lowerPureConsumers( + source, + unit, + classRegistry: sameFileRegistry, + classCollectionFields: sameFileCollections, + ); + var current = lowered.text; if (hasProviderHint) { - final withDispose = addProviderDisposeAtCallSites( - source, - unit: unit, + current = addProviderDisposeAtCallSites( + current, + unit: identical(current, source) ? unit : null, classRegistry: sameFileRegistry, ); - if (identical(withDispose, source)) { - await buildStep.writeAsString(outputId, source); - return; - } - await buildStep.writeAsString(outputId, _formatter.format(withDispose)); + } + // A pure-consumer widget's `SignalBuilder` wrap (GAP 1 above) needs + // `flutter_solidart` imported — this no-annotation branch otherwise + // never touches imports at all (unlike `_renderOutput`'s + // `computeOutputImports` call), since `.value`-only lowering + // introduces no new identifier that isn't already resolvable through + // the existing imports. `lowered.emittedSignalBuilder` is + // [lowerPureConsumers]'s own report of whether it placed a wrap — not + // a substring scan of `current`, which would both misfire on a + // preserved comment containing the text `SignalBuilder(` AND treat ANY + // existing `flutter_solidart` import as sufficient even when that + // import's `show`/`hide` combinators don't actually expose the name. + if (lowered.emittedSignalBuilder) { + current = _ensureSignalBuilderResolves(current, unit); + } + if (identical(current, source)) { + await buildStep.writeAsString(outputId, source); return; } - await buildStep.writeAsString(outputId, source); + await buildStep.writeAsString(outputId, _formatter.format(current)); return; } @@ -1023,7 +1165,15 @@ Future _populateCrossFileTypes( if (decl is! ClassDeclaration) continue; for (final member in decl.members) { if (member is FieldDeclaration) { - if (member.isStatic) continue; + // `static` fields ARE seeded (issue #106 residual gap survey, GAP + // 2): a singleton-holder shape (`static final AuthRepository + // instance = …;`, consumed elsewhere as `Holder.instance.session`) + // is exactly as valid a DI source as an instance field. The + // original `isStatic` skip here carried no stated rationale in + // #104/#105's history — it silently mirrored `@SolidState`'s OWN + // instance-only restriction (Section 3.1), which is a rule about + // what `@SolidState` may annotate, not about what this unrelated + // seeding loop may harvest a type name from. final type = member.fields.type; if (type is! NamedType) continue; _seedWantedTypeRecursive(type, wantedTypes, classRegistry); @@ -1038,18 +1188,32 @@ Future _populateCrossFileTypes( } else if (inner is FieldFormalParameter) { paramType = inner.type; } else if (inner is SuperFormalParameter) { - // Only the explicit-type-annotation shape (`Foo(AuthRepository - // super.repo)`) is recoverable here: `inner.type` is populated - // straight from the source annotation, same as the two branches - // above. The far more common bare-shorthand form (`super.repo`, - // no type written) carries NO type annotation in the source at - // all — the type is only knowable by resolving `repo` against - // the superclass's matching parameter/field, which this - // syntactic AST walk does not do. That shape is a known, - // accepted gap (see CHANGELOG): such a parameter simply isn't a - // wantedTypes candidate from this loop, same as if the class had - // no matching field/param seeding it at all. + // The explicit-type-annotation shape (`Foo(AuthRepository + // super.repo)`) is recoverable syntactically: `inner.type` is + // populated straight from the source annotation, same as the + // two branches above. The far more common bare-shorthand form + // (`super.repo`, no type written) carries NO type annotation in + // the source at all — but on the RESOLVED path (this file has + // at least one `@Solid*` annotation or a provider hint of its + // own, so `unit` here is a fully-resolved `CompilationUnit`), + // `SuperFormalParameter.declaredFragment.element.type` IS + // populated even without a written annotation (issue #106 + // residual gap survey, GAP 3; verified empirically against + // `package:analyzer`). `_seedFromResolvedSuperFormal` is the + // fallback for exactly that case. + // + // This resolved-only fallback 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. paramType = inner.type; + if (paramType == null) { + _seedFromResolvedSuperFormal(inner, wantedTypes, classRegistry); + } } else { paramType = null; } @@ -1344,6 +1508,165 @@ void _seedWantedTypeRecursive( } } +/// Ensures the `SignalBuilder` identifier [lowerPureConsumers] (issue #106 +/// residual gap survey, GAP 1) reported emitting via `emittedSignalBuilder` +/// actually resolves in [text]. Callers already checked that flag — never a +/// substring scan of [text] for `SignalBuilder(`, which both risks a +/// false-positive on a preserved source comment and can't tell an EXPOSED +/// existing import from a RESTRICTED one. +/// +/// Three cases, keyed on whether [unit] already imports `flutter_solidart` +/// and, if so, whether its combinators actually expose the name: +/// * no `flutter_solidart` import at all → splice in a clean one +/// ([_ensureFlutterSolidartImport], pre-existing behavior). +/// * an import exists and already exposes `SignalBuilder` +/// ([_importExposesName], reused from the cross-file registry walk) → +/// no-op. This is the common case: an import added fresh by this same +/// function always exposes every name, so only a HAND-WRITTEN `show`/ +/// `hide` on a pre-existing import can narrow it. +/// * an import exists but does NOT expose `SignalBuilder` (a `show` list +/// that omits it, or a `hide SignalBuilder`) — e.g. a file that already +/// does `import '…/flutter_solidart.dart' show Signal;` for some +/// unrelated hand-rolled signal before ever gaining a pure-consumer +/// widget read — → repair that import's combinators in place +/// ([_repairImportToExposeName]) rather than adding a second import of +/// the same URI (which `dart analyze` would flag as `duplicate_import`). +/// +/// Known, accepted gap: an `as`-PREFIXED existing import (`import '…' as +/// sa;`) is not detected as non-exposing — [_importExposesName] only +/// inspects `show`/`hide` combinators, not prefixes — so a file whose only +/// `flutter_solidart` import is aliased would still emit an unresolvable +/// bare `SignalBuilder(...)`. No reported real-world shape does this (the +/// project's own convention forbids import aliases), and every one of this +/// generator's OWN rewriters emits bare (unprefixed) `flutter_solidart` +/// identifiers, so the gap only bites a hand-aliased pre-existing import. +String _ensureSignalBuilderResolves(String text, CompilationUnit unit) { + ImportDirective? existing; + for (final directive in unit.directives.whereType()) { + if (directive.uri.stringValue == flutterSolidartUri) { + existing = directive; + break; + } + } + if (existing == null) return _ensureFlutterSolidartImport(text, unit); + if (_importExposesName(existing, 'SignalBuilder')) return text; + return _repairImportToExposeName(text, existing, 'SignalBuilder'); +} + +/// Splices [name] into [directive]'s combinators so it becomes visible, +/// using the combinator nodes' own AST offsets rather than a fragile textual +/// regex — robust regardless of whitespace/formatting: +/// * an existing `show` combinator gains `, ` after its last shown +/// name; +/// * an existing `hide` combinator loses `` — the whole `hide` +/// clause is removed if [name] was its only hidden name, otherwise just +/// `` (and its separating comma) is spliced out. +/// +/// Only called after [_importExposesName] has already confirmed [directive] +/// does NOT expose [name], so exactly one of the two branches below applies +/// — an import with neither combinator, or with a `show`/`hide` that +/// already agrees the name is visible, would have made +/// [_importExposesName] return `true` and this function would never run. +/// Leaves [text] untouched (defensive fallback, not expected to be reached) +/// if neither combinator matches. +String _repairImportToExposeName( + String text, + ImportDirective directive, + String name, +) { + for (final combinator in directive.combinators) { + if (combinator is ShowCombinator) { + if (combinator.shownNames.any((id) => id.name == name)) continue; + final last = combinator.shownNames.last; + return '${text.substring(0, last.end)}, ' + '$name${text.substring(last.end)}'; + } + if (combinator is HideCombinator) { + final hidden = combinator.hiddenNames; + final idx = hidden.indexWhere((id) => id.name == name); + if (idx == -1) continue; + if (hidden.length == 1) { + return text.substring(0, combinator.offset) + + text.substring(combinator.end); + } + if (idx == 0) { + return text.substring(0, hidden[0].offset) + + text.substring(hidden[1].offset); + } + return text.substring(0, hidden[idx - 1].end) + + text.substring(hidden[idx].end); + } + } + return text; +} + +/// Ensures `package:flutter_solidart/flutter_solidart.dart` is imported in +/// [text] when no existing import of it is present at all. Called by +/// [_ensureSignalBuilderResolves] for that one case; the other two (an +/// existing import that already exposes the name, or one that needs +/// combinator repair) are handled directly there. +/// +/// Unlike `_renderOutput`'s main annotated path, this no-annotation branch +/// never runs [computeOutputImports] against a freshly reassembled import +/// block — there is no per-class rewrite result to aggregate. This is a +/// narrow, text-level splice instead: [unit]'s import-directive offsets are +/// still valid against [text] because every edit the pure-consumer lowering +/// passes make lands inside a class member, strictly after the last import +/// directive — so the import section itself is never touched by them. +String _ensureFlutterSolidartImport(String text, CompilationUnit unit) { + final directives = unit.directives.whereType().toList(); + final sourceUris = []; + final sourceDirectiveText = {}; + for (final directive in directives) { + final uri = directive.uri.stringValue; + if (uri == null) continue; + sourceUris.add(uri); + sourceDirectiveText[uri] = directive.toSource(); + } + final imports = computeOutputImports( + sourceUris, + addSolidart: true, + referencesSolidAnnotations: true, + ); + final importBlock = imports + .map((uri) => sourceDirectiveText[uri] ?? "import '$uri';") + .join('\n'); + if (directives.isEmpty) { + return '$importBlock\n\n$text'; + } + return text.substring(0, directives.first.offset) + + importBlock + + text.substring(directives.last.end); +} + +/// Resolved-path fallback for a BARE `super.` formal parameter (issue #106 +/// residual gap survey, GAP 3): [param]'s own AST carries no type +/// annotation, but on a fully-resolved [CompilationUnit], +/// `declaredFragment.element.type` still resolves to the forwarded +/// parameter's real type (verified empirically against `package:analyzer` +/// directly — the analyzer resolves a super-initializer parameter's type +/// from the superclass constructor's matching parameter even when nothing +/// is written at this position). Seeds [wantedTypes] the same way +/// [_seedWantedTypeRecursive] would if the source had spelled the type out. +/// +/// No-ops when the unit isn't resolved (`declaredFragment` unpopulated) or +/// 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( + SuperFormalParameter param, + Set wantedTypes, + Map> classRegistry, +) { + final type = param.declaredFragment?.element.type; + if (type is! InterfaceType) return; + final typeText = type.element.name; + if (typeText == null) return; + if (_coreSdkTypeNames.contains(typeText)) return; + if (classRegistry.containsKey(typeText)) return; + wantedTypes.add(typeText); +} + /// Translates a `source/` AssetId to its `lib/` sibling. The /// inverse of [_resolveImportToSourceAsset]'s `lib/` → `source/` redirect. /// Passes non-`source/` AssetIds through unchanged. diff --git a/packages/solid_generator/lib/src/build_rewriter.dart b/packages/solid_generator/lib/src/build_rewriter.dart index 304bd4e..de1fa17 100644 --- a/packages/solid_generator/lib/src/build_rewriter.dart +++ b/packages/solid_generator/lib/src/build_rewriter.dart @@ -23,6 +23,13 @@ class SourceEdit { final String replacement; } +/// Return type of [rewriteBuildMethod]: the rewritten build-method text +/// (`text`) plus whether a `SignalBuilder` wrap was actually placed +/// (`emittedWrap`). See [rewriteBuildMethod]'s doc comment for why callers +/// should use `emittedWrap` instead of scanning `text` for the substring +/// `SignalBuilder(`. +typedef BuildMethodRewrite = ({String text, bool emittedWrap}); + /// Rewrites the `build()` method source text by applying reactive-read rules. /// /// The rewrite comprises four passes composed in the order they are visible @@ -81,10 +88,17 @@ class SourceEdit { /// collection-signal slice: `.` shapes skip /// `.value` and resolve through the mixin on the receiver chain directly. /// -/// [source] is the full source text of the input file. The returned string -/// is the rewritten build method (from `@override` through the closing `}`), -/// ready for the caller to embed into the emitted `State` class. -String rewriteBuildMethod( +/// [source] is the full source text of the input file. The returned +/// record's `text` is the rewritten build method (from `@override` through +/// the closing `}`), ready for the caller to embed into the emitted `State` +/// class. Its `emittedWrap` is true iff this call placed at least one +/// `SignalBuilder` wrap (an outermost wrap node, or the unanchored-reads +/// fallback) — callers use this to decide whether `flutter_solidart` needs +/// importing without re-deriving the same fact by scanning the returned +/// text for the substring `SignalBuilder(`, which can misfire on a +/// preserved source comment or string literal that happens to contain that +/// text. +BuildMethodRewrite rewriteBuildMethod( MethodDeclaration buildMethod, Set reactiveFields, String source, { @@ -216,7 +230,7 @@ String rewriteBuildMethod( methodStart: methodStart, ); } - return result; + return (text: result, emittedWrap: wrapNodes.isNotEmpty || hasUnanchored); } /// Wraps the build method's body in an outer `SignalBuilder` when one or diff --git a/packages/solid_generator/lib/src/cross_file_consumer_rewriter.dart b/packages/solid_generator/lib/src/cross_file_consumer_rewriter.dart new file mode 100644 index 0000000..539b046 --- /dev/null +++ b/packages/solid_generator/lib/src/cross_file_consumer_rewriter.dart @@ -0,0 +1,246 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:solid_generator/src/ast_compat.dart'; +import 'package:solid_generator/src/build_rewriter.dart'; +import 'package:solid_generator/src/class_kind.dart'; +import 'package:solid_generator/src/value_rewriter.dart'; + +/// Combined result of [lowerPureConsumers]: the fully-lowered `text` and +/// whether the widget pass actually placed a `SignalBuilder` wrap anywhere +/// in the file. +/// +/// `emittedSignalBuilder` is sourced from [rewriteBuildMethod]'s own +/// `emittedWrap` flag — never re-derived by scanning `text` for the +/// substring `SignalBuilder(`, which would misfire on a source comment or +/// string literal that happens to contain that text. `builder.dart` uses +/// this flag to decide whether the `flutter_solidart` import needs +/// splicing in (or repairing) — see `_ensureSignalBuilderResolves`. +typedef PureConsumerLowering = ({String text, bool emittedSignalBuilder}); + +/// Lowers BOTH pure-consumer shapes — a plain-class consumer (issue #106) +/// and a `StatelessWidget`/`State` consumer (issue #106 residual gap +/// survey, GAP 1) — in a single text transformation. +/// +/// [classRegistry]/[classCollectionFields] already contain the cross-file +/// entries `builder.dart::_populateCrossFileTypes` seeds by the time this +/// runs. +/// +/// STRUCTURAL NOTE (why this is one function, not two run in sequence): +/// an earlier version ran [collectPureConsumerWidgetEdits] first against +/// [text] and [unit], applied its edits to produce a new string, then ran +/// [collectPureConsumerCrossFileEdits] against THAT string — forcing it +/// onto a fresh, UNRESOLVED re-parse (`unit: null`) whenever the widget +/// pass had made any change at all. That silently degraded any tier-1-ONLY +/// read (a static-holder receiver like `Holder.instance.session`, a for-in +/// loop variable, `.first`) belonging to a DIFFERENT class in the same +/// file: such reads need `Expression.staticType` from a genuinely resolved +/// unit, which the AST-only fallback tiers of an unresolved re-parse cannot +/// supply — the read stayed un-lowered with no error, exactly the shape of +/// the original #106 bug, just reproduced whenever a file happened to ALSO +/// contain a widget pure consumer. +/// +/// The fix is to never let either pass observe a text mutation from the +/// other before deciding what to rewrite: both [collectPureConsumerWidgetEdits] +/// and [collectPureConsumerCrossFileEdits] collect their edits against the +/// SAME pristine [text] and the SAME resolved [unit], and this function +/// merges the two edit lists and applies them together in one +/// [applyEditsToRange] call. This is safe because the two passes visit +/// disjoint class-kind sets — [ClassKind.plainClass] vs +/// [ClassKind.statelessWidget]/[ClassKind.stateClass] — so their edit +/// ranges can never overlap; there is no ordering dependency left to get +/// wrong. +/// +/// Returns `text` unchanged (by reference) when neither pass has anything +/// to rewrite — callers rely on that identity to skip re-formatting an +/// untouched file. +PureConsumerLowering lowerPureConsumers( + String text, + CompilationUnit unit, { + required Map> classRegistry, + required Map> classCollectionFields, +}) { + final widgetResult = collectPureConsumerWidgetEdits( + unit, + text, + classRegistry: classRegistry, + classCollectionFields: classCollectionFields, + ); + final crossFileEdits = collectPureConsumerCrossFileEdits( + unit, + text, + classRegistry: classRegistry, + classCollectionFields: classCollectionFields, + ); + if (widgetResult.edits.isEmpty && crossFileEdits.isEmpty) { + return (text: text, emittedSignalBuilder: false); + } + final mergedEdits = [...widgetResult.edits, ...crossFileEdits]; + final rewritten = applyEditsToRange(text, mergedEdits, 0); + return ( + text: rewritten, + emittedSignalBuilder: widgetResult.emittedSignalBuilder, + ); +} + +/// Collects `.value`-lowering edits for a PURE CONSUMER plain class — one +/// that reaches a cross-file `@SolidState`-bearing class only through +/// constructor injection or a plain instance field, while declaring NO +/// `@Solid*` annotation of its own (issue #106). +/// +/// Such a class never reaches `rewritePlainClass` / `rewriteStateClass` / +/// `rewriteStatelessWidget` — those only run for a class with at least one +/// reactive member of its OWN, because they unconditionally add `implements +/// Disposable` and synthesize a `dispose()`, which would be wrong here: a +/// pure consumer owns no reactive member and disposes nothing of its own. +/// Instead, this applies the exact same name-based `.value` rewrite +/// `rewriteUserMethod` applies to a NON-annotated member of an +/// already-annotated class — using an EMPTY same-class reactive-name set +/// (there is none) and [classRegistry] (already merged with the cross-file +/// entries `builder.dart::_populateCrossFileTypes` seeds) for the +/// cross-class branch. +/// +/// Scope, matched to parity with the annotated-class pipeline: +/// * Only [ClassKind.plainClass] declarations are visited here. A +/// pure-consumer `StatelessWidget`/`State` whose `build()` reads a +/// cross-file signal needs a `SignalBuilder` wrap around the tracked +/// subtree to rebuild reactively, not just a bare `.value` read with no +/// subscription (which would look fixed while staying silently +/// non-reactive) — see [collectPureConsumerWidgetEdits] for that shape +/// (issue #106 residual gap survey, GAP 1). +/// * Only constructors and methods (including getters/setters) are visited. +/// Field initializers are left verbatim — `rewritePlainClass` applies the +/// same rule to non-annotated fields on an already-annotated class. +/// * Factory constructors are skipped — `rewritePlainClass` round-trips +/// them verbatim for the same reason (a factory body is a delegating +/// return, not an instance-scoped body). +/// * Bare top-level functions reading a cross-file signal outside any class +/// are a known, accepted gap: the reproduced issue and every reported +/// real-world shape are DI-shaped (a class holding another class). +/// +/// Returns an empty edit list when [classRegistry] is empty or no visited +/// member contains a rewrite. +List collectPureConsumerCrossFileEdits( + CompilationUnit unit, + String text, { + required Map> classRegistry, + required Map> classCollectionFields, +}) { + if (classRegistry.isEmpty) return const []; + final edits = []; + for (final decl in unit.declarations) { + if (decl is! ClassDeclaration) continue; + if (classKindOf(decl) != ClassKind.plainClass) continue; + for (final member in decl.members) { + if (member is ConstructorDeclaration) { + if (member.factoryKeyword != null) continue; + } else if (member is! MethodDeclaration) { + continue; + } + final result = collectValueEdits( + member, + const {}, + text, + classRegistry: classRegistry, + classCollectionFields: classCollectionFields, + ); + edits.addAll(result.edits); + } + } + return edits; +} + +/// Collects `.value`-lowering edits AND `SignalBuilder`-wrap placement for a +/// PURE CONSUMER `StatelessWidget`/`State` class — the widget-class +/// sibling of [collectPureConsumerCrossFileEdits] (issue #106 residual gap +/// survey, GAP 1). +/// +/// A pure-consumer widget's `build()` needs BOTH the `.value` lowering AND +/// reactive rebuild, exactly like an `@SolidEnvironment`-consuming widget's +/// `build()` — but with none of the state-ownership machinery: the injected +/// dependency arrives through plain constructor injection (a field, not +/// `context.read()`), so no `BuildContext` is needed and no +/// `StatelessWidget`→`StatefulWidget` split is required. The exact same +/// `rewriteBuildMethod` used by `stateless_rewriter.dart` / +/// `state_class_rewriter.dart` for an ANNOTATED widget's `build()` is reused +/// verbatim here — it already resolves `.value` lowering, tracked-read +/// collection, and `SignalBuilder` wrap placement (SPEC §7) from a bare +/// [MethodDeclaration] plus a `classRegistry`, with no dependency on the +/// class owning any reactive member of its own. +/// +/// Scope, matched to parity with [collectPureConsumerCrossFileEdits]: +/// * Only [ClassKind.statelessWidget] and [ClassKind.stateClass] +/// declarations are visited — a bare [ClassKind.statefulWidget] shell has +/// no `build()` of its own (that lives on its `State` sibling), and +/// [ClassKind.plainClass] is the sibling function's scope. +/// * The `build()` method is rewritten through [rewriteBuildMethod] (which +/// internally applies the wrap-placement rules and reports whether it +/// placed one via its `emittedWrap` flag); every other constructor / +/// method is rewritten through the same per-identifier [collectValueEdits] +/// path [collectPureConsumerCrossFileEdits] uses, since only `build()`'s +/// return value is a widget subtree that can host a `SignalBuilder`. +/// * `reactiveFields`/`environmentFields`/`widgetBoundFields` are all empty +/// — a pure consumer owns no reactive member of its own, no +/// `@SolidEnvironment` field, and (unlike the Stateless→Stateful LIFT +/// rewriters) never moves its body to a different class scope, so no +/// `widget.`-prefixing is needed. +/// +/// Returns `(edits: const [], emittedSignalBuilder: false)` when +/// [classRegistry] is empty or no visited member contains a rewrite. +({List edits, bool emittedSignalBuilder}) +collectPureConsumerWidgetEdits( + CompilationUnit unit, + String text, { + required Map> classRegistry, + required Map> classCollectionFields, +}) { + if (classRegistry.isEmpty) { + return (edits: const [], emittedSignalBuilder: false); + } + final edits = []; + var emittedSignalBuilder = false; + for (final decl in unit.declarations) { + if (decl is! ClassDeclaration) continue; + final kind = classKindOf(decl); + if (kind != ClassKind.statelessWidget && kind != ClassKind.stateClass) { + continue; + } + for (final member in decl.members) { + if (member is MethodDeclaration && member.name.lexeme == 'build') { + final original = text.substring(member.offset, member.end); + final rewritten = rewriteBuildMethod( + member, + const {}, + text, + classRegistry: classRegistry, + classCollectionFields: classCollectionFields, + ); + if (rewritten.emittedWrap) emittedSignalBuilder = true; + if (rewritten.text != original) { + edits.add(ValueEdit(member.offset, member.end, rewritten.text)); + } + continue; + } + if (member is ConstructorDeclaration) { + if (member.factoryKeyword != null) continue; + final result = collectValueEdits( + member, + const {}, + text, + classRegistry: classRegistry, + classCollectionFields: classCollectionFields, + ); + edits.addAll(result.edits); + continue; + } + if (member is! MethodDeclaration) continue; + final result = collectValueEdits( + member, + const {}, + text, + classRegistry: classRegistry, + classCollectionFields: classCollectionFields, + ); + edits.addAll(result.edits); + } + } + return (edits: edits, emittedSignalBuilder: emittedSignalBuilder); +} diff --git a/packages/solid_generator/lib/src/state_class_rewriter.dart b/packages/solid_generator/lib/src/state_class_rewriter.dart index 090040a..a91a14b 100644 --- a/packages/solid_generator/lib/src/state_class_rewriter.dart +++ b/packages/solid_generator/lib/src/state_class_rewriter.dart @@ -142,7 +142,7 @@ RewriteResult rewriteStateClass( if (member is MethodDeclaration) { final name = member.name.lexeme; if (name == 'build') { - final buildText = rewriteBuildMethod( + final buildRewrite = rewriteBuildMethod( member, reactiveNames, source, @@ -152,8 +152,12 @@ RewriteResult rewriteStateClass( collectionFields: collectionNames, classCollectionFields: classCollectionFields, ); - pieces.add(buildText); - if (buildText.contains('SignalBuilder(')) { + pieces.add(buildRewrite.text); + // `buildRewrite.emittedWrap` is sourced from `rewriteBuildMethod`'s + // own wrap-placement decision — not re-derived by scanning the + // returned text for the substring `SignalBuilder(`, which could + // misfire on a preserved source comment containing that text. + if (buildRewrite.emittedWrap) { buildHasSignalBuilder = true; } } else if (name == 'initState') { diff --git a/packages/solid_generator/lib/src/stateless_rewriter.dart b/packages/solid_generator/lib/src/stateless_rewriter.dart index f108b11..0111f17 100644 --- a/packages/solid_generator/lib/src/stateless_rewriter.dart +++ b/packages/solid_generator/lib/src/stateless_rewriter.dart @@ -100,7 +100,7 @@ RewriteResult rewriteStatelessWidget( final widgetBoundForBuild = widgetBoundNames.difference( partitionExcludeNames, ); - final buildMethodText = rewriteBuildMethod( + final buildRewrite = rewriteBuildMethod( members.buildMethod, reactiveNames, source, @@ -111,6 +111,7 @@ RewriteResult rewriteStatelessWidget( collectionFields: collectionNames, classCollectionFields: classCollectionFields, ); + final buildMethodText = buildRewrite.text; final reactiveBlock = _emitReactiveBlock( classDecl, @@ -188,13 +189,15 @@ RewriteResult rewriteStatelessWidget( // own same-class reactive declaration OR the build body wraps a // cross-class tracked read in `SignalBuilder` (the env-injected receiver // shape — the consumer has no own state but reads through to a sibling - // class's `@SolidState`). The textual scan on `buildMethodText` catches - // the cross-class case without re-walking the AST. + // class's `@SolidState`). `buildRewrite.emittedWrap` catches the + // cross-class case without re-walking the AST or scanning `buildMethodText` + // for the substring `SignalBuilder(` (which could misfire on a preserved + // source comment containing that text). final hasReactive = solidFields.isNotEmpty || solidGetters.isNotEmpty || solidQueries.isNotEmpty; - final buildEmitsSignalBuilder = buildMethodText.contains('SignalBuilder('); + final buildEmitsSignalBuilder = buildRewrite.emittedWrap; // A field's emitted ctor is one of: Signal / ListSignal / SetSignal / // MapSignal. Collection emitters bypass `Signal` entirely, so the import // set follows the per-field decision rather than blanket-adding `Signal`. diff --git a/packages/solid_generator/lib/src/value_rewriter.dart b/packages/solid_generator/lib/src/value_rewriter.dart index 511c4b4..27916db 100644 --- a/packages/solid_generator/lib/src/value_rewriter.dart +++ b/packages/solid_generator/lib/src/value_rewriter.dart @@ -826,18 +826,87 @@ class _ValueRewriteVisitor extends RecursiveAstVisitor { /// and the `this.` branch (tier 4) of [_resolveReceiverTypeName]. /// Walks [anchor]'s enclosing [ClassDeclaration] for a matching non-static /// [FieldDeclaration] named [name] and returns the declared [NamedType]'s - /// lexeme. Returns `null` for `var`/inferred-typed fields, static fields, - /// and when no field on the enclosing class matches [name]. + /// lexeme. Returns `null` when no field on the enclosing class matches + /// [name] at all. + /// + /// When a matching field IS found but is declared with no type annotation + /// (`final _service;` — infers `dynamic` under Dart's own rules, which + /// does NOT consult a same-named constructor parameter's explicit type), + /// falls back to [_resolveFieldTypeFromConstructorParams] (issue #106 + /// residual gap survey, GAP 4) rather than returning `null` outright. String? _resolveInstanceFieldTypeNameByName(String name, AstNode anchor) { final classDecl = anchor.thisOrAncestorOfType(); if (classDecl == null) return null; + var untypedFieldFound = false; for (final member in classDecl.members) { if (member is! FieldDeclaration) continue; if (member.isStatic) continue; - final type = member.fields.type; - if (type is! NamedType) continue; for (final variable in member.fields.variables) { - if (variable.name.lexeme == name) return type.name.lexeme; + if (variable.name.lexeme != name) continue; + final type = member.fields.type; + if (type is NamedType) return type.name.lexeme; + untypedFieldFound = true; + } + } + if (!untypedFieldFound) return null; + return _resolveFieldTypeFromConstructorParams(classDecl, name); + } + + /// Fallback for an untyped instance field (`final _service;` — infers + /// `dynamic`): recovers the real type from a constructor that initializes + /// it, covering two simple, unambiguous shapes: + /// + /// 1. A field-formal parameter with an explicit type + /// (`Foo(AuthService this._service)`). + /// 2. A plain typed parameter assigned to the field in the same + /// constructor's initializer list, via a bare-identifier RHS + /// (`Foo(AuthService service) : _service = service;`). + /// + /// Scans every constructor. If two constructors supply DIFFERENT types for + /// [fieldName], the result is ambiguous and this bails to `null` + /// (unresolved) rather than guess — the same "unknown wins over wrong + /// guess" posture the rest of this file's resolution tiers take. + String? _resolveFieldTypeFromConstructorParams( + ClassDeclaration classDecl, + String fieldName, + ) { + String? found; + for (final member in classDecl.members) { + if (member is! ConstructorDeclaration) continue; + final candidate = _fieldTypeFromConstructor(member, fieldName); + if (candidate == null) continue; + if (found != null && found != candidate) return null; + found = candidate; + } + return found; + } + + /// Returns the type [fieldName] resolves to from [ctor]'s field-formal + /// parameter or initializer-list assignment, or `null` if neither shape + /// is present. See [_resolveFieldTypeFromConstructorParams] for the two + /// shapes covered. + String? _fieldTypeFromConstructor( + ConstructorDeclaration ctor, + String fieldName, + ) { + for (final param in ctor.parameters.parameters) { + final inner = param is DefaultFormalParameter ? param.parameter : param; + if (inner is FieldFormalParameter && inner.name.lexeme == fieldName) { + final type = inner.type; + if (type is NamedType) return type.name.lexeme; + } + } + for (final initializer in ctor.initializers) { + if (initializer is! ConstructorFieldInitializer) continue; + if (initializer.fieldName.name != fieldName) continue; + final expr = initializer.expression; + if (expr is! SimpleIdentifier) continue; + for (final param in ctor.parameters.parameters) { + final inner = param is DefaultFormalParameter ? param.parameter : param; + if (inner is SimpleFormalParameter && inner.name?.lexeme == expr.name) { + final type = inner.type; + if (type is NamedType) return type.name.lexeme; + } } } return null; diff --git a/packages/solid_generator/pubspec.yaml b/packages/solid_generator/pubspec.yaml index 0859c86..63b9dd4 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.3 +version: 3.0.0-dev.4 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_class_field_type_from_ctor_param.dart b/packages/solid_generator/test/golden/inputs/cross_class_field_type_from_ctor_param.dart new file mode 100644 index 0000000..2ef7dc3 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_class_field_type_from_ctor_param.dart @@ -0,0 +1,69 @@ +// GAP 4 (post-#106 residual gap survey): an instance field declared with NO +// type annotation (`final _service;`) infers `dynamic` under Dart's own +// type-inference rules — a constructor field-formal parameter's EXPLICIT +// type (`AuthRepository this._service`) does NOT propagate back onto the +// field's inferred type, confirmed empirically via `package:analyzer` +// against a real resolved unit (the field's `declaredFragment.element.type` +// is `dynamic`, not `AuthRepository`). None of `value_rewriter.dart`'s +// existing receiver-resolution tiers consult constructor parameters, so +// `_service.session` stayed un-lowered even though the type is spelled out +// right there in the constructor signature. +// +// This is a same-file fixture (not `cross_file_*`): the untyped-field → +// ctor-param fallback is a pure AST scan of the ENCLOSING class's own +// constructors, independent of where `AuthRepository` itself is declared. +// +// `SessionReader` covers the field-formal parameter shape +// (`AuthRepository this._service`) — the explicit type is spelled out only +// on the parameter. `SessionReaderViaInitializer` covers the sibling +// initializer-list shape: a plain (non-field-formal) typed parameter +// assigned to the untyped field in the constructor's initializer list +// (`SessionReaderViaInitializer(AuthRepository service) : _service = +// service;`). Both classes carry their own `@SolidState` field so they +// enter the per-member rewrite pipeline (a class with zero `@Solid*` +// annotations of its own, in a file where some OTHER class IS annotated, is +// passed through verbatim rather than rewritten — only a file where EVERY +// class lacks annotations takes the pure-consumer path). +// +// The untyped `_service` field is the whole point of this fixture — the +// lints below all restate the same fact (no explicit type, so `dynamic`) +// that this fixture exists to exercise. +// ignore_for_file: inference_failure_on_uninitialized_variable +// ignore_for_file: specify_nonobvious_property_types +// ignore_for_file: prefer_typing_uninitialized_variables +// ignore_for_file: strict_top_level_inference +// ignore_for_file: avoid_dynamic_calls +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} + +class SessionReader { + SessionReader(AuthRepository this._service); + + final _service; + + @SolidState() + int probeCount = 0; + + bool hasSession() { + probeCount = probeCount + 1; + return _service.session != null; + } +} + +class SessionReaderViaInitializer { + SessionReaderViaInitializer(AuthRepository service) : _service = service; + + final _service; + + @SolidState() + int probeCount = 0; + + bool hasSession() { + probeCount = probeCount + 1; + return _service.session != null; + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_collection_generic_arg_seeding/manager.dart b/packages/solid_generator/test/golden/inputs/cross_file_collection_generic_arg_seeding/manager.dart index 770c56f..74c638c 100644 --- a/packages/solid_generator/test/golden/inputs/cross_file_collection_generic_arg_seeding/manager.dart +++ b/packages/solid_generator/test/golden/inputs/cross_file_collection_generic_arg_seeding/manager.dart @@ -6,6 +6,11 @@ // (and even that was later filtered as an SDK name — see finding 5), so // `AuthRepository` never entered `wantedTypes` and the for-in loop variable // read below stayed un-lowered. +// +// `firstHasSession` covers the sibling `.first` receiver (same finding): +// previously verified manually but had no persisted fixture — see +// CHANGELOG. Same resolved-type mechanism as the for-in loop variable: +// `repos.first` resolves to `AuthRepository` via `Expression.staticType`. import 'package:solid_annotations/solid_annotations.dart'; import 'auth_repository.dart'; @@ -25,4 +30,6 @@ class Manager { } return false; } + + bool firstHasSession() => repos.first.session != null; } diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer/auth_repository.dart new file mode 100644 index 0000000..01d7b6b --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer/auth_repository.dart @@ -0,0 +1,16 @@ +// Cross-file `@SolidState` host for the PURE CONSUMER regression (issue +// #106). `session_reader.dart` reaches this class ONLY through a +// constructor-injected field — no `@SolidEnvironment` field and no +// same-file `.environment()` / `Provider()` call site anywhere in this +// fixture — so the cross-file registry seeding in +// `builder.dart::_populateCrossFileTypes` must pick up `AuthRepository` +// from the consumer's constructor parameter / instance field declared type +// to recognize it as Solid-lowered, exactly as in +// `cross_file_constructor_injected_holder`. + +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer/session_reader.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer/session_reader.dart new file mode 100644 index 0000000..4a9114a --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer/session_reader.dart @@ -0,0 +1,33 @@ +// PURE CONSUMER shape reported in issue #106: unlike EVERY fixture in the +// #104/#105 suite (`cross_file_constructor_injected_holder` and siblings), +// `SessionReader` carries NO `@SolidState`/`@SolidEffect`/`@SolidQuery`/ +// `@SolidEnvironment` annotation of its own, and no `Provider(...)` / +// `.environment()` call site — it just receives `AuthRepository` through +// its constructor and stores it as a plain field. Before the fix, the +// builder's `hasSolidAnnotation`/`hasProviderHint` fast bailout matched +// neither hint text in this file, so it never even PARSED this file — the +// #104/#105 registry seeding never ran, and `_authRepository.session` reads +// stayed silently un-lowered (no `.value`), the exact shape that let +// `dart fix` collapse a null-guard into dead code in a real router guard. +// +// Covers a null-check guard (`hasSession`), a `!`-chain access +// (`sessionLength`), and a bare assignment (`clearSession`). +// ignore_for_file: unnecessary_this + +import 'auth_repository.dart'; + +class SessionReader { + SessionReader(this._authRepository); + + final AuthRepository _authRepository; + + bool hasSession() { + return _authRepository.session != null; + } + + int? sessionLength() => _authRepository.session!.length; + + void clearSession() { + _authRepository.session = null; + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_no_state/plain_settings.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_no_state/plain_settings.dart new file mode 100644 index 0000000..3f27077 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_no_state/plain_settings.dart @@ -0,0 +1,6 @@ +// Negative-fixture companion for `settings_reader.dart`: `PlainSettings` +// carries no `@Solid*` annotations at all — a genuinely solid-free type. + +class PlainSettings { + int volume = 50; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_no_state/settings_reader.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_no_state/settings_reader.dart new file mode 100644 index 0000000..cdb6898 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_no_state/settings_reader.dart @@ -0,0 +1,21 @@ +// Negative fixture for issue #106: `SettingsReader` has the exact PURE +// CONSUMER shape (constructor-injected field, zero `@Solid*` annotations, +// zero provider call sites of its own) as `cross_file_pure_consumer/ +// session_reader.dart`, but `PlainSettings` carries no `@SolidState` +// members. The new cross-file probe seeds `PlainSettings` into +// `wantedTypes` (the seeding is annotation-blind by design — it only +// proposes the receiver's type name as a candidate) and walks the import, +// 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 — proving the fast path survives for +// genuinely solid-free files. + +import 'plain_settings.dart'; + +class SettingsReader { + SettingsReader(this._settings); + + final PlainSettings _settings; + + int readVolume() => _settings.volume; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_router_guard/auth_guard.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_router_guard/auth_guard.dart new file mode 100644 index 0000000..1a046ff --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_router_guard/auth_guard.dart @@ -0,0 +1,27 @@ +// Router-guard shape from issue #106's real-world reproduction: an +// annotation-free `AuthGuard` reads a cross-file `@SolidState` session +// field through an `if (... == null) { … } else { … }` branch. Before the +// fix, `_authRepository.session` stayed un-lowered (a bare `String?` +// field read, not a `Signal` read), so `dart fix`'s +// `dead_code`/`unnecessary_null_comparison` passes would see the +// un-lowered comparison as trivially always-true or always-false and +// collapse one branch away — reproduced in a real app as a generated +// authentication bypass. `SessionReader` carries no `@SolidState` field of +// its own — the ONLY thing that could seed the cross-file registry for +// this file is the #106 pure-consumer probe. + +import 'auth_repository.dart'; + +class AuthGuard { + AuthGuard(this._authRepository); + + final AuthRepository _authRepository; + + String onNavigation() { + if (_authRepository.session == null) { + return '/login'; + } else { + return '/home'; + } + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_router_guard/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_router_guard/auth_repository.dart new file mode 100644 index 0000000..c2e6787 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_router_guard/auth_repository.dart @@ -0,0 +1,10 @@ +// Cross-file `@SolidState` host for the router-guard variant of the PURE +// CONSUMER regression (issue #106) — see `cross_file_pure_consumer/ +// auth_repository.dart` for the base shape. + +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/auth_repository.dart new file mode 100644 index 0000000..2ef0982 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/auth_repository.dart @@ -0,0 +1,14 @@ +// Cross-file `@SolidState` host for the pure-consumer WIDGET regression +// (GAP 1 of the post-#106 residual gap survey). Every consumer in this +// fixture reaches this class through plain constructor injection — no +// `@SolidEnvironment` field, no `.environment()`/`Provider()` call site — +// so the cross-file registry seeding already fixed for #104/#105 picks up +// `AuthRepository`; what's new here is that some of the consumers are +// WIDGETS whose `build()` needs a `SignalBuilder` wrap, not just a `.value` +// lowering. +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/mixed_consumer.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/mixed_consumer.dart new file mode 100644 index 0000000..d0d194a --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/mixed_consumer.dart @@ -0,0 +1,29 @@ +// GAP 1 of the post-#106 residual gap survey — a MIXED file: a plain-class +// pure consumer (`SessionLogger`, the #106 shape already fixed) and a +// widget pure consumer (`WelcomeBanner`, the new shape this gap closes) in +// the SAME file. Proves the two lowering passes +// (`lowerPureConsumerWidgetReads` and `lowerPureConsumerCrossFileReads`) +// compose correctly against one shared `classRegistry` and a single +// `flutter_solidart` import gets added exactly once. +import 'package:flutter/widgets.dart'; + +import 'auth_repository.dart'; + +class SessionLogger { + SessionLogger(this._authRepository); + + final AuthRepository _authRepository; + + bool hasSession() => _authRepository.session != null; +} + +class WelcomeBanner extends StatelessWidget { + const WelcomeBanner(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + Widget build(BuildContext context) { + return Text(authRepository.session ?? 'anonymous'); + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/profile_screen.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/profile_screen.dart new file mode 100644 index 0000000..4a9dfc3 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/profile_screen.dart @@ -0,0 +1,28 @@ +// GAP 1 of the post-#106 residual gap survey: a `StatelessWidget` pure +// consumer — constructor-injected `final AuthRepository authRepository;`, +// no `@Solid*` annotation of its own — reads `authRepository.session` +// inside `build()`. Before the fix, `cross_file_consumer_rewriter.dart` +// only visited `ClassKind.plainClass` declarations; a widget class needed +// BOTH the `.value` lowering AND a `SignalBuilder` wrap around the tracked +// subtree to actually rebuild when `session` changes, so it was skipped +// verbatim rather than emitted half-reactive. +// +// The wrap reuses the existing `rewriteBuildMethod` machinery verbatim — no +// widget-class Stateless→Stateful lift is needed here: unlike +// `@SolidEnvironment` (which needs `context.read()` and therefore a +// `State`), a constructor-injected field needs no `BuildContext` at all, +// so the class stays a `StatelessWidget`. +import 'package:flutter/widgets.dart'; + +import 'auth_repository.dart'; + +class ProfileScreen extends StatelessWidget { + const ProfileScreen(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + Widget build(BuildContext context) { + return Text(authRepository.session ?? 'anonymous'); + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/search_page.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/search_page.dart new file mode 100644 index 0000000..6ac2bf8 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget/search_page.dart @@ -0,0 +1,49 @@ +// GAP 1 of the post-#106 residual gap survey — the `State` shape. The +// `State` subclass copies the constructor-injected `AuthRepository` off its +// `widget` in `initState()` into its OWN field and carries no `@Solid*` +// annotation of its own, so `_SearchPageState.build()` needs the same +// `.value` + `SignalBuilder` treatment as the `StatelessWidget` shape in +// `profile_screen.dart`. +// +// This deliberately reads its OWN field (`authRepository.session`) rather +// than the usual Flutter `widget.` indirection directly inside +// `build()`. The `widget..` shape is a THREE-level +// chain (`PropertyAccess` whose target is a `PrefixedIdentifier`, not a bare +// identifier or a `this.` `PropertyAccess`) that only +// `value_rewriter.dart`'s tier-1 `Expression.staticType` resolution can +// recognize — this test sandbox has no `flutter` SDK dependency, so +// `package:flutter/widgets.dart` never actually resolves and every file +// importing it falls back to the unresolved parsed AST (see +// `builder.dart::_resolveUnit`'s doc comment). Proving the `widget.` +// shape end-to-end needs either a real resolver or a new AST tier for a +// `PrefixedIdentifier`-target `PropertyAccess` chain — out of scope for +// this fixture; this file instead pins the shape that already resolves via +// the existing tier-3 AST fallback regardless of resolution, which is +// exactly parallel to `cross_file_pure_consumer`'s plain-class coverage. +import 'package:flutter/widgets.dart'; + +import 'auth_repository.dart'; + +class SearchPage extends StatefulWidget { + const SearchPage(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + State createState() => _SearchPageState(); +} + +class _SearchPageState extends State { + late final AuthRepository authRepository; + + @override + void initState() { + super.initState(); + authRepository = widget.authRepository; + } + + @override + Widget build(BuildContext context) { + return Text(authRepository.session ?? 'anonymous'); + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_and_static/app.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_and_static/app.dart new file mode 100644 index 0000000..39a2b0d --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_and_static/app.dart @@ -0,0 +1,50 @@ +// Pass-ordering regression: a WIDGET pure consumer (`WelcomeBanner`, GAP 1 +// of the post-#106 residual gap survey) and a static-holder-mediated PLAIN +// pure consumer (`RouterGuard`, GAP 2 of the same survey) in the SAME FILE. +// +// Before the fix, `builder.dart`'s no-annotation branch ran +// `lowerPureConsumerWidgetReads` first against the pristine source, then fed +// its OUTPUT into `lowerPureConsumerCrossFileReads` — but only passed that +// second call the still-resolved `unit` when the first call had made NO +// edit at all (`unit: identical(current, source) ? unit : null`). Because +// `WelcomeBanner.build()` DOES get edited (it needs both `.value` lowering +// and a `SignalBuilder` wrap), the plain-class pass was forced onto a fresh, +// UNRESOLVED re-parse of the widget-edited text. +// +// `RouterGuard.resolve()`'s `Holder.instance.session` read is a TIER-1-ONLY +// shape (see `cross_file_static_holder_read` for the isolated case): only +// `Expression.staticType` on the `PrefixedIdentifier` target +// (`Holder.instance`) resolves the receiver to `AuthRepository`, and that +// tier is unavailable on an unresolved unit. So whenever this file ALSO +// contained a widget pure consumer, the guard's read silently stayed +// un-lowered — reproducing the original #106 authentication-bypass shape +// (`dart fix`'s `unnecessary_null_comparison`/`dead_code` passes can +// collapse the null check once the `Signal` object looks always-non-null). +import 'package:flutter/widgets.dart'; + +import 'auth_repository.dart'; + +class Holder { + static final AuthRepository instance = AuthRepository(); +} + +class RouterGuard { + String resolve() { + if (Holder.instance.session == null) { + return '/login'; + } else { + return '/home'; + } + } +} + +class WelcomeBanner extends StatelessWidget { + const WelcomeBanner(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + Widget build(BuildContext context) { + return Text(authRepository.session ?? 'anonymous'); + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_and_static/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_and_static/auth_repository.dart new file mode 100644 index 0000000..01e0495 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_and_static/auth_repository.dart @@ -0,0 +1,8 @@ +// Cross-file `@SolidState` host for the pass-ordering regression below (see +// `app.dart` in this fixture directory for the actual reproduction). +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_show_restricted_import/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_show_restricted_import/auth_repository.dart new file mode 100644 index 0000000..1bdbff9 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_show_restricted_import/auth_repository.dart @@ -0,0 +1,8 @@ +// Cross-file `@SolidState` host for the combinator-blind import-repair +// regression below (see `profile_screen.dart` in this fixture directory). +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_show_restricted_import/profile_screen.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_show_restricted_import/profile_screen.dart new file mode 100644 index 0000000..ed385db --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_widget_show_restricted_import/profile_screen.dart @@ -0,0 +1,30 @@ +// Combinator-blind import-repair regression: this file already imports +// `flutter_solidart` with a `show Signal` combinator (for an unrelated, +// hand-rolled signal) BEFORE it ever gains a cross-file pure-consumer +// WIDGET read. Before the fix, `builder.dart`'s no-annotation branch +// decided whether to repair the import by checking +// `!current.contains(flutterSolidartUri)` — which is FALSE here (the +// existing `show Signal` import's directive text already contains that +// URI), so the repair step never ran at all. The lowered `build()` method +// below emits a bare `SignalBuilder(...)` call that the `show Signal` +// import does NOT expose — non-compiling generated code. +import 'package:flutter/widgets.dart'; +import 'package:flutter_solidart/flutter_solidart.dart' show Signal; + +import 'auth_repository.dart'; + +/// An unrelated, hand-rolled signal — not `@SolidState`-generated — that +/// explains why this file already imports `flutter_solidart` (restricted to +/// `Signal`) before ever gaining a cross-file pure-consumer widget read. +final refreshTick = Signal(0); + +class ProfileScreen extends StatelessWidget { + const ProfileScreen(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + Widget build(BuildContext context) { + return Text(authRepository.session ?? 'anonymous'); + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_with_provider/app.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_with_provider/app.dart new file mode 100644 index 0000000..924f2c2 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_with_provider/app.dart @@ -0,0 +1,52 @@ +// Combo regression: this ONE file both (a) creates a cross-file +// `@SolidState`-bearing `AuthRepository` at a `Provider(...)` +// call site with `dispose:` omitted — so `addProviderDisposeAtCallSites` +// EDITS the text — and (b) reads that same cross-file class's reactive field +// through a for-in LOOP-VARIABLE null-guard on an annotation-free plain +// class (`RepoGuard`) — the #106 pure-consumer shape. +// +// Before the fix, the builder ran the dispose pass FIRST. Because that pass +// edited the text, `lowerPureConsumerCrossFileReads` received `unit: null` +// (offsets shifted out from under the resolved unit) and fell back to a +// freshly re-parsed, UNRESOLVED `CompilationUnit`. `r` is a for-in loop +// variable, not a parameter or field — the ONLY tier of +// `value_rewriter.dart`'s receiver resolution that can type it is tier 1 +// (`Expression.staticType`), which requires a resolved unit. So `r.session` +// stayed un-lowered — a bare `String?` field read — and `dart fix`'s +// `dead_code`/`unnecessary_null_comparison` passes could collapse the null +// guard: the #106 auth-bypass class, reintroduced through this second path. +import 'package:flutter/widgets.dart'; +import 'package:provider/provider.dart'; + +import 'auth_repository.dart'; + +class AuthGate extends StatelessWidget { + const AuthGate({super.key, required this.child}); + + final Widget child; + + @override + Widget build(BuildContext context) { + return Provider( + create: (_) => AuthRepository(), + child: child, + ); + } +} + +class RepoGuard { + RepoGuard(this.repos); + + final List repos; + + String check() { + for (final r in repos) { + if (r.session == null) { + return 'missing'; + } else { + return 'present'; + } + } + return 'empty'; + } +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_with_provider/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_with_provider/auth_repository.dart new file mode 100644 index 0000000..66a7623 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_pure_consumer_with_provider/auth_repository.dart @@ -0,0 +1,10 @@ +// Cross-file `@SolidState` host for the combo regression below — see +// `app.dart` in this fixture directory for the shape that reproduces the +// bug. + +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_static_holder_read/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_static_holder_read/auth_repository.dart new file mode 100644 index 0000000..a0130c8 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_static_holder_read/auth_repository.dart @@ -0,0 +1,14 @@ +// Cross-file `@SolidState` host for the STATIC-FIELD-MEDIATED DI regression +// (GAP 2 of the post-#106 residual gap survey). `holder_reader.dart` reaches +// this class ONLY through a `static final AuthRepository instance = …;` +// singleton-holder field — no `@SolidEnvironment` field, no plain instance +// field/constructor parameter, no `.environment()`/`Provider()` call site — +// so the cross-file registry seeding in +// `builder.dart::_populateCrossFileTypes` must stop skipping `static` +// fields to recognize `AuthRepository` as Solid-lowered. +import 'package:solid_annotations/solid_annotations.dart'; + +class AuthRepository { + @SolidState() + String? session; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_static_holder_read/holder_reader.dart b/packages/solid_generator/test/golden/inputs/cross_file_static_holder_read/holder_reader.dart new file mode 100644 index 0000000..ae87783 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_static_holder_read/holder_reader.dart @@ -0,0 +1,30 @@ +// Static-field-mediated DI / singleton-holder shape (GAP 2 of the post-#106 +// residual gap survey): `Holder.instance` is the receiver, not a +// constructor-injected field. Before the fix, the DI-seeding loop in +// `builder.dart::_populateCrossFileTypes` had `if (member.isStatic) +// continue;` with no stated rationale (see git history for #104/#105), so +// `AuthRepository` never entered `wantedTypes` from `Holder.instance`'s +// declared type and `Holder.instance.session` reads stayed silently +// un-lowered. +// +// Receiver resolution needed NO new AST tier: `Holder.instance.session` +// parses as `PropertyAccess(target: PrefixedIdentifier(Holder, instance), +// propertyName: session)`, and the existing tier-1 +// `Expression.staticType` resolution on the `PrefixedIdentifier` target +// already resolves to `AuthRepository` under a real resolved unit (verified +// empirically against `package:analyzer` directly) — once `AuthRepository` +// is seeded into the registry, `_maybeRewriteCrossClassPropertyAccess` +// already fires. +import 'auth_repository.dart'; + +class Holder { + static final AuthRepository instance = AuthRepository(); +} + +class SessionReader { + bool hasSession() { + return Holder.instance.session != null; + } + + int? sessionLength() => Holder.instance.session!.length; +} diff --git a/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_with_annotation/auth_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_with_annotation/auth_repository.dart new file mode 100644 index 0000000..ac74545 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_with_annotation/auth_repository.dart @@ -0,0 +1,9 @@ +// Cross-file `@SolidState` host for the bare-`super.` regression (GAP 3 of +// the post-#106 residual gap survey). See `customers_repository.dart` for +// the shape under test. +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_with_annotation/base_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_with_annotation/base_repository.dart new file mode 100644 index 0000000..af2c0e1 --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_with_annotation/base_repository.dart @@ -0,0 +1,12 @@ +// Plain, unannotated base class that owns the `authRepository` field. +// `customers_repository.dart` never redeclares this field — it only +// forwards to it through a BARE super-initializer parameter (no type +// written) — so any read of `authRepository` in the subclass is an +// INHERITED-member read. See GAP 3 of the post-#106 residual gap survey. +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_with_annotation/customers_repository.dart b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_with_annotation/customers_repository.dart new file mode 100644 index 0000000..2f1fa4c --- /dev/null +++ b/packages/solid_generator/test/golden/inputs/cross_file_super_param_bare_with_annotation/customers_repository.dart @@ -0,0 +1,48 @@ +// GAP 3 of the post-#106 residual gap survey: a BARE `super.` formal +// parameter (`super.authRepository`, no type written) forwards +// `AuthRepository` to `BaseRepository`'s constructor. Unlike +// `cross_file_super_param_type_only` (the already-fixed EXPLICIT-typed +// shape), the type is not present in the source at the parameter position +// at all — the syntactic AST walk in `_populateCrossFileTypes` genuinely +// cannot recover it from text alone. +// +// This class carries its OWN `@SolidState` field (`loadCount`), so the file +// enters the builder's MAIN lowering path with a fully RESOLVED +// `CompilationUnit` (`_resolveUnit`) rather than the no-annotation +// fast-path's unresolved syntactic probe. On that resolved path, +// `SuperFormalParameter.declaredFragment.element.type` IS populated even +// though the AST's own `.type` (explicit annotation) is null — verified +// empirically against `package:analyzer` directly — so the seeding loop +// falls back to the resolved element's type for this one shape. +// +// A PURE consumer whose ONLY link to a cross-file `@SolidState` class is a +// bare `super.x` (no annotation, no provider hint anywhere in the file) +// takes the no-annotation fast path instead, which probes an UNRESOLVED +// unit first — `declaredFragment`/`declaredElement` are not populated +// there, so this resolved-only enhancement cannot help that narrower +// shape; such a file still short-circuits to a verbatim copy before any +// resolved unit is ever requested. That remains a documented, accepted gap +// (see CHANGELOG) — this fixture instead proves the case the fix DOES +// reach: a file already entering the pipeline for another reason. +import 'package:solid_annotations/solid_annotations.dart'; + +// The bare `super.authRepository` shorthand never spells `AuthRepository` +// out in this file's text, so Dart itself has no use for this import — but +// the generator's cross-file registry walk needs an import directive +// pointing at wherever `AuthRepository` is declared to find it and pull in +// its `@SolidState` members. +// ignore: unused_import +import 'auth_repository.dart'; +import 'base_repository.dart'; + +class CustomersRepository extends BaseRepository { + CustomersRepository(super.authRepository); + + @SolidState() + int loadCount = 0; + + bool hasSession() { + loadCount = loadCount + 1; + return authRepository.session != null; + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_class_field_type_from_ctor_param.g.dart b/packages/solid_generator/test/golden/outputs/cross_class_field_type_from_ctor_param.g.dart new file mode 100644 index 0000000..f806b3a --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_class_field_type_from_ctor_param.g.dart @@ -0,0 +1,47 @@ +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(); + } +} + +class SessionReader implements Disposable { + SessionReader(AuthRepository this._service); + + final _service; + + final probeCount = Signal(0, name: 'probeCount'); + + bool hasSession() { + probeCount.value = probeCount.value + 1; + return _service.session.value != null; + } + + @override + void dispose() { + probeCount.dispose(); + } +} + +class SessionReaderViaInitializer implements Disposable { + SessionReaderViaInitializer(AuthRepository service) : _service = service; + + final _service; + + final probeCount = Signal(0, name: 'probeCount'); + + bool hasSession() { + probeCount.value = probeCount.value + 1; + return _service.session.value != null; + } + + @override + void dispose() { + probeCount.dispose(); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_collection_generic_arg_seeding/manager.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_collection_generic_arg_seeding/manager.g.dart index 5bd2bba..d4f9da4 100644 --- a/packages/solid_generator/test/golden/outputs/cross_file_collection_generic_arg_seeding/manager.g.dart +++ b/packages/solid_generator/test/golden/outputs/cross_file_collection_generic_arg_seeding/manager.g.dart @@ -17,6 +17,8 @@ class Manager implements Disposable { return false; } + bool firstHasSession() => repos.first.session.value != null; + @override void dispose() { loadCount.dispose(); diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_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_pure_consumer/session_reader.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer/session_reader.g.dart new file mode 100644 index 0000000..ba1f4cb --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer/session_reader.g.dart @@ -0,0 +1,33 @@ +// PURE CONSUMER shape reported in issue #106: unlike EVERY fixture in the +// #104/#105 suite (`cross_file_constructor_injected_holder` and siblings), +// `SessionReader` carries NO `@SolidState`/`@SolidEffect`/`@SolidQuery`/ +// `@SolidEnvironment` annotation of its own, and no `Provider(...)` / +// `.environment()` call site — it just receives `AuthRepository` through +// its constructor and stores it as a plain field. Before the fix, the +// builder's `hasSolidAnnotation`/`hasProviderHint` fast bailout matched +// neither hint text in this file, so it never even PARSED this file — the +// #104/#105 registry seeding never ran, and `_authRepository.session` reads +// stayed silently un-lowered (no `.value`), the exact shape that let +// `dart fix` collapse a null-guard into dead code in a real router guard. +// +// Covers a null-check guard (`hasSession`), a `!`-chain access +// (`sessionLength`), and a bare assignment (`clearSession`). +// ignore_for_file: unnecessary_this + +import 'auth_repository.dart'; + +class SessionReader { + SessionReader(this._authRepository); + + final AuthRepository _authRepository; + + bool hasSession() { + return _authRepository.session.value != null; + } + + int? sessionLength() => _authRepository.session.value!.length; + + void clearSession() { + _authRepository.session.value = null; + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_no_state/plain_settings.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_no_state/plain_settings.g.dart new file mode 100644 index 0000000..3f27077 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_no_state/plain_settings.g.dart @@ -0,0 +1,6 @@ +// Negative-fixture companion for `settings_reader.dart`: `PlainSettings` +// carries no `@Solid*` annotations at all — a genuinely solid-free type. + +class PlainSettings { + int volume = 50; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_no_state/settings_reader.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_no_state/settings_reader.g.dart new file mode 100644 index 0000000..cdb6898 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_no_state/settings_reader.g.dart @@ -0,0 +1,21 @@ +// Negative fixture for issue #106: `SettingsReader` has the exact PURE +// CONSUMER shape (constructor-injected field, zero `@Solid*` annotations, +// zero provider call sites of its own) as `cross_file_pure_consumer/ +// session_reader.dart`, but `PlainSettings` carries no `@SolidState` +// members. The new cross-file probe seeds `PlainSettings` into +// `wantedTypes` (the seeding is annotation-blind by design — it only +// proposes the receiver's type name as a candidate) and walks the import, +// 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 — proving the fast path survives for +// genuinely solid-free files. + +import 'plain_settings.dart'; + +class SettingsReader { + SettingsReader(this._settings); + + final PlainSettings _settings; + + int readVolume() => _settings.volume; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_router_guard/auth_guard.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_router_guard/auth_guard.g.dart new file mode 100644 index 0000000..1eb18e3 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_router_guard/auth_guard.g.dart @@ -0,0 +1,27 @@ +// Router-guard shape from issue #106's real-world reproduction: an +// annotation-free `AuthGuard` reads a cross-file `@SolidState` session +// field through an `if (... == null) { … } else { … }` branch. Before the +// fix, `_authRepository.session` stayed un-lowered (a bare `String?` +// field read, not a `Signal` read), so `dart fix`'s +// `dead_code`/`unnecessary_null_comparison` passes would see the +// un-lowered comparison as trivially always-true or always-false and +// collapse one branch away — reproduced in a real app as a generated +// authentication bypass. `SessionReader` carries no `@SolidState` field of +// its own — the ONLY thing that could seed the cross-file registry for +// this file is the #106 pure-consumer probe. + +import 'auth_repository.dart'; + +class AuthGuard { + AuthGuard(this._authRepository); + + final AuthRepository _authRepository; + + String onNavigation() { + if (_authRepository.session.value == null) { + return '/login'; + } else { + return '/home'; + } + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_router_guard/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_router_guard/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_router_guard/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_pure_consumer_widget/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/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_pure_consumer_widget/mixed_consumer.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/mixed_consumer.g.dart new file mode 100644 index 0000000..52b1f3f --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/mixed_consumer.g.dart @@ -0,0 +1,33 @@ +// GAP 1 of the post-#106 residual gap survey — a MIXED file: a plain-class +// pure consumer (`SessionLogger`, the #106 shape already fixed) and a +// widget pure consumer (`WelcomeBanner`, the new shape this gap closes) in +// the SAME file. Proves the two lowering passes +// (`lowerPureConsumerWidgetReads` and `lowerPureConsumerCrossFileReads`) +// compose correctly against one shared `classRegistry` and a single +// `flutter_solidart` import gets added exactly once. +import 'package:flutter/widgets.dart'; +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'auth_repository.dart'; + +class SessionLogger { + SessionLogger(this._authRepository); + + final AuthRepository _authRepository; + + bool hasSession() => _authRepository.session.value != null; +} + +class WelcomeBanner extends StatelessWidget { + const WelcomeBanner(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + Widget build(BuildContext context) { + return SignalBuilder( + builder: (context, child) { + return Text(authRepository.session.value ?? 'anonymous'); + }, + ); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/profile_screen.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/profile_screen.g.dart new file mode 100644 index 0000000..5232d18 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/profile_screen.g.dart @@ -0,0 +1,32 @@ +// GAP 1 of the post-#106 residual gap survey: a `StatelessWidget` pure +// consumer — constructor-injected `final AuthRepository authRepository;`, +// no `@Solid*` annotation of its own — reads `authRepository.session` +// inside `build()`. Before the fix, `cross_file_consumer_rewriter.dart` +// only visited `ClassKind.plainClass` declarations; a widget class needed +// BOTH the `.value` lowering AND a `SignalBuilder` wrap around the tracked +// subtree to actually rebuild when `session` changes, so it was skipped +// verbatim rather than emitted half-reactive. +// +// The wrap reuses the existing `rewriteBuildMethod` machinery verbatim — no +// widget-class Stateless→Stateful lift is needed here: unlike +// `@SolidEnvironment` (which needs `context.read()` and therefore a +// `State`), a constructor-injected field needs no `BuildContext` at all, +// so the class stays a `StatelessWidget`. +import 'package:flutter/widgets.dart'; +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'auth_repository.dart'; + +class ProfileScreen extends StatelessWidget { + const ProfileScreen(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + Widget build(BuildContext context) { + return SignalBuilder( + builder: (context, child) { + return Text(authRepository.session.value ?? 'anonymous'); + }, + ); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/search_page.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/search_page.g.dart new file mode 100644 index 0000000..3ab0f62 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget/search_page.g.dart @@ -0,0 +1,53 @@ +// GAP 1 of the post-#106 residual gap survey — the `State` shape. The +// `State` subclass copies the constructor-injected `AuthRepository` off its +// `widget` in `initState()` into its OWN field and carries no `@Solid*` +// annotation of its own, so `_SearchPageState.build()` needs the same +// `.value` + `SignalBuilder` treatment as the `StatelessWidget` shape in +// `profile_screen.dart`. +// +// This deliberately reads its OWN field (`authRepository.session`) rather +// than the usual Flutter `widget.` indirection directly inside +// `build()`. The `widget..` shape is a THREE-level +// chain (`PropertyAccess` whose target is a `PrefixedIdentifier`, not a bare +// identifier or a `this.` `PropertyAccess`) that only +// `value_rewriter.dart`'s tier-1 `Expression.staticType` resolution can +// recognize — this test sandbox has no `flutter` SDK dependency, so +// `package:flutter/widgets.dart` never actually resolves and every file +// importing it falls back to the unresolved parsed AST (see +// `builder.dart::_resolveUnit`'s doc comment). Proving the `widget.` +// shape end-to-end needs either a real resolver or a new AST tier for a +// `PrefixedIdentifier`-target `PropertyAccess` chain — out of scope for +// this fixture; this file instead pins the shape that already resolves via +// the existing tier-3 AST fallback regardless of resolution, which is +// exactly parallel to `cross_file_pure_consumer`'s plain-class coverage. +import 'package:flutter/widgets.dart'; +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'auth_repository.dart'; + +class SearchPage extends StatefulWidget { + const SearchPage(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + State createState() => _SearchPageState(); +} + +class _SearchPageState extends State { + late final AuthRepository authRepository; + + @override + void initState() { + super.initState(); + authRepository = widget.authRepository; + } + + @override + Widget build(BuildContext context) { + return SignalBuilder( + builder: (context, child) { + return Text(authRepository.session.value ?? 'anonymous'); + }, + ); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_and_static/app.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_and_static/app.g.dart new file mode 100644 index 0000000..915366f --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_and_static/app.g.dart @@ -0,0 +1,54 @@ +// Pass-ordering regression: a WIDGET pure consumer (`WelcomeBanner`, GAP 1 +// of the post-#106 residual gap survey) and a static-holder-mediated PLAIN +// pure consumer (`RouterGuard`, GAP 2 of the same survey) in the SAME FILE. +// +// Before the fix, `builder.dart`'s no-annotation branch ran +// `lowerPureConsumerWidgetReads` first against the pristine source, then fed +// its OUTPUT into `lowerPureConsumerCrossFileReads` — but only passed that +// second call the still-resolved `unit` when the first call had made NO +// edit at all (`unit: identical(current, source) ? unit : null`). Because +// `WelcomeBanner.build()` DOES get edited (it needs both `.value` lowering +// and a `SignalBuilder` wrap), the plain-class pass was forced onto a fresh, +// UNRESOLVED re-parse of the widget-edited text. +// +// `RouterGuard.resolve()`'s `Holder.instance.session` read is a TIER-1-ONLY +// shape (see `cross_file_static_holder_read` for the isolated case): only +// `Expression.staticType` on the `PrefixedIdentifier` target +// (`Holder.instance`) resolves the receiver to `AuthRepository`, and that +// tier is unavailable on an unresolved unit. So whenever this file ALSO +// contained a widget pure consumer, the guard's read silently stayed +// un-lowered — reproducing the original #106 authentication-bypass shape +// (`dart fix`'s `unnecessary_null_comparison`/`dead_code` passes can +// collapse the null check once the `Signal` object looks always-non-null). +import 'package:flutter/widgets.dart'; +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'auth_repository.dart'; + +class Holder { + static final AuthRepository instance = AuthRepository(); +} + +class RouterGuard { + String resolve() { + if (Holder.instance.session.value == null) { + return '/login'; + } else { + return '/home'; + } + } +} + +class WelcomeBanner extends StatelessWidget { + const WelcomeBanner(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + Widget build(BuildContext context) { + return SignalBuilder( + builder: (context, child) { + return Text(authRepository.session.value ?? 'anonymous'); + }, + ); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_and_static/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_and_static/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_and_static/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_pure_consumer_widget_show_restricted_import/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_show_restricted_import/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_show_restricted_import/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_pure_consumer_widget_show_restricted_import/profile_screen.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_show_restricted_import/profile_screen.g.dart new file mode 100644 index 0000000..4f6cb0a --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_widget_show_restricted_import/profile_screen.g.dart @@ -0,0 +1,35 @@ +// Combinator-blind import-repair regression: this file already imports +// `flutter_solidart` with a `show Signal` combinator (for an unrelated, +// hand-rolled signal) BEFORE it ever gains a cross-file pure-consumer +// WIDGET read. Before the fix, `builder.dart`'s no-annotation branch +// decided whether to repair the import by checking +// `!current.contains(flutterSolidartUri)` — which is FALSE here (the +// existing `show Signal` import's directive text already contains that +// URI), so the repair step never ran at all. The lowered `build()` method +// below emits a bare `SignalBuilder(...)` call that the `show Signal` +// import does NOT expose — non-compiling generated code. +import 'package:flutter/widgets.dart'; +import 'package:flutter_solidart/flutter_solidart.dart' + show Signal, SignalBuilder; + +import 'auth_repository.dart'; + +/// An unrelated, hand-rolled signal — not `@SolidState`-generated — that +/// explains why this file already imports `flutter_solidart` (restricted to +/// `Signal`) before ever gaining a cross-file pure-consumer widget read. +final refreshTick = Signal(0); + +class ProfileScreen extends StatelessWidget { + const ProfileScreen(this.authRepository, {super.key}); + + final AuthRepository authRepository; + + @override + Widget build(BuildContext context) { + return SignalBuilder( + builder: (context, child) { + return Text(authRepository.session.value ?? 'anonymous'); + }, + ); + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_with_provider/app.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_with_provider/app.g.dart new file mode 100644 index 0000000..8333755 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_with_provider/app.g.dart @@ -0,0 +1,53 @@ +// Combo regression: this ONE file both (a) creates a cross-file +// `@SolidState`-bearing `AuthRepository` at a `Provider(...)` +// call site with `dispose:` omitted — so `addProviderDisposeAtCallSites` +// EDITS the text — and (b) reads that same cross-file class's reactive field +// through a for-in LOOP-VARIABLE null-guard on an annotation-free plain +// class (`RepoGuard`) — the #106 pure-consumer shape. +// +// Before the fix, the builder ran the dispose pass FIRST. Because that pass +// edited the text, `lowerPureConsumerCrossFileReads` received `unit: null` +// (offsets shifted out from under the resolved unit) and fell back to a +// freshly re-parsed, UNRESOLVED `CompilationUnit`. `r` is a for-in loop +// variable, not a parameter or field — the ONLY tier of +// `value_rewriter.dart`'s receiver resolution that can type it is tier 1 +// (`Expression.staticType`), which requires a resolved unit. So `r.session` +// stayed un-lowered — a bare `String?` field read — and `dart fix`'s +// `dead_code`/`unnecessary_null_comparison` passes could collapse the null +// guard: the #106 auth-bypass class, reintroduced through this second path. +import 'package:flutter/widgets.dart'; +import 'package:provider/provider.dart'; + +import 'auth_repository.dart'; + +class AuthGate extends StatelessWidget { + const AuthGate({super.key, required this.child}); + + final Widget child; + + @override + Widget build(BuildContext context) { + return Provider( + create: (_) => AuthRepository(), + child: child, + dispose: (context, provider) => provider.dispose(), + ); + } +} + +class RepoGuard { + RepoGuard(this.repos); + + final List repos; + + String check() { + for (final r in repos) { + if (r.session.value == null) { + return 'missing'; + } else { + return 'present'; + } + } + return 'empty'; + } +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_with_provider/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_with_provider/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_pure_consumer_with_provider/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_static_holder_read/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_static_holder_read/auth_repository.g.dart new file mode 100644 index 0000000..b835061 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_static_holder_read/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_static_holder_read/holder_reader.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_static_holder_read/holder_reader.g.dart new file mode 100644 index 0000000..7ad4114 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_static_holder_read/holder_reader.g.dart @@ -0,0 +1,30 @@ +// Static-field-mediated DI / singleton-holder shape (GAP 2 of the post-#106 +// residual gap survey): `Holder.instance` is the receiver, not a +// constructor-injected field. Before the fix, the DI-seeding loop in +// `builder.dart::_populateCrossFileTypes` had `if (member.isStatic) +// continue;` with no stated rationale (see git history for #104/#105), so +// `AuthRepository` never entered `wantedTypes` from `Holder.instance`'s +// declared type and `Holder.instance.session` reads stayed silently +// un-lowered. +// +// Receiver resolution needed NO new AST tier: `Holder.instance.session` +// parses as `PropertyAccess(target: PrefixedIdentifier(Holder, instance), +// propertyName: session)`, and the existing tier-1 +// `Expression.staticType` resolution on the `PrefixedIdentifier` target +// already resolves to `AuthRepository` under a real resolved unit (verified +// empirically against `package:analyzer` directly) — once `AuthRepository` +// is seeded into the registry, `_maybeRewriteCrossClassPropertyAccess` +// already fires. +import 'auth_repository.dart'; + +class Holder { + static final AuthRepository instance = AuthRepository(); +} + +class SessionReader { + bool hasSession() { + return Holder.instance.session.value != null; + } + + int? sessionLength() => Holder.instance.session.value!.length; +} diff --git a/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_with_annotation/auth_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_with_annotation/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_with_annotation/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_with_annotation/base_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_with_annotation/base_repository.g.dart new file mode 100644 index 0000000..af2c0e1 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_with_annotation/base_repository.g.dart @@ -0,0 +1,12 @@ +// Plain, unannotated base class that owns the `authRepository` field. +// `customers_repository.dart` never redeclares this field — it only +// forwards to it through a BARE super-initializer parameter (no type +// written) — so any read of `authRepository` in the subclass is an +// INHERITED-member read. See GAP 3 of the post-#106 residual gap survey. +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_with_annotation/customers_repository.g.dart b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_with_annotation/customers_repository.g.dart new file mode 100644 index 0000000..5456d01 --- /dev/null +++ b/packages/solid_generator/test/golden/outputs/cross_file_super_param_bare_with_annotation/customers_repository.g.dart @@ -0,0 +1,20 @@ +import 'package:flutter_solidart/flutter_solidart.dart'; +import 'package:solid_annotations/solid_annotations.dart'; +import 'auth_repository.dart'; +import 'base_repository.dart'; + +class CustomersRepository extends BaseRepository implements Disposable { + CustomersRepository(super.authRepository); + + final loadCount = Signal(0, name: 'loadCount'); + + bool hasSession() { + loadCount.value = loadCount.value + 1; + return authRepository.session.value != null; + } + + @override + void dispose() { + loadCount.dispose(); + } +} diff --git a/packages/solid_generator/test/integration/golden_helpers.dart b/packages/solid_generator/test/integration/golden_helpers.dart index 6194f5f..8d45c6e 100644 --- a/packages/solid_generator/test/integration/golden_helpers.dart +++ b/packages/solid_generator/test/integration/golden_helpers.dart @@ -145,6 +145,16 @@ const List goldenNames = [ 'cross_file_super_param_type_only', 'cross_file_collection_generic_arg_seeding', 'cross_file_computed_from_injected_state', + 'cross_file_pure_consumer', + 'cross_file_pure_consumer_no_state', + 'cross_file_pure_consumer_router_guard', + 'cross_file_pure_consumer_with_provider', + 'cross_class_field_type_from_ctor_param', + 'cross_file_static_holder_read', + 'cross_file_super_param_bare_with_annotation', + 'cross_file_pure_consumer_widget', + 'cross_file_pure_consumer_widget_and_static', + 'cross_file_pure_consumer_widget_show_restricted_import', ]; /// Memoized golden directory resolution. Resolved relative to the package