Implement MDS045 list-marker-style rule - #206
Conversation
Add new rule MDS045 that enforces consistent bullet markers for unordered lists. The rule pins one of `-`, `*`, or `+` as the marker and can optionally rotate markers by nesting depth. - Implement Check() to walk unordered lists and compute depth - Implement Fix() to replace marker bytes - Add Configurable interface support for style and nested settings - Register as MDS045 in category "list" (disabled by default) - Add comprehensive unit tests in rule_test.go - Add fixture tests (good/, bad/, fixed/) in MDS045-list-marker-style/ - Add rule documentation in README.md - Add import in integration test - Mark plan 109 as completed (✅) All tests pass. Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/ac3f9ff9-2b18-4354-a2b5-2e1aef7dd4aa Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #206 +/- ##
==========================================
- Coverage 94.02% 93.94% -0.08%
==========================================
Files 126 127 +1
Lines 13715 13898 +183
==========================================
+ Hits 12895 13056 +161
- Misses 497 514 +17
- Partials 323 328 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new opt-in Markdown lint rule (MDS045) to enforce a consistent unordered-list bullet marker (-, *, or +), with optional depth-based rotation, plus accompanying tests and fixtures.
Changes:
- Implements
internal/rules/listmarkerstylewithCheck,Fix, and configurable settings (style,nested). - Adds unit tests and integration fixtures for MDS045, and wires the rule into the integration test registry.
- Marks plan 109 complete.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| plan/109_list-marker-style.md | Marks plan status/tasks/acceptance criteria as complete for MDS045. |
| internal/rules/listmarkerstyle/rule.go | New MDS045 rule implementation (AST walk, diagnostics, fixer, settings). |
| internal/rules/listmarkerstyle/rule_test.go | Unit tests for rule metadata, checking, fixing, and settings validation. |
| internal/rules/listmarkerstyle/README.md | Adds rule documentation under the implementation package directory. |
| internal/rules/listmarkerstyle/good/dash.md | Adds a “good” fixture (dash style) under implementation directory. |
| internal/rules/listmarkerstyle/good/asterisk.md | Adds a “good” fixture (asterisk style) under implementation directory. |
| internal/rules/listmarkerstyle/good/plus.md | Adds a “good” fixture (plus style) under implementation directory. |
| internal/rules/listmarkerstyle/good/nested.md | Adds a “good” fixture (nested rotation) under implementation directory. |
| internal/rules/listmarkerstyle/bad/asterisk-with-dash.md | Adds a “bad” fixture (asterisk used when dash configured) under implementation directory. |
| internal/rules/listmarkerstyle/bad/dash-with-asterisk.md | Adds a “bad” fixture (dash used when asterisk configured) under implementation directory. |
| internal/rules/listmarkerstyle/bad/plus-with-dash.md | Adds a “bad” fixture (plus used when dash configured) under implementation directory. |
| internal/rules/listmarkerstyle/bad/nested-wrong.md | Adds a “bad” fixture (wrong nested marker) under implementation directory. |
| internal/rules/listmarkerstyle/fixed/asterisk-with-dash.md | Adds a “fixed” fixture output under implementation directory. |
| internal/rules/listmarkerstyle/fixed/dash-with-asterisk.md | Adds a “fixed” fixture output under implementation directory. |
| internal/rules/listmarkerstyle/fixed/plus-with-dash.md | Adds a “fixed” fixture output under implementation directory. |
| internal/rules/listmarkerstyle/fixed/nested-wrong.md | Adds a “fixed” fixture output under implementation directory. |
| internal/rules/MDS045-list-marker-style/README.md | Adds rule documentation in the canonical rule README location. |
| internal/rules/MDS045-list-marker-style/good/dash.md | Adds canonical “good” fixture (dash). |
| internal/rules/MDS045-list-marker-style/good/asterisk.md | Adds canonical “good” fixture (asterisk). |
| internal/rules/MDS045-list-marker-style/good/plus.md | Adds canonical “good” fixture (plus). |
| internal/rules/MDS045-list-marker-style/good/nested.md | Adds canonical “good” fixture (nested rotation). |
| internal/rules/MDS045-list-marker-style/bad/asterisk-with-dash.md | Adds canonical “bad” fixture (asterisk vs dash). |
| internal/rules/MDS045-list-marker-style/bad/dash-with-asterisk.md | Adds canonical “bad” fixture (dash vs asterisk). |
| internal/rules/MDS045-list-marker-style/bad/plus-with-dash.md | Adds canonical “bad” fixture (plus vs dash). |
| internal/rules/MDS045-list-marker-style/bad/nested-wrong.md | Adds canonical “bad” fixture (wrong nested marker). |
| internal/rules/MDS045-list-marker-style/fixed/asterisk-with-dash.md | Adds canonical “fixed” output for asterisk→dash. |
| internal/rules/MDS045-list-marker-style/fixed/dash-with-asterisk.md | Adds canonical “fixed” output for dash→asterisk. |
| internal/rules/MDS045-list-marker-style/fixed/plus-with-dash.md | Adds canonical “fixed” output for plus→dash. |
| internal/rules/MDS045-list-marker-style/fixed/nested-wrong.md | Adds canonical “fixed” output for nested correction. |
| internal/integration/rules_test.go | Registers the new rule package via blank import so fixtures run in integration tests. |
|
@copilot apply changes based on the comments in this thread |
…, per-item diagnostics, ToStringSlice, mixed-marker tests Agent-Logs-Url: https://github.com/jeduden/mdsmith/sessions/db9dbd84-c280-487a-9135-9bc191bf9eaf Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>
Applied all review changes in commit 4153ee8:
|
…c280-487a-9135-9bc191bf9eaf Co-authored-by: jeduden <1117699+jeduden@users.noreply.github.com>
|
🟢 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. |
|
This PR could not be merged into the batch branch without conflicts with Next: Rebase onto or merge |
| --- | ||
| # Bad nested with wrong inner marker | ||
|
|
||
| Outer uses dash (correct), inner should use asterisk but uses dash. |
There was a problem hiding this comment.
This file is in fixed/ but the explanatory sentence still says the inner list "uses dash" even though the content has been fixed to * at depth 1. Updating the prose to match the fixed output (or making it neutral) would keep fixtures self-consistent.
| Outer uses dash (correct), inner should use asterisk but uses dash. | |
| Outer uses dash (correct), and the inner list uses asterisk. |
| github.com/neurosnap/sentences v1.1.2 | ||
| github.com/spf13/pflag v1.0.10 | ||
| github.com/stretchr/testify v1.11.1 | ||
| github.com/tetratelabs/wazero v1.11.0 |
There was a problem hiding this comment.
github.com/tetratelabs/wazero is now listed as a direct dependency, but the only imports in this repo appear to be in build-tagged/ignored spike code (e.g., internal/rules/concisenessscoring/wasmclassifier/embed.go has //go:build spike_wasm_classifier). Unless MDS045 requires wazero (it doesn’t), this looks like an accidental go mod tidy side effect; consider reverting this change or moving the dependency management to the spike/tooling workflow so the main module deps stay minimal.
| github.com/tetratelabs/wazero v1.11.0 |
| # Bad plus with dash config | ||
|
|
||
| This list uses plus but dash is configured. |
There was a problem hiding this comment.
This file is under fixed/ (expected post-fix output), but the heading/body text still describes the pre-fix state ("uses plus but dash is configured") even though the list items have been rewritten to -. Consider updating the prose/title to match the fixed output (or make it neutral) to avoid confusing future readers.
| # Bad plus with dash config | |
| This list uses plus but dash is configured. | |
| # Dash list with dash config | |
| This list uses dashes because dash is configured. |
| # Bad dash with asterisk config | ||
|
|
||
| This list uses dashes but asterisk is configured. |
There was a problem hiding this comment.
This fixed/ fixture’s heading/body text still describes the bad input ("uses dashes but asterisk is configured") even though the content has already been fixed to * markers. Updating the prose/title to reflect the post-fix state (or removing the claim) would prevent confusion when inspecting fixtures.
| # Bad dash with asterisk config | |
| This list uses dashes but asterisk is configured. | |
| # Fixed asterisk list marker style | |
| This list uses asterisk markers as configured. |
| # Bad asterisk with dash config | ||
|
|
||
| This list uses asterisks but dash is configured. |
There was a problem hiding this comment.
This fixed/ fixture’s description claims the list "uses asterisks" even though the markers have been fixed to -. Consider adjusting the heading/body text so the fixture reads as fixed output (or use neutral wording).
| # Bad asterisk with dash config | |
| This list uses asterisks but dash is configured. | |
| # Fixed dash list with dash config | |
| This list uses dashes as configured. |
|
🟢 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. |
|
This PR could not be merged into the batch branch without conflicts with Next: Rebase onto or merge |
|
🟢 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 — requeued The merge queue hit a transient error while processing this PR:
Next: No action needed — the queue will retry automatically on the next run. |
|
⏳ Merge Queue — requeued The merge queue hit a transient error while processing this PR:
Next: No action needed — the queue will retry automatically on the next run. |
|
🟢 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 — CI failed The batch CI run failed with this PR in it. Next: Fix the failure, push updates, then re-add the |
|
🔍 Merge Queue — bisecting A larger batch failed CI. Bisection is isolating the culprit: this run tests up to 2 of 4 candidate PRs on Next: No action needed — you'll be notified when the culprit is isolated or this PR merges. |
|
🔍 Merge Queue — bisecting A larger batch failed CI. Bisection is isolating the culprit: this run tests up to 1 of 2 candidate PRs on Next: No action needed — you'll be notified when the culprit is isolated or this PR merges. |
|
✅ Merge Queue — merged This PR landed on Next: Done — nothing more to do here. |
Adds rule MDS045 to enforce consistent bullet characters (
-,*,+) for unordered lists, eliminating the three-way ambiguity in CommonMark that causes diff noise.Implementation
*ast.Listnodes with!IsOrdered(), computes nesting depth via ancestor count, validates marker against configured stylestyle:dash|asterisk|plus(default:dash)nested: Optional array for depth-based rotation (e.g.,[dash, asterisk]alternates by level)list, disabled by default (opt-in)Example
Flags and fixes:
Testing
MDS045-list-marker-style/(good/bad/fixed)