From 995673794183aac165ba0be2244e5bd9115d0945 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 27 Jul 2024 09:01:42 -0400 Subject: [PATCH] refactor(lint): move reporters to separate module (#24757) --- cli/tools/lint/mod.rs | 246 +---------------------------------- cli/tools/lint/reporters.rs | 252 ++++++++++++++++++++++++++++++++++++ 2 files changed, 256 insertions(+), 242 deletions(-) create mode 100644 cli/tools/lint/reporters.rs diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index e01b384307..efa11f03c8 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -2,7 +2,7 @@ //! This module provides file linting utilities using //! [`deno_lint`](https://github.com/denoland/deno_lint). -use deno_ast::diagnostics::Diagnostic; + use deno_ast::ModuleSpecifier; use deno_ast::ParsedSource; use deno_config::deno_json::LintRulesConfig; @@ -22,7 +22,8 @@ use deno_graph::ModuleGraph; use deno_lint::diagnostic::LintDiagnostic; use deno_lint::linter::LintConfig; use log::debug; -use log::info; +use reporters::create_reporter; +use reporters::LintReporter; use serde::Serialize; use std::collections::HashSet; use std::fs; @@ -37,7 +38,6 @@ use crate::args::CliOptions; use crate::args::Flags; use crate::args::LintFlags; use crate::args::LintOptions; -use crate::args::LintReporterKind; use crate::args::WorkspaceLintOptions; use crate::cache::Caches; use crate::cache::IncrementalCache; @@ -51,6 +51,7 @@ use crate::util::path::is_script_ext; use crate::util::sync::AtomicFlag; mod linter; +mod reporters; mod rules; pub use linter::CliLinter; @@ -61,14 +62,6 @@ pub use rules::LintRuleProvider; static STDIN_FILE_NAME: &str = "$deno$stdin.ts"; -fn create_reporter(kind: LintReporterKind) -> Box { - match kind { - LintReporterKind::Pretty => Box::new(PrettyLintReporter::new()), - LintReporterKind::Json => Box::new(JsonLintReporter::new()), - LintReporterKind::Compact => Box::new(CompactLintReporter::new()), - } -} - pub async fn lint( flags: Arc, lint_flags: LintFlags, @@ -537,239 +530,8 @@ fn handle_lint_result( } } -trait LintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic); - fn visit_error(&mut self, file_path: &str, err: &AnyError); - fn close(&mut self, check_count: usize); -} - #[derive(Serialize)] struct LintError { file_path: String, message: String, } - -struct PrettyLintReporter { - lint_count: u32, - fixable_diagnostics: u32, -} - -impl PrettyLintReporter { - fn new() -> PrettyLintReporter { - PrettyLintReporter { - lint_count: 0, - fixable_diagnostics: 0, - } - } -} - -impl LintReporter for PrettyLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic) { - self.lint_count += 1; - if !d.details.fixes.is_empty() { - self.fixable_diagnostics += 1; - } - - log::error!("{}\n", d.display()); - } - - fn visit_error(&mut self, file_path: &str, err: &AnyError) { - log::error!("Error linting: {file_path}"); - log::error!(" {err}"); - } - - fn close(&mut self, check_count: usize) { - let fixable_suffix = if self.fixable_diagnostics > 0 { - colors::gray(format!(" ({} fixable via --fix)", self.fixable_diagnostics)) - .to_string() - } else { - "".to_string() - }; - match self.lint_count { - 1 => info!("Found 1 problem{}", fixable_suffix), - n if n > 1 => { - info!("Found {} problems{}", self.lint_count, fixable_suffix) - } - _ => (), - } - - match check_count { - 1 => info!("Checked 1 file"), - n => info!("Checked {} files", n), - } - } -} - -struct CompactLintReporter { - lint_count: u32, -} - -impl CompactLintReporter { - fn new() -> CompactLintReporter { - CompactLintReporter { lint_count: 0 } - } -} - -impl LintReporter for CompactLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic) { - self.lint_count += 1; - - match &d.range { - Some(range) => { - let text_info = &range.text_info; - let range = &range.range; - let line_and_column = text_info.line_and_column_display(range.start); - log::error!( - "{}: line {}, col {} - {} ({})", - d.specifier, - line_and_column.line_number, - line_and_column.column_number, - d.message(), - d.code(), - ) - } - None => { - log::error!("{}: {} ({})", d.specifier, d.message(), d.code()) - } - } - } - - fn visit_error(&mut self, file_path: &str, err: &AnyError) { - log::error!("Error linting: {file_path}"); - log::error!(" {err}"); - } - - fn close(&mut self, check_count: usize) { - match self.lint_count { - 1 => info!("Found 1 problem"), - n if n > 1 => info!("Found {} problems", self.lint_count), - _ => (), - } - - match check_count { - 1 => info!("Checked 1 file"), - n => info!("Checked {} files", n), - } - } -} - -// WARNING: Ensure doesn't change because it's used in the JSON output -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct JsonDiagnosticLintPosition { - /// The 1-indexed line number. - pub line: usize, - /// The 0-indexed column index. - pub col: usize, - pub byte_pos: usize, -} - -impl JsonDiagnosticLintPosition { - pub fn new(byte_index: usize, loc: deno_ast::LineAndColumnIndex) -> Self { - JsonDiagnosticLintPosition { - line: loc.line_index + 1, - col: loc.column_index, - byte_pos: byte_index, - } - } -} - -// WARNING: Ensure doesn't change because it's used in the JSON output -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] -struct JsonLintDiagnosticRange { - pub start: JsonDiagnosticLintPosition, - pub end: JsonDiagnosticLintPosition, -} - -// WARNING: Ensure doesn't change because it's used in the JSON output -#[derive(Clone, Serialize)] -struct JsonLintDiagnostic { - pub filename: String, - pub range: Option, - pub message: String, - pub code: String, - pub hint: Option, -} - -#[derive(Serialize)] -struct JsonLintReporter { - diagnostics: Vec, - errors: Vec, -} - -impl JsonLintReporter { - fn new() -> JsonLintReporter { - JsonLintReporter { - diagnostics: Vec::new(), - errors: Vec::new(), - } - } -} - -impl LintReporter for JsonLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic) { - self.diagnostics.push(JsonLintDiagnostic { - filename: d.specifier.to_string(), - range: d.range.as_ref().map(|range| { - let text_info = &range.text_info; - let range = range.range; - JsonLintDiagnosticRange { - start: JsonDiagnosticLintPosition::new( - range.start.as_byte_index(text_info.range().start), - text_info.line_and_column_index(range.start), - ), - end: JsonDiagnosticLintPosition::new( - range.end.as_byte_index(text_info.range().start), - text_info.line_and_column_index(range.end), - ), - } - }), - message: d.message().to_string(), - code: d.code().to_string(), - hint: d.hint().map(|h| h.to_string()), - }); - } - - fn visit_error(&mut self, file_path: &str, err: &AnyError) { - self.errors.push(LintError { - file_path: file_path.to_string(), - message: err.to_string(), - }); - } - - fn close(&mut self, _check_count: usize) { - sort_diagnostics(&mut self.diagnostics); - let json = serde_json::to_string_pretty(&self); - #[allow(clippy::print_stdout)] - { - println!("{}", json.unwrap()); - } - } -} - -fn sort_diagnostics(diagnostics: &mut [JsonLintDiagnostic]) { - // Sort so that we guarantee a deterministic output which is useful for tests - diagnostics.sort_by(|a, b| { - use std::cmp::Ordering; - let file_order = a.filename.cmp(&b.filename); - match file_order { - Ordering::Equal => match &a.range { - Some(a_range) => match &b.range { - Some(b_range) => { - let line_order = a_range.start.line.cmp(&b_range.start.line); - match line_order { - Ordering::Equal => a_range.start.col.cmp(&b_range.start.col), - _ => line_order, - } - } - None => Ordering::Less, - }, - None => match &b.range { - Some(_) => Ordering::Greater, - None => Ordering::Equal, - }, - }, - _ => file_order, - } - }); -} diff --git a/cli/tools/lint/reporters.rs b/cli/tools/lint/reporters.rs new file mode 100644 index 0000000000..cb00ad2962 --- /dev/null +++ b/cli/tools/lint/reporters.rs @@ -0,0 +1,252 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use deno_ast::diagnostics::Diagnostic; +use deno_core::error::AnyError; +use deno_core::serde_json; +use deno_lint::diagnostic::LintDiagnostic; +use deno_runtime::colors; +use log::info; +use serde::Serialize; + +use crate::args::LintReporterKind; + +use super::LintError; + +pub fn create_reporter(kind: LintReporterKind) -> Box { + match kind { + LintReporterKind::Pretty => Box::new(PrettyLintReporter::new()), + LintReporterKind::Json => Box::new(JsonLintReporter::new()), + LintReporterKind::Compact => Box::new(CompactLintReporter::new()), + } +} + +pub trait LintReporter { + fn visit_diagnostic(&mut self, d: &LintDiagnostic); + fn visit_error(&mut self, file_path: &str, err: &AnyError); + fn close(&mut self, check_count: usize); +} + +struct PrettyLintReporter { + lint_count: u32, + fixable_diagnostics: u32, +} + +impl PrettyLintReporter { + fn new() -> PrettyLintReporter { + PrettyLintReporter { + lint_count: 0, + fixable_diagnostics: 0, + } + } +} + +impl LintReporter for PrettyLintReporter { + fn visit_diagnostic(&mut self, d: &LintDiagnostic) { + self.lint_count += 1; + if !d.details.fixes.is_empty() { + self.fixable_diagnostics += 1; + } + + log::error!("{}\n", d.display()); + } + + fn visit_error(&mut self, file_path: &str, err: &AnyError) { + log::error!("Error linting: {file_path}"); + log::error!(" {err}"); + } + + fn close(&mut self, check_count: usize) { + let fixable_suffix = if self.fixable_diagnostics > 0 { + colors::gray(format!(" ({} fixable via --fix)", self.fixable_diagnostics)) + .to_string() + } else { + "".to_string() + }; + match self.lint_count { + 1 => info!("Found 1 problem{}", fixable_suffix), + n if n > 1 => { + info!("Found {} problems{}", self.lint_count, fixable_suffix) + } + _ => (), + } + + match check_count { + 1 => info!("Checked 1 file"), + n => info!("Checked {} files", n), + } + } +} + +struct CompactLintReporter { + lint_count: u32, +} + +impl CompactLintReporter { + fn new() -> CompactLintReporter { + CompactLintReporter { lint_count: 0 } + } +} + +impl LintReporter for CompactLintReporter { + fn visit_diagnostic(&mut self, d: &LintDiagnostic) { + self.lint_count += 1; + + match &d.range { + Some(range) => { + let text_info = &range.text_info; + let range = &range.range; + let line_and_column = text_info.line_and_column_display(range.start); + log::error!( + "{}: line {}, col {} - {} ({})", + d.specifier, + line_and_column.line_number, + line_and_column.column_number, + d.message(), + d.code(), + ) + } + None => { + log::error!("{}: {} ({})", d.specifier, d.message(), d.code()) + } + } + } + + fn visit_error(&mut self, file_path: &str, err: &AnyError) { + log::error!("Error linting: {file_path}"); + log::error!(" {err}"); + } + + fn close(&mut self, check_count: usize) { + match self.lint_count { + 1 => info!("Found 1 problem"), + n if n > 1 => info!("Found {} problems", self.lint_count), + _ => (), + } + + match check_count { + 1 => info!("Checked 1 file"), + n => info!("Checked {} files", n), + } + } +} + +// WARNING: Ensure doesn't change because it's used in the JSON output +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct JsonDiagnosticLintPosition { + /// The 1-indexed line number. + pub line: usize, + /// The 0-indexed column index. + pub col: usize, + pub byte_pos: usize, +} + +impl JsonDiagnosticLintPosition { + pub fn new(byte_index: usize, loc: deno_ast::LineAndColumnIndex) -> Self { + JsonDiagnosticLintPosition { + line: loc.line_index + 1, + col: loc.column_index, + byte_pos: byte_index, + } + } +} + +// WARNING: Ensure doesn't change because it's used in the JSON output +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +struct JsonLintDiagnosticRange { + pub start: JsonDiagnosticLintPosition, + pub end: JsonDiagnosticLintPosition, +} + +// WARNING: Ensure doesn't change because it's used in the JSON output +#[derive(Clone, Serialize)] +struct JsonLintDiagnostic { + pub filename: String, + pub range: Option, + pub message: String, + pub code: String, + pub hint: Option, +} + +#[derive(Serialize)] +struct JsonLintReporter { + diagnostics: Vec, + errors: Vec, +} + +impl JsonLintReporter { + fn new() -> JsonLintReporter { + JsonLintReporter { + diagnostics: Vec::new(), + errors: Vec::new(), + } + } +} + +impl LintReporter for JsonLintReporter { + fn visit_diagnostic(&mut self, d: &LintDiagnostic) { + self.diagnostics.push(JsonLintDiagnostic { + filename: d.specifier.to_string(), + range: d.range.as_ref().map(|range| { + let text_info = &range.text_info; + let range = range.range; + JsonLintDiagnosticRange { + start: JsonDiagnosticLintPosition::new( + range.start.as_byte_index(text_info.range().start), + text_info.line_and_column_index(range.start), + ), + end: JsonDiagnosticLintPosition::new( + range.end.as_byte_index(text_info.range().start), + text_info.line_and_column_index(range.end), + ), + } + }), + message: d.message().to_string(), + code: d.code().to_string(), + hint: d.hint().map(|h| h.to_string()), + }); + } + + fn visit_error(&mut self, file_path: &str, err: &AnyError) { + self.errors.push(LintError { + file_path: file_path.to_string(), + message: err.to_string(), + }); + } + + fn close(&mut self, _check_count: usize) { + sort_diagnostics(&mut self.diagnostics); + let json = serde_json::to_string_pretty(&self); + #[allow(clippy::print_stdout)] + { + println!("{}", json.unwrap()); + } + } +} + +fn sort_diagnostics(diagnostics: &mut [JsonLintDiagnostic]) { + // Sort so that we guarantee a deterministic output which is useful for tests + diagnostics.sort_by(|a, b| { + use std::cmp::Ordering; + let file_order = a.filename.cmp(&b.filename); + match file_order { + Ordering::Equal => match &a.range { + Some(a_range) => match &b.range { + Some(b_range) => { + let line_order = a_range.start.line.cmp(&b_range.start.line); + match line_order { + Ordering::Equal => a_range.start.col.cmp(&b_range.start.col), + _ => line_order, + } + } + None => Ordering::Less, + }, + None => match &b.range { + Some(_) => Ordering::Greater, + None => Ordering::Equal, + }, + }, + _ => file_order, + } + }); +}