mirror of
https://github.com/denoland/deno.git
synced 2025-03-03 09:31:22 -05:00
refactor(lint): reuse lint rules (#11934)
This commit updated "deno_lint" crate to 0.15.1 and refactors "cli/tools/lint.rs" to create only a single vector of lint rules, instead of creating a vector for each linted file.
This commit is contained in:
parent
08e12380a0
commit
c04117134e
3 changed files with 101 additions and 132 deletions
4
Cargo.lock
generated
4
Cargo.lock
generated
|
@ -810,9 +810,9 @@ dependencies = [
|
|||
|
||||
[[package]]
|
||||
name = "deno_lint"
|
||||
version = "0.15.0"
|
||||
version = "0.15.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e7af2b640545a0b60268b1dfc4b78d78585e89446b7b20c5e0406bff872f2a01"
|
||||
checksum = "aab27202e3c7a5bcc959ede0cfe7aef75ce57bf43c1f2124d921c0383bbc58f8"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"deno_ast",
|
||||
|
|
|
@ -47,7 +47,7 @@ deno_ast = { version = "0.1.6", features = ["bundler", "codegen", "dep_graph", "
|
|||
deno_core = { version = "0.98.0", path = "../core" }
|
||||
deno_doc = "0.13.0"
|
||||
deno_graph = "0.4.0"
|
||||
deno_lint = { version = "0.15.0", features = ["docs"] }
|
||||
deno_lint = { version = "0.15.1", features = ["docs"] }
|
||||
deno_runtime = { version = "0.24.0", path = "../runtime" }
|
||||
deno_tls = { version = "0.3.0", path = "../ext/tls" }
|
||||
|
||||
|
|
|
@ -29,6 +29,8 @@ use std::path::PathBuf;
|
|||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
static STDIN_FILE_NAME: &str = "_stdin.ts";
|
||||
|
||||
pub enum LintReporterKind {
|
||||
Pretty,
|
||||
Json,
|
||||
|
@ -50,20 +52,11 @@ pub async fn lint_files(
|
|||
ignore: Vec<PathBuf>,
|
||||
json: bool,
|
||||
) -> Result<(), AnyError> {
|
||||
if args.len() == 1 && args[0].to_string_lossy() == "-" {
|
||||
return lint_stdin(
|
||||
json,
|
||||
maybe_lint_config.as_ref(),
|
||||
rules_tags,
|
||||
rules_include,
|
||||
rules_exclude,
|
||||
);
|
||||
}
|
||||
|
||||
// First, prepare final configuration.
|
||||
// Collect included and ignored files. CLI flags take precendence
|
||||
// over config file, ie. if there's `files.ignore` in config file
|
||||
// and `--ignore` CLI flag, only the flag value is taken into account.
|
||||
let mut include_files = args;
|
||||
let mut include_files = args.clone();
|
||||
let mut exclude_files = ignore;
|
||||
|
||||
if let Some(lint_config) = maybe_lint_config.as_ref() {
|
||||
|
@ -86,18 +79,15 @@ pub async fn lint_files(
|
|||
}
|
||||
}
|
||||
|
||||
let target_files =
|
||||
collect_files(&include_files, &exclude_files, is_supported_ext).and_then(
|
||||
|files| {
|
||||
if files.is_empty() {
|
||||
Err(generic_error("No target files found."))
|
||||
} else {
|
||||
Ok(files)
|
||||
}
|
||||
},
|
||||
)?;
|
||||
debug!("Found {} files", target_files.len());
|
||||
let target_files_len = target_files.len();
|
||||
// Try to get configured rules. CLI flags take precendence
|
||||
// over config file, ie. if there's `rules.include` in config file
|
||||
// and `--rules-include` CLI flag, only the flag value is taken into account.
|
||||
let lint_rules = get_configured_rules(
|
||||
maybe_lint_config.as_ref(),
|
||||
rules_tags,
|
||||
rules_include,
|
||||
rules_exclude,
|
||||
)?;
|
||||
|
||||
let has_error = Arc::new(AtomicBool::new(false));
|
||||
|
||||
|
@ -108,53 +98,53 @@ pub async fn lint_files(
|
|||
};
|
||||
let reporter_lock = Arc::new(Mutex::new(create_reporter(reporter_kind)));
|
||||
|
||||
// Try to get configured rules. CLI flags take precendence
|
||||
// over config file, ie. if there's `rules.include` in config file
|
||||
// and `--rules-include` CLI flag, only the flag value is taken into account.
|
||||
// TODO(bartlomieju): this is done multiple times for each file because
|
||||
// Vec<Box<dyn LintRule>> is not clonable, this should be optimized.
|
||||
get_configured_rules(
|
||||
maybe_lint_config.as_ref(),
|
||||
rules_tags.clone(),
|
||||
rules_include.clone(),
|
||||
rules_exclude.clone(),
|
||||
)?;
|
||||
let no_of_files_linted =
|
||||
if args.len() == 1 && args[0].to_string_lossy() == "-" {
|
||||
let r = lint_stdin(lint_rules);
|
||||
|
||||
run_parallelized(target_files, {
|
||||
let reporter_lock = reporter_lock.clone();
|
||||
let has_error = has_error.clone();
|
||||
move |file_path| {
|
||||
let r = lint_file(
|
||||
file_path.clone(),
|
||||
maybe_lint_config.as_ref(),
|
||||
rules_tags.clone(),
|
||||
rules_include.clone(),
|
||||
rules_exclude.clone(),
|
||||
handle_lint_result(
|
||||
STDIN_FILE_NAME,
|
||||
r,
|
||||
reporter_lock.clone(),
|
||||
has_error.clone(),
|
||||
);
|
||||
let mut reporter = reporter_lock.lock().unwrap();
|
||||
|
||||
match r {
|
||||
Ok((mut file_diagnostics, source)) => {
|
||||
sort_diagnostics(&mut file_diagnostics);
|
||||
for d in file_diagnostics.iter() {
|
||||
has_error.store(true, Ordering::Relaxed);
|
||||
reporter.visit_diagnostic(d, source.split('\n').collect());
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
has_error.store(true, Ordering::Relaxed);
|
||||
reporter.visit_error(&file_path.to_string_lossy().to_string(), &err);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
})
|
||||
.await?;
|
||||
1
|
||||
} else {
|
||||
let target_files =
|
||||
collect_files(&include_files, &exclude_files, is_supported_ext)
|
||||
.and_then(|files| {
|
||||
if files.is_empty() {
|
||||
Err(generic_error("No target files found."))
|
||||
} else {
|
||||
Ok(files)
|
||||
}
|
||||
})?;
|
||||
debug!("Found {} files", target_files.len());
|
||||
let target_files_len = target_files.len();
|
||||
|
||||
run_parallelized(target_files, {
|
||||
let reporter_lock = reporter_lock.clone();
|
||||
let has_error = has_error.clone();
|
||||
move |file_path| {
|
||||
let r = lint_file(file_path.clone(), lint_rules.clone());
|
||||
handle_lint_result(
|
||||
&file_path.to_string_lossy(),
|
||||
r,
|
||||
reporter_lock,
|
||||
has_error,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
})
|
||||
.await?;
|
||||
|
||||
target_files_len
|
||||
};
|
||||
|
||||
reporter_lock.lock().unwrap().close(no_of_files_linted);
|
||||
let has_error = has_error.load(Ordering::Relaxed);
|
||||
|
||||
reporter_lock.lock().unwrap().close(target_files_len);
|
||||
|
||||
if has_error {
|
||||
std::process::exit(1);
|
||||
}
|
||||
|
@ -162,27 +152,27 @@ pub async fn lint_files(
|
|||
Ok(())
|
||||
}
|
||||
|
||||
fn rule_to_json(rule: Box<dyn LintRule>) -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"code": rule.code(),
|
||||
"tags": rule.tags(),
|
||||
"docs": rule.docs(),
|
||||
})
|
||||
}
|
||||
|
||||
pub fn print_rules_list(json: bool) {
|
||||
let lint_rules = rules::get_recommended_rules();
|
||||
|
||||
if json {
|
||||
let json_rules: Vec<serde_json::Value> =
|
||||
lint_rules.into_iter().map(rule_to_json).collect();
|
||||
let json_rules: Vec<serde_json::Value> = lint_rules
|
||||
.iter()
|
||||
.map(|rule| {
|
||||
serde_json::json!({
|
||||
"code": rule.code(),
|
||||
"tags": rule.tags(),
|
||||
"docs": rule.docs(),
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
let json_str = serde_json::to_string_pretty(&json_rules).unwrap();
|
||||
println!("{}", json_str);
|
||||
} else {
|
||||
// The rules should still be printed even if `--quiet` option is enabled,
|
||||
// so use `println!` here instead of `info!`.
|
||||
println!("Available rules:");
|
||||
for rule in lint_rules {
|
||||
for rule in lint_rules.iter() {
|
||||
println!(" - {}", rule.code());
|
||||
println!(" help: https://lint.deno.land/#{}", rule.code());
|
||||
println!();
|
||||
|
@ -190,7 +180,10 @@ pub fn print_rules_list(json: bool) {
|
|||
}
|
||||
}
|
||||
|
||||
pub fn create_linter(syntax: Syntax, rules: Vec<Box<dyn LintRule>>) -> Linter {
|
||||
pub fn create_linter(
|
||||
syntax: Syntax,
|
||||
rules: Arc<Vec<Box<dyn LintRule>>>,
|
||||
) -> Linter {
|
||||
LinterBuilder::default()
|
||||
.ignore_file_directive("deno-lint-ignore-file")
|
||||
.ignore_diagnostic_directive("deno-lint-ignore")
|
||||
|
@ -201,24 +194,13 @@ pub fn create_linter(syntax: Syntax, rules: Vec<Box<dyn LintRule>>) -> Linter {
|
|||
|
||||
fn lint_file(
|
||||
file_path: PathBuf,
|
||||
maybe_lint_config: Option<&LintConfig>,
|
||||
rules_tags: Vec<String>,
|
||||
rules_include: Vec<String>,
|
||||
rules_exclude: Vec<String>,
|
||||
lint_rules: Arc<Vec<Box<dyn LintRule>>>,
|
||||
) -> Result<(Vec<LintDiagnostic>, String), AnyError> {
|
||||
let file_name = file_path.to_string_lossy().to_string();
|
||||
let source_code = fs::read_to_string(&file_path)?;
|
||||
let media_type = MediaType::from(&file_path);
|
||||
let syntax = deno_ast::get_syntax(media_type);
|
||||
|
||||
// Obtaining rules from config is infallible at this point.
|
||||
let lint_rules = get_configured_rules(
|
||||
maybe_lint_config,
|
||||
rules_tags,
|
||||
rules_include,
|
||||
rules_exclude,
|
||||
)
|
||||
.unwrap();
|
||||
let linter = create_linter(syntax, lint_rules);
|
||||
|
||||
let (_, file_diagnostics) = linter.lint(file_name, source_code.clone())?;
|
||||
|
@ -230,56 +212,43 @@ fn lint_file(
|
|||
/// Treats input as TypeScript.
|
||||
/// Compatible with `--json` flag.
|
||||
fn lint_stdin(
|
||||
json: bool,
|
||||
maybe_lint_config: Option<&LintConfig>,
|
||||
rules_tags: Vec<String>,
|
||||
rules_include: Vec<String>,
|
||||
rules_exclude: Vec<String>,
|
||||
) -> Result<(), AnyError> {
|
||||
let mut source = String::new();
|
||||
if stdin().read_to_string(&mut source).is_err() {
|
||||
lint_rules: Arc<Vec<Box<dyn LintRule>>>,
|
||||
) -> Result<(Vec<LintDiagnostic>, String), AnyError> {
|
||||
let mut source_code = String::new();
|
||||
if stdin().read_to_string(&mut source_code).is_err() {
|
||||
return Err(generic_error("Failed to read from stdin"));
|
||||
}
|
||||
|
||||
let reporter_kind = if json {
|
||||
LintReporterKind::Json
|
||||
} else {
|
||||
LintReporterKind::Pretty
|
||||
};
|
||||
let mut reporter = create_reporter(reporter_kind);
|
||||
let lint_rules = get_configured_rules(
|
||||
maybe_lint_config,
|
||||
rules_tags,
|
||||
rules_include,
|
||||
rules_exclude,
|
||||
)?;
|
||||
let syntax = deno_ast::get_syntax(MediaType::TypeScript);
|
||||
let linter = create_linter(syntax, lint_rules);
|
||||
let mut has_error = false;
|
||||
let pseudo_file_name = "_stdin.ts";
|
||||
match linter
|
||||
.lint(pseudo_file_name.to_string(), source.clone())
|
||||
.map_err(|e| e.into())
|
||||
{
|
||||
Ok((_, diagnostics)) => {
|
||||
for d in diagnostics {
|
||||
has_error = true;
|
||||
reporter.visit_diagnostic(&d, source.split('\n').collect());
|
||||
|
||||
let (_, file_diagnostics) =
|
||||
linter.lint(STDIN_FILE_NAME.to_string(), source_code.clone())?;
|
||||
|
||||
Ok((file_diagnostics, source_code))
|
||||
}
|
||||
|
||||
fn handle_lint_result(
|
||||
file_path: &str,
|
||||
result: Result<(Vec<LintDiagnostic>, String), AnyError>,
|
||||
reporter_lock: Arc<Mutex<Box<dyn LintReporter + Send>>>,
|
||||
has_error: Arc<AtomicBool>,
|
||||
) {
|
||||
let mut reporter = reporter_lock.lock().unwrap();
|
||||
|
||||
match result {
|
||||
Ok((mut file_diagnostics, source)) => {
|
||||
sort_diagnostics(&mut file_diagnostics);
|
||||
for d in file_diagnostics.iter() {
|
||||
has_error.store(true, Ordering::Relaxed);
|
||||
reporter.visit_diagnostic(d, source.split('\n').collect());
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
has_error = true;
|
||||
reporter.visit_error(pseudo_file_name, &err);
|
||||
has_error.store(true, Ordering::Relaxed);
|
||||
reporter.visit_error(file_path, &err);
|
||||
}
|
||||
}
|
||||
|
||||
reporter.close(1);
|
||||
|
||||
if has_error {
|
||||
std::process::exit(1);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
trait LintReporter {
|
||||
|
@ -470,7 +439,7 @@ fn get_configured_rules(
|
|||
rules_tags: Vec<String>,
|
||||
rules_include: Vec<String>,
|
||||
rules_exclude: Vec<String>,
|
||||
) -> Result<Vec<Box<dyn LintRule>>, AnyError> {
|
||||
) -> Result<Arc<Vec<Box<dyn LintRule>>>, AnyError> {
|
||||
if maybe_lint_config.is_none()
|
||||
&& rules_tags.is_empty()
|
||||
&& rules_include.is_empty()
|
||||
|
|
Loading…
Add table
Reference in a new issue