refactor(linter): update comments, improve tests, add variant All to LintFilterKind (#10259)

I split this PR into easily reviewable commits. While working on this
PR, I noticed two possible follow up changes:

`LintFilterKind::Rule` and `LintFilterKind::Generic` behave exactly the
same in `ConfigStoreBuilder::with_filter` because `LintFilterKind::Rule`
never actually uses or checks for the plugin it stores. Should I include
the check for plugin?

The proposed fifth `LintFilterKind` isn't implemented yet (see: `//
TODO: plugin + category? e.g -A react:correctness)`). Should I add it?

Author: ulrichstark

crates/oxc_linter/src/config/config_builder.rs

@@ -243,28 +243,18 @@ impl ConfigStoreBuilder {
                     self.upsert_where(severity, |r| r.category() == *category);
                 }
                 LintFilterKind::Rule(_, name) => self.upsert_where(severity, |r| r.name() == name),
-                LintFilterKind::Generic(name_or_category) => {
-                    if name_or_category == "all" {
-                        self.upsert_where(severity, |r| r.category() != RuleCategory::Nursery);
-                    } else {
-                        self.upsert_where(severity, |r| r.name() == name_or_category);
-                    }
+                LintFilterKind::Generic(name) => self.upsert_where(severity, |r| r.name() == name),
+                LintFilterKind::All => {
+                    self.upsert_where(severity, |r| r.category() != RuleCategory::Nursery);
                 }
             },
             AllowWarnDeny::Allow => match filter {
                 LintFilterKind::Category(category) => {
                     self.rules.retain(|rule| rule.category() != *category);
                 }
-                LintFilterKind::Rule(_, name) => {
-                    self.rules.retain(|rule| rule.name() != name);
-                }
-                LintFilterKind::Generic(name_or_category) => {
-                    if name_or_category == "all" {
-                        self.rules.clear();
-                    } else {
-                        self.rules.retain(|rule| rule.name() != name_or_category);
-                    }
-                }
+                LintFilterKind::Rule(_, name) => self.rules.retain(|rule| rule.name() != name),
+                LintFilterKind::Generic(name) => self.rules.retain(|rule| rule.name() != name),
+                LintFilterKind::All => self.rules.clear(),
             },
         }
 

crates/oxc_linter/src/options/filter.rs

@@ -78,8 +78,10 @@ impl<'a> From<&'a LintFilter> for (AllowWarnDeny, &'a LintFilterKind) {
 
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum LintFilterKind {
+    All,
+    /// e.g. `no-const-assign`
     Generic(Cow<'static, str>),
-    /// e.g. `no-const-assign` or `eslint/no-const-assign`
+    /// e.g. `eslint/no-const-assign`
     Rule(LintPlugins, Cow<'static, str>),
     /// e.g. `correctness`
     Category(RuleCategory),
@@ -143,7 +145,13 @@ impl LintFilterKind {
         } else {
             match RuleCategory::try_from(filter.as_ref()) {
                 Ok(category) => Ok(LintFilterKind::Category(category)),
-                Err(()) => Ok(LintFilterKind::Generic(filter)),
+                Err(()) => {
+                    if filter == "all" {
+                        Ok(LintFilterKind::All)
+                    } else {
+                        Ok(LintFilterKind::Generic(filter))
+                    }
+                }
             }
         }
     }
@@ -220,11 +228,7 @@ mod test {
     fn test_from_category() {
         let correctness: LintFilter = LintFilter::new(AllowWarnDeny::Warn, "correctness").unwrap();
         assert_eq!(correctness.severity(), AllowWarnDeny::Warn);
-        assert!(
-            matches!(correctness.kind(), LintFilterKind::Category(RuleCategory::Correctness)),
-            "{:?}",
-            correctness.kind()
-        );
+        assert_eq!(correctness.kind(), &LintFilterKind::Category(RuleCategory::Correctness));
     }
 
     #[test]
@@ -239,24 +243,23 @@ mod test {
             filter.kind(),
             &LintFilterKind::Rule(LintPlugins::from("eslint"), "no-const-assign".into())
         );
-        assert!(matches!(filter.kind(), LintFilterKind::Rule(_, _)));
     }
 
     #[test]
     fn test_parse() {
         #[rustfmt::skip]
         let test_cases: Vec<(&'static str, LintFilterKind)> = vec![
-            ("no-const-assign", LintFilterKind::Generic("no-const-assign".into())),
             ("eslint/no-const-assign", LintFilterKind::Rule(LintPlugins::ESLINT, "no-const-assign".into())),
             ("import/namespace", LintFilterKind::Rule(LintPlugins::IMPORT, "namespace".into())),
             ("react-hooks/exhaustive-deps", LintFilterKind::Rule(LintPlugins::REACT, "exhaustive-deps".into())),
             // categories
             ("correctness", LintFilterKind::Category(RuleCategory::Correctness)),
-            ("nursery", LintFilterKind::Category("nursery".try_into().unwrap())),
-            ("perf", LintFilterKind::Category("perf".try_into().unwrap())),
+            ("nursery", LintFilterKind::Category(RuleCategory::Nursery)),
+            ("perf", LintFilterKind::Category(RuleCategory::Perf)),
             // misc
+            ("no-const-assign", LintFilterKind::Generic("no-const-assign".into())),
             ("not-a-valid-filter", LintFilterKind::Generic("not-a-valid-filter".into())),
-            ("all", LintFilterKind::Generic("all".into())),
+            ("all", LintFilterKind::All),
         ];
 
         for (input, expected) in test_cases {