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(biome_css_analyzer): implement noInvalidPositionAtImportRule #2717

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a33755b
feat: noInvalidPositionAtImportRule
t-shiratori May 5, 2024
eb0c07d
chore: refactor
t-shiratori May 5, 2024
10e882e
fix: conflict crates/biome_configuration/src/linter/rules.rs
t-shiratori May 5, 2024
6e3c7e4
Update crates/biome_css_analyze/src/lint/nursery/no_invalid_position_…
t-shiratori May 5, 2024
f757457
Update crates/biome_css_analyze/src/lint/nursery/no_invalid_position_…
t-shiratori May 5, 2024
981c14b
Update crates/biome_css_analyze/src/lint/nursery/no_invalid_position_…
t-shiratori May 5, 2024
9f715bd
Update crates/biome_css_analyze/src/lint/nursery/no_invalid_position_…
t-shiratori May 5, 2024
e97271b
Update crates/biome_css_analyze/src/lint/nursery/no_invalid_position_…
t-shiratori May 5, 2024
7495e81
fix: Emphasis @import
t-shiratori May 5, 2024
c6f45ab
fix: correct the order of notes
t-shiratori May 5, 2024
9ca1793
chore: refactor
t-shiratori May 5, 2024
b3cfcf4
update: snap
t-shiratori May 5, 2024
f536251
chore: refactor
t-shiratori May 5, 2024
6f752ed
fix: multiple diagnostic
t-shiratori May 6, 2024
83b81ac
update:
t-shiratori May 7, 2024
85420e3
Merge branch 'main' into feature/noInvalidPositionAtImportRule
t-shiratori May 7, 2024
b3c385d
Merge branch 'biomejs:main' into feature/noInvalidPositionAtImportRule
t-shiratori May 8, 2024
370b78c
Merge branch 'main' into feature/noInvalidPositionAtImportRule
t-shiratori May 8, 2024
eb6210b
fix: test case validCharset
t-shiratori May 8, 2024
aa879c1
add: test case validCharsetComment
t-shiratori May 8, 2024
27b6ab5
fix: test case validCharsetMultipleImport
t-shiratori May 8, 2024
26d9f6e
Merge branch 'main' into feature/noInvalidPositionAtImportRule
t-shiratori May 9, 2024
e690c55
Merge branch 'main' into feature/noInvalidPositionAtImportRule
t-shiratori May 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 65 additions & 43 deletions crates/biome_configuration/src/linter/rules.rs

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions crates/biome_css_analyze/src/lint/nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub mod no_duplicate_at_import_rules;
pub mod no_duplicate_font_names;
pub mod no_duplicate_selectors_keyframe_block;
pub mod no_important_in_keyframe;
pub mod no_invalid_position_at_import_rule;
pub mod no_unknown_function;
pub mod no_unknown_property;
pub mod no_unknown_selector_pseudo_element;
Expand All @@ -25,6 +26,7 @@ declare_group! {
self :: no_duplicate_font_names :: NoDuplicateFontNames ,
self :: no_duplicate_selectors_keyframe_block :: NoDuplicateSelectorsKeyframeBlock ,
self :: no_important_in_keyframe :: NoImportantInKeyframe ,
self :: no_invalid_position_at_import_rule :: NoInvalidPositionAtImportRule ,
self :: no_unknown_function :: NoUnknownFunction ,
self :: no_unknown_property :: NoUnknownProperty ,
self :: no_unknown_selector_pseudo_element :: NoUnknownSelectorPseudoElement ,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
use biome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic, RuleSource};
use biome_console::markup;
use biome_css_syntax::{AnyCssRule, CssRuleList};
use biome_rowan::{AstNode, TextRange};

declare_rule! {
/// Disallow the use of `@import` at-rules in invalid positions.
///
/// Any `@import` rules must precede all other valid at-rules and style rules in a stylesheet (ignoring `@charset` and `@layer`), or else the `@import` rule is invalid.
///
/// ## Examples
///
/// ### Invalid
///
/// ```css,expect_diagnostic
/// a {}
/// @import 'foo.css';
/// ```
///
/// ### Valid
///
/// ```css
/// @import 'foo.css';
/// a {}
/// ```
///
pub NoInvalidPositionAtImportRule {
version: "next",
name: "noInvalidPositionAtImportRule",
recommended: true,
sources: &[RuleSource::Stylelint("no-invalid-position-at-import-rule")],
}
}

impl Rule for NoInvalidPositionAtImportRule {
type Query = Ast<CssRuleList>;
type State = TextRange;
type Signals = Vec<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Vec<Self::State> {
let node = ctx.query();
let mut is_invalid_position = false;
let mut invalid_import_list = Vec::new();

for rule in node {
let any_css_at_rule = match rule {
AnyCssRule::CssAtRule(item) => item.rule().ok(),
_ => None,
};

if let Some(any_css_at_rule) = any_css_at_rule {
// Ignore @charset, @layer
if any_css_at_rule.as_css_charset_at_rule().is_some() {
continue;
}
if any_css_at_rule.as_css_layer_at_rule().is_some() {
continue;
}

let import_rule = any_css_at_rule.as_css_import_at_rule();
if let Some(import_rule) = import_rule {
if is_invalid_position {
invalid_import_list.push(import_rule.range());
}
} else {
is_invalid_position = true;
}
} else {
is_invalid_position = true;
}
}
invalid_import_list
}

fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
Some(
RuleDiagnostic::new(
rule_category!(),
state,
markup! {
"This "<Emphasis>"@import"</Emphasis>" is in the wrong position."
},
)
.note(markup! {
"Any "<Emphasis>"@import"</Emphasis>" rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the "<Emphasis>"@import"</Emphasis>" rule is invalid."
}).note(markup! {
"Consider moving import position."
})
)
}
}
1 change: 1 addition & 0 deletions crates/biome_css_analyze/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub type NoDuplicateFontNames =
<lint::nursery::no_duplicate_font_names::NoDuplicateFontNames as biome_analyze::Rule>::Options;
pub type NoDuplicateSelectorsKeyframeBlock = < lint :: nursery :: no_duplicate_selectors_keyframe_block :: NoDuplicateSelectorsKeyframeBlock as biome_analyze :: Rule > :: Options ;
pub type NoImportantInKeyframe = < lint :: nursery :: no_important_in_keyframe :: NoImportantInKeyframe as biome_analyze :: Rule > :: Options ;
pub type NoInvalidPositionAtImportRule = < lint :: nursery :: no_invalid_position_at_import_rule :: NoInvalidPositionAtImportRule as biome_analyze :: Rule > :: Options ;
pub type NoUnknownFunction =
<lint::nursery::no_unknown_function::NoUnknownFunction as biome_analyze::Rule>::Options;
pub type NoUnknownProperty =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
a {}
@import 'foo.css';
@import 'bar.css';
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: invalid.css
---
# Input
```css
a {}
@import 'foo.css';
@import 'bar.css';

```

# Diagnostics
```
invalid.css:2:2 lint/nursery/noInvalidPositionAtImportRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This @import is in the wrong position.

1 │ a {}
> 2 │ @import 'foo.css';
│ ^^^^^^^^^^^^^^^^^
Comment on lines +20 to +21
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if you noticed, but the diagnostic isn't correct because it doesn't highlight the @.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review.
Sorry, I didn't know how to do it 🙇‍♂️
Is there a sample somewhere?

3 │ @import 'bar.css';
4 │

i Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.

i Consider moving import position.


```

```
invalid.css:3:2 lint/nursery/noInvalidPositionAtImportRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This @import is in the wrong position.

1 │ a {}
2 │ @import 'foo.css';
> 3 │ @import 'bar.css';
│ ^^^^^^^^^^^^^^^^^
4 │

i Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.

i Consider moving import position.


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@import 'foo.css';
a {}
@import 'bar1.css';
@import 'bar2.css';
a {}
@import 'bar3.css';
@import 'bar4.css';
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: invalidBetweenImport.css
---
# Input
```css
@import 'foo.css';
a {}
@import 'bar1.css';
@import 'bar2.css';
a {}
@import 'bar3.css';
@import 'bar4.css';

```

# Diagnostics
```
invalidBetweenImport.css:3:2 lint/nursery/noInvalidPositionAtImportRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This @import is in the wrong position.

1 │ @import 'foo.css';
2 │ a {}
> 3 │ @import 'bar1.css';
│ ^^^^^^^^^^^^^^^^^^
4 │ @import 'bar2.css';
5 │ a {}

i Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.

i Consider moving import position.


```

```
invalidBetweenImport.css:4:2 lint/nursery/noInvalidPositionAtImportRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This @import is in the wrong position.

2 │ a {}
3 │ @import 'bar1.css';
> 4 │ @import 'bar2.css';
│ ^^^^^^^^^^^^^^^^^^
5 │ a {}
6 │ @import 'bar3.css';

i Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.

i Consider moving import position.


```

```
invalidBetweenImport.css:6:2 lint/nursery/noInvalidPositionAtImportRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This @import is in the wrong position.

4 │ @import 'bar2.css';
5 │ a {}
> 6 │ @import 'bar3.css';
│ ^^^^^^^^^^^^^^^^^^
7 │ @import 'bar4.css';
8 │

i Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.

i Consider moving import position.


```

```
invalidBetweenImport.css:7:2 lint/nursery/noInvalidPositionAtImportRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This @import is in the wrong position.

5 │ a {}
6 │ @import 'bar3.css';
> 7 │ @import 'bar4.css';
│ ^^^^^^^^^^^^^^^^^^
8 │

i Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.

i Consider moving import position.


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@media print {}
@import url('foo.css');
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: invalidMediaImport.css
---
# Input
```css
@media print {}
@import url('foo.css');

```

# Diagnostics
```
invalidMediaImport.css:2:2 lint/nursery/noInvalidPositionAtImportRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This @import is in the wrong position.

1 │ @media print {}
> 2 │ @import url('foo.css');
│ ^^^^^^^^^^^^^^^^^^^^^^
3 │

i Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.

i Consider moving import position.


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@media print {}
@imPort URl('foo.css');
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: invalidMediaImportUpperCase.css
---
# Input
```css
@media print {}
@imPort URl('foo.css');

```

# Diagnostics
```
invalidMediaImportUpperCase.css:2:2 lint/nursery/noInvalidPositionAtImportRule ━━━━━━━━━━━━━━━━━━━━━

! This @import is in the wrong position.

1 │ @media print {}
> 2 │ @imPort URl('foo.css');
│ ^^^^^^^^^^^^^^^^^^^^^^
3 │

i Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.

i Consider moving import position.


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@import 'foo.css';
a {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: valid.css
---
# Input
```css
@import 'foo.css';
a {}

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@charset "utf-8";
@import 'foo.css';
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: validCharset.css
---
# Input
```css
@charset "utf-8";
@import 'foo.css';

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@charset "utf-8";
/* some comment */
@import 'foo.css';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: validCharsetComment.css
---
# Input
```css
@charset "utf-8";
/* some comment */
@import 'foo.css';

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@charset "utf-8";
@imPORT 'foo.css';
@import 'bar.css';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/biome_css_analyze/tests/spec_tests.rs
expression: validCharsetMultipleImport.css
---
# Input
```css
@charset "utf-8";
@imPORT 'foo.css';
@import 'bar.css';

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/* some comment */
@import 'foo.css';
Comment on lines +1 to +2
Copy link

Choose a reason for hiding this comment

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

nit

Maybe add a test case for

@charset "utf-8";
/* some comment */
@import 'foo.css';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix: aa879c1

Loading