Sitelet https://github.com/flutter/flutter/pull/190891
Skip to content

[flutter_tools] refactor wasm dry-run result handling in Dart2WasmTarget - #190891

Merged
auto-submit[bot] merged 4 commits into
flutter:masterfrom
kevmoo:web-cognitive-complexity
Aug 14, 2026
Merged

auto-submit[bot] merged 4 commits into
flutter:masterfrom
kevmoo:web-cognitive-complexity

Conversation

@kevmoo

@kevmoo kevmoo commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Reduce cognitive complexity in Dart2WasmTarget._handleDryRunResult by decomposing the monolithic routine into focused helper methods:

  • Add _DryRunOutcome enum and _logAndClassifyDryRunResult to cleanly map exit codes and process streams to execution outcomes.
  • Extract _collectFindingsInfo and _parseWasmFindings for stdout error code and URI parsing.
  • Extract _categorizePackages to partition hosted vs. private package configurations from PackageConfig.
  • Extract _classifyUris and _truncateAnalyticsBuffer to handle analytics finding truncation and hint formatting.
  • Drops cognitive complexity score of _handleDryRunResult from 48 to 4.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect between commas.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

Reduce cognitive complexity in Dart2WasmTarget._handleDryRunResult
by decomposing the monolithic routine into focused helper methods:

* Add _DryRunOutcome enum and _logAndClassifyDryRunResult to cleanly
  map exit codes and process streams to execution outcomes.
* Extract _collectFindingsInfo and _parseWasmFindings for stdout error
  code and URI parsing.
* Extract _categorizePackages to partition hosted vs. private package
  configurations from PackageConfig.
* Extract _classifyUris and _truncateAnalyticsBuffer to handle
  analytics finding truncation and hint formatting.
* Drops cognitive complexity score of _handleDryRunResult from 48 to 4.
@github-actions github-actions Bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 11, 2026
…in web.dart

Specify explicit types for destructured record variables in
_formatFindingsBuffer to satisfy repo-wide specify_nonobvious_local_variable_types lint.
@kevmoo
kevmoo marked this pull request as ready for review August 11, 2026 02:25

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Dart2WasmTarget class in web.dart by extracting several helper methods to improve modularity and readability. The review feedback highlights two improvement opportunities: guarding URI path segment access in _classifyUris to prevent potential StateError exceptions, and optimizing _parseWasmFindings by instantiating the RegExp outside of the loop.

Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart
* Hoist _wasmErrorCodePattern to static field in Dart2WasmTarget and defer
  Uri.parse in _parseWasmFindings until errorCode is non-null.
* Guard uri.pathSegments access in _classifyUris with uri.scheme == 'package'
  && uri.pathSegments.isNotEmpty.
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart Outdated
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart Outdated
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart Outdated
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart Outdated
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart Outdated
Comment thread packages/flutter_tools/lib/src/build_system/targets/web.dart
- Document _DryRunOutcome values with classification rules and sample input
- Add doc comments to all new dry-run helper functions
- Add stdout:/stderr: headers when logging crash/failure output
- Make logger and _collectFindingsInfo params required named
- Use (false, false) => '' in exhaustive hint switch; drop nullable hpHint
- Use containsKey/contains consistently in _classifyUris
- Restore the shuffle rationale comment truncated in flutter#179826

@harryterkelsen harryterkelsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's beautiful 🥲

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

Labels

CICD Run CI/CD tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants