feat: implement noEmptyDocumentation#10346
Conversation
🦋 Changeset detectedLatest commit: c47eac6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
da7e3d2 to
8fe003c
Compare
Merging this PR will degrade performance by 41.5%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | css_analyzer[bulma_5641719244145477318.css] |
1.2 s | 1.3 s | -9.68% |
| ❌ | css_analyzer[foundation_11602414662825430680.css] |
236.7 ms | 266.5 ms | -11.16% |
| ❌ | css_analyzer[pure_9395922602181450299.css] |
23.3 ms | 26.9 ms | -13.37% |
| ❌ | css_analyzer[tachyons_11778168428173736564.css] |
177.4 ms | 200.9 ms | -11.67% |
| ❌ | css_analyzer[bootstrap_18416142857265205439.css] |
576.8 ms | 618.7 ms | -6.77% |
| ❌ | html_analyzer[real/wikipedia-Unix.html] |
164.6 ms | 191 ms | -13.81% |
| ❌ | html_analyzer[index_1033418810622582172.html] |
472.9 µs | 535.7 µs | -11.72% |
| ❌ | html_analyzer[real/wikipedia-fr-Guerre_de_Canudos.html] |
447.6 ms | 539.9 ms | -17.09% |
| ❌ | html_analyzer[real/wikipedia-JavaScript.html] |
187.6 ms | 220.7 ms | -15% |
| ❌ | json_analyzer[big5-added_15586211152145260264.json] |
463.3 µs | 732.8 µs | -36.78% |
| ❌ | json_analyzer[package-lock_11240515041603712709.json] |
28.6 ms | 33.8 ms | -15.38% |
| ❌ | json_analyzer[eucjp_1600564308684076393.json] |
767 µs | 1,111 µs | -30.96% |
| ❌ | json_analyzer[canada_639262044961882324.json] |
165.7 ms | 283.2 ms | -41.5% |
Comparing Netail:feat/no-empty-documentation (c47eac6) with main (8d19e7f)
Footnotes
-
70 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
4f7b1fc to
2dbc91b
Compare
WalkthroughAdds a new nursery lint rule, noEmptyDocumentation, across CSS, GraphQL, HTML, JavaScript and JSONC. Introduces a shared options type and a public LeadingCommentTriviaPieces iterator, implements language-specific rules that collect leading comment trivia, detect empty documentation, dedupe ranges and emit a single diagnostic with detail entries, and adds valid/invalid test fixtures plus a changeset documenting the patch release. Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/wise-hoops-dream.md:
- Around line 5-32: Trim the changeset to 1–3 user-facing sentences describing
the addition of the new nursery rule `noEmptyDocumentation`; remove all embedded
code examples and implementation details, and replace the body with a concise
note such as: "Added the nursery rule `noEmptyDocumentation` for HTML, CSS,
JavaScript, JSON, and GraphQL. Biome now reports diagnostics for empty
documentation comments/descriptions to reduce noise and improve readability."
include the rule link
(`https://biomejs.dev/linter/rules/no-empty-documentation/`) if you want a
reference.
In `@crates/biome_css_analyze/src/lint/nursery/no_empty_documentation.rs`:
- Around line 105-111: The trailing_comments function is incorrectly using
leading_trivia() on the last token, capturing comments before the token; change
the call in trailing_comments (the code that obtains trivia from token returned
by SyntaxNode::last_token()) from .leading_trivia() to .trailing_trivia() so it
collects comments after the token (consistent with other language
implementations).
In `@crates/biome_graphql_analyze/src/lint/nursery/no_empty_documentation.rs`:
- Around line 147-153: The trailing_comments function currently checks
trailing_trivia() on syntax_node.first_token(), which can miss comments on the
final token; change the lookup to syntax_node.last_token() and update any
variable handling accordingly so you collect trailing_trivia().pieces() from the
node's last token (still returning Vec<SyntaxTriviaPiece<GraphqlLanguage>>) and
preserve behavior when there is no last_token().
In `@crates/biome_html_analyze/src/lint/nursery/no_empty_documentation.rs`:
- Around line 107-113: In trailing_comments, switch from reading trivia on the
node's first_token() to its last_token() so trailing comments attached to the
final token (e.g., near EOF) are captured; update the token lookup in the
trailing_comments function (currently using SyntaxNode::first_token) to use
SyntaxNode::last_token and keep the rest of the logic that calls
token.trailing_trivia().pieces() unchanged.
In `@crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rs`:
- Around line 130-137: The trailing_comments function currently calls
syntax_node.first_token() which misses true trailing trivia on the node; change
the token lookup to syntax_node.last_token() in trailing_comments so you collect
trailing_trivia() from the node's final token (preserve the existing
trailing_trivia().pieces().filter(...).collect() flow). Update the function
signature/logic in trailing_comments to mirror the pattern used in
use_sorted_interface_members.rs so end-of-line comments (e.g., "let x = 1; //
comment") are included.
In `@crates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rs`:
- Around line 126-135: The trailing_comments function currently reads trailing
trivia from syntax_node.first_token(), which misses comments at the end of the
node; change it to use syntax_node.last_token() so trailing_trivia() is
retrieved from the node's last token (mirror the leading_comments approach).
Update the function trailing_comments (signature trailing_comments(syntax_node:
&SyntaxNode<JsonLanguage>) -> Vec<SyntaxTriviaPiece<JsonLanguage>>) to call
last_token(), then .trailing_trivia().pieces().filter(|piece|
piece.is_comments()).collect() and keep the existing else branch.
🪄 Autofix (Beta)
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: d10789d5-6894-4358-b198-18e45ca326b7
⛔ Files ignored due to path filters (15)
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_css_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/noEmptyDocumentation/valid.css.snapis excluded by!**/*.snapand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/valid.graphql.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noEmptyDocumentation/valid.html.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jsonc.snapis excluded by!**/*.snapand included by**crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/valid.jsonc.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (18)
.changeset/wise-hoops-dream.mdcrates/biome_css_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_css_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.csscrates/biome_css_analyze/tests/specs/nursery/noEmptyDocumentation/valid.csscrates/biome_graphql_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/valid.graphqlcrates/biome_html_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_html_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.htmlcrates/biome_html_analyze/tests/specs/nursery/noEmptyDocumentation/valid.htmlcrates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jscrates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/valid.jscrates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jsonccrates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/valid.jsonccrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_empty_documentation.rs
| Added the new nursery rule [`noEmptyDocumentation`](https://biomejs.dev/linter/rules/no-empty-documentation), which disallows empty documentation (comments & descriptions) in HTML, CSS, JavaScript, JSON & GraphQL. | ||
|
|
||
| ```html | ||
| <!-- --> | ||
| <input/> | ||
| ``` | ||
|
|
||
| ```css | ||
| /* */ | ||
| .invalid {} | ||
| ``` | ||
|
|
||
| ```js | ||
| /* */ | ||
| let invalid = 1; | ||
| ``` | ||
|
|
||
| ```jsonc | ||
| { | ||
| /* */ | ||
| "name": "John Doe" | ||
| } | ||
| ``` | ||
|
|
||
| ```graphql | ||
| " " | ||
| query {} | ||
| ``` |
There was a problem hiding this comment.
Trim the changeset entry to user-facing release notes only.
This is currently much longer than a changeset needs to be (the embedded examples are overkill here). Keep it to 1–3 concise sentences.
Suggested edit
-Added the new nursery rule [`noEmptyDocumentation`](https://biomejs.dev/linter/rules/no-empty-documentation), which disallows empty documentation (comments & descriptions) in HTML, CSS, JavaScript, JSON & GraphQL.
-
-```html
-<!-- -->
-<input/>
-```
-
-```css
-/* */
-.invalid {}
-```
-
-```js
-/* */
-let invalid = 1;
-```
-
-```jsonc
-{
- /* */
- "name": "John Doe"
-}
-```
-
-```graphql
-" "
-query {}
-```
+Added the nursery rule [`noEmptyDocumentation`](https://biomejs.dev/linter/rules/no-empty-documentation/) for HTML, CSS, JavaScript, JSON, and GraphQL.
+Biome now reports diagnostics for empty documentation comments/descriptions to reduce noise and improve readability.As per coding guidelines: "Write changesets with focus on user-facing changes only, be concise (1-3 sentences)..."
📝 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 the new nursery rule [`noEmptyDocumentation`](https://biomejs.dev/linter/rules/no-empty-documentation), which disallows empty documentation (comments & descriptions) in HTML, CSS, JavaScript, JSON & GraphQL. | |
| ```html | |
| <!-- --> | |
| <input/> | |
| ``` | |
| ```css | |
| /* */ | |
| .invalid {} | |
| ``` | |
| ```js | |
| /* */ | |
| let invalid = 1; | |
| ``` | |
| ```jsonc | |
| { | |
| /* */ | |
| "name": "John Doe" | |
| } | |
| ``` | |
| ```graphql | |
| " " | |
| query {} | |
| ``` | |
| Added the nursery rule [`noEmptyDocumentation`](https://biomejs.dev/linter/rules/no-empty-documentation/) for HTML, CSS, JavaScript, JSON, and GraphQL. | |
| Biome now reports diagnostics for empty documentation comments/descriptions to reduce noise and improve readability. |
🤖 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/wise-hoops-dream.md around lines 5 - 32, Trim the changeset to
1–3 user-facing sentences describing the addition of the new nursery rule
`noEmptyDocumentation`; remove all embedded code examples and implementation
details, and replace the body with a concise note such as: "Added the nursery
rule `noEmptyDocumentation` for HTML, CSS, JavaScript, JSON, and GraphQL. Biome
now reports diagnostics for empty documentation comments/descriptions to reduce
noise and improve readability." include the rule link
(`https://biomejs.dev/linter/rules/no-empty-documentation/`) if you want a
reference.
| fn trailing_comments( | ||
| syntax_node: &SyntaxNode<GraphqlLanguage>, | ||
| ) -> Vec<SyntaxTriviaPiece<GraphqlLanguage>> { | ||
| if let Some(token) = syntax_node.first_token() { | ||
| token | ||
| .trailing_trivia() | ||
| .pieces() |
There was a problem hiding this comment.
Switch trailing trivia lookup to last_token().
Line 150 currently inspects trailing trivia on the first token, which can miss empty trailing comments on the final token and under-report diagnostics.
Suggested patch
fn trailing_comments(
syntax_node: &SyntaxNode<GraphqlLanguage>,
) -> Vec<SyntaxTriviaPiece<GraphqlLanguage>> {
- if let Some(token) = syntax_node.first_token() {
+ if let Some(token) = syntax_node.last_token() {
token
.trailing_trivia()
.pieces()
.filter(|piece| piece.is_comments())
.collect()📝 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.
| fn trailing_comments( | |
| syntax_node: &SyntaxNode<GraphqlLanguage>, | |
| ) -> Vec<SyntaxTriviaPiece<GraphqlLanguage>> { | |
| if let Some(token) = syntax_node.first_token() { | |
| token | |
| .trailing_trivia() | |
| .pieces() | |
| fn trailing_comments( | |
| syntax_node: &SyntaxNode<GraphqlLanguage>, | |
| ) -> Vec<SyntaxTriviaPiece<GraphqlLanguage>> { | |
| if let Some(token) = syntax_node.last_token() { | |
| token | |
| .trailing_trivia() | |
| .pieces() |
🤖 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_graphql_analyze/src/lint/nursery/no_empty_documentation.rs`
around lines 147 - 153, The trailing_comments function currently checks
trailing_trivia() on syntax_node.first_token(), which can miss comments on the
final token; change the lookup to syntax_node.last_token() and update any
variable handling accordingly so you collect trailing_trivia().pieces() from the
node's last token (still returning Vec<SyntaxTriviaPiece<GraphqlLanguage>>) and
preserve behavior when there is no last_token().
| fn trailing_comments(syntax_node: &SyntaxNode<JsLanguage>) -> Vec<SyntaxTriviaPiece<JsLanguage>> { | ||
| if let Some(token) = syntax_node.first_token() { | ||
| token | ||
| .trailing_trivia() | ||
| .pieces() | ||
| .filter(|piece| piece.is_comments()) | ||
| .collect() | ||
| } else { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rs | sed -n '120,150p'Repository: biomejs/biome
Length of output: 1275
🏁 Script executed:
# Also check the surrounding context and function signature
head -n 150 crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rs | tail -n 50Repository: biomejs/biome
Length of output: 1594
🏁 Script executed:
# Search for usage of trailing_comments function
rg "trailing_comments" crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rsRepository: biomejs/biome
Length of output: 214
🏁 Script executed:
rg -A5 -B5 "trailing_comments|no_empty_documentation" crates/biome_js_analyze/tests/ | head -100Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Search for test files related to no_empty_documentation
fd "no_empty_documentation" crates/biome_js_analyze/tests/Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Search for test files in the analyze directory more broadly
fd "test" crates/biome_js_analyze/tests/ | grep -i "no_empty\|nursery" | head -20Repository: biomejs/biome
Length of output: 1825
🏁 Script executed:
# Check the structure of test directories
ls -la crates/biome_js_analyze/tests/ 2>/dev/null | head -20Repository: biomejs/biome
Length of output: 547
🏁 Script executed:
# Look for snapshot test files
fd ".*\.rs" crates/biome_js_analyze/tests/ | head -10Repository: biomejs/biome
Length of output: 145
🏁 Script executed:
# Look for no_empty_documentation test specs
find crates/biome_js_analyze/tests/specs -type d -name "*noEmpty*"Repository: biomejs/biome
Length of output: 526
🏁 Script executed:
# Check nursery directory for no_empty_documentation
ls -la crates/biome_js_analyze/tests/specs/nursery/ | grep -i emptyRepository: biomejs/biome
Length of output: 129
🏁 Script executed:
# Let's look at the entire nursery test structure
ls crates/biome_js_analyze/tests/specs/nursery/ | grep -i "no" | head -20Repository: biomejs/biome
Length of output: 483
🏁 Script executed:
# Look at the test files for noEmptyDocumentation
ls -la crates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/Repository: biomejs/biome
Length of output: 427
🏁 Script executed:
# Read the test cases
cat crates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/*.js | head -80Repository: biomejs/biome
Length of output: 295
🏁 Script executed:
# Let's look at the exact structure of the rule to understand context
cat crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rs | head -100Repository: biomejs/biome
Length of output: 2705
🏁 Script executed:
# Look for where trailing_comments is called
grep -B10 -A10 "trailing_comments(syntax_node)" crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rsRepository: biomejs/biome
Length of output: 572
🏁 Script executed:
# Let's check the full implementation of the run method
sed -n '70,100p' crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rsRepository: biomejs/biome
Length of output: 972
🏁 Script executed:
# Let's verify the semantics by checking how Rowan syntax trees work
# Let's look at how leading_comments and trailing_comments are used in the context
sed -n '110,140p' crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rsRepository: biomejs/biome
Length of output: 881
🏁 Script executed:
# Let's create a simple test case to understand the issue better
# Check if there are any examples in the codebase of trailing comments on the same line
grep -r "trailing_trivia\|last_token" crates/biome_js_analyze/src/ | head -20Repository: biomejs/biome
Length of output: 3046
Use last_token() instead of first_token() to capture trailing comments.
The function currently reads trailing trivia from the first token, which only captures comments between the first and second tokens. Trailing comments at the end of a node (e.g., let x = 1; //comment) are missed entirely. The codebase uses this exact pattern elsewhere—see use_sorted_interface_members.rs for reference.
Suggested fix
fn trailing_comments(syntax_node: &SyntaxNode<JsLanguage>) -> Vec<SyntaxTriviaPiece<JsLanguage>> {
- if let Some(token) = syntax_node.first_token() {
+ if let Some(token) = syntax_node.last_token() {
token
.trailing_trivia()
.pieces()
.filter(|piece| piece.is_comments())
.collect()📝 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.
| fn trailing_comments(syntax_node: &SyntaxNode<JsLanguage>) -> Vec<SyntaxTriviaPiece<JsLanguage>> { | |
| if let Some(token) = syntax_node.first_token() { | |
| token | |
| .trailing_trivia() | |
| .pieces() | |
| .filter(|piece| piece.is_comments()) | |
| .collect() | |
| } else { | |
| fn trailing_comments(syntax_node: &SyntaxNode<JsLanguage>) -> Vec<SyntaxTriviaPiece<JsLanguage>> { | |
| if let Some(token) = syntax_node.last_token() { | |
| token | |
| .trailing_trivia() | |
| .pieces() | |
| .filter(|piece| piece.is_comments()) | |
| .collect() | |
| } else { |
🤖 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_js_analyze/src/lint/nursery/no_empty_documentation.rs` around
lines 130 - 137, The trailing_comments function currently calls
syntax_node.first_token() which misses true trailing trivia on the node; change
the token lookup to syntax_node.last_token() in trailing_comments so you collect
trailing_trivia() from the node's final token (preserve the existing
trailing_trivia().pieces().filter(...).collect() flow). Update the function
signature/logic in trailing_comments to mirror the pattern used in
use_sorted_interface_members.rs so end-of-line comments (e.g., "let x = 1; //
comment") are included.
| fn trailing_comments( | ||
| syntax_node: &SyntaxNode<JsonLanguage>, | ||
| ) -> Vec<SyntaxTriviaPiece<JsonLanguage>> { | ||
| if let Some(token) = syntax_node.first_token() { | ||
| token | ||
| .trailing_trivia() | ||
| .pieces() | ||
| .filter(|piece| piece.is_comments()) | ||
| .collect() | ||
| } else { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n crates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rs | sed -n '120,145p'Repository: biomejs/biome
Length of output: 921
🏁 Script executed:
rg "trailing_comments" crates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rs -A 3 -B 1Repository: biomejs/biome
Length of output: 369
🏁 Script executed:
cat -n crates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rs | sed -n '110,130p'Repository: biomejs/biome
Length of output: 725
🏁 Script executed:
rg "no_empty_documentation" crates/biome_json_analyze/src -lRepository: biomejs/biome
Length of output: 126
🏁 Script executed:
find crates/biome_json_analyze -name "*no_empty_documentation*" -o -name "*test*" -type f | head -20Repository: biomejs/biome
Length of output: 172
🏁 Script executed:
find . -path "*/tests/*" -name "*no_empty_documentation*" -o -path "*/json/*" -name "*no_empty_documentation*" 2>/dev/null | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
find . -type d -name "json" | grep testRepository: biomejs/biome
Length of output: 328
🏁 Script executed:
find crates/biome_json_analyze/tests -type d -name "*no_empty_documentation*" -o -type f -name "*no_empty_documentation*"Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
find crates/biome_json_analyze/tests -type d | head -20Repository: biomejs/biome
Length of output: 1635
🏁 Script executed:
ls -la crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/Repository: biomejs/biome
Length of output: 421
🏁 Script executed:
cat crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jsoncRepository: biomejs/biome
Length of output: 106
🏁 Script executed:
cat crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jsonc.snapRepository: biomejs/biome
Length of output: 864
🏁 Script executed:
cat -n crates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rs | head -80Repository: biomejs/biome
Length of output: 2801
🏁 Script executed:
cat -n crates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rs | sed -n '80,110p'Repository: biomejs/biome
Length of output: 1040
🏁 Script executed:
cat -n crates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rs | sed -n '138,170p'Repository: biomejs/biome
Length of output: 1210
🏁 Script executed:
cat crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/valid.jsoncRepository: biomejs/biome
Length of output: 125
Use last_token() for trailing_comments to capture end-of-node comments.
The function at line 129 reads trailing trivia from first_token(), which only captures comments between the first and second token. For trailing comments that come after the node, use last_token() instead. This mirrors the leading_comments function logic properly: leading reads from first token, trailing should read from last token.
Suggested fix
fn trailing_comments(
syntax_node: &SyntaxNode<JsonLanguage>,
) -> Vec<SyntaxTriviaPiece<JsonLanguage>> {
- if let Some(token) = syntax_node.first_token() {
+ if let Some(token) = syntax_node.last_token() {
token
.trailing_trivia()
.pieces()
.filter(|piece| piece.is_comments())
.collect()🤖 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_json_analyze/src/lint/nursery/no_empty_documentation.rs` around
lines 126 - 135, The trailing_comments function currently reads trailing trivia
from syntax_node.first_token(), which misses comments at the end of the node;
change it to use syntax_node.last_token() so trailing_trivia() is retrieved from
the node's last token (mirror the leading_comments approach). Update the
function trailing_comments (signature trailing_comments(syntax_node:
&SyntaxNode<JsonLanguage>) -> Vec<SyntaxTriviaPiece<JsonLanguage>>) to call
last_token(), then .trailing_trivia().pieces().filter(|piece|
piece.is_comments()).collect() and keep the existing else branch.
ematipico
left a comment
There was a problem hiding this comment.
The business logic of the rule can be shared in one single type, maybe in the analyser, or rule options crates.
As for the solution, create a struct that implements Iterator and return SyntaxTokenKind as Item, so that callers can use the filter function, which you're using in each different crate.
I would also revisit docs and business logic. I believe trailing comments shouldn't be part of the logic, however if we do think we should, we should add tests because at the moment there are none.
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added the new nursery rule [`noEmptyDocumentation`](https://biomejs.dev/linter/rules/no-empty-documentation), which disallows empty documentation (comments & descriptions) in HTML, CSS, JavaScript, JSON & GraphQL. |
There was a problem hiding this comment.
| Added the new nursery rule [`noEmptyDocumentation`](https://biomejs.dev/linter/rules/no-empty-documentation), which disallows empty documentation (comments & descriptions) in HTML, CSS, JavaScript, JSON & GraphQL. | |
| Added the new nursery rule [`noEmptyDocumentation`](https://biomejs.dev/linter/rules/no-empty-documentation), which disallows empty documentation (comments & descriptions) in HTML, CSS, JavaScript, JSONC & GraphQL. |
By default JSON doesn't support comments
| token | ||
| .leading_trivia() | ||
| .pieces() | ||
| .filter(|piece| piece.is_comments()) |
There was a problem hiding this comment.
In JavaScript we have the concept of multiline comments. I suggest using that to filter trivia
There was a problem hiding this comment.
// Hello
//
// Something else, but part of the full block
Something like this you mean?
There was a problem hiding this comment.
No.
// -> single line comment
/***/ -> multi line comment
Then again, it's all part of the business logic of the function and the misalignment between comment and documentation
There was a problem hiding this comment.
Ahh okay, but that one I got covered, both /* */ & /** */
| } | ||
| } | ||
|
|
||
| fn trailing_comments(syntax_node: &SyntaxNode<CssLanguage>) -> Vec<SyntaxTriviaPiece<CssLanguage>> { |
There was a problem hiding this comment.
Generally these kinds of comments are leading comments, so I would exclude trailing comments.
If you found trialing comments that match the criteria, add them in the tests
| /// Disallow empty documentation. | ||
| /// | ||
| /// Enforces that comments are not empty. This helps maintain code quality by preventing meaningless | ||
| /// or placeholder comments that don't provide any documentation value. |
There was a problem hiding this comment.
Empty comments and empty documentation are different though, semantically speaking. The rule swaps their meaning, which isn't correct
let foo; // this is a comment
/** This is docs **/
let bar;There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rs (1)
121-132: ⚡ Quick winSimplify redundant conditional logic.
Both branches of each
if let Some(stripped) = stripped.strip_suffix("*/")perform identical operations, making the conditionals unnecessary. You can directly pass the result ofstrip_prefixtois_empty_block_comment_content.♻️ Proposed simplification
- } else if let Some(stripped) = text.strip_prefix("/**") { - if let Some(stripped) = stripped.strip_suffix("*/") { - return is_empty_block_comment_content(stripped); - } else { - return is_empty_block_comment_content(stripped); - } - } else if let Some(stripped) = text.strip_prefix("/*") { - if let Some(stripped) = stripped.strip_suffix("*/") { - return is_empty_block_comment_content(stripped); - } else { - return is_empty_block_comment_content(stripped); - } + } else if let Some(stripped) = text.strip_prefix("/**").or_else(|| text.strip_prefix("/*")) { + let stripped = stripped.strip_suffix("*/").unwrap_or(stripped); + return is_empty_block_comment_content(stripped); }🤖 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_js_analyze/src/lint/nursery/no_empty_documentation.rs` around lines 121 - 132, The nested `if let` branches are redundant; when handling `text.strip_prefix("/**")` and `text.strip_prefix("/*")` drop the inner conditional and normalize the suffix with `strip_suffix("*/").unwrap_or(stripped)` (or equivalent map/unwrap) then call `is_empty_block_comment_content` once. In other words, for each branch that matches `strip_prefix` on `text`, compute a single `stripped` that falls back to itself if `strip_suffix("*/")` is None, and pass that `stripped` to `is_empty_block_comment_content`.
🤖 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_js_analyze/src/lint/nursery/no_empty_documentation.rs`:
- Around line 121-132: The nested `if let` branches are redundant; when handling
`text.strip_prefix("/**")` and `text.strip_prefix("/*")` drop the inner
conditional and normalize the suffix with
`strip_suffix("*/").unwrap_or(stripped)` (or equivalent map/unwrap) then call
`is_empty_block_comment_content` once. In other words, for each branch that
matches `strip_prefix` on `text`, compute a single `stripped` that falls back to
itself if `strip_suffix("*/")` is None, and pass that `stripped` to
`is_empty_block_comment_content`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 93382003-db27-4442-b140-fae1027d332a
⛔ Files ignored due to path filters (10)
crates/biome_css_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/nursery/noEmptyDocumentation/valid.css.snapis excluded by!**/*.snapand included by**crates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/valid.graphql.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noEmptyDocumentation/valid.html.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jsonc.snapis excluded by!**/*.snapand included by**crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/valid.jsonc.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (18)
.changeset/wise-hoops-dream.mdcrates/biome_analyze/src/lib.rscrates/biome_analyze/src/trivia.rscrates/biome_css_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_css_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.csscrates/biome_css_analyze/tests/specs/nursery/noEmptyDocumentation/valid.csscrates/biome_graphql_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/valid.graphqlcrates/biome_html_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_html_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.htmlcrates/biome_html_analyze/tests/specs/nursery/noEmptyDocumentation/valid.htmlcrates/biome_js_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jscrates/biome_js_analyze/tests/specs/nursery/noEmptyDocumentation/valid.jscrates/biome_json_analyze/src/lint/nursery/no_empty_documentation.rscrates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jsonccrates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/valid.jsonc
✅ Files skipped from review due to trivial changes (2)
- crates/biome_json_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.jsonc
- .changeset/wise-hoops-dream.md
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_graphql_analyze/tests/specs/nursery/noEmptyDocumentation/invalid.graphql
Summary
Port stylelint's comments-no-empty and added support for other languages
Closes #7696
Test Plan
Unit tests
Docs