feat(linter): useSingleJsDocAsterisk #5844
Conversation
github-actions
bot
added
A-CLI
| }; | ||
| use biome_console::markup; | ||
| use biome_js_syntax::{JsModule, JsSyntaxToken}; | ||
| use biome_module_graph::JsdocComment; |
There was a problem hiding this comment.
I feel bit weird using function from module graph, despite not using module graph itself. Should it be moved somewhere else?
There was a problem hiding this comment.
Heh, sorry about that! I knew this moment would come 😅
I just didn’t know a better place to move it to and creating a whole crate for just the one struct felt like overkill. But yeah, it’s starting to make more sense now. @ematipico Would you know a better place for it, or shall we create a biome_jsdoc crate?
There was a problem hiding this comment.
A new crate is an option. I can propose two other options without creating a new crate:
- having a
comments_ext.rsinside thebiome_js_syntaxcrate - make jsdocs part of the semantic model crate (OXC does this)
Unfortunately I don't know how the JsDoc structs work under the hood, so take my suggestions with a grain of salt
There was a problem hiding this comment.
I think I'd lean towards a new crate still... @minht11 you can do it in this PR, but feel free to leave it, and I'll do it as a follow-up later.
Remove inaccurate comment add invalid examples
minht11
force-pushed
the
feat/useSingleJsdocAsterix
branch
from
517d1ba to
98be97b
Compare
CodSpeed Performance ReportMerging #5844 will not alter performanceComparing Summary
|
| "@biomejs/biome": minor | ||
| --- | ||
|
|
||
| Add [useSingleJsDocAsterisk](https://biomejs.dev/linter/rules/use-single-js-doc-asterisk/) |
There was a problem hiding this comment.
Could you add a short description here about what the rule does?
| text.lines() | ||
| .take(invalid_line.line_index) | ||
| .fold(0, |acc, line| { | ||
| acc + line.len() + 1 // +1 for the line break |
There was a problem hiding this comment.
This may break on Windows, since you may have a \r\n sequence instead of only a single \n.
In this case, it may be better to split at \n than to use .lines(), so you'll count the \r as a regular character. It's probably best to add some test cases around this.
| }; | ||
| use biome_console::markup; | ||
| use biome_js_syntax::{JsModule, JsSyntaxToken}; | ||
| use biome_module_graph::JsdocComment; |
There was a problem hiding this comment.
I think I'd lean towards a new crate still... @minht11 you can do it in this PR, but feel free to leave it, and I'll do it as a follow-up later.
| if char_is_whitespace(b) { | ||
| continue; | ||
| } | ||
|
|
||
| // Found non-whitespace character | ||
| break; |
There was a problem hiding this comment.
I think you can make your intention clearer like this:
| if char_is_whitespace(b) { | |
| continue; | |
| } | |
| // Found non-whitespace character | |
| break; | |
| if !char_is_whitespace(b) { | |
| break; | |
| } |
| /** | ||
| Asterisk after text * * | ||
| */ |
There was a problem hiding this comment.
I feel this one should be invalid, no?
Summary
Closes #5784
Test Plan
Added snapshots