Skip to content

firehose breaking check crashes on Dart 3.13 shorthand factory constructors and corrupts PR Health Markdown table #460

Closed

Description

@kevmoo

When running the breaking health check (pkgs/firehose) on packages that use Dart 3.13 primary constructors and shorthand named factory constructors (e.g. factory fromSamples(...) / factory fromJson(...)), two issues occur (observed in kevmoo/bench_press#47 (comment)):

1. dart_apitool crashes on Dart 3.13 shorthand factory constructors

firehose pins bmw-tech/dart_apitool, which depends on analyzer: ^10.0.2. Under analyzer 10.2.0, shorthand factory declarations (factory fromSamples(...) without a class prefix ClassName.) are parsed as unnamed constructors (name: 'new'), causing PackageApiDiffer._diffIterables to throw ArgumentError: Multiple new items found with the same structure as the old item ExecutableDeclaration(..., name: new, ...).

2. firehose dumps raw multi-line stderr into the PR comment Markdown table

When dart_apitool exits without writing report.json:

  • runDashProcess already prints stderr (err) to the GitHub Actions step log.
  • Health.breakingChangeCheck (pkgs/firehose/lib/src/health/health.dart) also interpolates the entire multi-line $err (including full ExecutableDeclaration.toString() dumps and stack trace frames) directly into BreakingChange.explanation alongside neededVersion: Version(0, 0, 0).
  • BreakingChange.toMarkdownRow() emits **0.0.0** <br> $explanation directly inside a Markdown pipe-table cell without sanitizing newlines (\n) or pipes (|), shattering the Markdown table across dozens of lines in the PR comment.

Proposed Fix for firehose

  1. Keep the full stderr output in the workflow step logs (already printed by runDashProcess), and replace the PR comment table's error explanation with a concise single-line message pointing to the workflow logs (with neededVersion: null).
  2. Sanitize cells in BreakingChange.toMarkdownRow() (replacing newlines and escaping |) so multi-line explanations cannot corrupt the Markdown table structure.
  3. Update dart_apitool (or migrate to package:api_summary) once upstream supports analyzer >=12.0.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions