Skip to content

ci: #663 follow-ups — discarded_futures hygiene + high-pattern guard#689

Open
joshuakrueger-dfx wants to merge 1 commit into
ci/harden-analysis-and-coveragefrom
ci/663-followups-stacked
Open

ci: #663 follow-ups — discarded_futures hygiene + high-pattern guard#689
joshuakrueger-dfx wants to merge 1 commit into
ci/harden-analysis-and-coveragefrom
ci/663-followups-stacked

Conversation

@joshuakrueger-dfx
Copy link
Copy Markdown
Collaborator

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-lines is at 100 and the Coverage Floor Gate job is a required check on develop, so there was nothing to add).

Both changes here are behaviour-preserving.

What changed

1 — discarded_futures lint (advisory)

  • Enables discarded_futures in analysis_options.yaml.
  • Resolves all 58 production hits under lib/ by making the fire-and-forget explicit with unawaited(...) (a handful of BlocProvider(create: ...) cascades are rewritten to a block body with unawaited(...) instead of a ..method() cascade). No behaviour change — every site is in a non-async context (build methods, cubit constructors, sync callbacks) where awaiting is impossible anyway, so unawaited only documents intent. Genuinely awaitable futures inside async functions are already covered by the existing unawaited_futures lint.
  • Scopes the lint out of test/ via a new test/analysis_options.yaml (153 hits, idiomatic fire-and-forget in test setup — unawaited_futures stays active there).
  • Note: this is advisory only at CI. discarded_futures is an INFO lint and the gate runs flutter analyze --fatal-warnings (not --fatal-infos), so it does not block the build. It keeps lib/ 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_lint rules. custom_lint itself cannot be added today: its current release pins analyzer: ^8, while this repo overrides analyzer: ^10 for the strict-inference gate, so they cannot co-resolve. This guard reaches the same goal with the analyzer package already in the tree — purely syntactic (parseFile), fast, version-tolerant.

Rules:

  • hardcoded_swiss_tax_residenceswissTaxResidence: 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's qr_address_widget crash shape).
  • cross_flow_brokerbot_endpoint — a sell-flow file calling a buy-side brokerbot endpoint, or vice versa.

Wiring:

  • Runs as a fatal step in the Analyze & Test job (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.
  • 10 unit tests in test/tool/pattern_guard_test.dart (each rule fires on known-bad, stays silent on known-good, suppression honoured).
  • The 7 existing hits are baselined with // 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; subtitle is a full 42-char address at the only call site, but a guarded slice is the better shape (tracked).
    • sell_converter_cubit.onCurrencyChanged calls getBuyPrice (not getSellPrice); current behaviour is pinned by sell_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 found
  • dart run tool/lints/pattern_guard.dart → OK
  • full suite (flutter test --exclude-tags golden) green, incl. the 10 new guard tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant