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): add NoUnknownAtRule #4689

Merged
merged 12 commits into from
Dec 4, 2024

Conversation

togami2864
Copy link
Contributor

Summary

close #4688

Test Plan

added tests and updated snapshots

@togami2864 togami2864 self-assigned this Dec 3, 2024
@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter A-Parser Area: parser A-Tooling Area: internal tools L-CSS Language: CSS A-Diagnostic Area: diagnostocis labels Dec 3, 2024
@github-actions github-actions bot removed A-Parser Area: parser A-Tooling Area: internal tools labels Dec 3, 2024
Comment on lines +107 to +111
/* TODO: the parser not supported yet */
/* @position-try --foo {}
@view-transition {
navigation: auto;
} */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

filed as an issue #4685

Comment on lines 76 to 78
fn diagnostic(_: &RuleContext<Self>, node: &Self::State) -> Option<RuleDiagnostic> {
let span = node.range;
let name = &node.name;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fn diagnostic(_: &RuleContext<Self>, node: &Self::State) -> Option<RuleDiagnostic> {
let span = node.range;
let name = &node.name;
fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let span = state.range;
let name = &state.name;

Comment on lines 87 to 91
.note(markup! {
""<Emphasis>{ name }</Emphasis>" is not a standard CSS at-rule, which may lead to unexpected styling results or failure to interpret the styles as intended."
}).note(markup! {
"See "<Hyperlink href="https://developer.mozilla.org/en-US/docs/Web/CSS/At-rule">"MDN web docs"</Hyperlink>" for more details."
}),
Copy link
Member

Choose a reason for hiding this comment

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

At the end, we need to suggest a solution to the user (see the three rule pillars). In this case, we should suggest to remove it. (another alternative is to list all the known rules, but I think it's too much)

Copy link

codspeed-hq bot commented Dec 3, 2024

CodSpeed Performance Report

Merging #4689 will improve performances by 6.79%

Comparing togami2864:no-unkwnon-at-rule (a524cad) with main (348c5c2)

Summary

⚡ 1 improvements
✅ 96 untouched benchmarks

Benchmarks breakdown

Benchmark main togami2864:no-unkwnon-at-rule Change
tachyons_11778168428173736564.css[cached] 19.8 ms 18.5 ms +6.79%

@ematipico ematipico merged commit 66458a1 into biomejs:main Dec 4, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-CSS Language: CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📎 Implement stylelint/at-rule-no-unknown
2 participants