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

This also simplifies the Runner trait.
This commit is contained in:
Boshen 2023-08-17 22:28:34 +08:00 committed by GitHub
parent 5a73f0e1b5
commit 3110490f36
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 282 additions and 306 deletions

View file

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

View file

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

View file

@ -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 {

View file

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

View file

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

View file

@ -1,214 +0,0 @@
use std::{collections::BTreeMap, env, path::PathBuf};
use clap::ArgMatches;
use super::command::lint_command;
pub use super::{error::Error, isolated_handler::IsolatedLintHandler};
use crate::runner::RunnerOptions;
#[derive(Debug)]
#[allow(clippy::struct_excessive_bools)]
pub struct LintOptions {
pub paths: Vec<PathBuf>,
/// Allow / Deny rules in order. [("allow" / "deny", rule name)]
/// Defaults to [("deny", "correctness")]
pub rules: Vec<(AllowWarnDeny, String)>,
pub list_rules: bool,
pub fix: bool,
pub quiet: bool,
pub ignore_path: PathBuf,
pub no_ignore: bool,
pub ignore_pattern: Vec<String>,
pub max_warnings: Option<usize>,
pub print_execution_times: bool,
}
impl Default for LintOptions {
fn default() -> Self {
let default_matches = ArgMatches::default();
Self::from(&default_matches)
}
}
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub enum AllowWarnDeny {
Allow,
// Warn,
Deny,
}
impl From<&'static str> for AllowWarnDeny {
fn from(s: &'static str) -> Self {
match s {
"allow" => Self::Allow,
"deny" => Self::Deny,
_ => unreachable!(),
}
}
}
impl<'a> From<&'a ArgMatches> for LintOptions {
fn from(matches: &'a ArgMatches) -> Self {
let list_rules = matches.get_flag("rules");
Self {
paths: matches.get_many("path").map_or_else(
|| if list_rules { vec![] } else { vec![PathBuf::from(".")] },
|paths| paths.into_iter().cloned().collect(),
),
rules: Self::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"),
}
}
}
impl LintOptions {
/// 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()
}
}
}
impl RunnerOptions for LintOptions {
#[inline]
fn build_args(cmd: clap::Command) -> clap::Command {
lint_command(cmd)
}
}
#[cfg(test)]
mod test {
use std::path::PathBuf;
use clap::Command;
use super::{AllowWarnDeny, LintOptions};
use crate::runner::RunnerOptions;
#[test]
fn verify_command() {
LintOptions::build_args(Command::new("oxc")).debug_assert();
}
fn get_lint_options(arg: &str) -> LintOptions {
let matches = LintOptions::build_args(Command::new("oxc"))
.try_get_matches_from(arg.split(' '))
.unwrap();
LintOptions::from(&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);
}
}

View file

@ -8,9 +8,7 @@ static GLOBAL: jemallocator::Jemalloc = jemallocator::Jemalloc;
#[global_allocator]
static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;
use oxc_cli::{
command, CliRunResult, LintOptions, LintRunner, Runner, TypeCheckOptions, TypeCheckRunner,
};
use oxc_cli::{command, CliRunResult, LintRunner, Runner, TypeCheckRunner};
fn main() -> CliRunResult {
let matches = command().get_matches();
@ -21,16 +19,9 @@ fn main() -> CliRunResult {
let Some((subcommand, matches)) = matches.subcommand() else { return CliRunResult::None };
// todo: register commands in list then iterate
match subcommand {
LintRunner::NAME => {
let options = LintOptions::from(matches);
LintRunner::new(options).run()
}
TypeCheckRunner::NAME => {
let options = TypeCheckOptions::from(matches);
TypeCheckRunner::new(options).run()
}
LintRunner::NAME => LintRunner::new(matches).run(),
TypeCheckRunner::NAME => TypeCheckRunner::new(matches).run(),
_ => CliRunResult::None,
}
}

View file

@ -3,26 +3,30 @@ use std::{
process::{ExitCode, Termination},
};
use clap::Command;
use clap::{ArgMatches, Command};
pub trait RunnerOptions: for<'a> From<&'a clap::ArgMatches> {
/// Add CLI arguments to a [`Command`], usually created from a [`Runner`].
fn build_args(cmd: Command) -> Command;
}
/// A trait for exposing Oxide functionality to the CLI.
/// A trait for exposing functionality to the CLI.
pub trait Runner: Send + Sync {
/// The name of the runner. Used to create a subcommand.
const NAME: &'static str;
/// A short description about what the runner does
const ABOUT: &'static str;
type Options: RunnerOptions;
fn command() -> Command {
let cmd = Command::new(Self::NAME).about(Self::ABOUT);
Self::Options::build_args(cmd)
let mut command = Self::init_command();
if command.get_name().is_empty() {
command = command.name(Self::NAME);
}
if command.get_about().is_none() {
command = command.about(Self::ABOUT);
}
command
}
fn new(options: Self::Options) -> Self;
fn new(matches: &ArgMatches) -> Self;
fn init_command() -> Command;
/// Executes the runner, providing some result to the CLI.
fn run(&self) -> CliRunResult;
@ -137,20 +141,17 @@ mod tests {
}
}
impl RunnerOptions for TestRunnerOptions {
fn build_args(cmd: Command) -> Command {
cmd.arg(Arg::new("foo").short('f').action(ArgAction::SetTrue).required(false))
}
}
impl Runner for TestRunner {
type Options = TestRunnerOptions;
const ABOUT: &'static str = "some description";
const NAME: &'static str = "test";
fn new(options: Self::Options) -> Self {
Self { options }
fn init_command() -> Command {
Command::new("")
.arg(Arg::new("foo").short('f').action(ArgAction::SetTrue).required(false))
}
fn new(matches: &ArgMatches) -> Self {
Self { options: TestRunnerOptions::from(matches) }
}
fn run(&self) -> CliRunResult {
@ -167,8 +168,7 @@ mod tests {
fn smoke_test_runner() {
let cmd = TestRunner::command();
let matches = cmd.get_matches_from("test -f".split(' '));
let opts = TestRunnerOptions::from(&matches);
let runner = TestRunner::new(opts);
let runner = TestRunner::new(&matches);
assert!(runner.options.foo);
let result = runner.run();
assert!(matches!(result, CliRunResult::None));

View file

@ -6,10 +6,7 @@ use oxc_parser::Parser;
use oxc_span::SourceType;
use oxc_type_synthesis::synthesize_program;
use crate::{
runner::{Runner, RunnerOptions},
CliRunResult,
};
use crate::{runner::Runner, CliRunResult};
const PRELUDE: &str = "
type StringOrNumber = string | number;
@ -59,33 +56,38 @@ impl<'a> From<&'a ArgMatches> for TypeCheckOptions {
}
}
}
impl RunnerOptions for TypeCheckOptions {
fn build_args(cmd: Command) -> Command {
cmd.arg(Arg::new("path").value_name("PATH").num_args(1).help("File to type check"))
.arg(
Arg::new("print_expression_mappings")
.required(false)
.help("Print types of expressions"),
)
.arg(Arg::new("print_called_functions").required(false).help("Print called functions"))
}
}
pub struct TypeCheckRunner {
options: TypeCheckOptions,
}
impl Runner for TypeCheckRunner {
type Options = TypeCheckOptions;
const ABOUT: &'static str =
"NOTE: Experimental / work in progress. Check source code for type errors using Ezno";
const NAME: &'static str = "check";
fn new(options: TypeCheckOptions) -> Self {
fn new(matches: &ArgMatches) -> Self {
let options = TypeCheckOptions::from(matches);
Self { options }
}
fn init_command() -> Command {
Command::new(Self::NAME)
.arg(
Arg::new("path")
.value_name("PATH")
.num_args(1)
.required(true)
.help("File to type check"),
)
.arg(
Arg::new("print_expression_mappings")
.required(false)
.help("Print types of expressions"),
)
.arg(Arg::new("print_called_functions").required(false).help("Print called functions"))
}
/// # Panics
fn run(&self) -> CliRunResult {
let now = std::time::Instant::now();

View file

@ -3,7 +3,7 @@ use std::path::Path;
use ignore::{overrides::OverrideBuilder, DirEntry, WalkBuilder};
use oxc_span::VALID_EXTENSIONS;
use crate::LintOptions;
use oxc_linter::LintOptions;
pub struct Walk {
inner: ignore::Walk,

View file

@ -9,6 +9,7 @@ mod disable_directives;
mod fixer;
mod globals;
mod jest_ast_util;
mod options;
pub mod rule;
mod rule_timer;
mod rules;
@ -21,6 +22,7 @@ use rustc_hash::FxHashMap;
pub use crate::{
context::LintContext,
options::{AllowWarnDeny, LintOptions},
rule::RuleCategory,
rules::{RuleEnum, RULES},
};

View file

@ -0,0 +1,52 @@
use std::path::PathBuf;
#[derive(Debug)]
#[allow(clippy::struct_excessive_bools)]
pub struct LintOptions {
pub paths: Vec<PathBuf>,
/// Allow / Deny rules in order. [("allow" / "deny", rule name)]
/// Defaults to [("deny", "correctness")]
pub rules: Vec<(AllowWarnDeny, String)>,
pub list_rules: bool,
pub fix: bool,
pub quiet: bool,
pub ignore_path: PathBuf,
pub no_ignore: bool,
pub ignore_pattern: Vec<String>,
pub max_warnings: Option<usize>,
pub print_execution_times: bool,
}
impl Default for LintOptions {
fn default() -> Self {
Self {
paths: vec![],
rules: vec![(AllowWarnDeny::Deny, String::from("correctness"))],
list_rules: false,
fix: false,
quiet: false,
ignore_path: PathBuf::default(),
no_ignore: false,
ignore_pattern: vec![],
max_warnings: None,
print_execution_times: false,
}
}
}
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub enum AllowWarnDeny {
Allow,
// Warn,
Deny,
}
impl From<&'static str> for AllowWarnDeny {
fn from(s: &'static str) -> Self {
match s {
"allow" => Self::Allow,
"deny" => Self::Deny,
_ => unreachable!(),
}
}
}