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): noUnknownUnit #2535

Merged
merged 26 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1683c6a
chore: add noUnknownUnit rule
neoki07 Apr 19, 2024
b6a5279
test: add no-unknown-unit tests
neoki07 Apr 19, 2024
7eebe23
feat: implement no-unknown-unit rule
neoki07 Apr 20, 2024
4b420a9
test: fix css hack test
neoki07 Apr 20, 2024
5b437d8
test: add snapshots
neoki07 Apr 20, 2024
bfee475
fix: corresponding to edge cases
neoki07 Apr 20, 2024
306e4a4
test: update test cases
neoki07 Apr 20, 2024
9cb08e4
fix: break loop when allowing x
neoki07 Apr 20, 2024
f8af9e9
fix: remove dbg macro
neoki07 Apr 20, 2024
ba1aef2
refactor: change rule state name
neoki07 Apr 20, 2024
0953ba7
chore: fix conflicts
neoki07 Apr 21, 2024
1adcd00
fix: add noUnknownUnit to recommended rules
neoki07 Apr 21, 2024
4990cb9
refactor: sort expressions in match statement
neoki07 Apr 21, 2024
c52e807
fix: set recommended to true
neoki07 Apr 21, 2024
652ca86
fix: use ends_with instead of strip_vendor_prefix
neoki07 Apr 21, 2024
a47ad86
refactor: add documentation on CSS hack
neoki07 Apr 21, 2024
073f1b7
refactor: remove ambiguous document
neoki07 Apr 21, 2024
18cb89b
fix: update diagnostic
neoki07 Apr 21, 2024
1e89e19
fix: update document texts of rule
neoki07 Apr 21, 2024
a801b5d
test: update invalid.css.snap and remove snap.new file
neoki07 Apr 21, 2024
ac189f9
fix: remove processing for css hack
neoki07 Apr 21, 2024
c21d091
refactor: add comment as to why `x` is checked
neoki07 Apr 21, 2024
4ed73c5
refactor: fix comment as to why `x` is checked
neoki07 Apr 21, 2024
8685fdf
chore: update auto generated files
neoki07 Apr 21, 2024
da1338a
Merge remote-tracking branch 'origin/main' into feat/unit-no-unknown
ematipico Apr 22, 2024
1f077ac
chore: run codegen
ematipico Apr 22, 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
29 changes: 25 additions & 4 deletions crates/biome_configuration/src/linter/rules.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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 @@ -6,6 +6,7 @@ pub mod no_color_invalid_hex;
pub mod no_css_empty_block;
pub mod no_duplicate_font_names;
pub mod no_duplicate_selectors_keyframe_block;
pub mod no_unknown_unit;

declare_group! {
pub Nursery {
Expand All @@ -15,6 +16,7 @@ declare_group! {
self :: no_css_empty_block :: NoCssEmptyBlock ,
self :: no_duplicate_font_names :: NoDuplicateFontNames ,
self :: no_duplicate_selectors_keyframe_block :: NoDuplicateSelectorsKeyframeBlock ,
self :: no_unknown_unit :: NoUnknownUnit ,
]
}
}
187 changes: 187 additions & 0 deletions crates/biome_css_analyze/src/lint/nursery/no_unknown_unit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
use biome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic, RuleSource};
use biome_console::markup;
use biome_css_syntax::{
AnyCssDimension, CssFunction, CssGenericProperty, CssQueryFeaturePlain, CssSyntaxKind,
};
use biome_rowan::{SyntaxNodeCast, TextRange};

const RESOLUTION_MEDIA_FEATURE_NAMES: [&str; 3] =
["resolution", "min-resolution", "max-resolution"];

declare_rule! {
/// Disallow unknown CSS units.
///
/// For details on known CSS units, see the [MDN web docs](https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Values_and_units#lengths).
///
///
/// ## Examples
///
/// ### Invalid
///
/// ```css,expect_diagnostic
/// a {
/// width: 10pixels;
/// }
/// ```
///
/// ```css,expect_diagnostic
/// a {
/// width: calc(10px + 10pixels);
/// }
/// ```
///
/// ### Valid
///
/// ```css
/// a {
/// width: 10px;
/// }
/// ```
///
/// ```css
/// a {
/// width: 10Px;
/// }
/// ```
///
/// ```css
/// a {
/// width: 10pX;
/// }
/// ```
///
/// ```css
/// a {
/// width: calc(10px + 10px);
/// }
/// ```
///
pub NoUnknownUnit {
version: "next",
name: "noUnknownUnit",
recommended: true,
sources: &[RuleSource::Stylelint("unit-no-unknown")],
}
}

pub struct NoUnknownUnitState {
unit: String,
span: TextRange,
}

impl Rule for NoUnknownUnit {
type Query = Ast<AnyCssDimension>;
type State = NoUnknownUnitState;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
let node = ctx.query();

match node {
AnyCssDimension::CssUnknownDimension(dimension) => {
let unit_token = dimension.unit_token().ok()?;
let unit = unit_token.text_trimmed().to_string();

Some(NoUnknownUnitState {
unit,
span: unit_token.text_trimmed_range(),
})
}
AnyCssDimension::CssRegularDimension(dimension) => {
let unit_token = dimension.unit_token().ok()?;
let unit = unit_token.text_trimmed().to_string();

// The `x` unit is parsed as `CssRegularDimension`, but it is used for describing resolutions.
// This check is to disallow the use of the `x` unit outside this specific context.
if unit == "x" {
ematipico marked this conversation as resolved.
Show resolved Hide resolved
let mut allow_x = false;

for ancestor in dimension.unit_token().ok()?.ancestors() {
match ancestor.kind() {
CssSyntaxKind::CSS_FUNCTION => {
let function_name = ancestor
.cast::<CssFunction>()?
.name()
.ok()?
.value_token()
.ok()?
.text_trimmed()
.to_lowercase();

if function_name.ends_with("image-set") {
allow_x = true;
break;
}
}
CssSyntaxKind::CSS_GENERIC_PROPERTY => {
let property_name = ancestor
.cast::<CssGenericProperty>()?
.name()
.ok()?
.as_css_identifier()?
.value_token()
.ok()?
.text_trimmed()
.to_lowercase();

if property_name == "image-resolution" {
allow_x = true;
break;
}
}
CssSyntaxKind::CSS_QUERY_FEATURE_PLAIN => {
let feature_name = ancestor
.cast::<CssQueryFeaturePlain>()?
.name()
.ok()?
.value_token()
.ok()?
.text_trimmed()
.to_lowercase();

if RESOLUTION_MEDIA_FEATURE_NAMES.contains(&feature_name.as_str()) {
allow_x = true;
break;
}
}
_ => {}
}
}

if !allow_x {
return Some(NoUnknownUnitState {
unit,
span: unit_token.text_trimmed_range(),
});
}
}

None
}
_ => None,
}
}

fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
Some(
RuleDiagnostic::new(
rule_category!(),
state.span,
markup! {
"Unexpected unknown unit: "<Emphasis>{ state.unit }</Emphasis>
},
)
.note(markup! {
"See "<Hyperlink href="https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Values_and_units#lengths">"MDN web docs"</Hyperlink>" for more details."
})
.footer_list(
markup! {
"Use a known unit instead, such as:"
},
&["px", "em", "rem", "etc."],
),

)
}
}
2 changes: 2 additions & 0 deletions crates/biome_css_analyze/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ pub type NoCssEmptyBlock =
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 NoUnknownUnit =
<lint::nursery::no_unknown_unit::NoUnknownUnit as biome_analyze::Rule>::Options;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have brought stylelint tests (link). However, root { --margin: 10px + 10pix; } was not added because the syntax seems incorrect.

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
a { font-size: 13pp; }
a { margin: 13xpx; }
a { font-size: .5remm; }
a { font-size: 0.5remm; }
a { color: rgb(255pix, 0, 51); }
a { color: hsl(255pix, 0, 51); }
a { color: rgba(255pix, 0, 51, 1); }
a { color: hsla(255pix, 0, 51, 1); }
a { margin: calc(13pix + 10px); }
a { margin: calc(10pix*2); }
a { margin: calc(2*10pix); }
a { -webkit-transition-delay: 10pix; }
a { margin: -webkit-calc(13pix + 10px); }
a { margin: some-function(13pix + 10px); }
root { --margin: 10pix; }
@media (min-width: 13pix) {}
@media (min-width: 10px)\n and (max-width: 20PIX) {}
@media (width < 10.01REMS) {}
a { width: 1e4pz; }
a { flex: 0 9r9 auto; }
a { width: 400x; }
@media (resolution: 2x) and (min-width: 200x) {}
@media ( resolution: /* comment */ 2x ) and (min-width: 200x) {}
a { background: image-set('img1x.png' 1x, 'img2x.png' 2x) left 20x / 15% 60% repeat-x; }
a { background: /* comment */ image-set('img1x.png' 1x, 'img2x.png' 2x) left 20x / 15% 60% repeat-x; }
a { background-image: image-set('img1x.png' 1pix, 'img2x.png' 2x); }
@font-face { color: U+0100-024F; }
a { unicode-range: U+0100-024F; }
Loading