Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/oxc_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_cli/src/lint/command.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_cli/src/lint/isolated_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 14 additions & 18 deletions crates/oxc_cli/src/lint/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
}
174 changes: 161 additions & 13 deletions crates/oxc_cli/src/lint/mod.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -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);
}
}
Loading
Loading