diff --git a/Cargo.lock b/Cargo.lock index e72caf230dda2..4bda81ac22bca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3246,6 +3246,9 @@ name = "smallvec" version = "1.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" +dependencies = [ + "serde", +] [[package]] name = "smawk" diff --git a/Cargo.toml b/Cargo.toml index 02a2d82ff8568..133094aa07eee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -223,7 +223,7 @@ sha1 = "0.10.6" # SHA-1 hashing simdutf8 = { version = "0.1.5", features = ["aarch64_neon"] } # SIMD UTF-8 validation similar = "2.7.0" # Text diffing similar-asserts = "1.7.0" # Test diff assertions -smallvec = { version = "1.15.1", features = ["union"] } # Stack-allocated vectors +smallvec = { version = "1.15.1", features = ["union", "serde"] } # Stack-allocated vectors tempfile = "3.23.0" # Temporary files tokio = { version = "1.48.0", default-features = false } # Async runtime toml = { version = "0.9.8" } diff --git a/crates/oxc_linter/src/config/config_builder.rs b/crates/oxc_linter/src/config/config_builder.rs index b7765d2babdb4..f1610158c5a45 100644 --- a/crates/oxc_linter/src/config/config_builder.rs +++ b/crates/oxc_linter/src/config/config_builder.rs @@ -509,7 +509,8 @@ impl ConfigStoreBuilder { severity: *severity, config: rule_name_to_rule .get(&get_name(r.plugin_name(), r.name())) - .and_then(|r| r.config.clone()), + .map(|r| r.config.clone()) + .unwrap_or_default(), }) .collect(); diff --git a/crates/oxc_linter/src/config/config_store.rs b/crates/oxc_linter/src/config/config_store.rs index c688556c1d955..824c2ef8c5756 100644 --- a/crates/oxc_linter/src/config/config_store.rs +++ b/crates/oxc_linter/src/config/config_store.rs @@ -363,6 +363,7 @@ mod test { use rustc_hash::FxHashMap; use serde_json::Value; + use smallvec::smallvec; use super::{ConfigStore, ExternalRuleId, ResolvedOxlintOverrides}; use crate::{ @@ -1077,7 +1078,7 @@ mod test { // Base config has external rule with options A, severity warn let base_external_rule_id = store.lookup_rule_id("custom", "my-rule").unwrap(); let base_options_id = - store.add_options(ExternalRuleId::DUMMY, vec![serde_json::json!({ "opt": "A" })]); + store.add_options(ExternalRuleId::DUMMY, &smallvec![serde_json::json!({ "opt": "A" })]); let base = Config::new( vec![], @@ -1096,7 +1097,7 @@ mod test { base_external_rule_id, store.add_options( ExternalRuleId::DUMMY, - vec![serde_json::json!({ "opt": "B" })], + &smallvec![serde_json::json!({ "opt": "B" })], ), AllowWarnDeny::Deny, )], diff --git a/crates/oxc_linter/src/config/rules.rs b/crates/oxc_linter/src/config/rules.rs index c4ba7659a8910..70a34b08adf09 100644 --- a/crates/oxc_linter/src/config/rules.rs +++ b/crates/oxc_linter/src/config/rules.rs @@ -8,6 +8,7 @@ use serde::{ de::{self, Deserializer, Visitor}, ser::SerializeMap, }; +use smallvec::SmallVec; use oxc_diagnostics::{Error, OxcDiagnostic}; @@ -56,8 +57,8 @@ pub struct ESLintRule { /// Severity of the rule: `off`, `warn`, `error`, etc. pub severity: AllowWarnDeny, /// JSON configuration for the rule, if any. - /// If is `Some`, the `Vec` must not be empty. - pub config: Option>, + /// `SmallVec` with inline capacity 1, because most rules have only one options object. + pub config: SmallVec<[serde_json::Value; 1]>, } impl OxlintRules { @@ -96,14 +97,12 @@ impl OxlintRules { .find(|r| r.name() == rule_name && r.plugin_name() == plugin_name) }); if let Some(rule) = rule { - // Configs are stored as `Option>`, but `from_configuration` expects - // a single `Value` with `Value::Null` being the equivalent of `None` - let config = match &rule_config.config { - Some(config) => { - debug_assert!(!config.is_empty()); - serde_json::Value::Array(config.clone()) - } - None => serde_json::Value::Null, + // Configs are stored as `SmallVec<[Value; 1]>`, but `from_configuration` expects + // a single `Value` with `Value::Null` being the equivalent of empty config + let config = if rule_config.config.is_empty() { + serde_json::Value::Null + } else { + serde_json::Value::Array(rule_config.config.to_vec()) }; rules_to_replace.push((rule.from_configuration(config), severity)); } @@ -120,12 +119,8 @@ impl OxlintRules { external_plugin_store.lookup_rule_id(plugin_name, rule_name)?; // Add options to store and get options ID - let options_id = if let Some(config) = &rule_config.config { - external_plugin_store.add_options(external_rule_id, config.clone()) - } else { - // No options - use reserved index 0 - ExternalOptionsId::NONE - }; + let options_id = external_plugin_store + .add_options(external_rule_id, &rule_config.config); external_rules_for_override .entry(external_rule_id) @@ -197,16 +192,13 @@ impl Serialize for OxlintRules { for rule in &self.rules { let key = rule.full_name(); - match rule.config.as_ref() { - // e.g. unicorn/some-rule: ["warn", { foo: "bar" }] - Some(config) => { - let value = (rule.severity.as_str(), config); - rules.serialize_entry(&key, &value)?; - } + if rule.config.is_empty() { // e.g. unicorn/some-rule: "warn" - _ => { - rules.serialize_entry(&key, rule.severity.as_str())?; - } + rules.serialize_entry(&key, rule.severity.as_str())?; + } else { + // e.g. unicorn/some-rule: ["warn", { foo: "bar" }] + let value = (rule.severity.as_str(), &rule.config); + rules.serialize_entry(&key, &value)?; } } @@ -292,11 +284,11 @@ pub(super) fn unalias_plugin_name(plugin_name: &str, rule_name: &str) -> (String fn parse_rule_value( value: serde_json::Value, -) -> Result<(AllowWarnDeny, Option>), Error> { +) -> Result<(AllowWarnDeny, SmallVec<[serde_json::Value; 1]>), Error> { match value { serde_json::Value::String(_) | serde_json::Value::Number(_) => { let severity = AllowWarnDeny::try_from(&value)?; - Ok((severity, None)) + Ok((severity, SmallVec::new())) } serde_json::Value::Array(mut v) => { @@ -310,13 +302,19 @@ fn parse_rule_value( // The first item should be SeverityConf let severity = AllowWarnDeny::try_from(v.first().unwrap())?; - let config = if v.len() == 1 { + let config = match v.len() { + 0 => unreachable!(), // e.g. ["warn"], [0] - None - } else { - // e.g. ["error", "args", { type: "whatever" }, ["len", "also"]] - v.remove(0); - Some(v) + 1 => SmallVec::new(), + // e.g. ["error", { type: "whatever" }] + // Separate branch for this common case which uses the faster `SmallVec::from_buf`, + // and avoids shifting the first element off the vector. + 2 => SmallVec::from_buf([v.pop().unwrap()]), + // e.g. ["error", { type: "whatever" }, ["len", "also"]] + _ => { + v.remove(0); + SmallVec::from_vec(v) + } }; Ok((severity, config)) @@ -379,25 +377,25 @@ mod test { assert_eq!(r1.rule_name, "no-console"); assert_eq!(r1.plugin_name, "eslint"); assert!(r1.severity.is_allow()); - assert!(r1.config.is_none()); + assert!(r1.config.is_empty()); let r2 = rules.next().unwrap(); assert_eq!(r2.rule_name, "no-unused-vars"); assert_eq!(r2.plugin_name, "foo"); assert!(r2.severity.is_warn_deny()); - assert!(r2.config.is_none()); + assert!(r2.config.is_empty()); let r3 = rules.next().unwrap(); assert_eq!(r3.rule_name, "dummy"); assert_eq!(r3.plugin_name, "eslint"); assert!(r3.severity.is_warn_deny()); - assert_eq!(r3.config, Some(vec![serde_json::json!("arg1"), serde_json::json!("args2")])); + assert_eq!(r3.config.as_slice(), &[serde_json::json!("arg1"), serde_json::json!("args2")]); let r4 = rules.next().unwrap(); assert_eq!(r4.rule_name, "noop"); assert_eq!(r4.plugin_name, "nextjs"); assert!(r4.severity.is_warn_deny()); - assert!(r4.config.is_none()); + assert!(r4.config.is_empty()); } #[test] diff --git a/crates/oxc_linter/src/external_plugin_store.rs b/crates/oxc_linter/src/external_plugin_store.rs index 34bb14547bee5..0debcd340028a 100644 --- a/crates/oxc_linter/src/external_plugin_store.rs +++ b/crates/oxc_linter/src/external_plugin_store.rs @@ -4,6 +4,7 @@ use std::{ }; use rustc_hash::{FxHashMap, FxHashSet}; +use smallvec::SmallVec; use oxc_index::{IndexVec, define_index_type, index_vec}; use serde::{Serialize, Serializer}; @@ -42,7 +43,7 @@ pub struct ExternalPluginStore { rules: IndexVec, /// Options for a rule, indexed by `ExternalOptionsId`. /// The rule ID is also stored, so that can merge options with the rule's default options on JS side. - options: IndexVec)>, + options: IndexVec)>, /// `true` for `oxlint`, `false` for language server is_enabled: bool, @@ -56,7 +57,7 @@ impl Default for ExternalPluginStore { impl ExternalPluginStore { pub fn new(is_enabled: bool) -> Self { - let options = index_vec![(ExternalRuleId::DUMMY, vec![])]; + let options = index_vec![(ExternalRuleId::DUMMY, SmallVec::new())]; Self { registered_plugin_paths: FxHashSet::default(), @@ -145,13 +146,17 @@ impl ExternalPluginStore { } /// Add options to the store and return its [`ExternalOptionsId`]. + /// If `options` is empty, returns [`ExternalOptionsId::NONE`] without adding to the store. pub fn add_options( &mut self, rule_id: ExternalRuleId, - options: Vec, + options: &SmallVec<[serde_json::Value; 1]>, ) -> ExternalOptionsId { - debug_assert!(!options.is_empty(), "`options` should never be an empty `Vec`"); - self.options.push((rule_id, options)) + if options.is_empty() { + ExternalOptionsId::NONE + } else { + self.options.push((rule_id, options.clone())) + } } /// Send options to JS side.