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_analyze): implement noIrregularWhitespaceCss #3428

Merged
merged 6 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
187 changes: 103 additions & 84 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

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}',
];
Copy link

@Mouvedia Mouvedia Jul 15, 2024

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


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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

CSS isn't necessary. One rule can use the same name in multiple languages in Biome.

Suggested change
pub NoIrregularWhitespaceCss {
pub NoIrregularWhitespace {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when trying to rename this, I run into this issue (which is the reason why I opened a new PR for this):
image

Copy link
Member

Choose a reason for hiding this comment

The 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",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
version: "1.8.0",
version: "next",

name: "noIrregularWhitespaceCss",
language: "css",
recommended: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
recommended: false,
recommended: true,

Copy link
Member

Choose a reason for hiding this comment

The 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
//
Copy link
Contributor

Choose a reason for hiding this comment

The 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>>()
}
1 change: 1 addition & 0 deletions crates/biome_css_analyze/src/options.rs

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} */
@layer module, 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;
}
Loading