Skip to content

fix: prune model helpers by whole-identifier match, not substring#211

Merged
eseidel merged 1 commit into
eseidel:mainfrom
easymac:mc/fix-model-helper-prefix-collision
Jun 11, 2026
Merged

fix: prune model helpers by whole-identifier match, not substring#211
eseidel merged 1 commit into
eseidel:mainfrom
easymac:mc/fix-model-helper-prefix-collision

Conversation

@easymac

@easymac easymac commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Helper pruning used body.contains(name), so a body calling maybeParseDateTime also matched maybeParseDate (same for maybeParseUriTemplate / maybeParseUri). The unused helper got written into model_helpers.dart with no caller, which reads as uncovered lines on the generated package.

The maybeParse* helpers only show up for nullable fields, so any spec with a nullable date-time or uri-template field trips it. Hit it regenerating shorebird's code_push_server.

Fix

Match helper names as whole identifiers: _referencedModelHelpers tokenizes the body once and intersects with ModelHelpers.all, so the longer name no longer drags in the shorter. Both SchemaUsage.fromBody and ApiUsage.fromBody use it.

No gen_tests fixture has a nullable date-time field, which is why this slipped through. Added unit and e2e coverage.

Verification

547 tests pass, analyze and format clean. On the shorebird spec, maybeParseDate was emitted with zero call sites before and is gone after.

## Summary

Helper pruning used `body.contains(name)`, so a body calling `maybeParseDateTime` also matched `maybeParseDate` (same for `maybeParseUriTemplate` / `maybeParseUri`). The unused helper got written into `model_helpers.dart` with no caller, which reads as uncovered lines on the generated package.

The `maybeParse*` helpers only show up for nullable fields, so any spec with a nullable date-time or uri-template field trips it. Hit it regenerating shorebird's `code_push_server`.

## Fix

Match helper names as whole identifiers: `_referencedModelHelpers` tokenizes the body once and intersects with `ModelHelpers.all`, so the longer name no longer drags in the shorter. Both `SchemaUsage.fromBody` and `ApiUsage.fromBody` use it.

No `gen_tests` fixture has a nullable date-time field, which is why this slipped through. Added unit and e2e coverage.

## Verification

547 tests pass, analyze and format clean. On the shorebird spec, `maybeParseDate` was emitted with zero call sites before and is gone after.
@eseidel eseidel merged commit e358525 into eseidel:main Jun 11, 2026
4 checks passed
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.65%. Comparing base (528e467) to head (a555dbb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #211   +/-   ##
=======================================
  Coverage   94.64%   94.65%           
=======================================
  Files          21       21           
  Lines        5062     5066    +4     
=======================================
+ Hits         4791     4795    +4     
  Misses        271      271           

☔ View full report in Codecov by Harness.
📢 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.

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.

2 participants