refactor(cli,linter): move LintOptions from cli to linter (#753)

This also simplifies the Runner trait.

crates/oxc_cli/src/lib.rs

@@ -7,9 +7,9 @@ mod walk;
 use clap::{Arg, Command};
 
 pub use crate::{
-    lint::{LintOptions, LintRunner},
-    runner::{CliRunResult, Runner, RunnerOptions},
-    type_check::{TypeCheckOptions, TypeCheckRunner},
+    lint::LintRunner,
+    runner::{CliRunResult, Runner},
+    type_check::TypeCheckRunner,
     walk::Walk,
 };
 

crates/oxc_cli/src/lint/command.rs

@@ -1,7 +1,7 @@
 use clap::{builder::ValueParser, Arg, ArgAction, Command};
 
-pub(super) fn lint_command(command: Command) -> Command {
-    command
+pub(super) fn lint_command() -> Command {
+    Command::new("")
             .arg_required_else_help(true)
             .after_help(
                 "# Rule Selection

crates/oxc_cli/src/lint/isolated_handler.rs

@@ -16,12 +16,11 @@ use oxc_diagnostics::{
     thiserror::Error,
     Error, GraphicalReportHandler, Severity,
 };
-use oxc_linter::{Fixer, LintContext, Linter};
+use oxc_linter::{Fixer, LintContext, LintOptions, Linter};
 use oxc_parser::Parser;
 use oxc_semantic::SemanticBuilder;
 use oxc_span::SourceType;
 
-use super::options::LintOptions;
 use crate::{CliRunResult, Walk};
 
 pub struct IsolatedLintHandler {

crates/oxc_cli/src/lint/main.rs

@@ -9,23 +9,21 @@ static GLOBAL: jemallocator::Jemalloc = jemallocator::Jemalloc;
 static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;
 
 use clap::{Arg, Command};
-use oxc_cli::{CliRunResult, LintOptions, LintRunner, Runner, RunnerOptions};
+use oxc_cli::{CliRunResult, LintRunner, Runner};
 
 pub fn command() -> Command {
-    LintOptions::build_args(
-        Command::new("oxlint")
-            .bin_name("oxlint")
-            .version("alpha")
-            .author("Boshen")
-            .about("Linter for the JavaScript Oxidation Compiler")
-            .arg_required_else_help(true)
-            .arg(
-                Arg::new("threads")
-                    .long("threads")
-                    .value_parser(clap::value_parser!(usize))
-                    .help("Number of threads to use. Set to 1 for using only 1 CPU core."),
-            ),
-    )
+    LintRunner::command()
+        .bin_name("oxlint")
+        .version("alpha")
+        .author("Boshen")
+        .about("Linter for the JavaScript Oxidation Compiler")
+        .arg_required_else_help(true)
+        .arg(
+            Arg::new("threads")
+                .long("threads")
+                .value_parser(clap::value_parser!(usize))
+                .help("Number of threads to use. Set to 1 for using only 1 CPU core."),
+        )
 }
 
 fn main() -> CliRunResult {
@@ -35,7 +33,5 @@ fn main() -> CliRunResult {
         rayon::ThreadPoolBuilder::new().num_threads(*threads).build_global().unwrap();
     }
 
-    let options = LintOptions::from(&matches);
-
-    LintRunner::new(options).run()
+    LintRunner::new(&matches).run()
 }

crates/oxc_cli/src/lint/mod.rs

@@ -1,16 +1,18 @@
 mod command;
 mod error;
 mod isolated_handler;
-mod options;
 
+use clap::{ArgMatches, Command};
+use std::{collections::BTreeMap, env, path::PathBuf};
 use std::{io::BufWriter, sync::Arc, time::Duration};
 
+use self::command::lint_command;
+pub use self::{error::Error, isolated_handler::IsolatedLintHandler};
+
 use oxc_index::assert_impl_all;
-use oxc_linter::{Linter, RuleCategory, RuleEnum, RULES};
+use oxc_linter::{AllowWarnDeny, LintOptions, Linter, RuleCategory, RuleEnum, RULES};
 use rustc_hash::FxHashSet;
 
-pub use self::{error::Error, options::LintOptions};
-use self::{isolated_handler::IsolatedLintHandler, options::AllowWarnDeny};
 use crate::{CliRunResult, Runner};
 
 pub struct LintRunner {
@@ -19,25 +21,22 @@ pub struct LintRunner {
 }
 assert_impl_all!(LintRunner: Send, Sync);
 
-impl Default for LintRunner {
-    fn default() -> Self {
-        Self::new(LintOptions::default())
-    }
-}
-
 impl Runner for LintRunner {
-    type Options = LintOptions;
-
     const ABOUT: &'static str = "Lint this repository.";
     const NAME: &'static str = "lint";
 
-    fn new(options: LintOptions) -> Self {
+    fn new(matches: &ArgMatches) -> Self {
+        let options = parse_arg_matches(matches);
         let linter = Linter::from_rules(Self::derive_rules(&options))
             .with_fix(options.fix)
             .with_print_execution_times(options.print_execution_times);
         Self { options: Arc::new(options), linter: Arc::new(linter) }
     }
 
+    fn init_command() -> Command {
+        lint_command()
+    }
+
     fn run(&self) -> CliRunResult {
         if self.options.list_rules {
             Self::print_rules();
@@ -128,3 +127,152 @@ impl LintRunner {
         }
     }
 }
+
+fn parse_arg_matches(matches: &ArgMatches) -> LintOptions {
+    let list_rules = matches.get_flag("rules");
+    LintOptions {
+        paths: matches.get_many("path").map_or_else(
+            || if list_rules { vec![] } else { vec![PathBuf::from(".")] },
+            |paths| paths.into_iter().cloned().collect(),
+        ),
+        rules: get_rules(matches),
+        fix: matches.get_flag("fix"),
+        quiet: matches.get_flag("quiet"),
+        ignore_path: matches
+            .get_one::<PathBuf>("ignore-path")
+            .map_or_else(|| PathBuf::from(".eslintignore"), Clone::clone),
+        no_ignore: matches.get_flag("no-ignore"),
+        ignore_pattern: matches
+            .get_many::<String>("ignore-pattern")
+            .map(|patterns| patterns.into_iter().cloned().collect())
+            .unwrap_or_default(),
+        max_warnings: matches.get_one("max-warnings").copied(),
+        list_rules,
+        print_execution_times: matches!(env::var("TIMING"), Ok(x) if x == "true" || x == "1"),
+    }
+}
+
+/// Get all rules in order, e.g.
+/// `-A all -D no-var -D -eqeqeq` => [("allow", "all"), ("deny", "no-var"), ("deny", "eqeqeq")]
+/// Defaults to [("deny", "correctness")];
+fn get_rules(matches: &ArgMatches) -> Vec<(AllowWarnDeny, String)> {
+    let mut map: BTreeMap<usize, (AllowWarnDeny, String)> = BTreeMap::new();
+    for key in ["allow", "deny"] {
+        let allow_warn_deny = AllowWarnDeny::from(key);
+        if let Some(values) = matches.get_many::<String>(key) {
+            let indices = matches.indices_of(key).unwrap();
+            let zipped =
+                values.zip(indices).map(|(value, i)| (i, (allow_warn_deny, value.clone())));
+            map.extend(zipped);
+        }
+    }
+    if map.is_empty() {
+        vec![(AllowWarnDeny::Deny, "correctness".into())]
+    } else {
+        map.into_values().collect()
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use std::path::PathBuf;
+
+    use super::{lint_command, parse_arg_matches, AllowWarnDeny, LintOptions};
+
+    #[test]
+    fn verify_command() {
+        lint_command().debug_assert();
+    }
+
+    fn get_lint_options(arg: &str) -> LintOptions {
+        let matches = lint_command().try_get_matches_from(arg.split(' ')).unwrap();
+        parse_arg_matches(&matches)
+    }
+
+    #[test]
+    fn default() {
+        let options = get_lint_options("lint .");
+        assert_eq!(options.paths, vec![PathBuf::from(".")]);
+        assert!(!options.fix);
+        assert!(!options.quiet);
+        assert_eq!(options.ignore_path, PathBuf::from(".eslintignore"));
+        assert!(!options.no_ignore);
+        assert!(options.ignore_pattern.is_empty());
+        assert_eq!(options.max_warnings, None);
+    }
+
+    #[test]
+    fn multiple_paths() {
+        let options = get_lint_options("lint foo bar baz");
+        assert_eq!(
+            options.paths,
+            [PathBuf::from("foo"), PathBuf::from("bar"), PathBuf::from("baz")]
+        );
+    }
+
+    #[test]
+    fn rules_with_deny_and_allow() {
+        let options = get_lint_options(
+            "lint src -D suspicious --deny pedantic -A no-debugger --allow no-var",
+        );
+        assert_eq!(
+            options.rules,
+            vec![
+                (AllowWarnDeny::Deny, "suspicious".into()),
+                (AllowWarnDeny::Deny, "pedantic".into()),
+                (AllowWarnDeny::Allow, "no-debugger".into()),
+                (AllowWarnDeny::Allow, "no-var".into())
+            ]
+        );
+    }
+
+    #[test]
+    fn quiet_true() {
+        let options = get_lint_options("lint foo.js --quiet");
+        assert!(options.quiet);
+    }
+
+    #[test]
+    fn fix_true() {
+        let options = get_lint_options("lint foo.js --fix");
+        assert!(options.fix);
+    }
+
+    #[test]
+    fn max_warnings() {
+        let options = get_lint_options("lint --max-warnings 10 foo.js");
+        assert_eq!(options.max_warnings, Some(10));
+    }
+
+    #[test]
+    fn ignore_path() {
+        let options = get_lint_options("lint --ignore-path .xxx foo.js");
+        assert_eq!(options.ignore_path, PathBuf::from(".xxx"));
+    }
+
+    #[test]
+    fn no_ignore() {
+        let options = get_lint_options("lint --no-ignore foo.js");
+        assert!(options.no_ignore);
+    }
+
+    #[test]
+    fn single_ignore_pattern() {
+        let options = get_lint_options("lint --ignore-pattern ./test foo.js");
+        assert_eq!(options.ignore_pattern, vec![String::from("./test")]);
+    }
+
+    #[test]
+    fn multiple_ignore_pattern() {
+        let options =
+            get_lint_options("lint --ignore-pattern ./test --ignore-pattern bar.js foo.js");
+        assert_eq!(options.ignore_pattern, vec![String::from("./test"), String::from("bar.js")]);
+    }
+
+    #[test]
+    fn list_rules_true() {
+        let options = get_lint_options("lint --rules");
+        assert!(options.paths.is_empty());
+        assert!(options.list_rules);
+    }
+}