fix(solid_generator): lower cross-file @SolidState reads in pure-consumer files - #107
Merged
Merged
Conversation
…umer files Closes #106. Files with no @solid* annotation text and no provider hint took a verbatim-passthrough fast path and never entered the pipeline, so the #105 registry seeding never ran for them: a pure consumer's cross-file state reads stayed silently un-lowered, and dart fix could collapse the resulting always-non-null guards into dead code (reproduced as a generated authentication bypass in a real router guard). The bailout now probes the #105 syntactic seeding first and, when the cross-file registry is non-empty, runs a dedicated plain-class lowering pass (no Disposable/dispose synthesis) with the resolved unit BEFORE dispose call-site injection — ordering matters: injecting first would hand the lowering pass edited text and lose staticType receiver resolution (loop variables), reintroducing the bug. The probe's registries are threaded into the pipeline (no duplicate import walk); genuinely dependency-free files keep a zero-import-walk path. Documented residual gaps: widget-class pure consumers (SignalBuilder placement is a larger change) and static-field-mediated DI (seeding deliberately skips static fields). Four golden fixtures: pure consumer (reads/writes/!.-chain), router-guard if/else shape, provider-hint + loop-variable combo, and a solid-free negative proving the verbatim path survives.
Widget-class pure consumers now get the SignalBuilder-wrapped build() the @SolidEnvironment path already uses (no stateful lift — a constructor-injected field needs no context), with a combinator-aware import repair driven by the wrap's own emitted flag (a show/hide- restricted flutter_solidart import is widened, never substring-guessed). Static-field-mediated DI seeds the registry (the isStatic skip had no recorded rationale). Bare super.x params seed from the resolved element type on the main path; a pure consumer whose only link is a bare super.x still misses the syntactic probe gate — documented. A new value_rewriter tier resolves an untyped field's type from its field-formal or initializer-list constructor parameter, bailing on conflicting constructors. The .first collection receiver is pinned by fixture. Both pure-consumer lowering passes are now pure edit-collectors over the same pristine source and resolved unit, merged and applied in a single transformation — running either pass on the other's edited text starved it of staticType resolution and silently reintroduced the #106 guard collapse for tier-1-only reads co-located with a widget consumer (caught in review, RED-proven by fixture).
This was referenced Aug 26, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #106.
Problem
The verbatim-passthrough fast path (no
@Solid*annotation text, noProvider/.environmenthint) meant pure consumers — files that hold a@SolidState-bearing class via plain constructor injection but declare nothing solid themselves — never entered the pipeline at all. The #105 registry seeding therefore never ran for them: their cross-file state reads stayed silently un-lowered, anddart fix'sunnecessary_null_comparison/dead_codepasses collapsed the resulting always-non-null guards — reproduced as a generated authentication bypass in a real app's router guard (AuthGuard.onNavigationreduced to an unconditionalresolver.next()). Every #105 fixture masked the gap because each consumer carried a@SolidStatecontrol field.Fix
cross_file_consumer_rewriter.dart): plain-class-only.valuelowering for pure consumers — noimplements Disposable, no synthesizeddispose()(a naive pipeline entry would have wrongly stamped both).staticTypereceiver resolution (loop variables), and reintroduces the exact bug (caught by review, RED-proven).Review guide — fixtures (
test/golden/)cross_file_pure_consumer!-chain, write — zero own annotations (RED: verbatim passthrough)cross_file_pure_consumer_router_guardif/elseshape survivesdart fixpost-fixcross_file_pure_consumer_with_providerdart fixproposed collapsing the guard)cross_file_pure_consumer_no_stateKnown residual gaps (documented in CHANGELOG + SPEC §2)
build()reading a cross-file signal needs SignalBuilder placement — deliberately skipped whole (byte-identical passthrough), not partially rewritten.Holder.instance.session): seeding deliberately skips static fields.Verification
327/327 solid_generator tests (4 new fixtures + idempotency), 11/11 integration tests,
dart analyze --fatal-infosclean, format clean, all 5 example apps rebuild with zero fix-attributable delta. RED authenticity re-proven against the pre-#106 generator. Version cut:solid_generator 3.0.0-dev.4.Pre-existing, unrelated:
example/lib/main.dartis stale vsexample/source/main.dart(aconstdrift predating this branch) — left untouched, worth a separate cleanup.Update: residual gaps closed (second commit)
Per follow-up review, the residual gaps documented above are now fixed in the same PR (all part of the unreleased
3.0.0-dev.4):build()gets the same SignalBuilder wrap@SolidEnvironmentwidgets use (no stateful lift; const constructors preserved; a build with no tracked reads is correctly NOT wrapped). Imports are repaired combinator-aware: ashow Signal/hide SignalBuilderimport is widened based on the wrap's own emitted flag, never a URI substring guess.Holder.instance.session) — seeding no longer skips static fields (the skip had no recorded rationale); the receiver already resolves via the staticType tier.super.x— seeds from the resolved element type on the main path; the one remaining sliver (a pure consumer whose ONLY link is a baresuper.x, which misses the syntactic probe gate) is documented precisely.final _service;+AuthService this._service) — new value_rewriter resolution tier, bailing on conflicting constructors..firstreceiver — was already green; now pinned by fixture.Structural hardening from review (2 reproduced BLOCKERs): both pure-consumer lowering passes are now pure edit-collectors over the same pristine source + resolved unit, merged into a single application — any ordering where one pass consumed the other's edited text silently starved tier-1 resolution and reintroduced the guard-collapse bug for mixed files (fixture
cross_file_pure_consumer_widget_and_static).Suite: 339 tests (10 new fixtures across both commits + idempotency), 11/11 integration, analyze/format clean, examples byte-identical.