refactor(cli): split out group options (#760)

This commit is contained in:
Boshen 2023-08-19 12:09:35 +08:00 committed by GitHub
parent 6f1daa67b2
commit 1fdce7e517
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 230 additions and 198 deletions

View file

@ -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<usize>) {
@ -33,6 +31,8 @@ impl CliCommand {
}
}
// To add a header or footer, see
// <https://docs.rs/bpaf/latest/bpaf/struct.OptionParser.html#method.descr>
/// 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<usize>,
/// Number of threads to use. Set to 1 for using only 1 CPU core.
#[bpaf(argument("NUMBER"), hide_usage)]
pub threads: Option<usize>,
}
#[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<String>,
/// Single file, single path or list of paths
#[bpaf(positional("PATH"), many)]
pub paths: Vec<PathBuf>,
}
static FILTER_HELP: &str = r#"
To allow or deny a rule, use multiple -A <NAME> or -D <NAME>.
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<usize>,
}
#[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<PathBuf>,
}
// This is formatted according to
// <https://docs.rs/bpaf/latest/bpaf/params/struct.NamedArg.html#method.help>
/// 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<String>,
///
#[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<usize>,
}
#[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::<Vec<_>>();
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::<Vec<_>>();
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::<Vec<_>>();
@ -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::<Vec<_>>();
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")]);
}
}

View file

@ -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<CliOptions>,
walk_options: Arc<WalkOptions>,
warning_options: Arc<WarningOptions>,
linter: Arc<Linter>,
}
@ -37,24 +35,20 @@ pub struct IsolatedLintHandler {
pub struct MinifiedFileError(pub PathBuf);
impl IsolatedLintHandler {
pub(super) fn new(
cli_options: Arc<CliOptions>,
walk_options: Arc<WalkOptions>,
linter: Arc<Linter>,
) -> Self {
Self { cli_options, walk_options, linter }
pub(super) fn new(warning_options: Arc<WarningOptions>, linter: Arc<Linter>) -> 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<Error>)>();
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<AtomicUsize>,
tx_error: mpsc::Sender<(PathBuf, Vec<Error>)>,
) {
let (tx_path, rx_path) = mpsc::channel::<Box<Path>>();
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;
}

View file

@ -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<WalkOptions>,
cli_options: Arc<CliOptions>,
list_rules: bool,
linter: Arc<Linter>,
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::<Vec<_>>();
timings.sort_by_key(|x| x.1);
let total = timings.iter().map(|x| x.1).sum::<Duration>().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}");
}
}
}

View file

@ -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)]

View file

@ -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);

View file

@ -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);
}

View file

@ -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::<Vec<_>>();
timings.sort_by_key(|x| x.1);
let total = timings.iter().map(|x| x.1).sum::<Duration>().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)]