feat(markdown): port markdownlint MD040 fenced code language rule - #11267
feat(markdown): port markdownlint MD040 fenced code language rule#11267saberoueslati wants to merge 4 commits into
Conversation
|
✅ Organic activityNo automation signals detected in the analyzed events. This is an automated analysis by AgentScan |
github-actions
Bot
added
A-Project
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds the 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/biome_rule_options/src/use_fenced_code_language.rs (1)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd module and type rustdoc.
This new public options module has no
//!module description, andUseFencedCodeLanguageOptionshas no///type-level contract. Add both descriptions before the field-level documentation.As per coding guidelines, use
//!for module purpose and rustdoc for new rule options.Proposed documentation
+//! Configuration options for the Markdown `useFencedCodeLanguage` rule. +//! These options control accepted language names and the info string format. + use biome_deserialize_macros::Deserializable; use serde::{Deserialize, Serialize}; +/// Configuration for the `useFencedCodeLanguage` rule. #[derive(Default, Clone, Debug, Deserialize, Deserializable, Eq, PartialEq, Serialize)]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_rule_options/src/use_fenced_code_language.rs` around lines 1 - 6, Add rustdoc for the public options module using a `//!` description of its purpose, and add a `///` contract documenting `UseFencedCodeLanguageOptions` before its declaration. Keep the existing derives and serde configuration unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/fast-impalas-knock.md:
- Line 5: Update the changeset entry for useFencedCodeLanguage to include the
existing issue using the exact “Fixed [`#11264`](issue link): ...” format, while
retaining the rule documentation link.
In
`@crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/languageOnly.md`:
- Line 11: Update the fenced code marker in the languageOnly.md fixture to have
exactly two trailing spaces instead of three, preserving the test case while
satisfying the MD009 lint rule.
In
`@crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md`:
- Around line 7-11: Preserve both intentional parser fixtures while adding
scoped Markdownlint exceptions: in
crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md
lines 7-11, suppress MD048 and MD046 around the tilde-fence and indented-code
examples; in
crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/invalid.md
lines 7-9, suppress MD048 around the tilde-fence example.
---
Nitpick comments:
In `@crates/biome_rule_options/src/use_fenced_code_language.rs`:
- Around line 1-6: Add rustdoc for the public options module using a `//!`
description of its purpose, and add a `///` contract documenting
`UseFencedCodeLanguageOptions` before its declaration. Keep the existing derives
and serde configuration unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d3fd4b1b-c3af-4941-8a9b-cdc7bba82031
⛔ Files ignored due to path filters (12)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/allowedLanguages.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/allowedLanguages.options.json.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/combined.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/combined.options.json.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/invalid.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/languageOnly.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/languageOnly.options.json.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (13)
.changeset/fast-impalas-knock.mdcrates/biome_analyze/src/rule.rscrates/biome_markdown_analyze/src/lint/nursery/use_fenced_code_language.rscrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/allowedLanguages.mdcrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/allowedLanguages.options.jsoncrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/combined.mdcrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/combined.options.jsoncrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/invalid.mdcrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/languageOnly.mdcrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/languageOnly.options.jsoncrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.mdcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_fenced_code_language.rs
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added a new nursery rule [`useFencedCodeLanguage`](https://biomejs.dev/linter/rules/use-fenced-code-language/), which requires fenced code blocks in Markdown to declare a language. It is a port of markdownlint's `MD040`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Link issue #11264 in the changeset.
The PR objectives identify issue #11264, but Line 5 links only the rule documentation. Add the issue link to the release entry.
As per coding guidelines, user-facing changesets must include issue and rule links. Based on learnings, use the exact Fixed [#NUMBER](issue link): ... format for an existing issue.
Proposed changeset entry
-Added a new nursery rule [`useFencedCodeLanguage`](https://biomejs.dev/linter/rules/use-fenced-code-language/), which requires fenced code blocks in Markdown to declare a language. It is a port of markdownlint's `MD040`.
+Fixed [`#11264`](https://github.com/biomejs/biome/issues/11264): Added a new nursery rule [`useFencedCodeLanguage`](https://biomejs.dev/linter/rules/use-fenced-code-language/), which requires fenced code blocks in Markdown to declare a language. It is a port of markdownlint's `MD040`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Added a new nursery rule [`useFencedCodeLanguage`](https://biomejs.dev/linter/rules/use-fenced-code-language/), which requires fenced code blocks in Markdown to declare a language. It is a port of markdownlint's `MD040`. | |
| Fixed [`#11264`](https://github.com/biomejs/biome/issues/11264): Added a new nursery rule [`useFencedCodeLanguage`](https://biomejs.dev/linter/rules/use-fenced-code-language/), which requires fenced code blocks in Markdown to declare a language. It is a port of markdownlint's `MD040`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.changeset/fast-impalas-knock.md at line 5, Update the changeset entry for
useFencedCodeLanguage to include the existing issue using the exact “Fixed
[`#11264`](issue link): ...” format, while retaining the rule documentation link.
Sources: Coding guidelines, Learnings
There was a problem hiding this comment.
The changeset has been removed because Markdown support is not in production yet, so there is no release entry to update.
| ~~~python | ||
| print(1) | ||
| ~~~ | ||
|
|
||
| indented code blocks are not fenced code blocks and are ignored |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Preserve intentional parser coverage while satisfying Markdownlint.
Both fixtures use intentional non-default Markdown syntax that the repository's Markdownlint configuration rejects. Keep the test cases, but add scoped exceptions for the exact rules.
crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md#L7-L11: Suppress MD048 and MD046 around the tilde-fence and indented-code examples.crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/invalid.md#L7-L9: Suppress MD048 around the tilde-fence example.
🧰 Tools
🪛 GitHub Check: lint
[failure] 11-11: Code block style
crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md:11 MD046/code-block-style Code block style [Expected: fenced; Actual: indented] https://github.com/DavidAnson/markdownlint/blob/v0.40.0/doc/md046.md
[failure] 7-7: Code fence style
crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md:7 MD048/code-fence-style Code fence style [Expected: backtick; Actual: tilde] https://github.com/DavidAnson/markdownlint/blob/v0.40.0/doc/md048.md
📍 Affects 2 files
crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md#L7-L11(this comment)crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/invalid.md#L7-L9
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md`
around lines 7 - 11, Preserve both intentional parser fixtures while adding
scoped Markdownlint exceptions: in
crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/valid.md
lines 7-11, suppress MD048 and MD046 around the tilde-fence and indented-code
examples; in
crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/invalid.md
lines 7-9, suppress MD048 around the tilde-fence example.
Sources: Linters/SAST tools, Pipeline failures
There was a problem hiding this comment.
Addressed by excluding Markdown analyzer fixtures from repository Markdownlint, consistent with the existing parser and formatter fixture exclusions.
I did not add Markdownlint control comments because these files are rule inputs; the comments would change the parsed document and snapshots.
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
We usually don't add options at the first try, so please explain in PR description the reasons why they were added.
Also, I suggest you becoming familiar with the contribution guide of the analyser. The docs you submitted are incorrect.
I didn't review the code yet
| /// Rules from [markdownlint](https://github.com/DavidAnson/markdownlint) | ||
| Markdownlint(&'a str), |
There was a problem hiding this comment.
Reverted. This branch now uses the shared RuleSource::MarkdownLint implementation added in #11265 instead of adding a separate Markdownlint variant.
| use biome_rule_options::use_fenced_code_language::UseFencedCodeLanguageOptions; | ||
|
|
||
| declare_lint_rule! { | ||
| /// Enforce that fenced code blocks specify a language. |
There was a problem hiding this comment.
| /// Enforce that fenced code blocks specify a language. | |
| /// Enforce that fenced code blocks specify a code tag (language). |
There was a problem hiding this comment.
Applied. The rule summary now uses “code tag (language)”.
| declare_lint_rule! { | ||
| /// Enforce that fenced code blocks specify a language. | ||
| /// | ||
| /// Fenced code blocks without a language aren't syntax-highlighted when rendered to HTML, |
There was a problem hiding this comment.
This is wrong. A transformed code necessarily has this. Syntax highlighting is only supported if the client supports it.
Rephrase it
There was a problem hiding this comment.
Rephrased. The documentation now states that renderers can use a code tag to select syntax highlighting when they support the language.
| /// Enforce that fenced code blocks specify a language. | ||
| /// | ||
| /// Fenced code blocks without a language aren't syntax-highlighted when rendered to HTML, | ||
| /// which makes the code harder to read. Use `text` for snippets that are intentionally |
There was a problem hiding this comment.
This isn't correct either. The use of text should be used also when the syntax highlighting doesn't support a language
There was a problem hiding this comment.
Updated. The documentation now recommends text when syntax highlighting is unwanted or unsupported for the snippet’s language.
| /// which makes the code harder to read. Use `text` for snippets that are intentionally | ||
| /// plain text. | ||
| /// | ||
| /// This rule is a port of markdownlint's [`MD040`](https://github.com/DavidAnson/markdownlint/blob/main/doc/md040.md). |
There was a problem hiding this comment.
Removed. The sources metadata records the markdownlint relationship.
There was a problem hiding this comment.
Remove this. Markdown isn't even in production yet
|
@ematipico put both this PR and #11266 back to being drafts, so I can work on them more and read the contribution guide for analyser more thoroughly, I'll ping you as soon as both are ready again, sorry for any hassle caused and thank you for the review |
3d8ee7b to
765b5f2
Compare
|
@ematipico tried addressing all the review comments |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_markdown_parser/src/syntax/fenced_code_block.rs (1)
235-241: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an EOF regression fixture for the new boundary.
The current fixtures cover an opening fence followed by a physical newline. They do not cover an opening fence whose info string reaches EOF without a final newline. Add an
okfixture for this case to exercisep.at(T![EOF]).As per coding guidelines, parser changes must include appropriate tests and bug fixes must include a reproducing regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_markdown_parser/src/syntax/fenced_code_block.rs` around lines 235 - 241, Add an ok parser fixture for a fenced code block whose opening fence info string reaches EOF without a trailing physical newline, ensuring the input exercises the p.at(T![EOF]) branch in the fenced-code parsing flow. Keep the fixture minimal and consistent with the existing opening-fence fixtures.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/biome_markdown_parser/src/syntax/fenced_code_block.rs`:
- Around line 235-241: Add an ok parser fixture for a fenced code block whose
opening fence info string reaches EOF without a trailing physical newline,
ensuring the input exercises the p.at(T![EOF]) branch in the fenced-code parsing
flow. Keep the fixture minimal and consistent with the existing opening-fence
fixtures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b16c1807-e4dd-4043-a9ef-8e8210cde0f1
⛔ Files ignored due to path filters (10)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/allowedLanguages.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/combined.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/invalid.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/languageOnly.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_parser/tests/md_test_suite/ok/fenced_code_whitespace_info.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_parser/tests/md_test_suite/ok/fenced_code_whitespace_info_nested.md.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (9)
.github/workflows/pull_request_markdown.ymlcrates/biome_markdown_analyze/src/lint/nursery/use_fenced_code_language.rscrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/allowedLanguages.mdcrates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/languageOnly.mdcrates/biome_markdown_parser/src/syntax/fenced_code_block.rscrates/biome_markdown_parser/tests/md_test_suite/ok/fenced_code_whitespace_info.mdcrates/biome_markdown_parser/tests/md_test_suite/ok/fenced_code_whitespace_info_nested.mdcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_fenced_code_language.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/languageOnly.md
- crates/biome_markdown_analyze/tests/specs/nursery/useFencedCodeLanguage/allowedLanguages.md
- crates/biome_rule_options/src/use_fenced_code_language.rs
- crates/biome_rule_options/src/lib.rs
- crates/biome_markdown_analyze/src/lint/nursery/use_fenced_code_language.rs
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
The change in the parser goes against the spec. Tabs and whitespaces should be trimmed. A solution you could consider is to move those whitespaces into trivia. https://spec.commonmark.org/0.31.2/#fenced-code-blocks
|
@ematipico Done, I moved the leading tabs and whitespace into trivia, so the parsed content is trimmed per the spec. |
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
Please rebase. There are changes that are already in main
| let allowed_languages = options | ||
| .allowed_languages | ||
| .as_deref() | ||
| .unwrap_or_default() | ||
| .iter() | ||
| .map(|language| format!("\"{language}\"")) | ||
| .collect::<Vec<_>>() | ||
| .join(", "); |
There was a problem hiding this comment.
The RuleDiagnostic type already has an API to print a list, look for it. Some lint rules already use it
There was a problem hiding this comment.
Switched to RuleDiagnostic::footer_list, matching the pattern in useValidLang and regenerated the snapshots
a1cf994 to
87f7a66
Compare
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
The parsing is a bit brittle. Needs more cases, and the docs aren't correct.
Also I suspect you're using a coding agent. If so, disclose it
| m.complete(p, MD_CODE_NAME_LIST); | ||
| } | ||
|
|
||
| /// Move a run of spaces and tabs at the current position into the trivia list. |
|
|
||
| /// Move a run of spaces and tabs at the current position into the trivia list. | ||
| /// | ||
| /// CommonMark §4.5 trims the info string of leading and trailing whitespace, so |
There was a problem hiding this comment.
The wording isn't correct. The trimming happens during rendering into ast or html
| /// context first, which stops only at the end of the line: bumping a token | ||
| /// reverts the lexer to the regular context, where the remainder of an info |
There was a problem hiding this comment.
You can bump a token with a specific context, so you don't need to relex every single time
| } | ||
| } | ||
|
|
||
| /// Bump the info string's content, leaving a trailing run of spaces and tabs |
There was a problem hiding this comment.
Remove "run", use a better term
| /// Bump the info string's content, leaving a trailing run of spaces and tabs | ||
| /// behind for [`skip_info_string_whitespace`] to move into the trivia list. | ||
| /// | ||
| /// Callers must run [`skip_info_string_whitespace`] first, so the token starts |
There was a problem hiding this comment.
Run? The comment is even wrong. You're calling this function after...
| MdTextual { | ||
| value_token: MD_TEXTUAL_LITERAL@35..39 "js" [Whitespace(" ")] [], | ||
| }, |
There was a problem hiding this comment.
Can please expand the test. Add more spaces, and also add spaces after the info string
| /// string splits on construct characters and a run of trailing spaces can even | ||
| /// swallow the line's newline as a hard line break. | ||
| fn skip_info_string_whitespace(p: &mut MarkdownParser) { | ||
| loop { |
There was a problem hiding this comment.
These kinds of loops are prohibited in the parser, because the parsing could get stuck.
Either use a while loop, or use the progression check.
2 participants
Summary
Closes #11264
Adds the recommended Markdown nursery rule
useFencedCodeLanguage, based on markdownlint MD040. The rule reports fenced code blocks without a code tag (language). It supports MD040’s establishedallowedLanguagesandlanguageOnlyoptions to preserve configuration compatibility and ease migration.The rule is recommended because missing language tags have a low false-positive rate, while authors can explicitly use
textwhen syntax highlighting is unwanted or unsupported.The Markdown parser now keeps whitespace from an otherwise empty opening-fence info string in
MdCodeNameListinstead of treating it as code content. This allows the rule to report whitespace-only opening fences as missing a language tag, including fences nested in block quotes and lists.Markdown analyzer fixtures are excluded from repository Markdownlint because they intentionally contain invalid and non-default Markdown used as analyzer input, consistently with the existing parser and formatter fixture exclusions.
No changeset is included because Markdown linting has not been released.
This PR was implemented with the help of Codex.
Test Plan
Added analyzer tests and snapshots covering missing and valid language tags, whitespace-only info strings, nested and unterminated fences, and the individual and combined behavior of
allowedLanguagesandlanguageOnly.Added parser fixtures covering whitespace-only info strings at the document level and inside block quotes and lists.
Validated with:
cargo test -p biome_markdown_analyzecargo test -p biome_markdown_parserjust lint-rulesjust lDocs
Updated the inline rule documentation to follow the analyzer documentation structure: added the options introduction and placed each option’s default before its configuration and applied examples.