Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ impl RecessOrderMember {
match &self.0 {
AnyCssDeclarationOrRule::CssBogus(_) => NodeKindOrder::UnknownKind,
AnyCssDeclarationOrRule::CssMetavariable(_) => NodeKindOrder::UnknownKind,
AnyCssDeclarationOrRule::ScssDeclaration(_) => NodeKindOrder::UnknownKind,
AnyCssDeclarationOrRule::AnyCssRule(rule) => match rule {
AnyCssRule::CssAtRule(_) => NodeKindOrder::NestedRuleOrAtRule,
AnyCssRule::CssBogusRule(_) => NodeKindOrder::UnknownKind,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use biome_analyze::{
Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
};
use biome_console::markup;
use biome_css_syntax::{AnyCssRule, CssRuleList};
use biome_css_syntax::{AnyCssRootItem, AnyCssRule, CssRootItemList};
use biome_diagnostics::Severity;
use biome_rowan::{AstNode, TextRange};
use biome_rule_options::no_invalid_position_at_import_rule::NoInvalidPositionAtImportRuleOptions;
Expand Down Expand Up @@ -39,7 +39,7 @@ declare_lint_rule! {
}

impl Rule for NoInvalidPositionAtImportRule {
type Query = Ast<CssRuleList>;
type Query = Ast<CssRootItemList>;
type State = TextRange;
type Signals = Box<[Self::State]>;
type Options = NoInvalidPositionAtImportRuleOptions;
Expand All @@ -49,9 +49,9 @@ impl Rule for NoInvalidPositionAtImportRule {
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(),
for item in node {
let any_css_at_rule = match item {
AnyCssRootItem::AnyCssRule(AnyCssRule::CssAtRule(at_rule)) => at_rule.rule().ok(),
_ => None,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl Rule for NoEmptySource {
fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
let node = ctx.query();

if node.rules().len() > 0 {
if node.items().len() > 0 {
return None;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use biome_analyze::{
Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
};
use biome_console::markup;
use biome_css_syntax::{AnyCssAtRule, AnyCssRule, CssImportAtRule, CssRuleList};
use biome_css_syntax::{AnyCssAtRule, AnyCssRootItem, AnyCssRule, CssImportAtRule, CssRootItemList};
use biome_diagnostics::Severity;
use biome_rowan::AstNode;
use biome_rule_options::no_duplicate_at_import_rules::NoDuplicateAtImportRulesOptions;
Expand Down Expand Up @@ -59,65 +59,64 @@ declare_lint_rule! {
}

impl Rule for NoDuplicateAtImportRules {
type Query = Ast<CssRuleList>;
type Query = Ast<CssRootItemList>;
type State = CssImportAtRule;
type Signals = Option<Self::State>;
type Options = NoDuplicateAtImportRulesOptions;

fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
let node = ctx.query();
let mut import_url_map: HashMap<String, HashSet<String>> = HashMap::new();
for rule in node {
match rule {
AnyCssRule::CssAtRule(item) => match item.rule().ok()? {
AnyCssAtRule::CssImportAtRule(import_rule) => {
let import_url = import_rule
.url()
.ok()?
.to_trimmed_text()
.to_lowercase_cow()
.replace("url(", "")
.replace(')', "")
.replace('"', "'");
if let Some(media_query_set) = import_url_map.get_mut(&import_url) {
// if the current import_rule has no media queries or there are no queries saved in the
// media_query_set, this is always a duplicate
if import_rule.media().to_trimmed_text().is_empty()
|| media_query_set.is_empty()
{
return Some(import_rule);
}
for item in node {
let AnyCssRootItem::AnyCssRule(AnyCssRule::CssAtRule(at_rule)) = item else {
continue;
};
let AnyCssAtRule::CssImportAtRule(import_rule) = at_rule.rule().ok()? else {
continue;
};

for media in import_rule.media() {
match media {
Ok(media) => {
if !media_query_set.insert(
media.to_trimmed_text().to_lowercase_cow().into(),
) {
return Some(import_rule);
}
}
_ => return None,
}
}
} else {
let mut media_set: HashSet<String> = HashSet::new();
for media in import_rule.media() {
match media {
Ok(media) => {
media_set.insert(
media.to_trimmed_text().to_lowercase_cow().into(),
);
}
_ => return None,
}
let import_url = import_rule
.url()
.ok()?
.to_trimmed_text()
.to_lowercase_cow()
.replace("url(", "")
.replace(')', "")
.replace('"', "'");
if let Some(media_query_set) = import_url_map.get_mut(&import_url) {
// if the current import_rule has no media queries or there are no queries saved in the
// media_query_set, this is always a duplicate
if import_rule.media().to_trimmed_text().is_empty()
|| media_query_set.is_empty()
{
return Some(import_rule);
}

for media in import_rule.media() {
match media {
Ok(media) => {
if !media_query_set.insert(
media.to_trimmed_text().to_lowercase_cow().into(),
) {
return Some(import_rule);
}
import_url_map.insert(import_url, media_set);
}
_ => return None,
}
_ => return None,
},
_ => return None,
}
Comment on lines +95 to +106
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Media query parse errors also abort the scan.

The _ => return None branches on lines 104 and 116 will abort the entire function when a media query fails to parse. Consider using continue to skip problematic entries whilst still catching duplicates in the rest of the stylesheet.

🛠️ Proposed fix for both branches
                 for media in import_rule.media() {
                     match media {
                         Ok(media) => {
                             if !media_query_set.insert(
                                 media.to_trimmed_text().to_lowercase_cow().into(),
                             ) {
                                 return Some(import_rule);
                             }
                         }
-                        _ => return None,
+                        Err(_) => continue,
                     }
                 }
             } else {
                 let mut media_set: HashSet<String> = HashSet::new();
                 for media in import_rule.media() {
                     match media {
                         Ok(media) => {
                             media_set.insert(
                                 media.to_trimmed_text().to_lowercase_cow().into(),
                             );
                         }
-                        _ => return None,
+                        Err(_) => continue,
                     }
                 }

Also applies to: 109-118

🤖 Prompt for AI Agents
In `@crates/biome_css_analyze/src/lint/suspicious/no_duplicate_at_import_rules.rs`
around lines 95 - 106, The match arms that currently use `_ => return None`
inside the media parsing loop (the loop iterating `for media in
import_rule.media()` and the analogous branch around lines 109-118) prematurely
abort the entire scan on a parse error; change those arms to `_ => continue` so
that a single media parse failure skips that media entry but continues scanning
the rest of the media list and stylesheet, keeping the duplicate-detection logic
that uses
`media_query_set.insert(media.to_trimmed_text().to_lowercase_cow().into())`
intact.

} else {
let mut media_set: HashSet<String> = HashSet::new();
for media in import_rule.media() {
match media {
Ok(media) => {
media_set.insert(
media.to_trimmed_text().to_lowercase_cow().into(),
);
}
_ => return None,
}
}
import_url_map.insert(import_url, media_set);
}
}
None
Expand Down
107 changes: 103 additions & 4 deletions crates/biome_css_factory/src/generated/node_factory.rs

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

Loading