feat: useTopLevelHeading - #11286
Conversation
|
✅ Organic activityNo automation signals detected in the analyzed events. This is an automated analysis by AgentScan |
|
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 Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds the nursery Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/biome_markdown_syntax/src/header_ext.rs (1)
11-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd rustdoc for the new public rule API.
Document the public methods and options type.
crates/biome_markdown_syntax/src/header_ext.rs#L11-L21: Add contracts foris_level_1andis_level_2.crates/biome_rule_options/src/use_top_level_heading.rs#L3-L6: DocumentUseTopLevelHeadingOptionsas the configuration type foruseTopLevelHeading.As per coding guidelines, use rustdoc for new features and rule options.
🤖 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_syntax/src/header_ext.rs` around lines 11 - 21, Add rustdoc contracts above MdSetextHeader::is_level_1 and MdSetextHeader::is_level_2 in crates/biome_markdown_syntax/src/header_ext.rs (lines 11-21), and document UseTopLevelHeadingOptions as the configuration type for useTopLevelHeading in crates/biome_rule_options/src/use_top_level_heading.rs (lines 3-6); no other sites require changes.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/breezy-lines-peel.md:
- Line 5: Correct the Markdown link for the useTopLevelHeading rule in the
changeset so it points to that rule’s documentation page rather than
no-invalid-property-init-value, while preserving the existing rule text and
changeset format.
In
`@crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/heading-1.md`:
- Line 2: Update the headings in heading-1.md (line 2) and setext-heading-1.md
(line 2) to use “Top-level heading” consistently, replacing “Top level heading”
in both fixtures.
In
`@crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/yaml.md`:
- Line 8: Update the heading in the useTopLevelHeading YAML fixture from ATX
syntax to the repository’s setext heading style required by MD003, while
preserving its level and heading text.
---
Nitpick comments:
In `@crates/biome_markdown_syntax/src/header_ext.rs`:
- Around line 11-21: Add rustdoc contracts above MdSetextHeader::is_level_1 and
MdSetextHeader::is_level_2 in crates/biome_markdown_syntax/src/header_ext.rs
(lines 11-21), and document UseTopLevelHeadingOptions as the configuration type
for useTopLevelHeading in crates/biome_rule_options/src/use_top_level_heading.rs
(lines 3-6); no other sites require changes.
🪄 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
Run ID: 1d551b31-45da-4d13-a297-79d5c9de940b
⛔ Files ignored due to path filters (11)
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/useTopLevelHeading/invalid/heading-2.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/invalid/paragraph.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/invalid/setext-heading-2.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/heading-1.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/html.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/setext-heading-1.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/yaml.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/breezy-lines-peel.mdcrates/biome_markdown_analyze/src/lint/nursery/use_top_level_heading.rscrates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/invalid/heading-2.mdcrates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/invalid/paragraph.mdcrates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/invalid/setext-heading-2.mdcrates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/heading-1.mdcrates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/html.mdcrates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/setext-heading-1.mdcrates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/valid/yaml.mdcrates/biome_markdown_syntax/src/header_ext.rscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_top_level_heading.rsjustfile
Merging this PR will not alter performance
Comparing Footnotes
|
61d494a to
fdefa0e
Compare
| /// Leading HTML comments used as file preamble are ignored when determining | ||
| /// the first meaningful block. |
There was a problem hiding this comment.
If the markdown files starts with a comment, like we do in test files, we skip the comment to check what the next block is
There was a problem hiding this comment.
This seems like an implementation detail, doesn't it? Or, if not, reword it? "Meaningful block" is very technical. What's a meaningful block? That's not something I would write to our end users
| }, | ||
| ) | ||
| .note(markup! { | ||
| "The first block should be a top-level heading (h1). Add a "<Emphasis>"# Heading"</Emphasis>" (or a level-1 setext heading) to the start of the document." |
There was a problem hiding this comment.
The second rule pillar is missing. Or if there is, it should be reworded
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_markdown_syntax/src/block_ext.rs (1)
116-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new public helper.
MdHtmlBlock::is_html_commentis a new public API used by the Markdown rule. Add///rustdoc that defines the accepted delimiters and explains why missingcontent()orvalue_token()returnsfalse.As per coding guidelines, use rustdoc for new rule-related Rust features.
🤖 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_syntax/src/block_ext.rs` around lines 116 - 126, Add rustdoc to the public MdHtmlBlock::is_html_comment helper documenting that it recognizes content beginning with the HTML comment delimiter "<!--" and returns false when content() or value_token() is unavailable. Keep the implementation behavior unchanged.Source: Coding guidelines
xtask/codegen/src/generate_new_analyzer_rule.rs (1)
588-590: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the generated Markdown template.
Add or verify a focused test for
generate_rule_templatethat checks the Markdown example emitsmd, notmarkdown. If the output uses snapshots, update them with the repository'scargo instaworkflow.As per coding guidelines, all Rust code changes must include appropriate tests.
🤖 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 `@xtask/codegen/src/generate_new_analyzer_rule.rs` around lines 588 - 590, Add or update a focused test for generate_rule_template that verifies the generated Markdown example uses the md language tag rather than markdown. If this output is snapshot-tested, refresh the relevant snapshot using the repository’s cargo insta workflow.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_syntax/src/block_ext.rs`:
- Around line 116-126: Add rustdoc to the public MdHtmlBlock::is_html_comment
helper documenting that it recognizes content beginning with the HTML comment
delimiter "<!--" and returns false when content() or value_token() is
unavailable. Keep the implementation behavior unchanged.
In `@xtask/codegen/src/generate_new_analyzer_rule.rs`:
- Around line 588-590: Add or update a focused test for generate_rule_template
that verifies the generated Markdown example uses the md language tag rather
than markdown. If this output is snapshot-tested, refresh the relevant snapshot
using the repository’s cargo insta workflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1004c3b1-b92b-4db9-8b9c-94f90a60c0fe
⛔ Files ignored due to path filters (3)
crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/invalid/heading-2.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/invalid/paragraph.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_analyze/tests/specs/nursery/useTopLevelHeading/invalid/setext-heading-2.md.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_markdown_analyze/src/lint/nursery/use_top_level_heading.rscrates/biome_markdown_syntax/src/block_ext.rsxtask/codegen/src/generate_new_analyzer_rule.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_markdown_analyze/src/lint/nursery/use_top_level_heading.rs
| html_block.is_html_comment() | ||
| } | ||
| AnyMdBlock::AnyMdLeafBlock(AnyMdLeafBlock::MdParagraph(paragraph)) => { | ||
| let text = paragraph.syntax().text_trimmed().to_string(); |
Summary
Added Markdownlint's MD041 which requires markdown documents to start with a top-level heading (heading 1)
One thing that's different tho, is that markdownlint checks in the HTML block if the first tag is
<h1>, which would require some embedding (for now, I've marked a html block as fine)Closes #11263
Test Plan
Unit tests
Docs