Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter): implement useSymbolDescription #5232

Merged
merged 5 commits into from
Mar 3, 2025

Conversation

minht11
Copy link
Contributor

@minht11 minht11 commented Mar 2, 2025

Summary

Implement symbol-description. Makes sure that Symbol() constructor always has description param.

Closes #5231

Test Plan

@github-actions 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 Mar 2, 2025
@minht11 minht11 self-assigned this Mar 2, 2025
@minht11 minht11 requested review from a team March 2, 2025 01:36
Copy link

codspeed-hq bot commented Mar 2, 2025

CodSpeed Performance Report

Merging #5232 will not alter performance

Comparing feat/lint-symbol-description (34b92e0) with main (bf29bbc)

Summary

✅ 97 untouched benchmarks

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! All good, however the diagnostic needs a rework

Comment on lines 69 to 73
"Provide a description for the symbol."
},
)
.note(markup! {
"Explicit description can be useful in debugging and making purpose of the symbol clearer."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the rule pillars: https://biomejs.dev/linter/#rule-pillars

In this case the first one is missing, which should be at line 69. The phrase at line 69 is the third pillar.

@minht11 minht11 force-pushed the feat/lint-symbol-description branch from a9d1e65 to 14729a4 Compare March 2, 2025 20:50
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this rule should cover empty strings/template literals too. What do you think?

@minht11 minht11 force-pushed the feat/lint-symbol-description branch from ed36ad5 to 335a4d6 Compare March 2, 2025 23:38
@ematipico ematipico merged commit da7b99e into main Mar 3, 2025
14 checks passed
@ematipico ematipico deleted the feat/lint-symbol-description branch March 3, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📎 Implement useSymbolDescription
3 participants