Skip to content

Commit c51a380

Browse files
committed
refactor(linter): store rule options as Vecs (#16339)
Rule options are always an array, so store them as a `Vec<Value>` instead of a single `Value` which is always a `Value::Array`. This gives better type safety.
1 parent 165f59d commit c51a380

File tree

3 files changed

+18
-17
lines changed

3 files changed

+18
-17
lines changed

crates/oxc_linter/src/config/config_store.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,7 +1077,7 @@ mod test {
10771077
// Base config has external rule with options A, severity warn
10781078
let base_external_rule_id = store.lookup_rule_id("custom", "my-rule").unwrap();
10791079
let base_options_id =
1080-
store.add_options(ExternalRuleId::DUMMY, serde_json::json!([{ "opt": "A" }]));
1080+
store.add_options(ExternalRuleId::DUMMY, vec![serde_json::json!({ "opt": "A" })]);
10811081

10821082
let base = Config::new(
10831083
vec![],
@@ -1096,7 +1096,7 @@ mod test {
10961096
base_external_rule_id,
10971097
store.add_options(
10981098
ExternalRuleId::DUMMY,
1099-
serde_json::json!([{ "opt": "B" }]),
1099+
vec![serde_json::json!({ "opt": "B" })],
11001100
),
11011101
AllowWarnDeny::Deny,
11021102
)],

crates/oxc_linter/src/config/rules.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ pub struct ESLintRule {
5656
/// Severity of the rule: `off`, `warn`, `error`, etc.
5757
pub severity: AllowWarnDeny,
5858
/// JSON configuration for the rule, if any.
59-
pub config: Option<serde_json::Value>,
59+
/// If is `Some`, the `Vec` must not be empty.
60+
pub config: Option<Vec<serde_json::Value>>,
6061
}
6162

6263
impl OxlintRules {
@@ -95,7 +96,15 @@ impl OxlintRules {
9596
.find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
9697
});
9798
if let Some(rule) = rule {
98-
let config = rule_config.config.clone().unwrap_or_default();
99+
// Configs are stored as `Option<Vec<Value>>`, but `from_configuration` expects
100+
// a single `Value` with `Value::Null` being the equivalent of `None`
101+
let config = match &rule_config.config {
102+
Some(config) => {
103+
debug_assert!(!config.is_empty());
104+
serde_json::Value::Array(config.clone())
105+
}
106+
None => serde_json::Value::Null,
107+
};
99108
rules_to_replace.push((rule.from_configuration(config), severity));
100109
}
101110
} else {
@@ -190,7 +199,7 @@ impl Serialize for OxlintRules {
190199
let key = rule.full_name();
191200
match rule.config.as_ref() {
192201
// e.g. unicorn/some-rule: ["warn", { foo: "bar" }]
193-
Some(config) if !config.is_null() => {
202+
Some(config) => {
194203
let value = (rule.severity.as_str(), config);
195204
rules.serialize_entry(&key, &value)?;
196205
}
@@ -283,7 +292,7 @@ pub(super) fn unalias_plugin_name(plugin_name: &str, rule_name: &str) -> (String
283292

284293
fn parse_rule_value(
285294
value: serde_json::Value,
286-
) -> Result<(AllowWarnDeny, Option<serde_json::Value>), Error> {
295+
) -> Result<(AllowWarnDeny, Option<Vec<serde_json::Value>>), Error> {
287296
match value {
288297
serde_json::Value::String(_) | serde_json::Value::Number(_) => {
289298
let severity = AllowWarnDeny::try_from(&value)?;
@@ -307,7 +316,7 @@ fn parse_rule_value(
307316
} else {
308317
// e.g. ["error", "args", { type: "whatever" }, ["len", "also"]]
309318
v.remove(0);
310-
Some(serde_json::Value::Array(v))
319+
Some(v)
311320
};
312321

313322
Ok((severity, config))
@@ -382,7 +391,7 @@ mod test {
382391
assert_eq!(r3.rule_name, "dummy");
383392
assert_eq!(r3.plugin_name, "eslint");
384393
assert!(r3.severity.is_warn_deny());
385-
assert_eq!(r3.config, Some(serde_json::json!(["arg1", "args2"])));
394+
assert_eq!(r3.config, Some(vec![serde_json::json!("arg1"), serde_json::json!("args2")]));
386395

387396
let r4 = rules.next().unwrap();
388397
assert_eq!(r4.rule_name, "noop");

crates/oxc_linter/src/external_plugin_store.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,19 +145,11 @@ impl ExternalPluginStore {
145145
}
146146

147147
/// Add options to the store and return its [`ExternalOptionsId`].
148-
///
149-
/// `options` must be a `serde_json::Value::Array`.
150-
///
151-
/// # Panics
152-
/// Panics if `options` is not an array.
153148
pub fn add_options(
154149
&mut self,
155150
rule_id: ExternalRuleId,
156-
options: serde_json::Value,
151+
options: Vec<serde_json::Value>,
157152
) -> ExternalOptionsId {
158-
let serde_json::Value::Array(options) = options else {
159-
panic!("`options` must be an array");
160-
};
161153
debug_assert!(!options.is_empty(), "`options` should never be an empty `Vec`");
162154
self.options.push((rule_id, options))
163155
}

0 commit comments

Comments
 (0)