Skip to content

Commit 838d81f

Browse files
camc314taearls
authored andcommitted
fix(linter): prevent category rules from being reapplied to already-configured plugins in overrides (oxc-project#12875)
fixes oxc-project#12859
1 parent 8448197 commit 838d81f

File tree

1 file changed

+219
-8
lines changed

1 file changed

+219
-8
lines changed

crates/oxc_linter/src/config/config_store.rs

Lines changed: 219 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,15 +183,34 @@ impl Config {
183183

184184
let mut external_rules = FxHashMap::default();
185185

186+
// Track which plugins have already had their category rules applied.
187+
// Start with the root plugins since they already have categories applied in base_rules.
188+
let mut configured_plugins = self.base.config.plugins.builtin;
189+
186190
for override_config in overrides_to_apply {
187191
if let Some(override_plugins) = &override_config.plugins {
188192
if *override_plugins != plugins {
189-
for (rule, severity) in all_rules.iter().filter_map(|rule| {
190-
self.categories
191-
.get(&rule.category())
192-
.map(|severity| (rule.clone(), severity))
193-
}) {
194-
rules.entry(rule).or_insert(*severity);
193+
// Only apply categories to plugins that:
194+
// 1. Are in the current accumulated plugin set
195+
// 2. Have NOT been configured yet (not in root or previous overrides)
196+
let unconfigured_plugins = plugins.builtin & !configured_plugins;
197+
198+
if !unconfigured_plugins.is_empty() {
199+
for (rule, severity) in all_rules.iter().filter_map(|rule| {
200+
let rule_plugin = BuiltinLintPlugins::from(rule.plugin_name());
201+
// Only apply categories to rules from unconfigured plugins
202+
if unconfigured_plugins.contains(rule_plugin) {
203+
self.categories
204+
.get(&rule.category())
205+
.map(|severity| (rule.clone(), severity))
206+
} else {
207+
None
208+
}
209+
}) {
210+
rules.entry(rule).or_insert(*severity);
211+
}
212+
// Mark these plugins as configured
213+
configured_plugins |= unconfigured_plugins;
195214
}
196215
}
197216
}
@@ -326,14 +345,15 @@ mod test {
326345

327346
use super::{ConfigStore, ResolvedOxlintOverrides};
328347
use crate::{
329-
AllowWarnDeny, BuiltinLintPlugins, ExternalPluginStore, LintPlugins, RuleEnum,
348+
AllowWarnDeny, BuiltinLintPlugins, ExternalPluginStore, LintPlugins, RuleCategory,
349+
RuleEnum,
330350
config::{
331351
LintConfig, OxlintEnv, OxlintGlobals, OxlintSettings,
332352
categories::OxlintCategories,
333353
config_store::{Config, ResolvedOxlintOverride, ResolvedOxlintOverrideRules},
334354
overrides::GlobSet,
335355
},
336-
rules::{EslintNoUnusedVars, TypescriptNoExplicitAny},
356+
rules::{EslintNoUnusedVars, ReactJsxFilenameExtension, TypescriptNoExplicitAny},
337357
};
338358

339359
macro_rules! from_json {
@@ -711,4 +731,195 @@ mod test {
711731
assert!(!app.globals.is_enabled("React"));
712732
assert!(!app.globals.is_enabled("Secret"));
713733
}
734+
735+
#[test]
736+
fn test_override_rule_not_reset_by_later_override_with_different_plugins() {
737+
// This test reproduces the issue from https://github.com/oxc-project/oxc/issues/12859
738+
// When multiple overrides apply to a file and they have different plugins,
739+
// the later override should not reset rules that were explicitly set in earlier overrides.
740+
741+
// Root config with react, typescript, unicorn plugins and restriction category
742+
let base_config = LintConfig {
743+
plugins: LintPlugins::new(
744+
BuiltinLintPlugins::REACT
745+
| BuiltinLintPlugins::TYPESCRIPT
746+
| BuiltinLintPlugins::UNICORN,
747+
FxHashSet::default(),
748+
),
749+
env: OxlintEnv::default(),
750+
settings: OxlintSettings::default(),
751+
globals: OxlintGlobals::default(),
752+
path: None,
753+
};
754+
755+
// Set up categories to enable restriction rules
756+
let mut categories = OxlintCategories::default();
757+
categories.insert(RuleCategory::Restriction, AllowWarnDeny::Warn);
758+
759+
// Create overrides similar to the user's config
760+
let overrides = ResolvedOxlintOverrides::new(vec![
761+
// First override: typescript plugin for *.{ts,tsx,mts}
762+
ResolvedOxlintOverride {
763+
env: None,
764+
files: GlobSet::new(vec!["*.{ts,tsx,mts}"]).unwrap(),
765+
plugins: Some(LintPlugins::new(
766+
BuiltinLintPlugins::TYPESCRIPT,
767+
FxHashSet::default(),
768+
)),
769+
globals: None,
770+
rules: ResolvedOxlintOverrideRules {
771+
builtin_rules: vec![],
772+
external_rules: vec![],
773+
},
774+
},
775+
// Second override: react plugin for *.{ts,tsx} with jsx-filename-extension turned off
776+
ResolvedOxlintOverride {
777+
env: None,
778+
files: GlobSet::new(vec!["*.{ts,tsx}"]).unwrap(),
779+
plugins: Some(LintPlugins::new(BuiltinLintPlugins::REACT, FxHashSet::default())),
780+
globals: None,
781+
rules: ResolvedOxlintOverrideRules {
782+
builtin_rules: vec![(
783+
RuleEnum::ReactJsxFilenameExtension(ReactJsxFilenameExtension::default()),
784+
AllowWarnDeny::Allow,
785+
)],
786+
external_rules: vec![],
787+
},
788+
},
789+
// Third override: unicorn plugin for *.{ts,tsx,mts}
790+
ResolvedOxlintOverride {
791+
env: None,
792+
files: GlobSet::new(vec!["*.{ts,tsx,mts}"]).unwrap(),
793+
plugins: Some(LintPlugins::new(BuiltinLintPlugins::UNICORN, FxHashSet::default())),
794+
globals: None,
795+
rules: ResolvedOxlintOverrideRules {
796+
builtin_rules: vec![],
797+
external_rules: vec![],
798+
},
799+
},
800+
]);
801+
802+
// Create base rules - jsx-filename-extension should be enabled by restriction category
803+
let base_rules = vec![(
804+
RuleEnum::ReactJsxFilenameExtension(ReactJsxFilenameExtension::default()),
805+
AllowWarnDeny::Warn,
806+
)];
807+
808+
let store = ConfigStore::new(
809+
Config::new(base_rules, vec![], categories, base_config, overrides),
810+
FxHashMap::default(),
811+
ExternalPluginStore::default(),
812+
);
813+
814+
// Resolve rules for a .tsx file
815+
let rules_for_tsx = store.resolve("App.tsx".as_ref());
816+
817+
// The jsx-filename-extension rule should be disabled (Allow) because the second override
818+
// explicitly set it to Allow, and the third override should not reset it
819+
let jsx_filename_rule = rules_for_tsx
820+
.rules
821+
.iter()
822+
.find(|(rule, _)| matches!(rule, RuleEnum::ReactJsxFilenameExtension(_)));
823+
824+
// This test should fail with the current implementation
825+
// The bug causes the rule to be re-enabled (Warn) instead of staying disabled (Allow)
826+
assert!(
827+
jsx_filename_rule.is_none(),
828+
"jsx-filename-extension should be disabled (not present in active rules)"
829+
);
830+
}
831+
832+
#[test]
833+
fn test_categories_only_applied_to_new_plugins_not_in_root() {
834+
// Test that categories are only applied to plugins that weren't in the root config
835+
836+
// Root config with only typescript plugin
837+
let base_config = LintConfig {
838+
plugins: LintPlugins::new(BuiltinLintPlugins::TYPESCRIPT, FxHashSet::default()),
839+
env: OxlintEnv::default(),
840+
settings: OxlintSettings::default(),
841+
globals: OxlintGlobals::default(),
842+
path: None,
843+
};
844+
845+
// Set up categories
846+
let mut categories = OxlintCategories::default();
847+
categories.insert(RuleCategory::Restriction, AllowWarnDeny::Warn);
848+
849+
// Override adds react plugin (new plugin not in root)
850+
let overrides = ResolvedOxlintOverrides::new(vec![ResolvedOxlintOverride {
851+
env: None,
852+
files: GlobSet::new(vec!["*.tsx"]).unwrap(),
853+
plugins: Some(LintPlugins::new(BuiltinLintPlugins::REACT, FxHashSet::default())),
854+
globals: None,
855+
rules: ResolvedOxlintOverrideRules { builtin_rules: vec![], external_rules: vec![] },
856+
}]);
857+
858+
let store = ConfigStore::new(
859+
Config::new(vec![], vec![], categories, base_config, overrides),
860+
FxHashMap::default(),
861+
ExternalPluginStore::default(),
862+
);
863+
864+
// For .tsx files, react rules should be enabled by categories since react wasn't in root
865+
let rules_for_tsx = store.resolve("App.tsx".as_ref());
866+
867+
// Check that react rules are present (categories were applied to the new plugin)
868+
let has_react_rules =
869+
rules_for_tsx.rules.iter().any(|(rule, _)| rule.plugin_name() == "react");
870+
871+
assert!(has_react_rules, "React rules should be enabled by categories for new plugin");
872+
}
873+
874+
#[test]
875+
fn test_categories_not_reapplied_to_root_plugins() {
876+
// Test that categories are NOT re-applied to plugins that were already in root
877+
878+
// Root config with react plugin
879+
let base_config = LintConfig {
880+
plugins: LintPlugins::new(BuiltinLintPlugins::REACT, FxHashSet::default()),
881+
env: OxlintEnv::default(),
882+
settings: OxlintSettings::default(),
883+
globals: OxlintGlobals::default(),
884+
path: None,
885+
};
886+
887+
// Set up categories
888+
let mut categories = OxlintCategories::default();
889+
categories.insert(RuleCategory::Restriction, AllowWarnDeny::Warn);
890+
891+
// Base rules with jsx-filename-extension disabled
892+
let base_rules = vec![(
893+
RuleEnum::ReactJsxFilenameExtension(ReactJsxFilenameExtension::default()),
894+
AllowWarnDeny::Allow, // Disabled at root
895+
)];
896+
897+
// Override adds typescript plugin
898+
let overrides = ResolvedOxlintOverrides::new(vec![ResolvedOxlintOverride {
899+
env: None,
900+
files: GlobSet::new(vec!["*.tsx"]).unwrap(),
901+
plugins: Some(LintPlugins::new(BuiltinLintPlugins::TYPESCRIPT, FxHashSet::default())),
902+
globals: None,
903+
rules: ResolvedOxlintOverrideRules { builtin_rules: vec![], external_rules: vec![] },
904+
}]);
905+
906+
let store = ConfigStore::new(
907+
Config::new(base_rules, vec![], categories, base_config, overrides),
908+
FxHashMap::default(),
909+
ExternalPluginStore::default(),
910+
);
911+
912+
// For .tsx files, jsx-filename-extension should remain disabled
913+
let rules_for_tsx = store.resolve("App.tsx".as_ref());
914+
915+
let jsx_filename_rule = rules_for_tsx
916+
.rules
917+
.iter()
918+
.find(|(rule, _)| matches!(rule, RuleEnum::ReactJsxFilenameExtension(_)));
919+
920+
assert!(
921+
jsx_filename_rule.is_none(),
922+
"jsx-filename-extension should remain disabled (not re-enabled by categories)"
923+
);
924+
}
714925
}

0 commit comments

Comments
 (0)