-
-
Notifications
You must be signed in to change notification settings - Fork 524
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_analyze): implement noIrregularWhitespaceCss #3428
Changes from 2 commits
a5880b9
e5a3212
c8b79cd
4364614
2473653
f625d43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,105 @@ | ||||||
use biome_analyze::{ | ||||||
context::RuleContext, declare_lint_rule, Ast, Rule, RuleDiagnostic, RuleSource, | ||||||
}; | ||||||
use biome_console::markup; | ||||||
use biome_css_syntax::{AnyCssRule, CssLanguage}; | ||||||
use biome_rowan::{AstNode, Direction, SyntaxToken, TextRange}; | ||||||
|
||||||
const IRREGULAR_WHITESPACES: &[char; 22] = &[ | ||||||
'\u{c}', '\u{b}', '\u{85}', '\u{feff}', '\u{a0}', '\u{1680}', '\u{180e}', '\u{2000}', | ||||||
'\u{2001}', '\u{2002}', '\u{2003}', '\u{2004}', '\u{2005}', '\u{2006}', '\u{2007}', '\u{2008}', | ||||||
'\u{2009}', '\u{200a}', '\u{200b}', '\u{202f}', '\u{205f}', '\u{3000}', | ||||||
]; | ||||||
|
||||||
declare_lint_rule! { | ||||||
/// Disallows the use of irregular whitespace. | ||||||
/// | ||||||
/// Using irregular whitespace would lead to the failure of selecting the correct target. | ||||||
/// | ||||||
/// ## Examples | ||||||
/// | ||||||
/// ### Invalid | ||||||
/// | ||||||
/// ```css,expect_diagnostic | ||||||
/// .firstClass.secondClass { | ||||||
/// color: red; | ||||||
/// } | ||||||
/// ``` | ||||||
/// | ||||||
/// ```css,expect_diagnostic | ||||||
/// .firstClass .secondClass { | ||||||
/// color:red; | ||||||
/// } | ||||||
/// ``` | ||||||
/// ### Valid | ||||||
/// | ||||||
/// ```css | ||||||
/// .firstClass .secondClass { | ||||||
/// color: red; | ||||||
/// } | ||||||
/// ``` | ||||||
/// | ||||||
pub NoIrregularWhitespaceCss { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, problem. Keep the old name and I will handle that once this rule is merged |
||||||
version: "1.8.0", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
name: "noIrregularWhitespaceCss", | ||||||
language: "css", | ||||||
recommended: false, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should recommend it. It's a slow rule, and a niche one. |
||||||
sources: &[RuleSource::Stylelint("no-irregular-whitespace")], | ||||||
} | ||||||
} | ||||||
|
||||||
impl Rule for NoIrregularWhitespaceCss { | ||||||
type Query = Ast<AnyCssRule>; | ||||||
type State = TextRange; | ||||||
type Signals = Vec<Self::State>; | ||||||
type Options = (); | ||||||
|
||||||
fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||||||
let node = ctx.query(); | ||||||
get_irregular_whitespace(node) | ||||||
} | ||||||
|
||||||
fn diagnostic(_: &RuleContext<Self>, range: &Self::State) -> Option<RuleDiagnostic> { | ||||||
// | ||||||
// Read our guidelines to write great diagnostics: | ||||||
// https://docs.rs/biome_analyze/latest/biome_analyze/#what-a-rule-should-say-to-the-user | ||||||
// | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove the comment |
||||||
Some( | ||||||
RuleDiagnostic::new( | ||||||
rule_category!(), | ||||||
range, | ||||||
markup! { | ||||||
"Irregular whitespace found." | ||||||
}, | ||||||
) | ||||||
.note(markup! { | ||||||
"Replace the irregular whitespace with normal whitespaces." | ||||||
}), | ||||||
) | ||||||
} | ||||||
} | ||||||
|
||||||
fn get_irregular_whitespace(node: &AnyCssRule) -> Vec<TextRange> { | ||||||
let syntax = node.syntax(); | ||||||
let mut all_whitespaces_token: Vec<SyntaxToken<CssLanguage>> = vec![]; | ||||||
let matches_irregular_whitespace = |token: &SyntaxToken<CssLanguage>| { | ||||||
!token.has_leading_comments() | ||||||
&& !token.has_trailing_comments() | ||||||
&& token.text().chars().any(|char| { | ||||||
IRREGULAR_WHITESPACES | ||||||
.iter() | ||||||
.any(|irregular_whitespace| &char == irregular_whitespace) | ||||||
}) | ||||||
}; | ||||||
|
||||||
for token in syntax.descendants_tokens(Direction::Next) { | ||||||
if matches_irregular_whitespace(&token) { | ||||||
all_whitespaces_token.push(token); | ||||||
} | ||||||
} | ||||||
|
||||||
all_whitespaces_token | ||||||
.iter() | ||||||
.map(|token| token.text_range()) | ||||||
.collect::<Vec<TextRange>>() | ||||||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
/* \u{b} */ | ||
@import 'a.css'; | ||
/* \u{c} */ | ||
@layermodule, state; | ||
/* \u{feff} */ | ||
.firstClass.secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{a0} */ | ||
.firstClass .secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{1680} */ | ||
.firstClass .secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{2000} */ | ||
.firstClass .secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{2001} */ | ||
.firstClass .secondClass { | ||
flex: 1 1 100px; | ||
} | ||
/* \u{2002} */ | ||
.firstClass.secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{2003} */ | ||
.firstClass .secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{2004} */ | ||
.firstClass .secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{2005} */ | ||
.firstClass .secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{2006} */ | ||
.firstClass .secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{2007} */ | ||
.firstClass .secondClass { | ||
padding: 10px; | ||
} | ||
/* \u{2008} */ | ||
@view-transition { | ||
navigation: auto; | ||
} | ||
/* \u{2009} */ | ||
@layer state { | ||
body { | ||
padding: 10px; | ||
} | ||
} | ||
/* \u{200a} */ | ||
@layer state { | ||
body { | ||
padding: 10px; | ||
} | ||
} | ||
/* \u{200b} */ | ||
@keyframes slidein { | ||
from{ | ||
transform: translateX(0%); | ||
} | ||
|
||
to { | ||
transform: translateX(100%); | ||
} | ||
} | ||
/* \u{202f} */ | ||
@font-face { | ||
font-family: "Trickster"; | ||
src: | ||
local("Trickster"), | ||
url("trickster-COLRv1.otf") format("opentype") tech(color-COLRv1), | ||
url("trickster-outline.otf") format("opentype"), | ||
url("trickster-outline.woff") format("woff"); | ||
} | ||
/* \u{205f} */ | ||
@keyframes slidein { | ||
from { | ||
transform: translateX(0%); | ||
} | ||
|
||
to { | ||
transform: translateX(100%); | ||
} | ||
} | ||
/* \u{3000} */ | ||
@container (width < 15rem) { | ||
color: blue; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question
Are
\u{2028}
and\u{2029}
missing?see
w3c/csswg-drafts#6992
eslint/eslint#2316