Skip to content

Commit

Permalink
perf(linter): remove allocations for string comparisons (#4570)
Browse files Browse the repository at this point in the history
Refactors a lot of case-insensitive comparisons from
```rust
a.to_lowercase() == b.to_lowercase()
```

with
```rust
a.eq_ignore_ascii_case(b)
```

These mostly happened when checking JSX props, so I'm expecting the most benefit from JSX-related rules.
  • Loading branch information
DonIsaac committed Jul 31, 2024
1 parent 0914e47 commit 7585e16
Show file tree
Hide file tree
Showing 27 changed files with 92 additions and 95 deletions.
3 changes: 1 addition & 2 deletions crates/oxc_codegen/src/gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1267,8 +1267,7 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for RegExpLiteral<'a> {
let last = p.peek_nth(0);
// Avoid forming a single-line comment or "</script" sequence
if Some('/') == last
|| (Some('<') == last
&& self.regex.pattern.as_str().to_lowercase().starts_with("script"))
|| (Some('<') == last && self.regex.pattern.to_lowercase().starts_with("script"))
{
p.print_hard_space();
}
Expand Down
32 changes: 16 additions & 16 deletions crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
get_element_type, get_prop_value, get_string_literal_prop_value, has_jsx_prop_lowercase,
get_element_type, get_prop_value, get_string_literal_prop_value, has_jsx_prop_ignore_case,
object_has_accessible_child,
},
AstNode,
Expand Down Expand Up @@ -205,8 +205,8 @@ impl Rule for AltText {

// <input type="image">
if let Some(custom_tags) = &self.input_type_image {
let has_input_with_type_image = name.to_lowercase() == "input"
&& has_jsx_prop_lowercase(jsx_el, "type").map_or(false, |v| {
let has_input_with_type_image = name.eq_ignore_ascii_case("input")
&& has_jsx_prop_ignore_case(jsx_el, "type").map_or(false, |v| {
get_string_literal_prop_value(v).map_or(false, |v| v == "image")
});
if has_input_with_type_image || custom_tags.iter().any(|i| i == name) {
Expand Down Expand Up @@ -247,26 +247,26 @@ fn aria_label_has_value<'a>(item: &'a JSXAttributeItem<'a>) -> bool {
}

fn img_rule<'a>(node: &'a JSXOpeningElement<'a>, ctx: &LintContext<'a>) {
if let Some(alt_prop) = has_jsx_prop_lowercase(node, "alt") {
if let Some(alt_prop) = has_jsx_prop_ignore_case(node, "alt") {
if !is_valid_alt_prop(alt_prop) {
ctx.diagnostic(missing_alt_value(node.span));
}
return;
}

if has_jsx_prop_lowercase(node, "role").map_or(false, is_presentation_role) {
if has_jsx_prop_ignore_case(node, "role").map_or(false, is_presentation_role) {
ctx.diagnostic(prefer_alt(node.span));
return;
}

if let Some(aria_label_prop) = has_jsx_prop_lowercase(node, "aria-label") {
if let Some(aria_label_prop) = has_jsx_prop_ignore_case(node, "aria-label") {
if !aria_label_has_value(aria_label_prop) {
ctx.diagnostic(aria_label_value(node.span));
}
return;
}

if let Some(aria_labelledby_prop) = has_jsx_prop_lowercase(node, "aria-labelledby") {
if let Some(aria_labelledby_prop) = has_jsx_prop_ignore_case(node, "aria-labelledby") {
if !aria_label_has_value(aria_labelledby_prop) {
ctx.diagnostic(aria_labelled_by_value(node.span));
}
Expand All @@ -282,11 +282,11 @@ fn object_rule<'a>(
ctx: &LintContext<'a>,
) {
let has_aria_label =
has_jsx_prop_lowercase(node, "aria-label").map_or(false, aria_label_has_value);
has_jsx_prop_ignore_case(node, "aria-label").map_or(false, aria_label_has_value);
let has_aria_labelledby =
has_jsx_prop_lowercase(node, "aria-labelledby").map_or(false, aria_label_has_value);
has_jsx_prop_ignore_case(node, "aria-labelledby").map_or(false, aria_label_has_value);
let has_label = has_aria_label || has_aria_labelledby;
let has_title_attr = has_jsx_prop_lowercase(node, "title")
let has_title_attr = has_jsx_prop_ignore_case(node, "title")
.and_then(get_string_literal_prop_value)
.map_or(false, |v| !v.is_empty());

Expand All @@ -298,14 +298,14 @@ fn object_rule<'a>(

fn area_rule<'a>(node: &'a JSXOpeningElement<'a>, ctx: &LintContext<'a>) {
let has_aria_label =
has_jsx_prop_lowercase(node, "aria-label").map_or(false, aria_label_has_value);
has_jsx_prop_ignore_case(node, "aria-label").map_or(false, aria_label_has_value);
let has_aria_labelledby =
has_jsx_prop_lowercase(node, "aria-labelledby").map_or(false, aria_label_has_value);
has_jsx_prop_ignore_case(node, "aria-labelledby").map_or(false, aria_label_has_value);
let has_label = has_aria_label || has_aria_labelledby;
if has_label {
return;
}
has_jsx_prop_lowercase(node, "alt").map_or_else(
has_jsx_prop_ignore_case(node, "alt").map_or_else(
|| {
ctx.diagnostic(area(node.span));
},
Expand All @@ -319,14 +319,14 @@ fn area_rule<'a>(node: &'a JSXOpeningElement<'a>, ctx: &LintContext<'a>) {

fn input_type_image_rule<'a>(node: &'a JSXOpeningElement<'a>, ctx: &LintContext<'a>) {
let has_aria_label =
has_jsx_prop_lowercase(node, "aria-label").map_or(false, aria_label_has_value);
has_jsx_prop_ignore_case(node, "aria-label").map_or(false, aria_label_has_value);
let has_aria_labelledby =
has_jsx_prop_lowercase(node, "aria-labelledby").map_or(false, aria_label_has_value);
has_jsx_prop_ignore_case(node, "aria-labelledby").map_or(false, aria_label_has_value);
let has_label = has_aria_label || has_aria_labelledby;
if has_label {
return;
}
has_jsx_prop_lowercase(node, "alt").map_or_else(
has_jsx_prop_ignore_case(node, "alt").map_or_else(
|| {
ctx.diagnostic(input_type_image(node.span));
},
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/anchor_has_content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
get_element_type, has_jsx_prop_lowercase, is_hidden_from_screen_reader,
get_element_type, has_jsx_prop_ignore_case, is_hidden_from_screen_reader,
object_has_accessible_child,
},
AstNode,
Expand Down Expand Up @@ -79,7 +79,7 @@ impl Rule for AnchorHasContent {
}

for attr in ["title", "aria-label"] {
if has_jsx_prop_lowercase(&jsx_el.opening_element, attr).is_some() {
if has_jsx_prop_ignore_case(&jsx_el.opening_element, attr).is_some() {
return;
};
}
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_linter/src/rules/jsx_a11y/anchor_is_valid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use oxc_span::Span;
use crate::{
context::LintContext,
rule::Rule,
utils::{get_element_type, has_jsx_prop_lowercase},
utils::{get_element_type, has_jsx_prop_ignore_case},
AstNode,
};

Expand Down Expand Up @@ -134,15 +134,15 @@ impl Rule for AnchorIsValid {
};
if name == "a" {
if let Option::Some(herf_attr) =
has_jsx_prop_lowercase(&jsx_el.opening_element, "href")
has_jsx_prop_ignore_case(&jsx_el.opening_element, "href")
{
// Check if the 'a' element has a correct href attribute
match herf_attr {
JSXAttributeItem::Attribute(attr) => match &attr.value {
Some(value) => {
let is_empty = check_value_is_empty(value, &self.0.valid_hrefs);
if is_empty {
if has_jsx_prop_lowercase(&jsx_el.opening_element, "onclick")
if has_jsx_prop_ignore_case(&jsx_el.opening_element, "onclick")
.is_some()
{
ctx.diagnostic(cant_be_anchor(ident.span));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
context::LintContext,
globals::HTML_TAG,
rule::Rule,
utils::{get_element_type, has_jsx_prop_lowercase, is_interactive_element, parse_jsx_value},
utils::{get_element_type, has_jsx_prop_ignore_case, is_interactive_element, parse_jsx_value},
AstNode,
};

Expand Down Expand Up @@ -62,7 +62,7 @@ impl Rule for AriaActivedescendantHasTabindex {
return;
};

if has_jsx_prop_lowercase(jsx_opening_el, "aria-activedescendant").is_none() {
if has_jsx_prop_ignore_case(jsx_opening_el, "aria-activedescendant").is_none() {
return;
};

Expand All @@ -75,7 +75,7 @@ impl Rule for AriaActivedescendantHasTabindex {
};

if let Some(JSXAttributeItem::Attribute(tab_index_attr)) =
has_jsx_prop_lowercase(jsx_opening_el, "tabIndex")
has_jsx_prop_ignore_case(jsx_opening_el, "tabIndex")
{
if !is_valid_tab_index_attr(tab_index_attr) {
return;
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/autocomplete_valid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use phf::{phf_map, phf_set};
use crate::{
context::LintContext,
rule::Rule,
utils::{get_element_type, has_jsx_prop_lowercase},
utils::{get_element_type, has_jsx_prop_ignore_case},
AstNode,
};

Expand Down Expand Up @@ -183,7 +183,7 @@ impl Rule for AutocompleteValid {
return;
}

let Some(autocomplete_prop) = has_jsx_prop_lowercase(jsx_el, "autocomplete") else {
let Some(autocomplete_prop) = has_jsx_prop_ignore_case(jsx_el, "autocomplete") else {
return;
};
let attr = match autocomplete_prop {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/html_has_lang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use oxc_span::Span;
use crate::{
context::LintContext,
rule::Rule,
utils::{get_element_type, get_prop_value, has_jsx_prop_lowercase},
utils::{get_element_type, get_prop_value, has_jsx_prop_ignore_case},
AstNode,
};

Expand Down Expand Up @@ -70,7 +70,7 @@ impl Rule for HtmlHasLang {
return;
};

has_jsx_prop_lowercase(jsx_el, "lang").map_or_else(
has_jsx_prop_ignore_case(jsx_el, "lang").map_or_else(
|| ctx.diagnostic(missing_lang_prop(identifier.span)),
|lang_prop| {
if !is_valid_lang_prop(lang_prop) {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/iframe_has_title.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use oxc_span::Span;
use crate::{
context::LintContext,
rule::Rule,
utils::{get_element_type, get_prop_value, has_jsx_prop_lowercase},
utils::{get_element_type, get_prop_value, has_jsx_prop_ignore_case},
AstNode,
};

Expand Down Expand Up @@ -75,7 +75,7 @@ impl Rule for IframeHasTitle {
return;
}

let Some(alt_prop) = has_jsx_prop_lowercase(jsx_el, "title") else {
let Some(alt_prop) = has_jsx_prop_ignore_case(jsx_el, "title") else {
ctx.diagnostic(iframe_has_title_diagnostic(iden.span));
return;
};
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/img_redundant_alt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
get_element_type, get_prop_value, has_jsx_prop_lowercase, is_hidden_from_screen_reader,
get_element_type, get_prop_value, has_jsx_prop_ignore_case, is_hidden_from_screen_reader,
},
AstNode,
};
Expand Down Expand Up @@ -120,7 +120,7 @@ impl Rule for ImgRedundantAlt {
return;
}

let Some(alt_prop) = has_jsx_prop_lowercase(jsx_el, "alt") else {
let Some(alt_prop) = has_jsx_prop_ignore_case(jsx_el, "alt") else {
return;
};

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/lang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use oxc_span::Span;
use crate::{
context::LintContext,
rule::Rule,
utils::{get_element_type, get_prop_value, has_jsx_prop_lowercase},
utils::{get_element_type, get_prop_value, has_jsx_prop_ignore_case},
AstNode,
};

Expand Down Expand Up @@ -75,7 +75,7 @@ impl Rule for Lang {
return;
};

has_jsx_prop_lowercase(jsx_el, "lang").map_or_else(
has_jsx_prop_ignore_case(jsx_el, "lang").map_or_else(
|| ctx.diagnostic(lang_diagnostic(identifier.span)),
|lang_prop| {
if !is_valid_lang_prop(lang_prop) {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jsx_a11y/media_has_caption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl Rule for MediaHasCaption {
if let JSXAttributeName::Identifier(iden) = &attr.name {
if let Some(JSXAttributeValue::StringLiteral(s)) = &attr.value {
return iden.name == "kind"
&& s.value.to_lowercase() == "captions";
&& s.value.eq_ignore_ascii_case("captions");
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/no_access_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, utils::has_jsx_prop_lowercase, AstNode};
use crate::{context::LintContext, rule::Rule, utils::has_jsx_prop_ignore_case, AstNode};

fn no_access_key_diagnostic(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("No access key attribute allowed.")
Expand Down Expand Up @@ -42,7 +42,8 @@ impl Rule for NoAccessKey {
let AstKind::JSXOpeningElement(jsx_el) = node.kind() else {
return;
};
if let Some(JSXAttributeItem::Attribute(attr)) = has_jsx_prop_lowercase(jsx_el, "accessKey")
if let Some(JSXAttributeItem::Attribute(attr)) =
has_jsx_prop_ignore_case(jsx_el, "accessKey")
{
match attr.value.as_ref() {
Some(JSXAttributeValue::StringLiteral(_)) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use oxc_span::Span;
use crate::{
context::LintContext,
rule::Rule,
utils::{get_element_type, has_jsx_prop_lowercase, parse_jsx_value},
utils::{get_element_type, has_jsx_prop_ignore_case, parse_jsx_value},
AstNode,
};

Expand Down Expand Up @@ -46,7 +46,7 @@ impl Rule for NoAriaHiddenOnFocusable {
let AstKind::JSXOpeningElement(jsx_el) = node.kind() else {
return;
};
if let Some(aria_hidden_prop) = has_jsx_prop_lowercase(jsx_el, "aria-hidden") {
if let Some(aria_hidden_prop) = has_jsx_prop_ignore_case(jsx_el, "aria-hidden") {
if is_aria_hidden_true(aria_hidden_prop) && is_focusable(ctx, jsx_el) {
if let JSXAttributeItem::Attribute(boxed_attr) = aria_hidden_prop {
ctx.diagnostic(no_aria_hidden_on_focusable_diagnostic(boxed_attr.span));
Expand Down Expand Up @@ -89,16 +89,16 @@ fn is_focusable(ctx: &LintContext, element: &JSXOpeningElement) -> bool {
return false;
};

if let Some(JSXAttributeItem::Attribute(attr)) = has_jsx_prop_lowercase(element, "tabIndex") {
if let Some(JSXAttributeItem::Attribute(attr)) = has_jsx_prop_ignore_case(element, "tabIndex") {
if let Some(attr_value) = &attr.value {
return parse_jsx_value(attr_value).map_or(false, |num| num >= 0.0);
}
}

match tag_name.as_str() {
"a" | "area" => has_jsx_prop_lowercase(element, "href").is_some(),
"a" | "area" => has_jsx_prop_ignore_case(element, "href").is_some(),
"button" | "input" | "select" | "textarea" => {
has_jsx_prop_lowercase(element, "disabled").is_none()
has_jsx_prop_ignore_case(element, "disabled").is_none()
}
_ => false,
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/no_redundant_roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use phf::phf_map;
use crate::{
context::LintContext,
rule::Rule,
utils::{get_element_type, has_jsx_prop_lowercase},
utils::{get_element_type, has_jsx_prop_ignore_case},
AstNode,
};

Expand Down Expand Up @@ -55,7 +55,7 @@ impl Rule for NoRedundantRoles {
if let AstKind::JSXOpeningElement(jsx_el) = node.kind() {
if let Some(component) = get_element_type(ctx, jsx_el) {
if let Some(JSXAttributeItem::Attribute(attr)) =
has_jsx_prop_lowercase(jsx_el, "role")
has_jsx_prop_ignore_case(jsx_el, "role")
{
if let Some(JSXAttributeValue::StringLiteral(role_values)) = &attr.value {
let roles: Vec<String> = role_values
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/prefer_tag_over_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use phf::phf_map;
use crate::{
context::LintContext,
rule::Rule,
utils::{get_element_type, has_jsx_prop_lowercase},
utils::{get_element_type, has_jsx_prop_ignore_case},
AstNode,
};

Expand Down Expand Up @@ -90,7 +90,7 @@ impl Rule for PreferTagOverRole {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::JSXOpeningElement(jsx_el) = node.kind() {
if let Some(name) = get_element_type(ctx, jsx_el) {
if let Some(role_prop) = has_jsx_prop_lowercase(jsx_el, "role") {
if let Some(role_prop) = has_jsx_prop_ignore_case(jsx_el, "role") {
Self::check_roles(role_prop, &ROLE_TO_TAG_MAP, &name, ctx);
}
}
Expand Down
Loading

0 comments on commit 7585e16

Please sign in to comment.