Skip to content

Commit

Permalink
perf(linter): change react rules and utils to use Cow and `CompactS…
Browse files Browse the repository at this point in the history
…tr` instead of `String` (#4603)
  • Loading branch information
DonIsaac committed Aug 3, 2024
1 parent fd2d9da commit 6ff200d
Show file tree
Hide file tree
Showing 18 changed files with 215 additions and 123 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion crates/oxc_linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ doctest = false
[dependencies]
oxc_allocator = { workspace = true }
oxc_parser = { workspace = true }
oxc_span = { workspace = true }
oxc_span = { workspace = true, features = ["schemars"] }
oxc_ast = { workspace = true }
oxc_cfg = { workspace = true }
oxc_diagnostics = { workspace = true }
Expand Down
6 changes: 5 additions & 1 deletion crates/oxc_linter/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ fn transform_rule_and_plugin_name<'a>(
mod test {
use std::env;

use oxc_span::CompactStr;
use rustc_hash::FxHashSet;
use serde::Deserialize;

Expand Down Expand Up @@ -235,7 +236,10 @@ mod test {

let OxlintConfig { rules, settings, env, globals } = config.unwrap();
assert!(!rules.is_empty());
assert_eq!(settings.jsx_a11y.polymorphic_prop_name, Some("role".to_string()));
assert_eq!(
settings.jsx_a11y.polymorphic_prop_name.as_ref().map(CompactStr::as_str),
Some("role")
);
assert_eq!(env.iter().count(), 1);
assert!(globals.is_enabled("foo"));
}
Expand Down
5 changes: 3 additions & 2 deletions crates/oxc_linter/src/config/settings/jsx_a11y.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use oxc_span::CompactStr;
use rustc_hash::FxHashMap;
use schemars::JsonSchema;
use serde::Deserialize;
Expand All @@ -6,7 +7,7 @@ use serde::Deserialize;
#[derive(Debug, Deserialize, Default, JsonSchema)]
pub struct JSXA11yPluginSettings {
#[serde(rename = "polymorphicPropName")]
pub polymorphic_prop_name: Option<String>,
pub polymorphic_prop_name: Option<CompactStr>,
#[serde(default)]
pub components: FxHashMap<String, String>,
pub components: FxHashMap<CompactStr, CompactStr>,
}
32 changes: 23 additions & 9 deletions crates/oxc_linter/src/config/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,21 @@ pub struct OxlintSettings {

#[cfg(test)]
mod test {
use std::borrow::Cow;

use oxc_span::CompactStr;
use serde::Deserialize;

use crate::config::settings::react::ComponentAttrs;

use super::OxlintSettings;

fn as_attrs<S: Into<CompactStr>, I: IntoIterator<Item = S>>(
attrs: I,
) -> ComponentAttrs<'static> {
ComponentAttrs::from(Cow::Owned(attrs.into_iter().map(Into::into).collect()))
}

#[test]
fn test_parse_settings() {
let settings = OxlintSettings::deserialize(&serde_json::json!({
Expand Down Expand Up @@ -62,21 +73,24 @@ mod test {
}))
.unwrap();

assert_eq!(settings.jsx_a11y.polymorphic_prop_name, Some("role".to_string()));
assert_eq!(settings.jsx_a11y.components.get("Link"), Some(&"Anchor".to_string()));
assert_eq!(settings.jsx_a11y.polymorphic_prop_name, Some("role".into()));
assert_eq!(settings.jsx_a11y.components.get("Link"), Some(&"Anchor".into()));
assert!(settings.next.get_root_dirs().contains(&"app".to_string()));
assert_eq!(settings.react.get_form_component_attrs("CustomForm"), Some(vec![]));
assert_eq!(
settings.react.get_form_component_attrs("SimpleForm"),
Some(vec!["endpoint".to_string()])
settings.react.get_form_component_attrs("CustomForm").unwrap(),
as_attrs::<CompactStr, _>(vec![])
);
assert_eq!(
settings.react.get_form_component_attrs("SimpleForm").unwrap(),
as_attrs(["endpoint"])
);
assert_eq!(
settings.react.get_form_component_attrs("Form"),
Some(vec!["registerEndpoint".to_string(), "loginEndpoint".to_string()])
settings.react.get_form_component_attrs("Form").unwrap(),
as_attrs(["registerEndpoint", "loginEndpoint"])
);
assert_eq!(
settings.react.get_link_component_attrs("Link"),
Some(vec!["to".to_string(), "href".to_string()])
settings.react.get_link_component_attrs("Link").unwrap(),
as_attrs(["to", "href"])
);
assert_eq!(settings.react.get_link_component_attrs("Noop"), None);
}
Expand Down
47 changes: 28 additions & 19 deletions crates/oxc_linter/src/config/settings/react.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
use std::borrow::Cow;

use oxc_span::CompactStr;
use schemars::JsonSchema;
use serde::Deserialize;

Expand All @@ -14,12 +17,13 @@ pub struct ReactPluginSettings {
// TODO: More properties should be added
}

pub type ComponentAttrs<'c> = Cow<'c, Vec<CompactStr>>;
impl ReactPluginSettings {
pub fn get_form_component_attrs(&self, name: &str) -> Option<Vec<String>> {
pub fn get_form_component_attrs(&self, name: &str) -> Option<ComponentAttrs<'_>> {
get_component_attrs_by_name(&self.form_components, name)
}

pub fn get_link_component_attrs(&self, name: &str) -> Option<Vec<String>> {
pub fn get_link_component_attrs(&self, name: &str) -> Option<ComponentAttrs<'_>> {
get_component_attrs_by_name(&self.link_components, name)
}
}
Expand All @@ -29,35 +33,40 @@ impl ReactPluginSettings {
#[derive(Clone, Debug, Deserialize, JsonSchema)]
#[serde(untagged)]
enum CustomComponent {
NameOnly(String),
NameOnly(CompactStr),
ObjectWithOneAttr {
name: String,
name: CompactStr,
#[serde(alias = "formAttribute", alias = "linkAttribute")]
attribute: String,
attribute: CompactStr,
},
ObjectWithManyAttrs {
name: String,
name: CompactStr,
#[serde(alias = "formAttribute", alias = "linkAttribute")]
attributes: Vec<String>,
attributes: Vec<CompactStr>,
},
}

fn get_component_attrs_by_name(
components: &Vec<CustomComponent>,
fn get_component_attrs_by_name<'c>(
components: &'c Vec<CustomComponent>,
name: &str,
) -> Option<Vec<String>> {
) -> Option<ComponentAttrs<'c>> {
for item in components {
let comp = match item {
CustomComponent::NameOnly(name) => (name, vec![]),
CustomComponent::ObjectWithOneAttr { name, attribute } => {
(name, vec![attribute.to_string()])
match item {
CustomComponent::NameOnly(comp_name) if comp_name == name => {
return Some(Cow::Owned(vec![]))
}
CustomComponent::ObjectWithOneAttr { name: comp_name, attribute }
if comp_name == name =>
{
return Some(Cow::Owned(vec![attribute.clone()]));
}
CustomComponent::ObjectWithManyAttrs { name, attributes } => (name, attributes.clone()),
CustomComponent::ObjectWithManyAttrs { name: comp_name, attributes }
if comp_name == name =>
{
return Some(Cow::Borrowed(attributes));
}
_ => {}
};

if comp.0 == name {
return Some(comp.1);
}
}

None
Expand Down
38 changes: 20 additions & 18 deletions crates/oxc_linter/src/rules/jsx_a11y/autocomplete_valid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ use oxc_ast::{
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use oxc_span::{CompactStr, Span};
use phf::{phf_map, phf_set};
use rustc_hash::FxHashSet;
use serde_json::Value;

use crate::{
context::LintContext,
Expand Down Expand Up @@ -43,7 +45,7 @@ declare_oxc_lint!(

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AutocompleteValidConfig {
input_components: Vec<String>,
input_components: FxHashSet<CompactStr>,
}

impl std::ops::Deref for AutocompleteValid {
Expand All @@ -56,7 +58,7 @@ impl std::ops::Deref for AutocompleteValid {

impl std::default::Default for AutocompleteValidConfig {
fn default() -> Self {
Self { input_components: vec!["input".to_string()] }
Self { input_components: FxHashSet::from_iter([CompactStr::new("input")]) }
}
}

Expand Down Expand Up @@ -157,29 +159,29 @@ fn is_valid_autocomplete_value(value: &str) -> bool {
}

impl Rule for AutocompleteValid {
fn from_configuration(value: serde_json::Value) -> Self {
let mut input_components: Vec<String> = vec!["input".to_string()];
if let Some(config) = value.get(0) {
if let Some(serde_json::Value::Array(components)) = config.get("inputComponents") {
input_components = components
fn from_configuration(config: Value) -> Self {
config
.get(0)
.and_then(|c| c.get("inputComponents"))
.and_then(Value::as_array)
.map(|components| {
components
.iter()
.filter_map(|c| c.as_str().map(std::string::ToString::to_string))
.collect();
}
}

// Add default input component
input_components.push("input".to_string());

Self(Box::new(AutocompleteValidConfig { input_components }))
.filter_map(Value::as_str)
.map(CompactStr::from)
.chain(Some("input".into()))
.collect()
})
.map(|input_components| Self(Box::new(AutocompleteValidConfig { input_components })))
.unwrap_or_default()
}

fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::JSXOpeningElement(jsx_el) = node.kind() {
let Some(name) = &get_element_type(ctx, jsx_el) else {
return;
};
if !self.input_components.contains(name) {
if !self.input_components.contains(name.as_ref()) {
return;
}

Expand Down
53 changes: 34 additions & 19 deletions crates/oxc_linter/src/rules/jsx_a11y/media_has_caption.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use std::borrow::Cow;

use oxc_ast::{
ast::{JSXAttributeItem, JSXAttributeName, JSXAttributeValue, JSXChild, JSXExpression},
AstKind,
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use serde_json::Value;

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

Expand All @@ -19,17 +22,17 @@ pub struct MediaHasCaption(Box<MediaHasCaptionConfig>);

#[derive(Debug, Clone)]
pub struct MediaHasCaptionConfig {
audio: Vec<String>,
video: Vec<String>,
track: Vec<String>,
audio: Vec<Cow<'static, str>>,
video: Vec<Cow<'static, str>>,
track: Vec<Cow<'static, str>>,
}

impl Default for MediaHasCaptionConfig {
fn default() -> Self {
Self {
audio: vec!["audio".to_string()],
video: vec!["video".to_string()],
track: vec!["track".to_string()],
audio: vec![Cow::Borrowed("audio")],
video: vec![Cow::Borrowed("video")],
track: vec![Cow::Borrowed("track")],
}
}
}
Expand Down Expand Up @@ -58,26 +61,38 @@ declare_oxc_lint!(
);

impl Rule for MediaHasCaption {
fn from_configuration(value: serde_json::Value) -> Self {
fn from_configuration(value: Value) -> Self {
let mut config = MediaHasCaptionConfig::default();

if let Some(arr) = value.as_array() {
for v in arr {
if let serde_json::Value::Object(rule_config) = v {
if let Some(audio) = rule_config.get("audio").and_then(|v| v.as_array()) {
config
.audio
.extend(audio.iter().filter_map(|v| v.as_str().map(String::from)));
if let Some(audio) = rule_config.get("audio").and_then(Value::as_array) {
config.audio.extend(
audio
.iter()
.filter_map(Value::as_str)
.map(String::from)
.map(Into::into),
);
}
if let Some(video) = rule_config.get("video").and_then(|v| v.as_array()) {
config
.video
.extend(video.iter().filter_map(|v| v.as_str().map(String::from)));
if let Some(video) = rule_config.get("video").and_then(Value::as_array) {
config.video.extend(
video
.iter()
.filter_map(Value::as_str)
.map(String::from)
.map(Into::into),
);
}
if let Some(track) = rule_config.get("track").and_then(|v| v.as_array()) {
config
.track
.extend(track.iter().filter_map(|v| v.as_str().map(String::from)));
if let Some(track) = rule_config.get("track").and_then(Value::as_array) {
config.track.extend(
track
.iter()
.filter_map(Value::as_str)
.map(String::from)
.map(Into::into),
);
}
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn is_aria_hidden_true(attr: &JSXAttributeItem) -> bool {
/// # Returns
///
/// `true` if the element is focusable, `false` otherwise.
fn is_focusable(ctx: &LintContext, element: &JSXOpeningElement) -> bool {
fn is_focusable<'a>(ctx: &LintContext<'a>, element: &JSXOpeningElement<'a>) -> bool {
let Some(tag_name) = get_element_type(ctx, element) else {
return false;
};
Expand All @@ -95,7 +95,7 @@ fn is_focusable(ctx: &LintContext, element: &JSXOpeningElement) -> bool {
}
}

match tag_name.as_str() {
match tag_name.as_ref() {
"a" | "area" => has_jsx_prop_ignore_case(element, "href").is_some(),
"button" | "input" | "select" | "textarea" => {
has_jsx_prop_ignore_case(element, "disabled").is_none()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ impl Rule for NoDistractingElements {
return;
};

let name = element_type.as_str();

if let "marquee" | "blink" = name {
if let "marquee" | "blink" = element_type.as_ref() {
ctx.diagnostic(no_distracting_elements_diagnostic(iden.span));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl Rule for RoleSupportsAriaProps {
if let Some(el_type) = get_element_type(ctx, jsx_el) {
let role = has_jsx_prop_ignore_case(jsx_el, "role");
let role_value = role.map_or_else(
|| get_implicit_role(jsx_el, el_type.as_str()),
|| get_implicit_role(jsx_el, &el_type),
|i| get_string_literal_prop_value(i),
);
let is_implicit = role_value.is_some() && role.is_none();
Expand Down
Loading

0 comments on commit 6ff200d

Please sign in to comment.