Skip to content

fix: format generated content in-process at write time#210

Merged
eseidel merged 5 commits into
mainfrom
es/format-in-process
Apr 30, 2026
Merged

fix: format generated content in-process at write time#210
eseidel merged 5 commits into
mainfrom
es/format-in-process

Conversation

@eseidel

@eseidel eseidel commented Apr 30, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up to #209. The emit-time long-line check from #209 fires against pre-format rendered content, which has long lines that dart format later reflows under 80. Result:

  1. Helper sees pre-format long lines → adds the 4-line directive block.
  2. dart format reflows the long lines under 80.
  3. dart fix --apply sees unnecessary_ignore on the now-pointless directive → strips line 4.
  4. Lines 1-3 (the justification comments) are unrelated // comments to dart fix — orphaned.

Confirmed 61 orphan-comment files in the shorebird codepush regen. Also surfaced a separate bug: the helper had a carve-out for /// doc lines, but the analyzer's lines_longer_than_80_chars lint actually does flag long doc comments (only URIs / imports / exports are exempt). github regen had 37 false-negative dartdoc lines.

Fix

Add package:dart_style as a dep and run DartFormatter in-process from _renderTemplate / _renderSpecTemplate before applying transforms. Files land on disk in canonical shape; the long-line decision matches what the analyzer will see.

Also drop the bogus /// carve-out from maybeAddLongLineIgnore.

The global formatAndFix step stays for now (dart fix --apply still does real work — removing unused imports, adding const/required, etc.), but its dart format calls are now near no-ops on files we wrote. Slated for removal once every emit path is on this contract.

Verification

  • github regen dart analyze: No issues found
  • github regen now has 728 files with the directive (down from ~3000 with the previous post-walk), all genuinely needed
  • 0 orphan comment blocks (vs 61 in shorebird codepush regen pre-fix)
  • 0 false-negative long doc comments (vs 37 pre-fix)
  • 530 unit tests pass (test for /// long lines flipped from "passes through" to "fires")

#209's emit-time long-line check fired against pre-format rendered
content, which had long lines that dart format later reflows under
80. Result: directive added → dart fix --apply strips it as
unnecessary_ignore → 3-line justification comment orphans behind.
Confirmed 61 such orphans on the shorebird codepush regen.

Add dart_style as a dep and run DartFormatter in-process from
_renderTemplate / _renderSpecTemplate before applying transforms.
Files land on disk in canonical shape; the directive decision
matches what the analyzer will actually see.

Also drop the bogus carve-out for /// dartdoc lines: the
lines_longer_than_80_chars lint excludes URIs (imports/exports) but
DOES flag long doc comments. github regen surfaced 37 false
negatives from this carve-out — all real long doc lines that
needed the directive.

The global formatAndFix step stays for now (dart fix still does
real work like removing unused imports), but its dart format calls
are now near no-ops on files we wrote — slated for removal once
every emit path is on this contract.

Verification: github regen dart analyze clean; 728 files now carry
the directive (down from a noisier ~3000 with the post-walk), 0
orphan comment blocks, 0 false negatives on long doc comments.
530 unit tests pass.
@codecov

codecov Bot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.69421% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.64%. Comparing base (c933581) to head (a688a51).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/src/render/formatting.dart 96.49% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #210      +/-   ##
==========================================
- Coverage   94.65%   94.64%   -0.01%     
==========================================
  Files          20       21       +1     
  Lines        4992     5062      +70     
==========================================
+ Hits         4725     4791      +66     
- Misses        267      271       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

eseidel added a commit to shorebirdtech/shorebird that referenced this pull request Apr 30, 2026
Pin space_gen to git ref c8937bd248495f5e32b6358dd3c5e42e62ee77d0
(eseidel/space_gen#210 tip), which fixes both regressions surfaced
by the previous regen attempt:

- eseidel/space_gen#209: scope long-line directive to files we emit
  + skip URI imports/exports when measuring (fixes the spurious
  directive on this package's hand-written
  shorebird_code_push_protocol.dart barrel and the unnecessary_ignore
  hits on three import-only files).
- eseidel/space_gen#210: format generated content in-process at write
  time via DartFormatter, so the directive decision matches what the
  analyzer sees (fixes the orphan justification comments left after
  dart fix --apply stripped unnecessary directives that were emitted
  against pre-format content).

The git pin is temporary — once the next pub.dev release ships,
switch to a versioned dep.

Regenerates lib/src/ from
/Users/eseidel/Documents/GitHub/_shorebird/packages/code_push_server/public/openapi.yaml
since api.shorebird.dev doesn't yet serve the latest spec.

Verification:
- dart analyze: No issues found
- dart test: all 216 tests pass
- 44 files changed (vs 74 in the previous regen — 30 orphan comments
  gone)
- Hand-written barrel untouched
The previous commit's in-process format pinned to
DartFormatter.latestLanguageVersion (currently 3.13.0), which forces
tall style and ignores the consuming package's own analysis_options
configuration. On consumers like shorebird_code_push_protocol — which
specifies 'formatter: trailing_commas: preserve' in its workspace
analysis_options.yaml — this strips trailing commas at write time,
and the global 'dart format' that runs after can't put them back
once removed.

Walk up from fileWriter.outDir collecting:
- pubspec.lock 'sdks.dart' lower bound (preferred — reflects the most
  recent 'pub get' resolution); pubspec.yaml 'environment.sdk' lower
  bound as a fallback before any 'pub get'.
- analysis_options.yaml 'formatter:' page_width and trailing_commas.
  Follows a single relative-path 'include:' so workspace setups (where
  the package's analysis_options.yaml only inherits from the workspace
  root's) resolve correctly. 'package:' includes are skipped — those
  would need to run pub.

Cache once per FileRenderer instance; the values are invariant for
the lifetime of a regen.

Verification:
- github regen dart analyze clean.
- 530 unit tests pass.
eseidel added a commit to shorebirdtech/shorebird that referenced this pull request Apr 30, 2026
Pin space_gen to git ref a3afdf7d63a1f820a4549aca69df7af58385fc58
(eseidel/space_gen#210 tip), which fixes the regressions surfaced
by the previous regen attempts:

- eseidel/space_gen#209: scope long-line directive to files we emit
  + skip URI imports/exports when measuring (fixes the spurious
  directive on this package's hand-written
  shorebird_code_push_protocol.dart barrel and the unnecessary_ignore
  hits on three import-only files).
- eseidel/space_gen#210 (initial commit): format generated content
  in-process at write time via DartFormatter, so the directive
  decision matches what the analyzer sees (fixes the orphan
  justification comments left after dart fix --apply stripped
  unnecessary directives that were emitted against pre-format
  content).
- eseidel/space_gen#210 (review): in-process format now reads the
  consumer's language version from pubspec.lock/pubspec.yaml and the
  formatter:section from analysis_options.yaml (following relative
  include: paths so workspace setups resolve correctly). Without
  this, the formatter ran in tall style with default trailing-comma
  handling and stripped trailing commas this package's
  analysis_options.yaml asks to preserve.

The git pin is temporary — once the next pub.dev release ships,
switch to a versioned dep.

Regenerates lib/src/ from
/Users/eseidel/Documents/GitHub/_shorebird/packages/code_push_server/public/openapi.yaml
since api.shorebird.dev doesn't yet serve the latest spec.

Verification:
- dart analyze: No issues found
- dart test: all 216 tests pass
- 36 files changed, hand-written barrel untouched, 0 orphan
  comments, trailing commas preserved per analysis_options.yaml
The previous emit-time helper only collected top-level type names
(class / enum / mixin / extension type). A bracket ref like `[hash]`
to a class field was treated as unresolved → directive added →
dart fix --apply then strips it as unnecessary_ignore (the analyzer
DOES resolve member refs through enclosing class scope) → 5-line
justification comment orphaned.

Surfaced by shorebird_code_push_protocol's
create_patch_artifact_request.dart: `/// The signature of the [hash].`
refers to `final String hash;` and was leaving an orphan rationale
comment after every regen.

Extend _topLevelDeclarations to also pick up identifiers that look
like field declarations: final / late final / const / var / static
followed by a type and an identifier. Two new tests pin the
behavior:
- ref to a same-file field is a no-op (no directive added).
- placeholder refs that look like fields but have no matching
  declaration (the MIT license template's [year] [fullname] case)
  still trigger the directive.

Methods/getters/setters aren't tracked — the regex would over-match
too easily — so a [fooMethod] ref that resolves only via a method
declaration still trips the directive. Same behavior as before; the
gap is harmless (per-file directive only suppresses
comment_references) and can be closed if a real-world case shows up.

Verification: github regen dart analyze clean, 32 files now carry
the comment_references directive (down from ~45 — member refs now
correctly resolve), 0 orphan justification comments. 532 unit tests
pass.
eseidel added a commit to shorebirdtech/shorebird that referenced this pull request Apr 30, 2026
Pin space_gen to git ref 4d44b7ebb2d4b9237cd97cce2b88abb1fecd3403
(eseidel/space_gen#210 tip), which fixes a series of regressions
surfaced by this regen:

- eseidel/space_gen#209: scope long-line directive to files we emit
  + skip URI imports/exports when measuring (fixes the spurious
  directive on this package's hand-written
  shorebird_code_push_protocol.dart barrel and the unnecessary_ignore
  hits on three import-only files).
- eseidel/space_gen#210 (initial): format generated content in-process
  at write time via DartFormatter, so the directive decision matches
  what the analyzer sees (fixes the orphan justification comments
  left after dart fix --apply stripped unnecessary directives that
  were emitted against pre-format content).
- eseidel/space_gen#210 (style): in-process format reads the
  consumer's language version from pubspec.lock/pubspec.yaml and the
  formatter:section from analysis_options.yaml (following relative
  include: paths so workspace setups resolve correctly). Without
  this, the formatter ran in tall style with default trailing-comma
  handling and stripped trailing commas this package's
  analysis_options.yaml asks to preserve.
- eseidel/space_gen#210 (members): comment_references resolution now
  also looks at field declarations, not just top-level types. Fixes
  the same orphan-justification problem for the comment_references
  directive surfaced in create_patch_artifact_request.dart, where
  '/// The signature of the [hash].' refers to 'final String hash;'
  and the analyzer correctly resolves it through the enclosing
  class.

The git pin is temporary — once the next pub.dev release ships,
switch to a versioned dep.

Regenerates lib/src/ from
/Users/eseidel/Documents/GitHub/_shorebird/packages/code_push_server/public/openapi.yaml
since api.shorebird.dev doesn't yet serve the latest spec.

Verification:
- dart analyze: No issues found
- dart test: all 216 tests pass
- 12 files changed, hand-written barrel untouched, 0 orphan comments
  of either flavor, trailing commas preserved per
  analysis_options.yaml
eseidel added 2 commits April 30, 2026 12:51
The in-process formatter, the ignore-directive helpers, and the
subprocess Formatter all lived as siblings inside file_renderer.dart.
That mixed layers — FileRenderer's job is wiring templates and
visitor walks, not knowing how to read pubspec.lock or compose a
`// ignore_for_file:` directive.

New module lib/src/render/formatting.dart holds:
- longLineIgnoreBlock / commentReferencesIgnoreBlock — the per-file
  directive blocks.
- maybeAddLongLineIgnore / maybeAddCommentReferencesIgnore /
  maybeAddIgnoreDirectives — the gen-time helpers wired into
  _renderSpecTemplate.
- Formatter — the subprocess wrapper that runs dart pub get / format
  / fix --apply at the end of a regen.
- FormatContext / FormatterConfig — the dart_style settings resolved
  from the consuming package's pubspec and analysis_options.yaml.
- DartFileFormatter — the in-process DartFormatter wrapper with the
  cached resolution. Used by FileRenderer at every .dart write.

FileRenderer now holds a DartFileFormatter field, calls into it
from _renderTemplate / _renderSpecTemplate, and re-exports Formatter
+ RunProcess for the existing FileRendererConfig contract.
The previously-private _FormatContext / _FormatterConfig become
public so cross-file callers (and tests) can name them.

Tests for the helpers move to test/render/formatting_test.dart
alongside ~12 new DartFileFormatter tests covering: pass-through on
non-Dart paths, formatting Dart content, pubspec.lock vs pubspec.yaml
priority, formatter: page_width / trailing_commas read from
analysis_options.yaml, relative include: following (workspace setup),
package: include skip, walk-up to workspace root, default fallbacks,
single resolution caching across many calls.

Verification:
- dart analyze clean.
- dart test: 544 pass.
- github regen dart analyze clean.
@eseidel eseidel merged commit 75251bb into main Apr 30, 2026
8 checks passed
@eseidel eseidel deleted the es/format-in-process branch April 30, 2026 20:05
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