From 3c95fedf22eb795171664915020bca19beadc487 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 30 Jan 2024 15:28:17 +0000 Subject: [PATCH 1/7] refactor(cli): traversal --- Cargo.lock | 1 + crates/biome_cli/src/execute/process_file.rs | 163 +++++++++++-------- crates/biome_cli/src/execute/traverse.rs | 20 ++- crates/biome_fs/Cargo.toml | 1 + crates/biome_fs/src/fs.rs | 12 ++ crates/biome_fs/src/fs/memory.rs | 16 +- crates/biome_fs/src/fs/os.rs | 20 ++- crates/biome_service/src/workspace.rs | 23 +++ 8 files changed, 181 insertions(+), 75 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ea389f85ee2..be7b56fca863 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -454,6 +454,7 @@ version = "0.5.7" dependencies = [ "biome_diagnostics", "crossbeam", + "dashmap", "directories", "indexmap 2.2.6", "oxc_resolver", diff --git a/crates/biome_cli/src/execute/process_file.rs b/crates/biome_cli/src/execute/process_file.rs index 8d0b67338f81..a4aafc2958e9 100644 --- a/crates/biome_cli/src/execute/process_file.rs +++ b/crates/biome_cli/src/execute/process_file.rs @@ -5,7 +5,7 @@ mod organize_imports; mod search; pub(crate) mod workspace_file; -use crate::execute::diagnostics::{ResultExt, UnhandledDiagnostic}; +use crate::execute::diagnostics::{ResultExt, ResultIoExt, UnhandledDiagnostic}; use crate::execute::traverse::TraversalOptions; use crate::execute::TraversalMode; use biome_diagnostics::{category, DiagnosticExt, DiagnosticTags, Error}; @@ -18,9 +18,9 @@ use search::search; use std::marker::PhantomData; use std::ops::Deref; use std::path::Path; - #[derive(Debug)] pub(crate) enum FileStatus { + Stored, /// File changed and it was a success Changed, /// File unchanged, and it was a success @@ -84,7 +84,7 @@ where } } -/// The return type for [process_file], with the following semantics: +/// The return type for [store_file], with the following semantics: /// - `Ok(Success)` means the operation was successful (the file is added to /// the `processed` counter) /// - `Ok(Message(_))` means the operation was successful but a message still @@ -124,7 +124,7 @@ impl<'ctx, 'app> Deref for SharedTraversalOptions<'ctx, 'app> { /// diagnostics were emitted, or compare the formatted code with the original /// content of the file and emit a diff or write the new content to the disk if /// write mode is enabled -pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { +pub(crate) fn store_file(ctx: &TraversalOptions, path: &Path) -> FileResult { tracing::trace_span!("process_file", path = ?path).in_scope(move || { let biome_path = BiomePath::new(path); let file_features = ctx @@ -150,8 +150,8 @@ 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) + TraversalMode::Check { .. } | TraversalMode::CI { .. } => file_features + .as_lint_support() .and_then(|support_kind| { if support_kind.is_not_enabled() { Some(support_kind) @@ -159,51 +159,16 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { 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 - .support_kind_for(&FeatureKind::Lint) - .and_then(|support_kind| { + .and(file_features.as_format_support().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) + .as_organize_imports_support() .and_then(|support_kind| { if support_kind.is_not_enabled() { Some(support_kind) @@ -212,8 +177,9 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { } }), ), - TraversalMode::Format { .. } => file_features.support_kind_for(&FeatureKind::Format), - TraversalMode::Lint { .. } => file_features.support_kind_for(&FeatureKind::Lint), + + TraversalMode::Format { .. } => file_features.as_format_support(), + TraversalMode::Lint { .. } => file_features.as_lint_support(), TraversalMode::Migrate { .. } => None, TraversalMode::Search { .. } => file_features.support_kind_for(&FeatureKind::Search), }; @@ -235,27 +201,96 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { }; } - let shared_context = &SharedTraversalOptions::new(ctx); + let rome_path = RomePath::new(path); + let open_options = OpenOptions::default() + .read(true) + .write(ctx.execution.requires_write_access()); + let mut file = ctx + .fs + .open_with_options(path, open_options) + .with_file_path(path.display().to_string())?; - match ctx.execution.traversal_mode { - TraversalMode::Lint { .. } => { - // the unsupported case should be handled already at this point - lint(shared_context, path) - } - TraversalMode::Format { .. } => { - // the unsupported case should be handled already at this point - format(shared_context, path) - } - TraversalMode::Check { .. } | TraversalMode::CI { .. } => { - check_file(shared_context, path, &file_features) - } - TraversalMode::Migrate { .. } => { - unreachable!("The migration should not be called for this file") - } + let mut input = String::new(); + file.read_to_string(&mut input) + .with_file_path(path.display().to_string())?; + + ctx.workspace + .open_file(OpenFileParams { + path: rome_path, + version: 0, + content: input.clone(), + language_hint: Language::default(), + }) + .with_file_path_and_code(path.display().to_string(), category!("internalError/fs"))?; + + Ok(FileStatus::Stored) + + // let shared_context = &SharedTraversalOptions::new(ctx); + // + // ctx.increment_processed(); + // + // match ctx.execution.traversal_mode { + // TraversalMode::Lint { .. } => { + // // the unsupported case should be handled already at this point + // lint(shared_context, path) + // } + // TraversalMode::Format { .. } => { + // // the unsupported case should be handled already at this point + // format(shared_context, path) + // } + // TraversalMode::Check { .. } => { + // check_file(shared_context, path, &file_features, category!("check")) + // } + // TraversalMode::CI { .. } => { + // check_file(shared_context, path, &file_features, category!("ci")) + // } + // TraversalMode::Migrate { .. } => { + // unreachable!("The migration should not be called for this file") + // } + // } + }) +} + + +pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { + let rome_path = RomePath::new(path); + let file_features = ctx + .workspace + .file_features(SupportsFeatureParams { + path: rome_path, + feature: FeaturesBuilder::new() + .with_formatter() + .with_linter() + .with_organize_imports() + .build(), + }) + .with_file_path_and_code_and_tags( + path.display().to_string(), + category!("files/missingHandler"), + DiagnosticTags::VERBOSE, + )?; + let shared_context = &SharedTraversalOptions::new(ctx); + + ctx.increment_processed(); + + match ctx.execution.traversal_mode { + TraversalMode::Lint { .. } => { + // the unsupported case should be handled already at this point + lint(shared_context, path) + } + TraversalMode::Format { .. } => { + // the unsupported case should be handled already at this point + format(shared_context, path) + } + TraversalMode::Check { .. } | + TraversalMode::CI { .. } => { + check_file(shared_context, path, &file_features) + } + TraversalMode::Migrate { .. } => { + unreachable!("The migration should not be called for this file")} TraversalMode::Search { ref pattern, .. } => { // the unsupported case should be handled already at this point search(shared_context, path, pattern) - } } - }) + } } diff --git a/crates/biome_cli/src/execute/traverse.rs b/crates/biome_cli/src/execute/traverse.rs index d08e1db75d2a..96d02e5d486d 100644 --- a/crates/biome_cli/src/execute/traverse.rs +++ b/crates/biome_cli/src/execute/traverse.rs @@ -1,4 +1,4 @@ -use super::process_file::{process_file, DiffKind, FileStatus, Message}; +use super::process_file::{store_file, DiffKind, FileStatus, Message, FileResult, process_file}; use super::{Execution, TraversalMode}; use crate::cli_options::CliOptions; use crate::execute::diagnostics::{ @@ -28,6 +28,7 @@ use std::{ thread, time::{Duration, Instant}, }; +use std::panic::UnwindSafe; pub(crate) fn traverse( execution: &Execution, @@ -162,6 +163,10 @@ fn traverse_inputs(fs: &dyn FileSystem, inputs: Vec, ctx: &TraversalOp } })); + fs.for_each_path(Box::new(move | path| { + ctx.handle_file(path) + })); + start.elapsed() } @@ -589,21 +594,26 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { } fn handle_file(&self, path: &Path) { - handle_file(self, path) + handle_file(self, path, process_file) + } + + fn store_file(&self, path: &Path) { + handle_file(self, path, store_file) } } -/// This function wraps the [process_file] function implementing the traversal +/// This function wraps the [store_file] function implementing the traversal /// in a [catch_unwind] block and emit diagnostics in case of error (either the /// traversal function returns Err or panics) -fn handle_file(ctx: &TraversalOptions, path: &Path) { - match catch_unwind(move || process_file(ctx, path)) { +fn handle_file(ctx: &TraversalOptions, path: &Path, func: F) where F: Fn(&TraversalOptions, &Path) -> FileResult + UnwindSafe { + match catch_unwind(move || func(ctx, path)) { Ok(Ok(FileStatus::Changed)) => { ctx.increment_changed(); } Ok(Ok(FileStatus::Unchanged)) => { ctx.increment_unchanged(); } + Ok(Ok(FileStatus::Stored)) => {} Ok(Ok(FileStatus::Message(msg))) => { ctx.increment_unchanged(); ctx.push_message(msg); diff --git a/crates/biome_fs/Cargo.toml b/crates/biome_fs/Cargo.toml index f790c4c4fe5d..4ff2ec0031ee 100644 --- a/crates/biome_fs/Cargo.toml +++ b/crates/biome_fs/Cargo.toml @@ -24,6 +24,7 @@ rustc-hash = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true } tracing = { workspace = true } +dashmap = { workspace = true} [features] serde = ["schemars", "biome_diagnostics/schema"] diff --git a/crates/biome_fs/src/fs.rs b/crates/biome_fs/src/fs.rs index bc453844ea43..5e244d7ca081 100644 --- a/crates/biome_fs/src/fs.rs +++ b/crates/biome_fs/src/fs.rs @@ -37,6 +37,9 @@ impl ConfigName { type AutoSearchResultAlias = Result, FileSystemDiagnostic>; +type ForEachPath<'fs, 'scope> = Box; + + pub trait FileSystem: Send + Sync + RefUnwindSafe { /// It opens a file with the given set of options fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result>; @@ -47,6 +50,9 @@ pub trait FileSystem: Send + Sync + RefUnwindSafe { /// efficiently batch many filesystem read operations fn traversal<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>); + /// TODO: add documentation + fn for_each_path(&self, func: ForEachPath); + // TODO: remove once we remove `rome.json` support [2.0] /// Returns the temporary configuration files that are supported fn deprecated_config_name(&self) -> &str { @@ -324,12 +330,18 @@ pub trait TraversalContext: Sync { /// This method will be called by the traversal for each file it finds /// where [TraversalContext::can_handle] returned true fn handle_file(&self, path: &Path); + + + fn store_file(&self, path: &Path); } impl FileSystem for Arc where T: FileSystem + Send, { + fn for_each_path(&self, func: ForEachPath) { + T::for_each_path(self, func) + } fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result> { T::open_with_options(self, path, options) } diff --git a/crates/biome_fs/src/fs/memory.rs b/crates/biome_fs/src/fs/memory.rs index 40e2f3709f1f..8c2534d56375 100644 --- a/crates/biome_fs/src/fs/memory.rs +++ b/crates/biome_fs/src/fs/memory.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use biome_diagnostics::{Error, Severity}; use parking_lot::{lock_api::ArcMutexGuard, Mutex, RawMutex, RwLock}; -use crate::fs::OpenOptions; +use crate::fs::{ForEachPath, OpenOptions}; use crate::{BiomePath, FileSystem, TraversalContext, TraversalScope}; use super::{BoxedTraversal, ErrorKind, File, FileSystemDiagnostic}; @@ -184,7 +184,6 @@ impl FileSystem for MemoryFileSystem { version: 0, })) } - fn traversal<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>) { func(&MemoryTraversalScope { fs: self }) } @@ -237,6 +236,15 @@ impl FileSystem for MemoryFileSystem { ) -> Result { todo!() } + + fn for_each_path(&self, func: ForEachPath) { + let files = self.files.0.read(); + + for file in files.keys() { + + func(file.as_path()) + } + } } struct MemoryFile { @@ -533,6 +541,10 @@ mod tests { fn handle_file(&self, path: &Path) { self.visited.lock().push(path.into()) } + + fn store_file(&self, path: &Path) { + self.visited.lock().push(path.into()) + } } let (interner, _) = PathInterner::new(); diff --git a/crates/biome_fs/src/fs/os.rs b/crates/biome_fs/src/fs/os.rs index f5a20e4bff12..dbd1c4e49c0e 100644 --- a/crates/biome_fs/src/fs/os.rs +++ b/crates/biome_fs/src/fs/os.rs @@ -1,5 +1,5 @@ //! Implementation of the [FileSystem] and related traits for the underlying OS filesystem -use super::{BoxedTraversal, ErrorKind, File, FileSystemDiagnostic}; +use super::{BoxedTraversal, ErrorKind, File, FileSystemDiagnostic, ForEachPath}; use crate::fs::OpenOptions; use crate::{ fs::{TraversalContext, TraversalScope}, @@ -53,6 +53,7 @@ impl Default for OsFileSystem { } } + impl FileSystem for OsFileSystem { fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result> { tracing::debug_span!("OsFileSystem::open_with_options", path = ?path, options = ?options) @@ -71,6 +72,16 @@ impl FileSystem for OsFileSystem { }) } + fn for_each_path(&self, func: ForEachPath) { + let paths = self.paths.0.read(); + let iter = paths.iter(); + for path in iter { + OsTraversalScope::with(|_| { + func(path.as_path()) + }) + } + } + fn working_directory(&self) -> Option { self.working_directory.clone() } @@ -345,9 +356,10 @@ fn handle_any_file<'scope>( } if file_type.is_file() { - scope.spawn(move |_| { - ctx.handle_file(&path); - }); + scope.spawn(move |_| { + ctx.handle_file(&path); + // ctx.store_file(&path); + }); return; } diff --git a/crates/biome_service/src/workspace.rs b/crates/biome_service/src/workspace.rs index 0a24fb787a18..979ecd9b0c06 100644 --- a/crates/biome_service/src/workspace.rs +++ b/crates/biome_service/src/workspace.rs @@ -272,6 +272,29 @@ impl FileFeaturesResult { } } + pub fn as_format_support(&self) -> Option<&SupportKind> { + self.support_kind_for(&FeatureName::Format) + } + + pub fn as_lint_support(&self) -> Option<&SupportKind> { + self.support_kind_for(&FeatureName::Lint) + } + pub fn as_organize_imports_support(&self) -> Option<&SupportKind> { + self.support_kind_for(&FeatureName::OrganizeImports) + } + + pub fn supports_lint(&self) -> bool { + self.supports_for(&FeatureName::Lint) + } + + pub fn supports_format(&self) -> bool { + self.supports_for(&FeatureName::Format) + } + + pub fn supports_organize_imports(&self) -> bool { + self.supports_for(&FeatureName::OrganizeImports) + } + pub fn support_kind_for(&self, feature: &FeatureKind) -> Option<&SupportKind> { self.features_supported.get(feature) } From cb5e47d8ec7afec9ba8f9fb0ee4f157e6ad2dbbe Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 20 May 2024 12:40:06 +0100 Subject: [PATCH 2/7] chore: use different functions instead --- crates/biome_cli/src/execute/process_file.rs | 142 +++++-------------- crates/biome_cli/src/execute/traverse.rs | 33 +++-- crates/biome_fs/src/fs.rs | 22 +-- crates/biome_fs/src/fs/memory.rs | 24 ++-- crates/biome_fs/src/fs/os.rs | 37 +++-- crates/biome_service/src/workspace.rs | 16 +-- crates/biome_service/src/workspace/server.rs | 4 +- 7 files changed, 118 insertions(+), 160 deletions(-) diff --git a/crates/biome_cli/src/execute/process_file.rs b/crates/biome_cli/src/execute/process_file.rs index a4aafc2958e9..aa2056dfeafa 100644 --- a/crates/biome_cli/src/execute/process_file.rs +++ b/crates/biome_cli/src/execute/process_file.rs @@ -9,8 +9,10 @@ use crate::execute::diagnostics::{ResultExt, ResultIoExt, UnhandledDiagnostic}; use crate::execute::traverse::TraversalOptions; use crate::execute::TraversalMode; use biome_diagnostics::{category, DiagnosticExt, DiagnosticTags, Error}; -use biome_fs::BiomePath; -use biome_service::workspace::{FeatureKind, SupportKind, SupportsFeatureParams}; +use biome_fs::{BiomePath, OpenOptions}; +use biome_service::workspace::{ + FeatureKind, FeaturesBuilder, OpenFileParams, SupportKind, SupportsFeatureParams, +}; use check::check_file; use format::format; use lint::lint; @@ -84,7 +86,7 @@ where } } -/// The return type for [store_file], with the following semantics: +/// The return type for [process_file], with the following semantics: /// - `Ok(Success)` means the operation was successful (the file is added to /// the `processed` counter) /// - `Ok(Message(_))` means the operation was successful but a message still @@ -124,7 +126,7 @@ impl<'ctx, 'app> Deref for SharedTraversalOptions<'ctx, 'app> { /// diagnostics were emitted, or compare the formatted code with the original /// content of the file and emit a diff or write the new content to the disk if /// write mode is enabled -pub(crate) fn store_file(ctx: &TraversalOptions, path: &Path) -> FileResult { +pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { tracing::trace_span!("process_file", path = ?path).in_scope(move || { let biome_path = BiomePath::new(path); let file_features = ctx @@ -151,7 +153,7 @@ pub(crate) fn store_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 { .. } | TraversalMode::CI { .. } => file_features - .as_lint_support() + .support_kind_for(&FeatureName::Lint) .and_then(|support_kind| { if support_kind.is_not_enabled() { Some(support_kind) @@ -159,16 +161,20 @@ pub(crate) fn store_file(ctx: &TraversalOptions, path: &Path) -> FileResult { None } }) - .and(file_features.as_format_support().and_then(|support_kind| { - if support_kind.is_not_enabled() { - Some(support_kind) - } else { - None - } - })) .and( file_features - .as_organize_imports_support() + .support_kind_for(&FeatureName::Format) + .and_then(|support_kind| { + if support_kind.is_not_enabled() { + Some(support_kind) + } else { + None + } + }), + ) + .and( + file_features + .support_kind_for(&FeatureName::OrganizeImports) .and_then(|support_kind| { if support_kind.is_not_enabled() { Some(support_kind) @@ -177,9 +183,8 @@ pub(crate) fn store_file(ctx: &TraversalOptions, path: &Path) -> FileResult { } }), ), - - TraversalMode::Format { .. } => file_features.as_format_support(), - TraversalMode::Lint { .. } => file_features.as_lint_support(), + TraversalMode::Format { .. } => file_features.support_kind_for(&FeatureName::Format), + TraversalMode::Lint { .. } => file_features.support_kind_for(&FeatureName::Lint), TraversalMode::Migrate { .. } => None, TraversalMode::Search { .. } => file_features.support_kind_for(&FeatureKind::Search), }; @@ -201,96 +206,27 @@ pub(crate) fn store_file(ctx: &TraversalOptions, path: &Path) -> FileResult { }; } - let rome_path = RomePath::new(path); - let open_options = OpenOptions::default() - .read(true) - .write(ctx.execution.requires_write_access()); - let mut file = ctx - .fs - .open_with_options(path, open_options) - .with_file_path(path.display().to_string())?; - - let mut input = String::new(); - file.read_to_string(&mut input) - .with_file_path(path.display().to_string())?; + let shared_context = &SharedTraversalOptions::new(ctx); - ctx.workspace - .open_file(OpenFileParams { - path: rome_path, - version: 0, - content: input.clone(), - language_hint: Language::default(), - }) - .with_file_path_and_code(path.display().to_string(), category!("internalError/fs"))?; - - Ok(FileStatus::Stored) - - // let shared_context = &SharedTraversalOptions::new(ctx); - // - // ctx.increment_processed(); - // - // match ctx.execution.traversal_mode { - // TraversalMode::Lint { .. } => { - // // the unsupported case should be handled already at this point - // lint(shared_context, path) - // } - // TraversalMode::Format { .. } => { - // // the unsupported case should be handled already at this point - // format(shared_context, path) - // } - // TraversalMode::Check { .. } => { - // check_file(shared_context, path, &file_features, category!("check")) - // } - // TraversalMode::CI { .. } => { - // check_file(shared_context, path, &file_features, category!("ci")) - // } - // TraversalMode::Migrate { .. } => { - // unreachable!("The migration should not be called for this file") - // } - // } - }) -} - - -pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { - let rome_path = RomePath::new(path); - let file_features = ctx - .workspace - .file_features(SupportsFeatureParams { - path: rome_path, - feature: FeaturesBuilder::new() - .with_formatter() - .with_linter() - .with_organize_imports() - .build(), - }) - .with_file_path_and_code_and_tags( - path.display().to_string(), - category!("files/missingHandler"), - DiagnosticTags::VERBOSE, - )?; - let shared_context = &SharedTraversalOptions::new(ctx); - - ctx.increment_processed(); - - match ctx.execution.traversal_mode { - TraversalMode::Lint { .. } => { - // the unsupported case should be handled already at this point - lint(shared_context, path) - } - TraversalMode::Format { .. } => { - // the unsupported case should be handled already at this point - format(shared_context, path) - } - TraversalMode::Check { .. } | - TraversalMode::CI { .. } => { - check_file(shared_context, path, &file_features) - } - TraversalMode::Migrate { .. } => { - unreachable!("The migration should not be called for this file")} + match ctx.execution.traversal_mode { + TraversalMode::Lint { .. } => { + // the unsupported case should be handled already at this point + lint(shared_context, path) + } + TraversalMode::Format { .. } => { + // the unsupported case should be handled already at this point + format(shared_context, path) + } + TraversalMode::Check { .. } | TraversalMode::CI { .. } => { + check_file(shared_context, path, &file_features) + } + TraversalMode::Migrate { .. } => { + unreachable!("The migration should not be called for this file") + } TraversalMode::Search { ref pattern, .. } => { // the unsupported case should be handled already at this point search(shared_context, path, pattern) + } } - } + }) } diff --git a/crates/biome_cli/src/execute/traverse.rs b/crates/biome_cli/src/execute/traverse.rs index 96d02e5d486d..874424f26e89 100644 --- a/crates/biome_cli/src/execute/traverse.rs +++ b/crates/biome_cli/src/execute/traverse.rs @@ -1,4 +1,4 @@ -use super::process_file::{store_file, DiffKind, FileStatus, Message, FileResult, process_file}; +use super::process_file::{process_file, process_file, DiffKind, FileResult, FileStatus, Message}; use super::{Execution, TraversalMode}; use crate::cli_options::CliOptions; use crate::execute::diagnostics::{ @@ -15,7 +15,9 @@ 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::panic::UnwindSafe; use std::sync::atomic::AtomicU32; +use std::sync::RwLock; use std::{ env::current_dir, ffi::OsString, @@ -28,7 +30,6 @@ use std::{ thread, time::{Duration, Instant}, }; -use std::panic::UnwindSafe; pub(crate) fn traverse( execution: &Execution, @@ -103,6 +104,7 @@ pub(crate) fn traverse( skipped: &skipped, messages: sender, remaining_diagnostics: &remaining_diagnostics, + paths: RwLock::default(), }, ); // wait for the main thread to finish @@ -163,8 +165,11 @@ fn traverse_inputs(fs: &dyn FileSystem, inputs: Vec, ctx: &TraversalOp } })); - fs.for_each_path(Box::new(move | path| { - ctx.handle_file(path) + let paths = ctx.paths(); + fs.check(Box::new(move |scope: &dyn TraversalScope| { + for path in paths { + scope.handle(ctx, path.to_path_buf()); + } })); start.elapsed() @@ -488,6 +493,9 @@ 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) paths: RwLock>, } impl<'ctx, 'app> TraversalOptions<'ctx, 'app> { @@ -523,6 +531,10 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { &self.interner } + fn paths(&self) -> &[PathBuf] { + self.paths.read().unwrap().as_slice() + } + fn push_diagnostic(&self, error: Error) { self.push_message(error); } @@ -593,19 +605,22 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { } } - fn handle_file(&self, path: &Path) { + fn handle_path(&self, path: &Path) { handle_file(self, path, process_file) } - fn store_file(&self, path: &Path) { - handle_file(self, path, store_file) + fn store_path(&self, path: &Path) { + self.paths.write().unwrap().push(path.to_path_buf()) } } -/// This function wraps the [store_file] function implementing the traversal +/// This function wraps the [process_file] function implementing the traversal /// in a [catch_unwind] block and emit diagnostics in case of error (either the /// traversal function returns Err or panics) -fn handle_file(ctx: &TraversalOptions, path: &Path, func: F) where F: Fn(&TraversalOptions, &Path) -> FileResult + UnwindSafe { +fn handle_file(ctx: &TraversalOptions, path: &Path, func: F) +where + F: Fn(&TraversalOptions, &Path) -> FileResult + UnwindSafe, +{ match catch_unwind(move || func(ctx, path)) { Ok(Ok(FileStatus::Changed)) => { ctx.increment_changed(); diff --git a/crates/biome_fs/src/fs.rs b/crates/biome_fs/src/fs.rs index 5e244d7ca081..505b402d053e 100644 --- a/crates/biome_fs/src/fs.rs +++ b/crates/biome_fs/src/fs.rs @@ -37,9 +37,6 @@ impl ConfigName { type AutoSearchResultAlias = Result, FileSystemDiagnostic>; -type ForEachPath<'fs, 'scope> = Box; - - pub trait FileSystem: Send + Sync + RefUnwindSafe { /// It opens a file with the given set of options fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result>; @@ -51,7 +48,7 @@ pub trait FileSystem: Send + Sync + RefUnwindSafe { fn traversal<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>); /// TODO: add documentation - fn for_each_path(&self, func: ForEachPath); + fn check<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>); // TODO: remove once we remove `rome.json` support [2.0] /// Returns the temporary configuration files that are supported @@ -305,12 +302,14 @@ type BoxedTraversal<'fs, 'scope> = Box) + pub trait TraversalScope<'scope> { /// Spawn a new filesystem read task /// - /// If the provided path exists and is a file, then the [`handle_file`](TraversalContext::handle_file) + /// If the provided path exists and is a file, then the [`handle_file`](TraversalContext::handle_path) /// method of the provided [TraversalContext] will be called. If it's a /// directory, it will be recursively traversed and all the files the /// [`can_handle`](TraversalContext::can_handle) method of the context /// returns true for will be handled as well fn spawn(&self, context: &'scope dyn TraversalContext, path: PathBuf); + + fn handle(&self, context: &'scope dyn TraversalContext, path: PathBuf); } pub trait TraversalContext: Sync { @@ -329,19 +328,24 @@ pub trait TraversalContext: Sync { /// This method will be called by the traversal for each file it finds /// where [TraversalContext::can_handle] returned true - fn handle_file(&self, path: &Path); + fn handle_path(&self, path: &Path); + /// This method will be called by the traversal for each file it finds + /// where [TraversalContext::store_path] returned true + fn store_path(&self, path: &Path); - fn store_file(&self, path: &Path); + /// Returns the paths that should be handled + fn paths(&self) -> Vec; } impl FileSystem for Arc where T: FileSystem + Send, { - fn for_each_path(&self, func: ForEachPath) { - T::for_each_path(self, func) + fn check<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>) { + T::check(self, func) } + fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result> { T::open_with_options(self, path, options) } diff --git a/crates/biome_fs/src/fs/memory.rs b/crates/biome_fs/src/fs/memory.rs index 8c2534d56375..5b266affe877 100644 --- a/crates/biome_fs/src/fs/memory.rs +++ b/crates/biome_fs/src/fs/memory.rs @@ -237,13 +237,8 @@ impl FileSystem for MemoryFileSystem { todo!() } - fn for_each_path(&self, func: ForEachPath) { - let files = self.files.0.read(); - - for file in files.keys() { - - func(file.as_path()) - } + fn check<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>) { + func(&MemoryTraversalScope { fs: self }) } } @@ -319,7 +314,7 @@ impl<'scope> TraversalScope<'scope> for MemoryTraversalScope<'scope> { if !ctx.can_handle(&biome_path) { continue; } - ctx.handle_file(path); + ctx.store_path(path); } } } @@ -344,6 +339,10 @@ impl<'scope> TraversalScope<'scope> for MemoryTraversalScope<'scope> { } } } + + fn handle(&self, context: &'scope dyn TraversalContext, path: PathBuf) { + context.handle_path(&path); + } } #[cfg(test)] @@ -538,13 +537,18 @@ mod tests { true } - fn handle_file(&self, path: &Path) { + fn handle_path(&self, path: &Path) { self.visited.lock().push(path.into()) } - fn store_file(&self, path: &Path) { + fn store_path(&self, path: &Path) { self.visited.lock().push(path.into()) } + + fn paths(&self) -> Vec { + let lock = self.visited.lock(); + lock.to_vec() + } } let (interner, _) = PathInterner::new(); diff --git a/crates/biome_fs/src/fs/os.rs b/crates/biome_fs/src/fs/os.rs index dbd1c4e49c0e..62247e2eb837 100644 --- a/crates/biome_fs/src/fs/os.rs +++ b/crates/biome_fs/src/fs/os.rs @@ -53,7 +53,6 @@ impl Default for OsFileSystem { } } - impl FileSystem for OsFileSystem { fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result> { tracing::debug_span!("OsFileSystem::open_with_options", path = ?path, options = ?options) @@ -72,16 +71,22 @@ impl FileSystem for OsFileSystem { }) } - fn for_each_path(&self, func: ForEachPath) { - let paths = self.paths.0.read(); - let iter = paths.iter(); - for path in iter { - OsTraversalScope::with(|_| { - func(path.as_path()) - }) - } + fn check(&self, func: BoxedTraversal) { + OsTraversalScope::with(move |scope| { + func(scope); + }) } + // fn for_each_path(&self, func: ForEachPath) { + // let paths = self.paths.0.read(); + // let iter = paths.iter(); + // for path in iter { + // OsTraversalScope::with(|_| { + // func(path.as_path()) + // }) + // } + // } + fn working_directory(&self) -> Option { self.working_directory.clone() } @@ -220,6 +225,12 @@ impl<'scope> TraversalScope<'scope> for OsTraversalScope<'scope> { }; handle_any_file(&self.scope, ctx, path, file_type, None); } + + fn handle(&self, context: &'scope dyn TraversalContext, path: PathBuf) { + self.scope.spawn(move |_| { + context.handle_path(&path); + }); + } } // TODO: remove in Biome 2.0, and directly use `.gitignore` @@ -356,10 +367,10 @@ fn handle_any_file<'scope>( } if file_type.is_file() { - scope.spawn(move |_| { - ctx.handle_file(&path); - // ctx.store_file(&path); - }); + scope.spawn(move |_| { + ctx.handle_path(&path); + // ctx.store_file(&path); + }); return; } diff --git a/crates/biome_service/src/workspace.rs b/crates/biome_service/src/workspace.rs index 979ecd9b0c06..b11a6cba727e 100644 --- a/crates/biome_service/src/workspace.rs +++ b/crates/biome_service/src/workspace.rs @@ -283,18 +283,6 @@ impl FileFeaturesResult { self.support_kind_for(&FeatureName::OrganizeImports) } - pub fn supports_lint(&self) -> bool { - self.supports_for(&FeatureName::Lint) - } - - pub fn supports_format(&self) -> bool { - self.supports_for(&FeatureName::Format) - } - - pub fn supports_organize_imports(&self) -> bool { - self.supports_for(&FeatureName::OrganizeImports) - } - pub fn support_kind_for(&self, feature: &FeatureKind) -> Option<&SupportKind> { self.features_supported.get(feature) } @@ -320,14 +308,14 @@ impl FileFeaturesResult { .all(|support_kind| support_kind.is_protected()) } - /// The file is not supported if all the featured marked it as not supported + /// The file is not supported if all the features are unsupported pub fn is_not_supported(&self) -> bool { self.features_supported .values() .all(|support_kind| support_kind.is_not_supported()) } - /// The file is not enabled if all the features marked it as not enabled + /// The file is not enabled if all the features aren't enabled pub fn is_not_enabled(&self) -> bool { self.features_supported .values() diff --git a/crates/biome_service/src/workspace/server.rs b/crates/biome_service/src/workspace/server.rs index cd74e67b3450..bf4c23fe4ebe 100644 --- a/crates/biome_service/src/workspace/server.rs +++ b/crates/biome_service/src/workspace/server.rs @@ -390,8 +390,8 @@ impl Workspace for WorkspaceServer { } } - // If the file is not ignored by at least one feature, - // then check that the file is not protected. + // If the file is not ignored by at least one feature, then check that the file is not protected. + // // Protected files must be ignored. if !file_features.is_not_processed() && FileFeaturesResult::is_protected_file(path) { file_features.set_protected_for_all_features(); From 4e048870bc0784705369a74d49b36435770957cf Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 1 Aug 2024 11:33:03 +0100 Subject: [PATCH 3/7] refactor(cli): double traversal --- CHANGELOG.md | 23 ++++++- crates/biome_cli/src/execute/mod.rs | 5 +- crates/biome_cli/src/execute/process_file.rs | 19 +++--- crates/biome_cli/src/execute/traverse.rs | 57 ++++++++++------ crates/biome_cli/src/reporter/mod.rs | 12 ++++ crates/biome_cli/src/reporter/terminal.rs | 66 ++++++++++++++++++- .../max_diagnostics_verbose.snap | 24 +++---- crates/biome_diagnostics/src/advice.rs | 22 +++++++ crates/biome_fs/src/fs.rs | 23 +++---- crates/biome_fs/src/fs/memory.rs | 14 ++-- crates/biome_fs/src/fs/os.rs | 13 +--- crates/biome_service/src/workspace.rs | 11 ---- 12 files changed, 203 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86ba6618d143..6e7128491caa 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 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ℹ List of files that were evaluated + + - biome/biome.json + - biome/packages/@biomejs/cli-win32-arm64/package.json + - biome/packages/tailwindcss-config-analyzer/package.json + + VERBOSE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ℹ List of files that were 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..68e3186f0119 100644 --- a/crates/biome_cli/src/execute/mod.rs +++ b/crates/biome_cli/src/execute/mod.rs @@ -431,7 +431,8 @@ pub fn execute_mode( }; migrate::run(payload) } else { - let (summary_result, diagnostics) = traverse(&execution, &mut session, cli_options, paths)?; + let (summary_result, evaluated_paths, fixed_paths, diagnostics) = + traverse(&execution, &mut session, cli_options, paths)?; let console = session.app.console; let errors = summary_result.errors; let skipped = summary_result.skipped; @@ -460,6 +461,8 @@ pub fn execute_mode( diagnostics, }, execution: execution.clone(), + evaluated_paths, + fixed_paths, }; reporter.write(&mut ConsoleReporterVisitor(console))?; } diff --git a/crates/biome_cli/src/execute/process_file.rs b/crates/biome_cli/src/execute/process_file.rs index aa2056dfeafa..ba7ed6467958 100644 --- a/crates/biome_cli/src/execute/process_file.rs +++ b/crates/biome_cli/src/execute/process_file.rs @@ -5,14 +5,12 @@ mod organize_imports; mod search; pub(crate) mod workspace_file; -use crate::execute::diagnostics::{ResultExt, ResultIoExt, UnhandledDiagnostic}; +use crate::execute::diagnostics::{ResultExt, UnhandledDiagnostic}; use crate::execute::traverse::TraversalOptions; use crate::execute::TraversalMode; use biome_diagnostics::{category, DiagnosticExt, DiagnosticTags, Error}; -use biome_fs::{BiomePath, OpenOptions}; -use biome_service::workspace::{ - FeatureKind, FeaturesBuilder, OpenFileParams, SupportKind, SupportsFeatureParams, -}; +use biome_fs::BiomePath; +use biome_service::workspace::{FeatureKind, SupportKind, SupportsFeatureParams}; use check::check_file; use format::format; use lint::lint; @@ -22,7 +20,6 @@ use std::ops::Deref; use std::path::Path; #[derive(Debug)] pub(crate) enum FileStatus { - Stored, /// File changed and it was a success Changed, /// File unchanged, and it was a success @@ -153,7 +150,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 { .. } | TraversalMode::CI { .. } => file_features - .support_kind_for(&FeatureName::Lint) + .support_kind_for(&FeatureKind::Lint) .and_then(|support_kind| { if support_kind.is_not_enabled() { Some(support_kind) @@ -163,7 +160,7 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { }) .and( file_features - .support_kind_for(&FeatureName::Format) + .support_kind_for(&FeatureKind::Format) .and_then(|support_kind| { if support_kind.is_not_enabled() { Some(support_kind) @@ -174,7 +171,7 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { ) .and( file_features - .support_kind_for(&FeatureName::OrganizeImports) + .support_kind_for(&FeatureKind::OrganizeImports) .and_then(|support_kind| { if support_kind.is_not_enabled() { Some(support_kind) @@ -183,8 +180,8 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { } }), ), - TraversalMode::Format { .. } => file_features.support_kind_for(&FeatureName::Format), - TraversalMode::Lint { .. } => file_features.support_kind_for(&FeatureName::Lint), + TraversalMode::Format { .. } => file_features.support_kind_for(&FeatureKind::Format), + TraversalMode::Lint { .. } => file_features.support_kind_for(&FeatureKind::Lint), TraversalMode::Migrate { .. } => None, TraversalMode::Search { .. } => file_features.support_kind_for(&FeatureKind::Search), }; diff --git a/crates/biome_cli/src/execute/traverse.rs b/crates/biome_cli/src/execute/traverse.rs index 874424f26e89..5b40e3361232 100644 --- a/crates/biome_cli/src/execute/traverse.rs +++ b/crates/biome_cli/src/execute/traverse.rs @@ -1,4 +1,4 @@ -use super::process_file::{process_file, process_file, DiffKind, FileResult, FileStatus, Message}; +use super::process_file::{process_file, DiffKind, FileResult, FileStatus, Message}; use super::{Execution, TraversalMode}; use crate::cli_options::CliOptions; use crate::execute::diagnostics::{ @@ -36,7 +36,7 @@ pub(crate) fn traverse( session: &mut CliSession, cli_options: &CliOptions, mut inputs: Vec, -) -> Result<(TraversalSummary, Vec), CliDiagnostic> { +) -> Result<(TraversalSummary, Vec, Vec, Vec), CliDiagnostic> { init_thread_pool(); if inputs.is_empty() { @@ -83,7 +83,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, fixed_paths, diagnostics) = thread::scope(|s| { let handler = thread::Builder::new() .name(String::from("biome::console")) .spawn_scoped(s, || printer.run(receiver, recv_files)) @@ -91,7 +91,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, fixed_paths) = traverse_inputs( fs, inputs, &TraversalOptions { @@ -104,13 +104,14 @@ pub(crate) fn traverse( skipped: &skipped, messages: sender, remaining_diagnostics: &remaining_diagnostics, - paths: RwLock::default(), + evaluated_paths: RwLock::default(), + fixed_paths: RwLock::default(), }, ); // wait for the main thread to finish let diagnostics = handler.join().unwrap(); - (elapsed, diagnostics) + (elapsed, evaluated_paths, fixed_paths, diagnostics) }); // Make sure patterns are always cleaned up at the end of traversal. @@ -138,6 +139,8 @@ pub(crate) fn traverse( suggested_fixes_skipped, diagnostics_not_printed, }, + evaluated_paths, + fixed_paths, diagnostics, )) } @@ -157,22 +160,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, Vec, Vec) { 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.paths(); - fs.check(Box::new(move |scope: &dyn TraversalScope| { - for path in paths { + 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, ctx.fixed_paths()) } // struct DiagnosticsReporter<'ctx> {} @@ -495,12 +503,17 @@ pub(crate) struct TraversalOptions<'ctx, 'app> { pub(crate) remaining_diagnostics: &'ctx AtomicU32, /// List of paths that should be processed - pub(crate) paths: RwLock>, + pub(crate) evaluated_paths: RwLock>, + + /// List of paths that were fixed by Biome + pub(crate) fixed_paths: RwLock>, } impl<'ctx, 'app> TraversalOptions<'ctx, 'app> { - pub(crate) fn increment_changed(&self) { + pub(crate) fn increment_changed(&self, path: impl Into) { self.changed.fetch_add(1, Ordering::Relaxed); + let mut fixed_paths = self.fixed_paths.write().unwrap(); + fixed_paths.push(path.into()); } pub(crate) fn increment_unchanged(&self) { self.unchanged.fetch_add(1, Ordering::Relaxed); @@ -531,8 +544,12 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { &self.interner } - fn paths(&self) -> &[PathBuf] { - self.paths.read().unwrap().as_slice() + fn evaluated_paths(&self) -> Vec { + self.evaluated_paths.read().unwrap().clone() + } + + fn fixed_paths(&self) -> Vec { + self.fixed_paths.read().unwrap().clone() } fn push_diagnostic(&self, error: Error) { @@ -610,7 +627,10 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { } fn store_path(&self, path: &Path) { - self.paths.write().unwrap().push(path.to_path_buf()) + self.evaluated_paths + .write() + .unwrap() + .push(path.to_path_buf()) } } @@ -623,12 +643,11 @@ where { match catch_unwind(move || func(ctx, path)) { Ok(Ok(FileStatus::Changed)) => { - ctx.increment_changed(); + ctx.increment_changed(path); } Ok(Ok(FileStatus::Unchanged)) => { ctx.increment_unchanged(); } - Ok(Ok(FileStatus::Stored)) => {} Ok(Ok(FileStatus::Message(msg))) => { ctx.increment_unchanged(); ctx.push_message(msg); diff --git a/crates/biome_cli/src/reporter/mod.rs b/crates/biome_cli/src/reporter/mod.rs index 6318b1a88740..0af614c0e3a1 100644 --- a/crates/biome_cli/src/reporter/mod.rs +++ b/crates/biome_cli/src/reporter/mod.rs @@ -8,6 +8,7 @@ use crate::execute::Execution; use biome_diagnostics::{Error, Severity}; use serde::Serialize; use std::io; +use std::path::PathBuf; use std::time::Duration; pub struct DiagnosticsPayload { @@ -47,6 +48,17 @@ pub trait ReporterVisitor { summary: TraversalSummary, ) -> io::Result<()>; + /// Writes the paths that were handled during a run. + /// The fist list represents the paths that were evaluated, the second list represents the paths that were fixed + fn report_handled_paths( + &mut self, + evaluated_paths: Vec, + fixed_paths: Vec, + ) -> io::Result<()> { + let _ = (evaluated_paths, fixed_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..5733245f9b0c 100644 --- a/crates/biome_cli/src/reporter/terminal.rs +++ b/crates/biome_cli/src/reporter/terminal.rs @@ -3,23 +3,54 @@ 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 std::io; +use std::path::PathBuf; use std::time::Duration; pub(crate) struct ConsoleReporter { pub(crate) summary: TraversalSummary, pub(crate) diagnostics_payload: DiagnosticsPayload, pub(crate) execution: Execution, + pub(crate) evaluated_paths: Vec, + pub(crate) fixed_paths: Vec, } 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, self.fixed_paths)?; + } Ok(()) } } + +#[derive(Debug, Diagnostic)] +#[diagnostic( + tags(VERBOSE), + severity = Information, + message = "List of files that were evaluated:" +)] +struct EvaluatedPathsDiagnostic { + #[advice] + list: ListAdvice, +} + +#[derive(Debug, Diagnostic)] +#[diagnostic( + tags(VERBOSE), + severity = Information, + message = "List of files that were 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: Vec, + fixed_paths: Vec, + ) -> io::Result<()> { + let evaluated_paths_diagnostic = EvaluatedPathsDiagnostic { + list: ListAdvice { + list: evaluated_paths + .into_iter() + .map(|p| p.display().to_string()) + .collect(), + }, + }; + + let fixed_paths_diagnostic = FixedPathsDiagnostic { + list: ListAdvice { + list: fixed_paths + .into_iter() + .map(|p| p.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..1f814a416c82 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,6 +115,18 @@ 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 diff --git a/crates/biome_diagnostics/src/advice.rs b/crates/biome_diagnostics/src/advice.rs index 71efee787699..4520a2fcd328 100644 --- a/crates/biome_diagnostics/src/advice.rs +++ b/crates/biome_diagnostics/src/advice.rs @@ -99,6 +99,28 @@ impl Advices for LogAdvice { } } +/// Utility advice that prints a list of items. +#[derive(Debug)] +pub struct ListAdvice { + pub list: Vec, +} + +impl Advices for ListAdvice { + fn record(&self, visitor: &mut dyn Visit) -> io::Result<()> { + if self.list.is_empty() { + visitor.record_log(LogCategory::Warn, &"The list is empty.") + } else { + let pattern_list: Vec<_> = self + .list + .iter() + .map(|pattern| pattern as &dyn Display) + .collect(); + + visitor.record_list(&pattern_list) + } + } +} + /// Utility type implementing [Advices] that emits a single code frame /// advice with the provided path, span and source code. #[derive(Debug)] diff --git a/crates/biome_fs/src/fs.rs b/crates/biome_fs/src/fs.rs index 505b402d053e..a38b9d02fc48 100644 --- a/crates/biome_fs/src/fs.rs +++ b/crates/biome_fs/src/fs.rs @@ -47,9 +47,6 @@ pub trait FileSystem: Send + Sync + RefUnwindSafe { /// efficiently batch many filesystem read operations fn traversal<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>); - /// TODO: add documentation - fn check<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>); - // TODO: remove once we remove `rome.json` support [2.0] /// Returns the temporary configuration files that are supported fn deprecated_config_name(&self) -> &str { @@ -300,15 +297,20 @@ impl FileSystemExt for T where T: FileSystem {} type BoxedTraversal<'fs, 'scope> = Box) + Send + 'fs>; pub trait TraversalScope<'scope> { - /// Spawn a new filesystem read task + /// Spawn a new filesystem read task. /// /// If the provided path exists and is a file, then the [`handle_file`](TraversalContext::handle_path) /// method of the provided [TraversalContext] will be called. If it's a /// directory, it will be recursively traversed and all the files the - /// [`can_handle`](TraversalContext::can_handle) method of the context + /// [TraversalContext::can_handle] method of the context /// returns true for will be handled as well - fn spawn(&self, context: &'scope dyn TraversalContext, path: PathBuf); + fn evaluate(&self, context: &'scope dyn TraversalContext, path: PathBuf); + /// Spawn a new filesystem read task. + /// + /// It's assumed that the provided already exist and was already evaluated via [TraversalContext::can_handle]. + /// + /// This method will call [TraversalContext::handle_path]. fn handle(&self, context: &'scope dyn TraversalContext, path: PathBuf); } @@ -335,17 +337,16 @@ pub trait TraversalContext: Sync { fn store_path(&self, path: &Path); /// Returns the paths that should be handled - fn paths(&self) -> Vec; + fn evaluated_paths(&self) -> Vec; + + /// Returns the paths that were fixed/changed + fn fixed_paths(&self) -> Vec; } impl FileSystem for Arc where T: FileSystem + Send, { - fn check<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>) { - T::check(self, func) - } - fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result> { T::open_with_options(self, path, options) } diff --git a/crates/biome_fs/src/fs/memory.rs b/crates/biome_fs/src/fs/memory.rs index 5b266affe877..1775407d649c 100644 --- a/crates/biome_fs/src/fs/memory.rs +++ b/crates/biome_fs/src/fs/memory.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use biome_diagnostics::{Error, Severity}; use parking_lot::{lock_api::ArcMutexGuard, Mutex, RawMutex, RwLock}; -use crate::fs::{ForEachPath, OpenOptions}; +use crate::fs::OpenOptions; use crate::{BiomePath, FileSystem, TraversalContext, TraversalScope}; use super::{BoxedTraversal, ErrorKind, File, FileSystemDiagnostic}; @@ -236,10 +236,6 @@ impl FileSystem for MemoryFileSystem { ) -> Result { todo!() } - - fn check<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>) { - func(&MemoryTraversalScope { fs: self }) - } } struct MemoryFile { @@ -293,7 +289,7 @@ pub struct MemoryTraversalScope<'scope> { } impl<'scope> TraversalScope<'scope> for MemoryTraversalScope<'scope> { - fn spawn(&self, ctx: &'scope dyn TraversalContext, base: PathBuf) { + fn evaluate(&self, ctx: &'scope dyn TraversalContext, base: PathBuf) { // Traversal is implemented by iterating on all keys, and matching on // those that are prefixed with the provided `base` path { @@ -545,7 +541,7 @@ mod tests { self.visited.lock().push(path.into()) } - fn paths(&self) -> Vec { + fn evaluated_paths(&self) -> Vec { let lock = self.visited.lock(); lock.to_vec() } @@ -559,7 +555,7 @@ mod tests { // Traverse a directory fs.traversal(Box::new(|scope| { - scope.spawn(&ctx, PathBuf::from("dir1")); + scope.evaluate(&ctx, PathBuf::from("dir1")); })); let mut visited = Vec::new(); @@ -571,7 +567,7 @@ mod tests { // Traverse a single file fs.traversal(Box::new(|scope| { - scope.spawn(&ctx, PathBuf::from("dir2/file2")); + scope.evaluate(&ctx, PathBuf::from("dir2/file2")); })); let mut visited = Vec::new(); diff --git a/crates/biome_fs/src/fs/os.rs b/crates/biome_fs/src/fs/os.rs index 62247e2eb837..f8fc6fd304e8 100644 --- a/crates/biome_fs/src/fs/os.rs +++ b/crates/biome_fs/src/fs/os.rs @@ -1,5 +1,5 @@ //! Implementation of the [FileSystem] and related traits for the underlying OS filesystem -use super::{BoxedTraversal, ErrorKind, File, FileSystemDiagnostic, ForEachPath}; +use super::{BoxedTraversal, ErrorKind, File, FileSystemDiagnostic}; use crate::fs::OpenOptions; use crate::{ fs::{TraversalContext, TraversalScope}, @@ -71,12 +71,6 @@ impl FileSystem for OsFileSystem { }) } - fn check(&self, func: BoxedTraversal) { - OsTraversalScope::with(move |scope| { - func(scope); - }) - } - // fn for_each_path(&self, func: ForEachPath) { // let paths = self.paths.0.read(); // let iter = paths.iter(); @@ -213,7 +207,7 @@ impl<'scope> OsTraversalScope<'scope> { } impl<'scope> TraversalScope<'scope> for OsTraversalScope<'scope> { - fn spawn(&self, ctx: &'scope dyn TraversalContext, path: PathBuf) { + fn evaluate(&self, ctx: &'scope dyn TraversalContext, path: PathBuf) { let file_type = match path.metadata() { Ok(meta) => meta.file_type(), Err(err) => { @@ -368,8 +362,7 @@ fn handle_any_file<'scope>( if file_type.is_file() { scope.spawn(move |_| { - ctx.handle_path(&path); - // ctx.store_file(&path); + ctx.store_path(&path); }); return; } diff --git a/crates/biome_service/src/workspace.rs b/crates/biome_service/src/workspace.rs index b11a6cba727e..6a2ed1314e62 100644 --- a/crates/biome_service/src/workspace.rs +++ b/crates/biome_service/src/workspace.rs @@ -272,17 +272,6 @@ impl FileFeaturesResult { } } - pub fn as_format_support(&self) -> Option<&SupportKind> { - self.support_kind_for(&FeatureName::Format) - } - - pub fn as_lint_support(&self) -> Option<&SupportKind> { - self.support_kind_for(&FeatureName::Lint) - } - pub fn as_organize_imports_support(&self) -> Option<&SupportKind> { - self.support_kind_for(&FeatureName::OrganizeImports) - } - pub fn support_kind_for(&self, feature: &FeatureKind) -> Option<&SupportKind> { self.features_supported.get(feature) } From 8c4769a8d4ef52225ea79b949ccc47ad9f5b3cda Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 1 Aug 2024 11:56:41 +0100 Subject: [PATCH 4/7] update snapshots and fix clippy --- Cargo.lock | 1 - crates/biome_cli/src/execute/mod.rs | 28 +++++++++++-------- crates/biome_cli/src/execute/traverse.rs | 18 ++++++++---- .../max_diagnostics_verbose.snap | 20 +++++++++++++ .../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 +++++++++++++ .../main_commands_lint/print_verbose.snap | 20 +++++++++++++ .../unsupported_file_verbose.snap | 20 +++++++++++++ crates/biome_fs/Cargo.toml | 1 - crates/biome_fs/src/fs/memory.rs | 9 +++++- crates/biome_fs/src/fs/os.rs | 10 ------- crates/biome_service/src/workspace/server.rs | 4 +-- 17 files changed, 239 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index be7b56fca863..9ea389f85ee2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -454,7 +454,6 @@ version = "0.5.7" dependencies = [ "biome_diagnostics", "crossbeam", - "dashmap", "directories", "indexmap 2.2.6", "oxc_resolver", diff --git a/crates/biome_cli/src/execute/mod.rs b/crates/biome_cli/src/execute/mod.rs index 68e3186f0119..8ef4bc292c32 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,19 +431,23 @@ pub fn execute_mode( }; migrate::run(payload) } else { - let (summary_result, evaluated_paths, fixed_paths, diagnostics) = - traverse(&execution, &mut session, cli_options, paths)?; + let TraverseResult { + summary, + evaluated_paths, + fixed_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, @@ -454,7 +458,7 @@ 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, @@ -472,7 +476,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, @@ -480,7 +484,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| { @@ -520,7 +524,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/traverse.rs b/crates/biome_cli/src/execute/traverse.rs index 5b40e3361232..8a702d846368 100644 --- a/crates/biome_cli/src/execute/traverse.rs +++ b/crates/biome_cli/src/execute/traverse.rs @@ -31,12 +31,19 @@ use std::{ time::{Duration, Instant}, }; +pub(crate) struct TraverseResult { + pub(crate) summary: TraversalSummary, + pub(crate) evaluated_paths: Vec, + pub(crate) fixed_paths: Vec, + pub(crate) diagnostics: Vec, +} + pub(crate) fn traverse( execution: &Execution, session: &mut CliSession, cli_options: &CliOptions, mut inputs: Vec, -) -> Result<(TraversalSummary, Vec, Vec, Vec), CliDiagnostic> { +) -> Result { init_thread_pool(); if inputs.is_empty() { @@ -128,8 +135,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, @@ -139,10 +146,11 @@ pub(crate) fn traverse( suggested_fixes_skipped, diagnostics_not_printed, }, + evaluated_paths, fixed_paths, diagnostics, - )) + }) } /// This function will setup the global Rayon thread pool the first time it's called @@ -176,7 +184,7 @@ fn traverse_inputs( fs.traversal(Box::new(|scope: &dyn TraversalScope| { for path in paths.clone() { - scope.handle(ctx, path.to_path_buf()); + scope.handle(ctx, path.clone()); } })); 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 1f814a416c82..f7c87b2cb273 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 @@ -133,3 +133,23 @@ src/file.js format ━━━━━━━━━━━━━━━━━━━━ Checked 1 file in