Plan 85: extract shared type-conversion helpers (Phase 1) - #150
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #150 +/- ##
==========================================
+ Coverage 86.76% 87.06% +0.30%
==========================================
Files 96 97 +1
Lines 10446 10346 -100
==========================================
- Hits 9063 9008 -55
+ Misses 900 856 -44
+ Partials 483 482 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR consolidates repeated “untyped settings map → typed Go value” coercion logic across multiple Markdown lint rules into a shared internal/rules/settings package, and updates affected rules/tests accordingly to reduce duplication and improve consistency.
Changes:
- Added
internal/rules/settingswith exported coercion helpers (ToInt,ToFloat,ToStringSlice) plus table-driven tests. - Refactored several rules to use the shared helpers and renamed
ApplySettings(settings ...)params to avoid package-name shadowing. - Removed now-redundant per-rule
toInt/toFloathelpers and their coverage-only tests; updated plan status tracking.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/rules/settings/settings.go |
Introduces shared coercion helpers used by multiple rule packages. |
internal/rules/settings/settings_test.go |
Adds table-driven unit tests for the shared coercion helpers. |
internal/rules/tokenbudget/rule.go |
Replaces local numeric coercion with settings.ToInt/ToFloat; avoids settings name shadowing. |
internal/rules/tablereadability/rule.go |
Switches numeric coercion to shared helpers; avoids settings name shadowing. |
internal/rules/tablereadability/rule_coverage_test.go |
Removes coverage-only tests for deleted local coercion helpers. |
internal/rules/tableformat/rule.go |
Uses shared settings.ToInt and removes local toInt. |
internal/rules/tableformat/rule_coverage_test.go |
Removes coverage-only tests for deleted local toInt. |
internal/rules/paragraphstructure/rule.go |
Switches to shared settings.ToInt; avoids settings name shadowing. |
internal/rules/paragraphreadability/rule.go |
Switches to shared settings.ToInt/ToFloat; avoids settings name shadowing. |
internal/rules/nomultipleblanks/rule.go |
Switches to shared settings.ToInt; removes local toInt. |
internal/rules/maxfilelength/rule.go |
Switches to shared settings.ToInt; removes local toInt. |
internal/rules/linelength/rule.go |
Switches numeric coercion to shared settings.ToInt; leaves local toStringSlice. |
internal/rules/firstlineheading/rule.go |
Switches to shared settings.ToInt; removes local toInt. |
internal/rules/concisenessscoring/rule.go |
Switches to shared settings.ToInt/ToFloat; removes local helpers. |
plan/85_coverage-to-95-percent.md |
Updates plan status and checks off completed Phase 1 tasks. |
PLAN.md |
Updates the generated plan catalog row to reflect the new plan status. |
Phase 1 of plan 85: Replace 10 duplicated private toInt copies and 4 toFloat copies with a single internal/rules/settings package covered by table-driven unit tests. Each rule package's ApplySettings parameter is renamed from settings to s so the package can be imported under its natural name. The new settings package has 100% statement coverage. The emptysectionbody and maxsectionlength variants are kept local because they reject non-whole floats.
22b4b7f to
8cee9dd
Compare
- ToInt/ToFloat reject NaN, +/-Inf, and out-of-range floats so invalid config surfaces deterministically instead of yielding implementation-dependent int values. - linelength.applyExclude now uses settings.ToStringSlice so rule state does not alias the caller's config slice; drops the local toStringSlice duplicate.
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
|
🔵 Merge Queue — CI running Merged into batch branch Next: No action needed — you'll be notified when CI completes. |
|
✅ Merge Queue — merged This PR landed on Next: Done — nothing more to do here. |
PR #150 landed the settings helpers during our PR's review window. Swap MDS037's local toInt and toStringSlice for the package-level versions so the rule picks up the project-wide coercion semantics: settings.ToInt truncates fractional floats (instead of rejecting them) and rejects NaN/Inf/out-of-range. Update the fractional-float test accordingly.
PR #150 landed the settings helpers during our PR's review window. Swap MDS037's local toInt and toStringSlice for the package-level versions so the rule picks up the project-wide coercion semantics: settings.ToInt truncates fractional floats (instead of rejecting them) and rejects NaN/Inf/out-of-range. Update the fractional-float test accordingly.
PR #150 landed the settings helpers during our PR's review window. Swap MDS037's local toInt and toStringSlice for the package-level versions so the rule picks up the project-wide coercion semantics: settings.ToInt truncates fractional floats (instead of rejecting them) and rejects NaN/Inf/out-of-range. Update the fractional-float test accordingly.
Summary
Consolidate duplicated type conversion logic across 10 rule packages into a new shared
internal/rules/settingspackage. This eliminates code duplication and provides a single, well-tested implementation for coercing YAML-decoded values to Go types.Key Changes
New package: Created
internal/rules/settings/settings.gowith three exported helper functions:ToInt(v any) (int, bool)- Coerces int, int64, or float64 to int (truncating floats toward zero)ToFloat(v any) (float64, bool)- Coerces int, int64, or float64 to float64ToStringSlice(v any) ([]string, bool)- Coerces []string or []any of strings to []string (returns a copy)Comprehensive tests: Added
internal/rules/settings/settings_test.gowith table-driven tests covering all type branches (int, float64, int64, string, bool, nil, slices)Refactored 10 rule packages to use the shared helpers:
Removed duplicate code: Deleted private
toInt()andtoFloat()implementations from each rule packageImplementation Details
ToStringSlicereturns a fresh copy to prevent caller mutations from affecting the original inputsettingstosinApplySettingsmethods to avoid shadowing the package namehttps://claude.ai/code/session_01EJJ9LCv67GJ39SnJNpYGx3