diff --git a/crates/oxc_cli/src/command.rs b/crates/oxc_cli/src/command.rs index 4543e44b3..6bba8a57b 100644 --- a/crates/oxc_cli/src/command.rs +++ b/crates/oxc_cli/src/command.rs @@ -1,4 +1,4 @@ -use bpaf::Bpaf; +use bpaf::{doc::Style, Bpaf}; use oxc_linter::AllowWarnDeny; use std::{ffi::OsString, path::PathBuf}; @@ -15,15 +15,13 @@ pub enum CliCommand { } impl CliCommand { - pub fn cli_options(&self) -> &CliOptions { - match self { - Self::Lint(options) => &options.cli, - Self::Check(options) => &options.cli, - } - } - pub fn handle_threads(&self) { - Self::set_rayon_threads(self.cli_options().threads); + match self { + Self::Lint(options) => { + Self::set_rayon_threads(options.misc_options.threads); + } + Self::Check(_) => {} + } } fn set_rayon_threads(threads: Option) { @@ -33,6 +31,8 @@ impl CliCommand { } } +// To add a header or footer, see +// /// Linter for the JavaScript Oxidation Compiler #[derive(Debug, Clone, Bpaf)] #[bpaf(options)] @@ -43,71 +43,13 @@ pub struct LintCommand { impl LintCommand { pub fn handle_threads(&self) { - CliCommand::set_rayon_threads(self.lint_options.cli.threads); + CliCommand::set_rayon_threads(self.lint_options.misc_options.threads); } } +/// Miscellaneous #[derive(Debug, Clone, Bpaf)] -pub struct CliOptions { - /// Disable reporting on warnings, only errors are reported - #[bpaf(switch, hide_usage)] - pub quiet: bool, - - /// Specify a warning threshold, - /// which can be used to force exit with an error status if there are too many warning-level rule violations in your project - #[bpaf(argument("NUMBER"), hide_usage)] - pub max_warnings: Option, - - /// Number of threads to use. Set to 1 for using only 1 CPU core. - #[bpaf(argument("NUMBER"), hide_usage)] - pub threads: Option, -} - -#[derive(Debug, Clone, Bpaf)] -pub struct WalkOptions { - /// Disables excluding of files from .eslintignore files - #[bpaf(switch)] - pub no_ignore: bool, - - /// Specify the file to use as your .eslintignore - #[bpaf(argument("PATH"), fallback(".eslintignore".into()))] - pub ignore_path: OsString, - - /// Specify patterns of files to ignore (in addition to those in .eslintignore) - #[bpaf(argument("PATTERN"), many)] - pub ignore_pattern: Vec, - - /// Single file, single path or list of paths - #[bpaf(positional("PATH"), many)] - pub paths: Vec, -} - -static FILTER_HELP: &str = r#" -To allow or deny a rule, use multiple -A or -D . - -For example "-D correctness -A no-debugger" or "-A all -D no-debugger". - -The categories are: - * correctness - code that is outright wrong or useless - * suspicious - code that is most likely wrong or useless - * pedantic - lints which are rather strict or have occasional false positives - * style - code that should be written in a more idiomatic way - * nursery - new lints that are still under development - * restriction - lints which prevent the use of language and library features - * all - all the categories listed above - -The default category is "-D correctness". -"#; - -#[derive(Debug, Clone, Bpaf)] -pub struct LintOptions { - #[bpaf(external(lint_filter), map(LintFilter::into_tuple), many, group_help(FILTER_HELP))] - pub filter: Vec<(AllowWarnDeny, String)>, - - /// Fix as many issues as possible. Only unfixed issues are reported in the output - #[bpaf(switch)] - pub fix: bool, - +pub struct MiscOptions { /// Display the execution time of each lint rule #[bpaf(switch, env("TIMING"), hide_usage)] pub timing: bool, @@ -116,23 +58,60 @@ pub struct LintOptions { #[bpaf(switch, hide_usage)] pub rules: bool, - #[bpaf(external(cli_options), hide_usage)] - pub cli: CliOptions, - - #[bpaf(external(walk_options), hide_usage)] - pub walk: WalkOptions, + /// Number of threads to use. Set to 1 for using only 1 CPU core + #[bpaf(argument("INT"), hide_usage)] + pub threads: Option, } +#[derive(Debug, Clone, Bpaf)] +pub struct LintOptions { + #[bpaf(external(lint_filter), map(LintFilter::into_tuple), many)] + pub filter: Vec<(AllowWarnDeny, String)>, + + #[bpaf(external)] + pub fix_options: FixOptions, + + #[bpaf(external)] + pub ignore_options: IgnoreOptions, + + #[bpaf(external)] + pub warning_options: WarningOptions, + + #[bpaf(external)] + pub misc_options: MiscOptions, + + /// Single file, single path or list of paths + #[bpaf(positional("PATH"), many)] + pub paths: Vec, +} + +// This is formatted according to +// +/// Allowing / Denying Multiple Lints +/// For example `-D correctness -A no-debugger` or `-A all -D no-debugger`. +/// ㅤ +/// The default category is "-D correctness". +/// Use "--rules" for rule names. +/// Use "--help --help" for rule categories. +/// +/// The categories are: +/// * correctness - code that is outright wrong or useless +/// * suspicious - code that is most likely wrong or useless +/// * pedantic - lints which are rather strict or have occasional false positives +/// * style - code that should be written in a more idiomatic way +/// * nursery - new lints that are still under development +/// * restriction - lints which prevent the use of language and library features +/// * all - all the categories listed above #[derive(Debug, Clone, Bpaf)] pub enum LintFilter { Allow( - /// Allow the rule or category - #[bpaf(short('A'), long("allow"), argument("RULE|CATEGORY"))] + /// Allow the rule or category (suppress the lint) + #[bpaf(short('A'), long("allow"), argument("NAME"))] String, ), Deny( - /// Deny the rule or category - #[bpaf(short('D'), long("deny"), argument("RULE|CATEGORY"))] + /// Deny the rule or category (emit an error) + #[bpaf(short('D'), long("deny"), argument("NAME"))] String, ), } @@ -146,11 +125,56 @@ impl LintFilter { } } +/// Fix Problems +#[derive(Debug, Clone, Bpaf)] +pub struct FixOptions { + /// Fix as many issues as possible. Only unfixed issues are reported in the output + #[bpaf(switch)] + pub fix: bool, +} + +const NO_IGNORE_HELP: &[(&str, Style)] = &[ + ("Disables excluding of files from .eslintignore files, ", Style::Text), + ("--ignore-path", Style::Literal), + (" flags and ", Style::Text), + ("--ignore-pattern", Style::Literal), + (" flags", Style::Text), +]; + +/// Ignore Files +#[derive(Debug, Clone, Bpaf)] +pub struct IgnoreOptions { + /// Specify the file to use as your .eslintignore + #[bpaf(argument("PATH"), fallback(".eslintignore".into()), hide_usage)] + pub ignore_path: OsString, + + /// Specify patterns of files to ignore (in addition to those in .eslintignore) + /// + /// The supported syntax is the same as for .eslintignore and .gitignore files + /// You should quote your patterns in order to avoid shell interpretation of glob patterns + #[bpaf(argument("PAT"), many, hide_usage)] + pub ignore_pattern: Vec, + + /// + #[bpaf(switch, hide_usage, help(NO_IGNORE_HELP))] + pub no_ignore: bool, +} + +/// Handle Warnings +#[derive(Debug, Clone, Bpaf)] +pub struct WarningOptions { + /// Disable reporting on warnings, only errors are reported + #[bpaf(switch, hide_usage)] + pub quiet: bool, + + /// Specify a warning threshold, + /// which can be used to force exit with an error status if there are too many warning-level rule violations in your project + #[bpaf(argument("INT"), hide_usage)] + pub max_warnings: Option, +} + #[derive(Debug, Clone, Bpaf)] pub struct CheckOptions { - #[bpaf(external(cli_options), hide_usage)] - pub cli: CliOptions, - /// Print called functions #[bpaf(switch, hide_usage)] pub print_called_functions: bool, @@ -165,30 +189,66 @@ pub struct CheckOptions { } #[cfg(test)] -mod cli_options { - use super::{lint_command, CliOptions}; +mod misc_options { + use super::{lint_command, MiscOptions}; - fn get_cli_options(arg: &str) -> CliOptions { + fn get_misc_options(arg: &str) -> MiscOptions { let args = arg.split(' ').map(std::string::ToString::to_string).collect::>(); - lint_command().run_inner(args.as_slice()).unwrap().lint_options.cli + lint_command().run_inner(args.as_slice()).unwrap().lint_options.misc_options } #[test] fn default() { - let options = get_cli_options("."); + let options = get_misc_options("."); + assert!(!options.timing); + assert!(!options.rules); + assert!(options.threads.is_none()); + } + + #[test] + fn timing() { + let options = get_misc_options("--timing ."); + assert!(options.timing); + } + + #[test] + fn threads() { + let options = get_misc_options("--threads 4 ."); + assert_eq!(options.threads, Some(4)); + } + + #[test] + fn list_rules() { + let options = get_misc_options("--rules"); + assert!(options.rules); + } +} + +#[cfg(test)] +mod warning_options { + use super::{lint_command, WarningOptions}; + + fn get_warning_options(arg: &str) -> WarningOptions { + let args = arg.split(' ').map(std::string::ToString::to_string).collect::>(); + lint_command().run_inner(args.as_slice()).unwrap().lint_options.warning_options + } + + #[test] + fn default() { + let options = get_warning_options("."); assert!(!options.quiet); assert_eq!(options.max_warnings, None); } #[test] fn quiet() { - let options = get_cli_options("--quiet ."); + let options = get_warning_options("--quiet ."); assert!(options.quiet); } #[test] fn max_warnings() { - let options = get_cli_options("--max-warnings 10 ."); + let options = get_warning_options("--max-warnings 10 ."); assert_eq!(options.max_warnings, Some(10)); } } @@ -197,6 +257,7 @@ mod cli_options { mod lint_options { use super::{lint_command, LintOptions}; use oxc_linter::AllowWarnDeny; + use std::path::PathBuf; fn get_lint_options(arg: &str) -> LintOptions { let args = arg.split(' ').map(std::string::ToString::to_string).collect::>(); @@ -206,20 +267,23 @@ mod lint_options { #[test] fn default() { let options = get_lint_options("."); - assert!(!options.fix); - assert!(!options.rules); + assert_eq!(options.paths, vec![PathBuf::from(".")]); + assert!(!options.fix_options.fix); } #[test] - fn list_rules() { - let options = get_lint_options("--rules"); - assert!(options.rules); + fn multiple_paths() { + let options = get_lint_options("foo bar baz"); + assert_eq!( + options.paths, + [PathBuf::from("foo"), PathBuf::from("bar"), PathBuf::from("baz")] + ); } #[test] fn fix() { let options = get_lint_options("--fix test.js"); - assert!(options.fix); + assert!(options.fix_options.fix); } #[test] @@ -239,54 +303,44 @@ mod lint_options { } #[cfg(test)] -mod walk_options { - use super::{lint_command, WalkOptions}; +mod ignore_options { + use super::{lint_command, IgnoreOptions}; use std::{ffi::OsString, path::PathBuf}; - fn get_walk_options(arg: &str) -> WalkOptions { + fn get_ignore_options(arg: &str) -> IgnoreOptions { let args = arg.split(' ').map(std::string::ToString::to_string).collect::>(); - lint_command().run_inner(args.as_slice()).unwrap().lint_options.walk + lint_command().run_inner(args.as_slice()).unwrap().lint_options.ignore_options } #[test] fn default() { - let options = get_walk_options("."); - assert_eq!(options.paths, vec![PathBuf::from(".")]); + let options = get_ignore_options("."); assert_eq!(options.ignore_path, OsString::from(".eslintignore")); assert!(!options.no_ignore); assert!(options.ignore_pattern.is_empty()); } - #[test] - fn multiple_paths() { - let options = get_walk_options("foo bar baz"); - assert_eq!( - options.paths, - [PathBuf::from("foo"), PathBuf::from("bar"), PathBuf::from("baz")] - ); - } - #[test] fn ignore_path() { - let options = get_walk_options("--ignore-path .xxx foo.js"); + let options = get_ignore_options("--ignore-path .xxx foo.js"); assert_eq!(options.ignore_path, PathBuf::from(".xxx")); } #[test] fn no_ignore() { - let options = get_walk_options("--no-ignore foo.js"); + let options = get_ignore_options("--no-ignore foo.js"); assert!(options.no_ignore); } #[test] fn single_ignore_pattern() { - let options = get_walk_options("--ignore-pattern ./test foo.js"); + let options = get_ignore_options("--ignore-pattern ./test foo.js"); assert_eq!(options.ignore_pattern, vec![String::from("./test")]); } #[test] fn multiple_ignore_pattern() { - let options = get_walk_options("--ignore-pattern ./test --ignore-pattern bar.js foo.js"); + let options = get_ignore_options("--ignore-pattern ./test --ignore-pattern bar.js foo.js"); assert_eq!(options.ignore_pattern, vec![String::from("./test"), String::from("bar.js")]); } } diff --git a/crates/oxc_cli/src/lint/isolated_handler.rs b/crates/oxc_cli/src/lint/isolated_handler.rs index 05c2d2998..35d2343da 100644 --- a/crates/oxc_cli/src/lint/isolated_handler.rs +++ b/crates/oxc_cli/src/lint/isolated_handler.rs @@ -21,12 +21,10 @@ use oxc_parser::Parser; use oxc_semantic::SemanticBuilder; use oxc_span::SourceType; -use crate::{CliOptions, CliRunResult, Walk, WalkOptions}; +use crate::{CliRunResult, Walk, WarningOptions}; pub struct IsolatedLintHandler { - cli_options: Arc, - - walk_options: Arc, + warning_options: Arc, linter: Arc, } @@ -37,24 +35,20 @@ pub struct IsolatedLintHandler { pub struct MinifiedFileError(pub PathBuf); impl IsolatedLintHandler { - pub(super) fn new( - cli_options: Arc, - walk_options: Arc, - linter: Arc, - ) -> Self { - Self { cli_options, walk_options, linter } + pub(super) fn new(warning_options: Arc, linter: Arc) -> Self { + Self { warning_options, linter } } /// # Panics /// /// * When `mpsc::channel` fails to send. - pub(super) fn run(&self) -> CliRunResult { + pub(super) fn run(&self, walk: Walk) -> CliRunResult { let now = std::time::Instant::now(); let number_of_files = Arc::new(AtomicUsize::new(0)); let (tx_error, rx_error) = mpsc::channel::<(PathBuf, Vec)>(); - self.process_paths(&number_of_files, tx_error); + self.process_paths(walk, &number_of_files, tx_error); let (number_of_warnings, number_of_errors) = self.process_diagnostics(&rx_error); CliRunResult::LintResult { @@ -64,7 +58,7 @@ impl IsolatedLintHandler { number_of_warnings, number_of_errors, max_warnings_exceeded: self - .cli_options + .warning_options .max_warnings .map_or(false, |max_warnings| number_of_warnings > max_warnings), } @@ -72,12 +66,12 @@ impl IsolatedLintHandler { fn process_paths( &self, + walk: Walk, number_of_files: &Arc, tx_error: mpsc::Sender<(PathBuf, Vec)>, ) { let (tx_path, rx_path) = mpsc::channel::>(); - let walk = Walk::new(&self.walk_options); let number_of_files = Arc::clone(number_of_files); rayon::spawn(move || { let mut count = 0; @@ -127,11 +121,11 @@ impl IsolatedLintHandler { } // The --quiet flag follows ESLint's --quiet behavior as documented here: https://eslint.org/docs/latest/use/command-line-interface#--quiet // Note that it does not disable ALL diagnostics, only Warning diagnostics - if self.cli_options.quiet { + if self.warning_options.quiet { continue; } - if let Some(max_warnings) = self.cli_options.max_warnings { + if let Some(max_warnings) = self.warning_options.max_warnings { if number_of_warnings > max_warnings { continue; } diff --git a/crates/oxc_cli/src/lint/mod.rs b/crates/oxc_cli/src/lint/mod.rs index d56a0fc37..8e2bccfac 100644 --- a/crates/oxc_cli/src/lint/mod.rs +++ b/crates/oxc_cli/src/lint/mod.rs @@ -1,23 +1,17 @@ mod error; mod isolated_handler; -use std::{io::BufWriter, sync::Arc, time::Duration}; +use std::{io::BufWriter, sync::Arc}; pub use self::{error::Error, isolated_handler::IsolatedLintHandler}; use oxc_index::assert_impl_all; use oxc_linter::{LintOptions, Linter}; -use crate::{ - command::{CliOptions, LintOptions as CliLintOptions, WalkOptions}, - CliRunResult, Runner, -}; +use crate::{command::LintOptions as CliLintOptions, walk::Walk, CliRunResult, Runner}; pub struct LintRunner { - walk_options: Arc, - cli_options: Arc, - list_rules: bool, - linter: Arc, + options: CliLintOptions, } assert_impl_all!(LintRunner: Send, Sync); @@ -25,64 +19,34 @@ impl Runner for LintRunner { type Options = CliLintOptions; fn new(options: Self::Options) -> Self { - let list_rules = options.rules; - let filter = - if options.filter.is_empty() { LintOptions::default().filter } else { options.filter }; - let lint_options = LintOptions { filter, fix: options.fix, timing: options.timing }; - let linter = Linter::from_options(lint_options); - Self { - cli_options: Arc::new(options.cli), - walk_options: Arc::new(options.walk), - list_rules, - linter: Arc::new(linter), - } + Self { options } } - fn run(&self) -> CliRunResult { - if self.list_rules { - Self::print_rules(); + fn run(self) -> CliRunResult { + if self.options.misc_options.rules { + let mut stdout = BufWriter::new(std::io::stdout()); + Linter::print_rules(&mut stdout); return CliRunResult::None; } - let result = IsolatedLintHandler::new( - Arc::clone(&self.cli_options), - Arc::clone(&self.walk_options), - Arc::clone(&self.linter), - ) - .run(); + let CliLintOptions { + paths, + filter, + warning_options, + ignore_options, + fix_options, + misc_options, + } = self.options; + let walk = Walk::new(&paths, &ignore_options); + let filter = if filter.is_empty() { LintOptions::default().filter } else { filter }; + let lint_options = + LintOptions { filter, fix: fix_options.fix, timing: misc_options.timing }; + let linter = Arc::new(Linter::from_options(lint_options)); + let result = + IsolatedLintHandler::new(Arc::new(warning_options), Arc::clone(&linter)).run(walk); - if self.linter.options().timing { - self.print_execution_times(); - } + linter.print_execution_times_if_enable(); result } } - -impl LintRunner { - fn print_rules() { - let mut stdout = BufWriter::new(std::io::stdout()); - Linter::print_rules(&mut stdout); - } - - fn print_execution_times(&self) { - let mut timings = self - .linter - .rules() - .iter() - .map(|rule| (rule.name(), rule.execute_time())) - .collect::>(); - - timings.sort_by_key(|x| x.1); - let total = timings.iter().map(|x| x.1).sum::().as_secs_f64(); - - println!("Rule timings in milliseconds:"); - println!("Total: {:.2}ms", total * 1000.0); - println!("{:>7} | {:>5} | Rule", "Time", "%"); - for (name, duration) in timings.iter().rev() { - let millis = duration.as_secs_f64() * 1000.0; - let relative = duration.as_secs_f64() / total * 100.0; - println!("{millis:>7.2} | {relative:>4.1}% | {name}"); - } - } -} diff --git a/crates/oxc_cli/src/runner.rs b/crates/oxc_cli/src/runner.rs index 65d141e9f..135991937 100644 --- a/crates/oxc_cli/src/runner.rs +++ b/crates/oxc_cli/src/runner.rs @@ -10,7 +10,7 @@ pub trait Runner: Send + Sync { fn new(matches: Self::Options) -> Self; /// Executes the runner, providing some result to the CLI. - fn run(&self) -> CliRunResult; + fn run(self) -> CliRunResult; } #[derive(Debug)] diff --git a/crates/oxc_cli/src/type_check/mod.rs b/crates/oxc_cli/src/type_check/mod.rs index f57965feb..e69514722 100644 --- a/crates/oxc_cli/src/type_check/mod.rs +++ b/crates/oxc_cli/src/type_check/mod.rs @@ -62,7 +62,7 @@ impl Runner for TypeCheckRunner { } /// # Panics - fn run(&self) -> CliRunResult { + fn run(self) -> CliRunResult { let now = std::time::Instant::now(); let path = Path::new(&self.options.path); diff --git a/crates/oxc_cli/src/walk.rs b/crates/oxc_cli/src/walk.rs index bc9327a62..6168dfdcd 100644 --- a/crates/oxc_cli/src/walk.rs +++ b/crates/oxc_cli/src/walk.rs @@ -1,9 +1,9 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use ignore::{overrides::OverrideBuilder, DirEntry, WalkBuilder}; use oxc_span::VALID_EXTENSIONS; -use crate::WalkOptions; +use crate::IgnoreOptions; pub struct Walk { inner: ignore::Walk, @@ -11,10 +11,10 @@ pub struct Walk { impl Walk { /// # Panics - pub fn new(options: &WalkOptions) -> Self { - let mut inner = WalkBuilder::new(&options.paths[0]); + pub fn new(paths: &[PathBuf], options: &IgnoreOptions) -> Self { + let mut inner = WalkBuilder::new(&paths[0]); - if let Some(paths) = options.paths.get(1..) { + if let Some(paths) = paths.get(1..) { for path in paths { inner.add(path); } diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 71a3885c3..60008efde 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -14,7 +14,7 @@ pub mod rule; mod rule_timer; mod rules; -use std::{self, fs, io::Write, rc::Rc}; +use std::{self, fs, io::Write, rc::Rc, time::Duration}; pub use fixer::{FixResult, Fixer, Message}; pub(crate) use oxc_semantic::AstNode; @@ -147,6 +147,26 @@ impl Linter { } writeln!(writer, "Total: {}", RULES.len()).unwrap(); } + + pub fn print_execution_times_if_enable(&self) { + if !self.options.timing { + return; + } + let mut timings = + self.rules().iter().map(|rule| (rule.name(), rule.execute_time())).collect::>(); + + timings.sort_by_key(|x| x.1); + let total = timings.iter().map(|x| x.1).sum::().as_secs_f64(); + + println!("Rule timings in milliseconds:"); + println!("Total: {:.2}ms", total * 1000.0); + println!("{:>7} | {:>5} | Rule", "Time", "%"); + for (name, duration) in timings.iter().rev() { + let millis = duration.as_secs_f64() * 1000.0; + let relative = duration.as_secs_f64() / total * 100.0; + println!("{millis:>7.2} | {relative:>4.1}% | {name}"); + } + } } #[cfg(test)]