ci: #663 follow-ups — discarded_futures hygiene + high-pattern guard#689
Open
joshuakrueger-dfx wants to merge 1 commit into
Open
ci: #663 follow-ups — discarded_futures hygiene + high-pattern guard#689joshuakrueger-dfx wants to merge 1 commit into
joshuakrueger-dfx wants to merge 1 commit into
Conversation
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.
Why
Follow-ups to #663, which hardened the static-analysis gate after the Big Brother audit (#657). #663 deliberately deferred three items as separate PRs — this is two of them (the third, activating the coverage-floor gate, turned out to be already shipped:
.coverage-floor-linesis at 100 and theCoverage Floor Gatejob is a required check ondevelop, so there was nothing to add).Both changes here are behaviour-preserving.
What changed
1 —
discarded_futureslint (advisory)discarded_futuresinanalysis_options.yaml.lib/by making the fire-and-forget explicit withunawaited(...)(a handful ofBlocProvider(create: ...)cascades are rewritten to a block body withunawaited(...)instead of a..method()cascade). No behaviour change — every site is in a non-asynccontext (build methods, cubit constructors, sync callbacks) where awaiting is impossible anyway, sounawaitedonly documents intent. Genuinely awaitable futures insideasyncfunctions are already covered by the existingunawaited_futureslint.test/via a newtest/analysis_options.yaml(153 hits, idiomatic fire-and-forget in test setup —unawaited_futuresstays active there).discarded_futuresis an INFO lint and the gate runsflutter analyze --fatal-warnings(not--fatal-infos), so it does not block the build. It keepslib/clean and flags new occurrences in editors/PR review.2 — High-pattern guard (
tool/lints/pattern_guard.dart)A lightweight AST check for the three recurring HIGH defect shapes the audit named for
custom_lintrules.custom_lintitself cannot be added today: its current release pinsanalyzer: ^8, while this repo overridesanalyzer: ^10for the strict-inference gate, so they cannot co-resolve. This guard reaches the same goal with theanalyzerpackage already in the tree — purely syntactic (parseFile), fast, version-tolerant.Rules:
hardcoded_swiss_tax_residence—swissTaxResidence:passed a boolean literal instead of a value.fixed_index_address_substring—.substring(<int>, <int>)with two constant indices and an end index>= 6(assumes a fixed length → RangeError on a shorter string; this is the audit'sqr_address_widgetcrash shape).cross_flow_brokerbot_endpoint— a sell-flow file calling a buy-side brokerbot endpoint, or vice versa.Wiring:
Analyze & Testjob (dart run tool/lints/pattern_guard.dart), so it blocks the build on any new occurrence. Placed there rather than in its own job to avoid duplicating pub get + codegen; can be promoted to a separately-required check later, like the floor gate.test/tool/pattern_guard_test.dart(each rule fires on known-bad, stays silent on known-good, suppression honoured).// realunit-lint:ignore <rule> — <reason>so the guard passes on current code and only fails on NEW occurrences. Two of these are flagged for product review rather than silently accepted:qr_address_widget— the audit's RangeError site;subtitleis a full 42-char address at the only call site, but a guarded slice is the better shape (tracked).sell_converter_cubit.onCurrencyChangedcallsgetBuyPrice(notgetSellPrice); current behaviour is pinned bysell_converter_cubit_test.dart. Baselined and flagged for product review of whether a currency change in the sell flow should price via the sell endpoint. Not changed here to keep this PR behaviour-neutral.Verification
flutter analyze --fatal-warnings→ No issues founddart run tool/lints/pattern_guard.dart→ OKflutter test --exclude-tags golden) green, incl. the 10 new guard tests