Add MDS034 markdown-flavor rule for flavor validation - #146
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
==========================================
+ Coverage 87.06% 87.32% +0.26%
==========================================
Files 97 107 +10
Lines 10346 13220 +2874
==========================================
+ Hits 9008 11545 +2537
- Misses 856 1218 +362
+ Partials 482 457 -25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new opt-in meta lint rule (MDS034) that validates a Markdown file’s syntax against a configured “flavor” (commonmark, gfm, goldmark) and reports unsupported feature usage, backed by unit + fixture tests and rule documentation.
Changes:
- Introduces
internal/rules/markdownflavorwith flavor/feature registry, a shared Goldmark parser configured with extensions, AST/regex-based detection, and the MDS034 rule implementation. - Adds unit tests + integration fixtures/docs for MDS034 and registers the rule in the CLI + rule index.
- Updates integration test harness to suppress MDS033’s once-per-process warning leakage, and updates plan status/progress tracking.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| plan/86_markdown-flavor-validation.md | Marks plan as in-progress and checks off completed subtasks for the initial MDS034 increment. |
| internal/rules/markdownflavor/features.go | Defines Flavor/Feature enums and the support matrix used by MDS034. |
| internal/rules/markdownflavor/features_test.go | Unit tests for flavor parsing, stringification, and support matrix expectations. |
| internal/rules/markdownflavor/parser.go | Adds a cached Goldmark parser with relevant built-in extensions enabled for detection. |
| internal/rules/markdownflavor/parser_test.go | Validates the dual-parser detects extension nodes as expected. |
| internal/rules/markdownflavor/detect.go | Implements feature detection (dual-parser walk + bare URL scan) and finding positioning. |
| internal/rules/markdownflavor/detect_test.go | Unit tests covering detection for the implemented feature set and exclusions. |
| internal/rules/markdownflavor/rule.go | Registers and implements MDS034 (configurable, opt-in, warning diagnostics). |
| internal/rules/markdownflavor/rule_test.go | Unit tests for rule identity, settings parsing, and flavor-specific behavior. |
| internal/rules/index.md | Adds MDS034 to the published rules index. |
| internal/rules/directorystructure/rule.go | Adds SilenceConfigWarningForTesting() to pre-consume MDS033’s sync.Once warning gate. |
| internal/rules/MDS034-markdown-flavor/good/commonmark.md | “Good” fixture for commonmark flavor. |
| internal/rules/MDS034-markdown-flavor/good/gfm.md | “Good” fixture for gfm flavor. |
| internal/rules/MDS034-markdown-flavor/good/goldmark.md | “Good” fixture for goldmark flavor. |
| internal/rules/MDS034-markdown-flavor/bad/commonmark-bare-url.md | “Bad” fixture asserting bare-URL autolink diagnostics under commonmark. |
| internal/rules/MDS034-markdown-flavor/bad/commonmark-heading-id.md | “Bad” fixture asserting heading-ID diagnostics under commonmark. |
| internal/rules/MDS034-markdown-flavor/bad/commonmark-strikethrough.md | “Bad” fixture asserting strikethrough diagnostics under commonmark. |
| internal/rules/MDS034-markdown-flavor/bad/commonmark-table.md | “Bad” fixture asserting table diagnostics under commonmark. |
| internal/rules/MDS034-markdown-flavor/bad/commonmark-task-list.md | “Bad” fixture asserting task-list diagnostics under commonmark. |
| internal/rules/MDS034-markdown-flavor/bad/gfm-definition-list.md | “Bad” fixture asserting definition-list diagnostics under gfm. |
| internal/rules/MDS034-markdown-flavor/bad/gfm-footnote.md | “Bad” fixture asserting footnote diagnostics under gfm. |
| internal/rules/MDS034-markdown-flavor/bad/gfm-heading-id.md | “Bad” fixture asserting heading-ID diagnostics under gfm. |
| internal/rules/MDS034-markdown-flavor/README.md | Adds rule documentation including settings, support matrix, and examples. |
| internal/integration/rules_test.go | Imports MDS034 and silences MDS033 warning gate up front to avoid cross-fixture leakage. |
| cmd/mdsmith/main.go | Registers the new rule in the CLI binary. |
| PLAN.md | Marks plan 86 as in-progress in the plan catalog. |
jeduden
force-pushed
the
claude/plan-86-markdown-flavor-validation
branch
from
April 20, 2026 15:55
447185c to
71fa971
Compare
jeduden
commented
Apr 21, 2026
Implements the last of the five custom extensions. Block parser: a line of the form \`*[TERM]: EXPANSION\` (up to 3 spaces of indent) creates an AbbreviationDefinition node and records the term in a parser-context table keyed by the term's bytes. AST transformer: after all blocks have been parsed, walk the document, skip CodeSpan / FencedCodeBlock / CodeBlock / AbbreviationDefinition subtrees, and rewrite each Text node around whole-word matches of any defined term. Each hit becomes an AbbreviationReference node containing a Text with the term's source span; gaps and prefix/suffix remain as plain Text siblings. The transformer runs at the document level so a \`*[TERM]: ...\` definition placed after the paragraph that uses it still marks every inline reference, matching PHP Markdown Extra / MyST behaviour. Tests cover: definition recognition, reference marking, missing-definition no-op, whole-word boundary (XHTML does not match HTML), multiple occurrences, and inline-code exclusion.
Parser() now enables Superscript, Subscript, MathBlock, MathInline, and Abbreviation alongside the built-in goldmark extensions and the PI block parser. The shared singleton now covers all twelve MDS034 features from one dual parse. detectFromDual gained handlers for the new AST node kinds: - SuperscriptNode / SubscriptNode / MathInlineNode use a shared markerInlineFinding that backs up the single opening marker byte so the diagnostic points at \`^\` / \`~\` / \`$\` rather than the first content character. - MathBlockNode / AbbreviationDefinition report at the block's first-line column. - AbbreviationReference uses inlineFinding so the diagnostic lands on the exact source column of the abbreviated term within its paragraph. Switch dispatch split into builtinFindingFor and customFindingFor to keep each under the funlen threshold and make the mapping easy to scan. anyDualFeatureAccepted now enumerates all 11 dual- parse features so DetectFiltered can still skip the re-parse when none are wanted. Fixtures: add five commonmark bad fixtures covering superscript, subscript, math block, inline math, and abbreviation (definition + reference). Plan: check off the dual-parser, detector, ACs 1–3, and update the MDS034 README's "Detected features" section to list all twelve features.
Three threads: - mathblock.go Open() doc said a \`\$\$\` fence only opens "at the document root" but the code doesn't check the parent block. Reword the comment so it describes the real behaviour: opens regardless of parent so long as the line starts with \`\$\$\` at up to 3 columns of indent. - abbreviation.go Transform() doc promised "paragraph"- only rewriting, but the walk visits every Text node outside code / def nodes. Reword to match: code spans, fenced / indented code blocks, and AbbreviationDefinition nodes are skipped; everything else (paragraphs, headings, list items, …) is eligible. - abbreviation.go buildReference had an unused source parameter that every caller passed nil for. Drop the parameter so the helper no longer pretends it might need the source bytes.
jeduden
force-pushed
the
claude/plan-86-markdown-flavor-validation
branch
from
April 21, 2026 06:43
fde4ae2 to
4b3edf5
Compare
Per user request: support four additional common Markdown dialects plus a permissive "accept all" flavor so MDS034 can be applied to a wider range of docs without toggling off each feature by hand. New flavors and their support sets: - any — accepts every tracked feature; use when the target renderer is unknown or permissive. - pandoc — GFM + footnotes, definition lists, heading IDs, superscript, subscript, math block, and inline math. Rejects abbreviations (not a default Pandoc extension). - phpextra — PHP Markdown Extra: tables, footnotes, definition lists, heading IDs, and abbreviations. Rejects GFM features and math. - multimarkdown — PHP Extra + math block + inline math. - myst — MyST (Sphinx flavor): tables, strikethrough, footnotes, definition lists, heading IDs, math block, and inline math. Flavor.Supports short-circuits on FlavorAny so its matrix stays empty; every other flavor consults the explicit support table. Tests split into one per-flavor case plus a rule-level assertion that flavor: any silences all diagnostics. A phpextra integration test exercises a mixed document to verify selective rejection. Good fixtures land for every new flavor and four bad fixtures cover flavor-specific rejection paths (pandoc + abbreviation, phpextra + strikethrough, multimarkdown + task list, myst + abbreviation). README "Settings" section expands with the per- flavor explanation; "Detected features" table adds columns for the four new named flavors (any is documented separately to keep the table within MDS026's 8-column limit).
Copilot caught a race condition: the test-only
SilenceConfigWarningForTesting helper reassigned the
package-level \`configWarned\` sync.Once before
consuming it. If Rule.Check ran concurrently on
another goroutine (tests run the engine in parallel
for different files), the reassignment could race
with configWarned.Do inside Check.
The original intent was "mark the guard as already-
fired so later checks do not emit the warning".
A plain configWarned.Do(func(){}) achieves that
without touching the Once variable: after the first
call, Do is a no-op and never writes.
Copilot noted the doc said "seven AST-detected core features" but only listed five built-in extensions plus the heading-ID attribute parser (six enablements). Rewrite to match: five built-in extensions plus the heading-ID attribute parser cover six features; the seventh AST-tracked feature, bare-URL autolinks, is detected on the main CommonMark parse by detectBareURLs, not here.
Lift uncovered branches across the three packages touched by this PR: - internal/rules/markdownflavor/detect.go: drop the \`want\` parameter from nearestBlockAncestor. Every caller passed \`ast.NodeKind(0)\` so the kind-matching branch was dead; removing it simplifies the call sites and eliminates a 50%- covered helper. - internal/rules/markdownflavor/ext/stubs_test.go: new test that invokes the no-op interface methods (Close / CloseBlock / CanInterruptParagraph / CanAcceptIndentedLine) and Dump on every custom AST node. These methods exist only because goldmark's parser interfaces require them; the test exercises the bodies so coverage does not silently regress when a helper starts doing real work. - internal/rules/directorystructure/rule_test.go: add TestCategory and TestSilenceConfigWarningForTesting for the public methods that had no test yet. Coverage: markdownflavor 93.3% → 94.3%; ext 89.5% → 93.3%; directorystructure 90.2% → 93.4%.
codecov/project fired with -0.33% on commit 718690e because several rejection / EOF branches inside the new MDS034 block parsers and detect helpers stayed uncovered. Add targeted edge-case tests: - ext/mathblock_edge_test.go covers Open rejections (empty line, four-space indent, non-fence \`$\` line) and Continue's EOF path with an unclosed block. - ext/abbreviation_edge_test.go covers every Open rejection (empty, indent, wrong prefix, missing closing bracket, missing colon, empty term), the \`*[TERM]:\` (empty expansion) success, the transformer's no-definitions and empty-table early-returns, the first-match-at-paragraph-start rewrite branch, and the multi-term gap-text insertion. - detect_edge_test.go covers lineCol / lineStartOf clamp paths, the firstTextStart \`-1\` sentinel, and findHeadingID's no-attribute short-circuit. Local Go coverage now: markdownflavor 94.7% (from 94.3%), ext 94.4% (from 93.3%).
Last-mile coverage bumps for the branches codecov
still flags. Each of these paths is defensive — not
reachable from a goldmark-produced AST — so we
exercise them with orphan nodes built in the test.
- taskCheckBoxFinding / inlineExtFinding return a
(1, 1) fallback when the inline node has no block
ancestor.
- findingFromBlock returns the same fallback when
the block has no Lines appended.
- nodeByteRange clamps a negative firstTextStart
result (FootnoteLink has no children and no
segment) to 0.
- findHeadingID now has tests for every rejection
path: no Attributes(), Attributes() without an
"id" key (e.g. \`{.highlight}\`), and the empty-
source heading.
Reach 100% statement coverage in internal/rules/
markdownflavor and internal/rules/markdownflavor/ext.
Each branch covered here was a defensive guard,
early-exit, or unreachable fallback that natural
input never hit.
Covered branches:
- features.go: String() fallback for an unknown
Flavor; Name() fallback for an unknown Feature.
- detect.go:
- nearestBlockAncestor walks past a non-block
inline ancestor (Paragraph > Emphasis >
FootnoteLink).
- findHeadingID rejects a Heading with an id
attribute but no Lines appended.
- findHeadingID rejects a Heading whose source
line contains no '{'.
- ext/abbreviation.go:
- rewriteText on an empty-body Text node.
- rewriteText on an orphan Text with no parent.
- bestMatchAt rejects a term followed by a word
byte (suffix match, e.g. API in APIserver).
- ext/mathblock.go:
- Continue returns parser.Close when the block
was already closed in Open (same-line `$$x$$`).
- Continue returns parser.Close on EOF.
- ext/mathinline.go:
- Parse skips a `$` candidate closer that is
immediately followed by another `$` (`$$`
fence marker), pairing with a later closer.
jeduden
added
queue
Owner
Author
|
🟢 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. |
Owner
Author
|
🔵 Merge Queue — CI running Merged into batch branch Next: No action needed — you'll be notified when CI completes. |
jeduden
removed
the
queue:active
Owner
Author
|
✅ Merge Queue — merged This PR landed on Next: Done — nothing more to do here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements MDS034, a new linting rule that validates Markdown syntax against a declared target flavor (CommonMark, GFM, or Goldmark) and flags unsupported syntax features.
Key Changes
Feature detection system (
features.go):FlavorandFeatureenums and a support matrix. CommonMark rejects every tracked feature; GFM adds tables, task lists, strikethrough, and bare-URL autolinks; the goldmark profile adds heading IDs on top of GFM. Twelve features are tracked.Dual-parser architecture (
parser.go): shared goldmark parser with Table, Strikethrough, TaskList, Footnote, DefinitionList, the heading-ID attribute parser, and thelint.PIBlockParserPrioritizedblock parser so<?include?>blocks stay PI nodes. Also enables five custom MDS034 extensions (Superscript, Subscript, MathBlock, MathInline, Abbreviation) so all eleven AST-detected features come from one dual parse. Linkify is intentionally not enabled — bare-URL detection runs over the main CommonMark AST.Custom goldmark extensions (
internal/rules/markdownflavor/ext/): detection-only parsers with no renderers.SuperscriptExt— inline^text^, rejects^^and longer runs.SubscriptExt— inline~text~; coexists with built-in Strikethrough via a higher-priority slot that only accepts length-1~runs.MathBlockExt— block$$...$$fence (single-line or multi-line).MathInlineExt— inline$...$with Pandoctex_math_dollarsopen/close rules (no leading space, no trailing space, no trailing digit after the closer).AbbreviationExt— block*[TERM]: EXPANSIONplus an AST transformer that walks the document after block parsing and marks whole-word term occurrences inside paragraphs, skipping code spans, code blocks, and other definition nodes.AST-based detection (
detect.go):detectFromDualwalks the dual-parser tree for all eleven AST-detected features viabuiltinFindingForandcustomFindingFor. Block features (tables, footnotes, definition lists, math block, abbreviation definition) report at the first-line column; inline features (strikethrough, super/subscript, inline math) back up past their opening marker so the diagnostic points at the delimiter. Bare-URL autolinks are detected separately from the main CommonMark AST — regex-based scan over text nodes, skipping links, autolinks, code spans, and code blocks.DetectFiltered(f, accept)letsRule.Checkskip detectors whose features the configured flavor already supports.Rule implementation (
rule.go): Registers MDS034 as a configurable, opt-in meta rule.flavorsetting (case-sensitive:"commonmark","gfm","goldmark")."X is/are not supported by Y").rule.Configurableandrule.Defaultable.Test coverage:
internal/rules/MDS034-markdown-flavor/bad/covering every tracked feature, plus good fixtures for each flavor.Documentation: rule README lists all twelve features and their per-flavor support.
Implementation Details
AdjustDiagnosticsapplies any front-matterLineOffsetlater.Finding.Start/Endare best-effort anchors. They cover the feature span precisely only for heading IDs (viaExtra) and bare URLs; other findings use convenience anchors (line-start widening or zero-length) so a future Fix must recompute an exact span fromf.Source.HeadingIDExtrametadata carries attribute block positions for heading ID fixes.sync.Once) to avoid rebuilding on every rule invocation.mdsmith fix) is not implemented in this slice; every detected feature currently surfaces as a diagnostic only.https://claude.ai/code/session_01UR6YKkUmqFvGi9n8iKcybJ