From 15823402a6737ca992febfec72f54a78ac64b21e Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Sun, 4 Aug 2024 13:41:43 +0100 Subject: [PATCH] refactor(cli): traversal (#3563) --- CHANGELOG.md | 23 +++++- crates/biome_cli/src/execute/mod.rs | 27 +++--- crates/biome_cli/src/execute/process_file.rs | 34 +------- crates/biome_cli/src/execute/traverse.rs | 62 ++++++++++---- crates/biome_cli/src/reporter/mod.rs | 11 +++ crates/biome_cli/src/reporter/terminal.rs | 66 ++++++++++++++- .../max_diagnostics_verbose.snap | 44 +++++++--- .../does_not_conceal_previous_overrides.snap | 4 +- .../not_process_file_from_cli_verbose.snap | 20 +++++ ...file_linter_disabled_from_cli_verbose.snap | 20 +++++ ...process_ignored_file_from_cli_verbose.snap | 20 +++++ .../main_commands_check/print_verbose.snap | 20 +++++ .../unsupported_file_verbose.snap | 20 +++++ .../main_commands_ci/print_verbose.snap | 20 +++++ .../main_commands_format/print_verbose.snap | 20 +++++ ...treat_known_json_files_as_jsonc_files.snap | 4 +- .../main_commands_lint/print_verbose.snap | 20 +++++ .../unsupported_file_verbose.snap | 20 +++++ crates/biome_diagnostics/src/advice.rs | 22 +++++ crates/biome_fs/src/fs.rs | 82 +++++++++++++++++-- crates/biome_fs/src/fs/memory.rs | 43 ++++++---- crates/biome_fs/src/fs/os.rs | 10 ++- crates/biome_fs/src/lib.rs | 6 +- crates/biome_service/src/workspace.rs | 4 +- crates/biome_service/src/workspace/server.rs | 4 +- 25 files changed, 521 insertions(+), 105 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db7cf9ee1e60..fda4b081fda2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,28 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - `biome init` now generates a new config file with more options set. This change intends to improve discoverability of the options and to set the more commonly used options to their default values. - Contributed by Conaclos + Contributed by @Conaclos +- The `--verbose` flag how reports the list of files that were evaluated, and the list of files that were fixed. + The **evaluated** files are the those files that can be handled by Biome, files that are ignored, don't have an extension or have an extension that Biome can't evaluate are excluded by this list. + The **fixed** files are those files that were handled by Biome and *changed*. Files that stays the same after the process are excluded from this list. + + ```shell + VERBOSE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ℹ Files processed: + + - biome/biome.json + - biome/packages/@biomejs/cli-win32-arm64/package.json + - biome/packages/tailwindcss-config-analyzer/package.json + + VERBOSE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ℹ Files fixed: + + - biome/biome/packages/tailwindcss-config-analyzer/src/generate-tailwind-preset.ts + ``` + + Contributed by @ematipico #### Bug fixes diff --git a/crates/biome_cli/src/execute/mod.rs b/crates/biome_cli/src/execute/mod.rs index 2662d0fa41d0..719646feba43 100644 --- a/crates/biome_cli/src/execute/mod.rs +++ b/crates/biome_cli/src/execute/mod.rs @@ -8,7 +8,7 @@ use crate::cli_options::{CliOptions, CliReporter}; use crate::commands::MigrateSubCommand; use crate::diagnostics::ReportDiagnostic; use crate::execute::migrate::MigratePayload; -use crate::execute::traverse::traverse; +use crate::execute::traverse::{traverse, TraverseResult}; use crate::reporter::github::{GithubReporter, GithubReporterVisitor}; use crate::reporter::json::{JsonReporter, JsonReporterVisitor}; use crate::reporter::junit::{JunitReporter, JunitReporterVisitor}; @@ -431,18 +431,22 @@ pub fn execute_mode( }; migrate::run(payload) } else { - let (summary_result, diagnostics) = traverse(&execution, &mut session, cli_options, paths)?; + let TraverseResult { + summary, + evaluated_paths, + diagnostics, + } = traverse(&execution, &mut session, cli_options, paths)?; let console = session.app.console; - let errors = summary_result.errors; - let skipped = summary_result.skipped; - let processed = summary_result.changed + summary_result.unchanged; - let should_exit_on_warnings = summary_result.warnings > 0 && cli_options.error_on_warnings; + let errors = summary.errors; + let skipped = summary.skipped; + let processed = summary.changed + summary.unchanged; + let should_exit_on_warnings = summary.warnings > 0 && cli_options.error_on_warnings; match execution.report_mode { ReportMode::Terminal { with_summary } => { if with_summary { let reporter = SummaryReporter { - summary: summary_result, + summary, diagnostics_payload: DiagnosticsPayload { verbose: cli_options.verbose, diagnostic_level: cli_options.diagnostic_level, @@ -453,13 +457,14 @@ pub fn execute_mode( reporter.write(&mut SummaryReporterVisitor(console))?; } else { let reporter = ConsoleReporter { - summary: summary_result, + summary, diagnostics_payload: DiagnosticsPayload { verbose: cli_options.verbose, diagnostic_level: cli_options.diagnostic_level, diagnostics, }, execution: execution.clone(), + evaluated_paths, }; reporter.write(&mut ConsoleReporterVisitor(console))?; } @@ -469,7 +474,7 @@ pub fn execute_mode( "The ""--json"" option is ""unstable/experimental"" and its output might change between patches/minor releases." }); let reporter = JsonReporter { - summary: summary_result, + summary, diagnostics: DiagnosticsPayload { verbose: cli_options.verbose, diagnostic_level: cli_options.diagnostic_level, @@ -477,7 +482,7 @@ pub fn execute_mode( }, execution: execution.clone(), }; - let mut buffer = JsonReporterVisitor::new(summary_result); + let mut buffer = JsonReporterVisitor::new(summary); reporter.write(&mut buffer)?; if pretty { let content = serde_json::to_string(&buffer).map_err(|error| { @@ -517,7 +522,7 @@ pub fn execute_mode( } ReportMode::Junit => { let reporter = JunitReporter { - summary: summary_result, + summary, diagnostics_payload: DiagnosticsPayload { verbose: cli_options.verbose, diagnostic_level: cli_options.diagnostic_level, diff --git a/crates/biome_cli/src/execute/process_file.rs b/crates/biome_cli/src/execute/process_file.rs index 8d0b67338f81..ba7ed6467958 100644 --- a/crates/biome_cli/src/execute/process_file.rs +++ b/crates/biome_cli/src/execute/process_file.rs @@ -18,7 +18,6 @@ use search::search; use std::marker::PhantomData; use std::ops::Deref; use std::path::Path; - #[derive(Debug)] pub(crate) enum FileStatus { /// File changed and it was a success @@ -150,38 +149,7 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { // then we pick the specific features for this file let unsupported_reason = match ctx.execution.traversal_mode() { - TraversalMode::Check { .. } => file_features - .support_kind_for(&FeatureKind::Lint) - .and_then(|support_kind| { - if support_kind.is_not_enabled() { - Some(support_kind) - } else { - None - } - }) - .and( - file_features - .support_kind_for(&FeatureKind::Format) - .and_then(|support_kind| { - if support_kind.is_not_enabled() { - Some(support_kind) - } else { - None - } - }), - ) - .and( - file_features - .support_kind_for(&FeatureKind::OrganizeImports) - .and_then(|support_kind| { - if support_kind.is_not_enabled() { - Some(support_kind) - } else { - None - } - }), - ), - TraversalMode::CI { .. } => file_features + TraversalMode::Check { .. } | TraversalMode::CI { .. } => file_features .support_kind_for(&FeatureKind::Lint) .and_then(|support_kind| { if support_kind.is_not_enabled() { diff --git a/crates/biome_cli/src/execute/traverse.rs b/crates/biome_cli/src/execute/traverse.rs index d08e1db75d2a..63c75133bbc3 100644 --- a/crates/biome_cli/src/execute/traverse.rs +++ b/crates/biome_cli/src/execute/traverse.rs @@ -9,13 +9,14 @@ use crate::reporter::TraversalSummary; use crate::{CliDiagnostic, CliSession}; use biome_diagnostics::DiagnosticTags; use biome_diagnostics::{category, DiagnosticExt, Error, Resource, Severity}; -use biome_fs::{BiomePath, FileSystem, PathInterner}; +use biome_fs::{BiomePath, EvaluatedPath, FileSystem, PathInterner}; use biome_fs::{TraversalContext, TraversalScope}; use biome_service::workspace::{DropPatternParams, IsPathIgnoredParams}; use biome_service::{extension_error, workspace::SupportsFeatureParams, Workspace, WorkspaceError}; use crossbeam::channel::{unbounded, Receiver, Sender}; use rustc_hash::FxHashSet; use std::sync::atomic::AtomicU32; +use std::sync::RwLock; use std::{ env::current_dir, ffi::OsString, @@ -29,12 +30,18 @@ use std::{ time::{Duration, Instant}, }; +pub(crate) struct TraverseResult { + pub(crate) summary: TraversalSummary, + pub(crate) evaluated_paths: FxHashSet, + pub(crate) diagnostics: Vec, +} + pub(crate) fn traverse( execution: &Execution, session: &mut CliSession, cli_options: &CliOptions, mut inputs: Vec, -) -> Result<(TraversalSummary, Vec), CliDiagnostic> { +) -> Result { init_thread_pool(); if inputs.is_empty() { @@ -81,7 +88,7 @@ pub(crate) fn traverse( .with_diagnostic_level(cli_options.diagnostic_level) .with_max_diagnostics(max_diagnostics); - let (duration, diagnostics) = thread::scope(|s| { + let (duration, evaluated_paths, diagnostics) = thread::scope(|s| { let handler = thread::Builder::new() .name(String::from("biome::console")) .spawn_scoped(s, || printer.run(receiver, recv_files)) @@ -89,7 +96,7 @@ pub(crate) fn traverse( // The traversal context is scoped to ensure all the channels it // contains are properly closed once the traversal finishes - let elapsed = traverse_inputs( + let (elapsed, evaluated_paths) = traverse_inputs( fs, inputs, &TraversalOptions { @@ -102,12 +109,13 @@ pub(crate) fn traverse( skipped: &skipped, messages: sender, remaining_diagnostics: &remaining_diagnostics, + evaluated_paths: RwLock::default(), }, ); // wait for the main thread to finish let diagnostics = handler.join().unwrap(); - (elapsed, diagnostics) + (elapsed, evaluated_paths, diagnostics) }); // Make sure patterns are always cleaned up at the end of traversal. @@ -124,8 +132,8 @@ pub(crate) fn traverse( let skipped = skipped.load(Ordering::Relaxed); let suggested_fixes_skipped = printer.skipped_fixes(); let diagnostics_not_printed = printer.not_printed_diagnostics(); - Ok(( - TraversalSummary { + Ok(TraverseResult { + summary: TraversalSummary { changed, unchanged, duration, @@ -135,8 +143,9 @@ pub(crate) fn traverse( suggested_fixes_skipped, diagnostics_not_printed, }, + evaluated_paths, diagnostics, - )) + }) } /// This function will setup the global Rayon thread pool the first time it's called @@ -154,15 +163,27 @@ fn init_thread_pool() { /// Initiate the filesystem traversal tasks with the provided input paths and /// run it to completion, returning the duration of the process -fn traverse_inputs(fs: &dyn FileSystem, inputs: Vec, ctx: &TraversalOptions) -> Duration { +fn traverse_inputs( + fs: &dyn FileSystem, + inputs: Vec, + ctx: &TraversalOptions, +) -> (Duration, FxHashSet) { let start = Instant::now(); fs.traversal(Box::new(move |scope: &dyn TraversalScope| { for input in inputs { - scope.spawn(ctx, PathBuf::from(input)); + scope.evaluate(ctx, PathBuf::from(input)); + } + })); + + let paths = ctx.evaluated_paths(); + + fs.traversal(Box::new(|scope: &dyn TraversalScope| { + for path in paths.clone() { + scope.handle(ctx, path.to_path_buf()); } })); - start.elapsed() + (start.elapsed(), paths) } // struct DiagnosticsReporter<'ctx> {} @@ -483,11 +504,16 @@ pub(crate) struct TraversalOptions<'ctx, 'app> { /// The approximate number of diagnostics the console will print before /// folding the rest into the "skipped diagnostics" counter pub(crate) remaining_diagnostics: &'ctx AtomicU32, + + /// List of paths that should be processed + pub(crate) evaluated_paths: RwLock>, } impl<'ctx, 'app> TraversalOptions<'ctx, 'app> { - pub(crate) fn increment_changed(&self) { + pub(crate) fn increment_changed(&self, path: &Path) { self.changed.fetch_add(1, Ordering::Relaxed); + let mut evaluated_paths = self.evaluated_paths.write().unwrap(); + evaluated_paths.replace(EvaluatedPath::new_evaluated(path)); } pub(crate) fn increment_unchanged(&self) { self.unchanged.fetch_add(1, Ordering::Relaxed); @@ -518,6 +544,10 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { &self.interner } + fn evaluated_paths(&self) -> FxHashSet { + self.evaluated_paths.read().unwrap().clone() + } + fn push_diagnostic(&self, error: Error) { self.push_message(error); } @@ -588,9 +618,13 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { } } - fn handle_file(&self, path: &Path) { + fn handle_path(&self, path: &Path) { handle_file(self, path) } + + fn store_path(&self, path: &Path) { + self.evaluated_paths.write().unwrap().insert(path.into()); + } } /// This function wraps the [process_file] function implementing the traversal @@ -599,7 +633,7 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { fn handle_file(ctx: &TraversalOptions, path: &Path) { match catch_unwind(move || process_file(ctx, path)) { Ok(Ok(FileStatus::Changed)) => { - ctx.increment_changed(); + ctx.increment_changed(path); } Ok(Ok(FileStatus::Unchanged)) => { ctx.increment_unchanged(); diff --git a/crates/biome_cli/src/reporter/mod.rs b/crates/biome_cli/src/reporter/mod.rs index 6318b1a88740..a4660ba69660 100644 --- a/crates/biome_cli/src/reporter/mod.rs +++ b/crates/biome_cli/src/reporter/mod.rs @@ -6,6 +6,8 @@ pub(crate) mod terminal; use crate::execute::Execution; use biome_diagnostics::{Error, Severity}; +use biome_fs::EvaluatedPath; +use rustc_hash::FxHashSet; use serde::Serialize; use std::io; use std::time::Duration; @@ -47,6 +49,15 @@ pub trait ReporterVisitor { summary: TraversalSummary, ) -> io::Result<()>; + /// Writes the paths that were handled during a run. + fn report_handled_paths( + &mut self, + evaluated_paths: FxHashSet, + ) -> io::Result<()> { + let _ = evaluated_paths; + Ok(()) + } + /// Writes a diagnostics fn report_diagnostics( &mut self, diff --git a/crates/biome_cli/src/reporter/terminal.rs b/crates/biome_cli/src/reporter/terminal.rs index 3bb731ed94c8..220748e45b01 100644 --- a/crates/biome_cli/src/reporter/terminal.rs +++ b/crates/biome_cli/src/reporter/terminal.rs @@ -3,7 +3,10 @@ use crate::reporter::{DiagnosticsPayload, ReporterVisitor, TraversalSummary}; use crate::Reporter; use biome_console::fmt::Formatter; use biome_console::{fmt, markup, Console, ConsoleExt}; -use biome_diagnostics::PrintDiagnostic; +use biome_diagnostics::advice::ListAdvice; +use biome_diagnostics::{Diagnostic, PrintDiagnostic}; +use biome_fs::EvaluatedPath; +use rustc_hash::FxHashSet; use std::io; use std::time::Duration; @@ -11,15 +14,43 @@ pub(crate) struct ConsoleReporter { pub(crate) summary: TraversalSummary, pub(crate) diagnostics_payload: DiagnosticsPayload, pub(crate) execution: Execution, + pub(crate) evaluated_paths: FxHashSet, } impl Reporter for ConsoleReporter { fn write(self, visitor: &mut dyn ReporterVisitor) -> io::Result<()> { + let verbose = self.diagnostics_payload.verbose; visitor.report_diagnostics(&self.execution, self.diagnostics_payload)?; visitor.report_summary(&self.execution, self.summary)?; + if verbose { + visitor.report_handled_paths(self.evaluated_paths)?; + } Ok(()) } } + +#[derive(Debug, Diagnostic)] +#[diagnostic( + tags(VERBOSE), + severity = Information, + message = "Files processed:" +)] +struct EvaluatedPathsDiagnostic { + #[advice] + list: ListAdvice, +} + +#[derive(Debug, Diagnostic)] +#[diagnostic( + tags(VERBOSE), + severity = Information, + message = "Files fixed:" +)] +struct FixedPathsDiagnostic { + #[advice] + list: ListAdvice, +} + pub(crate) struct ConsoleReporterVisitor<'a>(pub(crate) &'a mut dyn Console); impl<'a> ReporterVisitor for ConsoleReporterVisitor<'a> { @@ -49,6 +80,39 @@ impl<'a> ReporterVisitor for ConsoleReporterVisitor<'a> { Ok(()) } + fn report_handled_paths( + &mut self, + evaluated_paths: FxHashSet, + ) -> io::Result<()> { + let evaluated_paths_diagnostic = EvaluatedPathsDiagnostic { + list: ListAdvice { + list: evaluated_paths + .iter() + .map(|p| p.as_ref().display().to_string()) + .collect(), + }, + }; + + let fixed_paths_diagnostic = FixedPathsDiagnostic { + list: ListAdvice { + list: evaluated_paths + .iter() + .filter(|p| p.is_fixed()) + .map(|p| p.as_ref().display().to_string()) + .collect(), + }, + }; + + self.0.log(markup! { + {PrintDiagnostic::verbose(&evaluated_paths_diagnostic)} + }); + self.0.log(markup! { + {PrintDiagnostic::verbose(&fixed_paths_diagnostic)} + }); + + Ok(()) + } + fn report_diagnostics( &mut self, execution: &Execution, diff --git a/crates/biome_cli/tests/snapshots/main_cases_diagnostics/max_diagnostics_verbose.snap b/crates/biome_cli/tests/snapshots/main_cases_diagnostics/max_diagnostics_verbose.snap index b7672740e982..58520ca8cf30 100644 --- a/crates/biome_cli/tests/snapshots/main_cases_diagnostics/max_diagnostics_verbose.snap +++ b/crates/biome_cli/tests/snapshots/main_cases_diagnostics/max_diagnostics_verbose.snap @@ -67,18 +67,6 @@ src/folder_6/package-lock.json project VERBOSE ━━━━━━━━━━ i You can hide this diagnostic by using --diagnostic-level=warn to increase the diagnostic level shown by CLI. -``` - -```block -src/file.js format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - × File content differs from formatting output - - 1 │ - ··statement(··)·· - 1 │ + statement(); - 2 │ + - - ``` ```block @@ -127,9 +115,41 @@ src/folder_0/package-lock.json project VERBOSE ━━━━━━━━━━ i You can hide this diagnostic by using --diagnostic-level=warn to increase the diagnostic level shown by CLI. +``` + +```block +src/file.js format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × File content differs from formatting output + + 1 │ - ··statement(··)·· + 1 │ + statement(); + 2 │ + + + ``` ```block Checked 1 file in