feat(linter): useSingleJsDocAsterisk #5844

minht11 · 2025-05-01T23:47:32Z

Summary

Implement useSingleJsDocAsterisk rule which ensures single asterisk used at the comment start.
Add new rule source EslintJsDoc.

Test Plan

Added snapshots

minht11 · 2025-05-01T23:49:53Z

crates/biome_js_analyze/src/lint/nursery/use_single_js_doc_asterisk.rs

+};
+use biome_console::markup;
+use biome_js_syntax::{JsModule, JsSyntaxToken};
+use biome_module_graph::JsdocComment;


I feel bit weird using function from module graph, despite not using module graph itself. Should it be moved somewhere else?

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?

A new crate is an option. I can propose two other options without creating a new crate:

having a comments_ext.rs inside the biome_js_syntax crate

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

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.

Added new crate

codspeed-hq · 2025-05-02T00:45:07Z

CodSpeed Performance Report

Merging #5844 will not alter performance

_{Comparing feat/useSingleJsdocAsterix (217ac02) with main (14551ba)}

Summary

✅ 95 untouched benchmarks

arendjr

Nice! Just a few comments.

.changeset/honest-bikes-smell.md

arendjr · 2025-05-02T07:56:45Z

crates/biome_js_analyze/src/lint/nursery/use_single_js_doc_asterisk.rs

+                        text.lines()
+                            .take(invalid_line.line_index)
+                            .fold(0, |acc, line| {
+                                acc + line.len() + 1 // +1 for the line break


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.

You were right in CRLF mode offset was wrong and fix would replace all comments whit wrong line ending..
Fixed that and added snapshot file, is that what you had in mind with testing?

Yeah, sounds good!

arendjr · 2025-05-02T08:00:41Z

crates/biome_js_analyze/src/lint/nursery/use_single_js_doc_asterisk.rs

+};
+use biome_console::markup;
+use biome_js_syntax::{JsModule, JsSyntaxToken};
+use biome_module_graph::JsdocComment;


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.

crates/biome_js_analyze/src/lint/nursery/use_single_js_doc_asterisk.rs

arendjr · 2025-05-02T08:17:54Z

crates/biome_js_analyze/tests/specs/nursery/useSingleJsDocAsterisk/valid.js

+/**
+ Asterisk after text * *
+*/


I feel this one should be invalid, no?

https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-multi-asterisks.md Allows it.

I am reaching a bit but could imagine comments like:

/** * https//google.com/* * Password **** */

Unintentional asterisks at the end of line (except last one) should be rare enough, so for now I would it leave as is, but no strong preference tho.

I don’t necessarily mind the asterisk at the end, but shouldn’t it fail because there isn’t an asterisk at the start of the line?

I copied the same behavior from no-multi-asterisks and did not think much of it, but seems like oversight.

We should catch it, though I would like to implement this in a separate PR not to stall this one.

Remove inaccurate comment add invalid examples

.changeset/honest-bikes-smell.md

ematipico

Please make sure that the diagnostics follow the rule pillars

ematipico · 2025-05-04T03:55:33Z

crates/biome_js_analyze/src/lint/nursery/use_single_js_doc_asterisk.rs

+            rule_category!(),
+            state.range,
+            markup! {
+                "JSDoc comment line should " {position} " with a single asterisk."


We're missing the second pillar , where we explain the error

crates/biome_js_analyze/src/lint/nursery/use_single_js_doc_asterisk.rs

crates/biome_js_analyze/tests/specs/nursery/useSingleJsDocAsterisk/valid.js

add comment

minht11 self-assigned this May 1, 2025

github-actions bot added A-CLI Area: CLI A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels May 1, 2025

minht11 commented May 1, 2025

View reviewed changes

minht11 force-pushed the feat/useSingleJsdocAsterix branch from 517d1ba to 98be97b Compare May 2, 2025 00:08

minht11 requested review from a team May 2, 2025 00:10

arendjr reviewed May 2, 2025

View reviewed changes

minht11 added 3 commits May 2, 2025 22:48

implement use_single_js_doc_asterisk

067ebd0

Remove inaccurate comment add invalid examples

move jsdoc comment into separate crate

32c1050

handle crlf lines better

cda9b64

minht11 force-pushed the feat/useSingleJsdocAsterix branch from 98be97b to cda9b64 Compare May 3, 2025 21:36

update snap

7bbec0f

dyc3 reviewed May 3, 2025

View reviewed changes

.changeset/honest-bikes-smell.md Outdated Show resolved Hide resolved

ematipico requested changes May 4, 2025

View reviewed changes

update diagnostic message

217ac02

add comment

arendjr approved these changes May 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(linter): useSingleJsDocAsterisk #5844

feat(linter): useSingleJsDocAsterisk #5844

minht11 commented May 1, 2025

minht11 May 1, 2025

arendjr May 2, 2025

ematipico May 2, 2025 •

edited

Loading

arendjr May 2, 2025

minht11 May 3, 2025

codspeed-hq bot commented May 2, 2025 •

edited

Loading

arendjr left a comment

arendjr May 2, 2025

minht11 May 3, 2025

arendjr May 4, 2025

arendjr May 2, 2025

arendjr May 2, 2025

minht11 May 3, 2025

arendjr May 4, 2025

minht11 May 4, 2025

ematipico left a comment

ematipico May 4, 2025

feat(linter): useSingleJsDocAsterisk #5844

Are you sure you want to change the base?

feat(linter): useSingleJsDocAsterisk #5844

Conversation

minht11 commented May 1, 2025

Summary

Test Plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ematipico May 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codspeed-hq bot commented May 2, 2025 • edited Loading

Merging #5844 will not alter performance

Summary

arendjr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ematipico left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ematipico May 2, 2025 •

edited

Loading

codspeed-hq bot commented May 2, 2025 •

edited

Loading