feat(cli): add -A all -D isolated-declarations (#211)

* feat(linter): add restriction category for lint rules

* feat(cli): add "--allow" and "--deny" commands

* feat(linter): use a single instance of linter

* feat(cli): derive rules from args

* feat(cli): print number of rules
This commit is contained in:
Boshen 2023-03-26 04:34:31 -07:00 committed by GitHub
parent d4af69930c
commit ef19895cc3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 235 additions and 37 deletions

1
Cargo.lock generated
View file

@ -877,6 +877,7 @@ dependencies = [
"oxc_parser",
"oxc_semantic",
"rayon",
"rustc-hash",
]
[[package]]

View file

@ -26,6 +26,7 @@ oxc_linter = { path = "../oxc_linter" }
clap = { workspace = true }
rayon = { workspace = true }
miette = { workspace = true, features = ["fancy-no-backtrace"] }
rustc-hash = { workspace = true }
num_cpus = "1.15.0"
ignore = { version = "0.4.20", features = ["simd-accel"] }

View file

@ -5,6 +5,16 @@ pub fn lint_command() -> Command {
.alias("check")
.about("Lint this repository.")
.arg_required_else_help(true)
.after_help(
"To allow or deny a rule, multiple -A <NAME> or -D <NAME>.
For example: -D correctness -A no-debugger.
The categories are:
* correctness - code that is outright wrong or useless
* nursery - new lints that are still under development
* all - all the categories listed above
The default category is -D correctness.")
.arg(
Arg::new("path")
.value_name("PATH")
@ -13,6 +23,22 @@ pub fn lint_command() -> Command {
.value_parser(ValueParser::path_buf())
.help("File or Directory paths to scan. Directories are scanned recursively.")
)
.arg(
Arg::new("allow")
.long("allow")
.short('A')
.required(false)
.action(ArgAction::Append)
.help("Allow a rule or a category")
)
.arg(
Arg::new("deny")
.long("deny")
.short('D')
.required(false)
.action(ArgAction::Append)
.help("Deny a rule or a category")
)
.arg(
Arg::new("fix")
.long("fix")

View file

@ -1,7 +1,8 @@
use std::path::PathBuf;
mod command;
mod runner;
use std::{collections::BTreeMap, path::PathBuf};
use clap::ArgMatches;
pub use self::{command::lint_command, runner::LintRunner};
@ -9,6 +10,9 @@ pub use self::{command::lint_command, runner::LintRunner};
#[derive(Debug)]
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 fix: bool,
pub quiet: bool,
pub ignore_path: PathBuf,
@ -17,6 +21,23 @@ pub struct LintOptions {
pub max_warnings: Option<usize>,
}
#[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 {
Self {
@ -24,6 +45,7 @@ impl<'a> From<&'a ArgMatches> for LintOptions {
|| 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
@ -39,11 +61,34 @@ impl<'a> From<&'a ArgMatches> for LintOptions {
}
}
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()
}
}
}
#[cfg(test)]
mod test {
use std::path::PathBuf;
use super::{lint_command, LintOptions};
use super::{lint_command, AllowWarnDeny, LintOptions};
#[test]
fn verify_command() {
@ -56,7 +101,7 @@ mod test {
}
#[test]
fn test_default() {
fn default() {
let options = get_lint_options("lint .");
assert_eq!(options.paths, vec![PathBuf::from(".")]);
assert!(!options.fix);
@ -68,7 +113,7 @@ mod test {
}
#[test]
fn test_multiple_paths() {
fn multiple_paths() {
let options = get_lint_options("lint foo bar baz");
assert_eq!(
options.paths,
@ -77,43 +122,59 @@ mod test {
}
#[test]
fn test_quiet_true() {
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 test_fix_true() {
fn fix_true() {
let options = get_lint_options("lint foo.js --fix");
assert!(options.fix);
}
#[test]
fn test_max_warnings() {
fn max_warnings() {
let options = get_lint_options("lint --max-warnings 10 foo.js");
assert_eq!(options.max_warnings, Some(10));
}
#[test]
fn test_ignore_path() {
fn ignore_path() {
let options = get_lint_options("lint --ignore-path .xxx foo.js");
assert_eq!(options.ignore_path, PathBuf::from(".xxx"));
}
#[test]
fn test_no_ignore() {
fn no_ignore() {
let options = get_lint_options("lint --no-ignore foo.js");
assert!(options.no_ignore);
}
#[test]
fn test_single_ignore_pattern() {
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 test_multiple_ignore_pattern() {
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")]);

View file

@ -13,21 +13,71 @@ use miette::NamedSource;
use oxc_allocator::Allocator;
use oxc_ast::SourceType;
use oxc_diagnostics::{Error, MinifiedFileError, Severity};
use oxc_linter::{Fixer, Linter};
use oxc_linter::{Fixer, Linter, RuleCategory, RuleEnum, RULES};
use oxc_parser::Parser;
use oxc_semantic::SemanticBuilder;
use rustc_hash::FxHashSet;
use super::LintOptions;
use super::{AllowWarnDeny, LintOptions};
use crate::{CliRunResult, Walk};
pub struct LintRunner {
options: LintOptions,
linter: Arc<Linter>,
}
impl LintRunner {
#[must_use]
pub fn new(options: LintOptions) -> Self {
Self { options }
let linter = Linter::from_rules(Self::derive_rules(&options)).with_fix(options.fix);
Self { options, linter: Arc::new(linter) }
}
fn derive_rules(options: &LintOptions) -> Vec<RuleEnum> {
let mut rules: FxHashSet<RuleEnum> = FxHashSet::default();
for (allow_warn_deny, name_or_category) in &options.rules {
let maybe_category = RuleCategory::from(name_or_category.as_str());
match allow_warn_deny {
AllowWarnDeny::Deny => {
match maybe_category {
Some(category) => rules.extend(
RULES.iter().filter(|rule| rule.category() == category).cloned(),
),
None => {
if name_or_category == "all" {
rules.extend(RULES.iter().cloned());
} else {
rules.extend(
RULES
.iter()
.filter(|rule| rule.name() == name_or_category)
.cloned(),
);
}
}
};
}
AllowWarnDeny::Allow => {
match maybe_category {
Some(category) => rules.retain(|rule| rule.category() != category),
None => {
if name_or_category == "all" {
rules.drain();
} else {
rules.retain(|rule| rule.name() == name_or_category);
}
}
};
}
}
}
let mut rules = rules.into_iter().collect::<Vec<_>>();
// for stable diagnostics output ordering
rules.sort_unstable_by_key(|rule| rule.name());
rules
}
/// # Panics
@ -45,6 +95,7 @@ impl LintRunner {
CliRunResult::LintResult {
duration: now.elapsed(),
number_of_rules: self.linter.number_of_rules(),
number_of_files: number_of_files.load(Ordering::Relaxed),
number_of_diagnostics,
number_of_warnings,
@ -73,12 +124,13 @@ impl LintRunner {
number_of_files.store(count, Ordering::Relaxed);
});
let fix = self.options.fix;
let linter = Arc::clone(&self.linter);
rayon::spawn(move || {
while let Ok(path) = rx_path.recv() {
let tx_error = tx_error.clone();
let linter = Arc::clone(&linter);
rayon::spawn(move || {
if let Some(diagnostics) = Self::lint_path(&path, fix) {
if let Some(diagnostics) = Self::lint_path(&linter, &path) {
tx_error.send(diagnostics).unwrap();
}
drop(tx_error);
@ -128,7 +180,7 @@ impl LintRunner {
(number_of_warnings, number_of_diagnostics)
}
fn lint_path(path: &Path, fix: bool) -> Option<(PathBuf, Vec<Error>)> {
fn lint_path(linter: &Linter, path: &Path) -> Option<(PathBuf, Vec<Error>)> {
let source_text = fs::read_to_string(path).unwrap_or_else(|_| panic!("{path:?} not found"));
let allocator = Allocator::default();
let source_type =
@ -147,13 +199,13 @@ impl LintRunner {
return Some(Self::wrap_diagnostics(path, &source_text, semantic_ret.errors));
};
let result = Linter::new().with_fix(fix).run(&Rc::new(semantic_ret.semantic));
let result = linter.run(&Rc::new(semantic_ret.semantic));
if result.is_empty() {
return None;
}
if fix {
if linter.has_fix() {
let fix_result = Fixer::new(&source_text, result).fix();
fs::write(path, fix_result.fixed_code.as_bytes()).unwrap();
let errors = fix_result.messages.into_iter().map(|m| m.error).collect();

View file

@ -10,11 +10,12 @@ pub enum CliRunResult {
paths: Vec<PathBuf>,
},
LintResult {
duration: std::time::Duration,
number_of_rules: usize,
number_of_files: usize,
number_of_warnings: usize,
number_of_diagnostics: usize,
max_warnings_exceeded: bool,
duration: std::time::Duration,
},
}
@ -27,15 +28,18 @@ impl Termination for CliRunResult {
ExitCode::from(1)
}
Self::LintResult {
duration,
number_of_rules,
number_of_files,
number_of_warnings,
number_of_diagnostics,
max_warnings_exceeded,
duration,
} => {
let ms = duration.as_millis();
let cpus = num_cpus::get();
println!("Checked {number_of_files} files in {ms}ms using {cpus} cores.");
println!(
"Finished in {ms}ms on {number_of_files} files with {number_of_rules} rules using {cpus} cores."
);
if max_warnings_exceeded {
println!("Exceeded maximum number of warnings. Found {number_of_warnings}.");

View file

@ -17,11 +17,13 @@ use std::{fs, rc::Rc};
pub use fixer::{Fixer, Message};
pub(crate) use oxc_semantic::AstNode;
use oxc_semantic::Semantic;
use rule::{Rule, RuleCategory};
use crate::{
context::LintContext,
rules::{early_error::javascript::EarlyErrorJavaScript, RuleEnum, RULES},
context::LintContext, rule::Rule, rules::early_error::javascript::EarlyErrorJavaScript,
};
pub use crate::{
rule::RuleCategory,
rules::{RuleEnum, RULES},
};
#[derive(Debug)]
@ -37,17 +39,11 @@ impl Linter {
#[must_use]
#[allow(clippy::new_without_default)]
pub fn new() -> Self {
// let rules_config = Self::read_rules_configuration();
// let rules = rules_config.map_or_else(
// || RULES.to_vec(),
// |rules_config| {
let rules = RULES
.iter()
.cloned()
.filter(|rule| rule.category() == RuleCategory::Correctness)
.collect::<Vec<_>>();
// },
// );
Self::from_rules(rules)
}
@ -56,6 +52,16 @@ impl Linter {
Self { rules, early_error_javascript: EarlyErrorJavaScript, fix: false }
}
#[must_use]
pub fn has_fix(&self) -> bool {
self.fix
}
#[must_use]
pub fn number_of_rules(&self) -> usize {
self.rules.len()
}
#[must_use]
pub fn with_fix(mut self, yes: bool) -> Self {
self.fix = yes;

View file

@ -4,12 +4,6 @@ use oxc_semantic::Symbol;
use crate::{context::LintContext, AstNode};
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RuleCategory {
Correctness,
Nursery,
}
pub trait Rule: Sized + Default + Debug {
/// Initialize from eslint json configuration
#[must_use]
@ -32,3 +26,30 @@ pub trait RuleMeta {
None
}
}
/// Rule categories defined by rust-clippy
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RuleCategory {
/// Code that is outright wrong or useless
Correctness,
/// Lints which prevent the use of language and library features
/// The restriction category should, emphatically, not be enabled as a whole.
/// The contained lints may lint against perfectly reasonable code, may not have an alternative suggestion,
/// and may contradict any other lints (including other categories).
/// Lints should be considered on a case-by-case basis before enabling.
Restriction,
/// New lints that are still under development
Nursery,
}
impl RuleCategory {
#[must_use]
pub fn from(input: &str) -> Option<Self> {
match input {
"correctness" => Some(Self::Correctness),
"restriction" => Some(Self::Restriction),
"nursery" => Some(Self::Nursery),
_ => None,
}
}
}

View file

@ -126,6 +126,32 @@ pub fn declare_all_lint_rules(metadata: AllLintRulesMeta) -> TokenStream {
}
}
impl std::hash::Hash for RuleEnum {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.name().hash(state);
}
}
impl PartialEq for RuleEnum {
fn eq(&self, other: &Self) -> bool {
self.name() == other.name()
}
}
impl Eq for RuleEnum {}
impl Ord for RuleEnum {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.name().cmp(&other.name())
}
}
impl PartialOrd for RuleEnum {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
Some(self.cmp(&other))
}
}
lazy_static::lazy_static! {
pub static ref RULES: Vec<RuleEnum> = vec![
#(RuleEnum::#struct_names(#struct_names::default())),*