Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_cli): prevent exploration of ignored directories #4276

Merged
merged 2 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions crates/rome_cli/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ pub(crate) struct Execution {
max_diagnostics: u16,
}

impl Execution {
pub(crate) fn as_feature_name(&self) -> FeatureName {
match self.traversal_mode {
TraversalMode::Format { .. } => FeatureName::Format,
_ => FeatureName::Lint,
}
}
}

pub(crate) enum TraversalMode {
/// This mode is enabled when running the command `rome check`
Check {
Expand Down
16 changes: 15 additions & 1 deletion crates/rome_cli/src/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rome_diagnostics::{
};
use rome_fs::{FileSystem, OpenOptions, PathInterner, RomePath};
use rome_fs::{TraversalContext, TraversalScope};
use rome_service::workspace::{SupportsFeatureResult, UnsupportedReason};
use rome_service::workspace::{IsPathIgnoredParams, SupportsFeatureResult, UnsupportedReason};
use rome_service::{
workspace::{
FeatureName, FileGuard, Language, OpenFileParams, RuleCategories, SupportsFeatureParams,
Expand Down Expand Up @@ -648,6 +648,20 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> {
}

fn can_handle(&self, rome_path: &RomePath) -> bool {
if rome_path.is_dir() {
let can_handle = !self
.workspace
.is_path_ignored(IsPathIgnoredParams {
rome_path: rome_path.clone(),
feature: self.execution.as_feature_name(),
})
.unwrap_or_else(|err| {
self.push_diagnostic(err.into());
false
});
return can_handle;
}

let can_lint = self.can_lint(rome_path);
let can_format = self.can_format(rome_path);

Expand Down
9 changes: 5 additions & 4 deletions crates/rome_fs/src/fs/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ impl<'scope> TraversalScope<'scope> for OsTraversalScope<'scope> {

/// Default list of ignored directories, in the future will be supplanted by
/// detecting and parsing .ignore files
/// TODO: add support for ignore files in Rome
const DEFAULT_IGNORE: &[&str; 5] = &[".git", ".svn", ".hg", ".yarn", "node_modules"];

/// Traverse a single directory
Expand Down Expand Up @@ -268,9 +267,11 @@ fn handle_dir_entry<'scope>(
}

if file_type.is_dir() {
scope.spawn(move |scope| {
handle_dir(scope, ctx, &path, origin_path);
});
if ctx.can_handle(&RomePath::new(path.clone())) {
scope.spawn(move |scope| {
handle_dir(scope, ctx, &path, origin_path);
});
}
return;
}

Expand Down
1 change: 1 addition & 0 deletions crates/rome_lsp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ impl ServerFactory {
builder = builder.custom_method("rome/rage", LSPServer::rage);

workspace_method!(builder, supports_feature);
workspace_method!(builder, is_path_ignored);
workspace_method!(builder, update_settings);
workspace_method!(builder, open_file);
workspace_method!(builder, get_syntax_tree);
Expand Down
17 changes: 16 additions & 1 deletion crates/rome_service/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub enum UnsupportedReason {
FileNotSupported,
}

#[derive(Debug, serde::Serialize, serde::Deserialize)]
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub enum FeatureName {
Format,
Expand Down Expand Up @@ -334,6 +334,13 @@ impl RageEntry {
}
}

#[derive(Debug, serde::Serialize, serde::Deserialize)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct IsPathIgnoredParams {
pub rome_path: RomePath,
pub feature: FeatureName,
}

pub trait Workspace: Send + Sync + RefUnwindSafe {
/// Checks whether a certain feature is supported. There are different conditions:
/// - Rome doesn't recognize a file, so it can't provide the feature;
Expand All @@ -344,6 +351,14 @@ pub trait Workspace: Send + Sync + RefUnwindSafe {
params: SupportsFeatureParams,
) -> Result<SupportsFeatureResult, WorkspaceError>;

/// Checks if the current path is ignored by the workspace, against a particular feature.
///
/// Takes as input the path of the file that workspace is currently processing and
/// a list of paths to match against.
///
/// If the file path matches, than `true` is returned and it should be considered ignored.
fn is_path_ignored(&self, params: IsPathIgnoredParams) -> Result<bool, WorkspaceError>;

/// Update the global settings for this workspace
fn update_settings(&self, params: UpdateSettingsParams) -> Result<(), WorkspaceError>;

Expand Down
18 changes: 11 additions & 7 deletions crates/rome_service/src/workspace/client.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use std::{
panic::RefUnwindSafe,
sync::atomic::{AtomicU64, Ordering},
use crate::workspace::{
IsPathIgnoredParams, RageParams, RageResult, ServerInfo, SupportsFeatureResult,
};

use crate::{TransportError, Workspace, WorkspaceError};
use rome_formatter::Printed;
use serde::{de::DeserializeOwned, Deserialize, Serialize};
use serde_json::json;

use crate::workspace::{RageParams, RageResult, ServerInfo, SupportsFeatureResult};
use crate::{TransportError, Workspace, WorkspaceError};
use std::{
panic::RefUnwindSafe,
sync::atomic::{AtomicU64, Ordering},
};

use super::{
ChangeFileParams, CloseFileParams, FixFileParams, FixFileResult, FormatFileParams,
Expand Down Expand Up @@ -106,6 +106,10 @@ where
self.request("rome/supports_feature", params)
}

fn is_path_ignored(&self, params: IsPathIgnoredParams) -> Result<bool, WorkspaceError> {
self.request("rome/is_path_ignored", params)
}

fn update_settings(&self, params: UpdateSettingsParams) -> Result<(), WorkspaceError> {
self.request("rome/update_settings", params)
}
Expand Down
73 changes: 39 additions & 34 deletions crates/rome_service/src/workspace/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use super::{
UpdateSettingsParams,
};
use crate::file_handlers::{Capabilities, FixAllParams, Language, LintParams};
use crate::workspace::{RageEntry, RageParams, RageResult, ServerInfo, SupportsFeatureResult};
use crate::workspace::{
IsPathIgnoredParams, RageEntry, RageParams, RageResult, ServerInfo, SupportsFeatureResult,
};
use crate::{
file_handlers::Features,
settings::{SettingsHandle, WorkspaceSettings},
Expand Down Expand Up @@ -123,7 +125,10 @@ impl WorkspaceServer {
feature: Option<FeatureName>,
) -> Result<AnyParse, WorkspaceError> {
let ignored = if let Some(feature) = feature {
self.is_file_ignored(&rome_path, &feature)
self.is_path_ignored(IsPathIgnoredParams {
rome_path: rome_path.clone(),
feature,
})?
} else {
false
};
Expand Down Expand Up @@ -179,37 +184,6 @@ impl WorkspaceServer {
}
}
}

/// Takes as input the path of the file that workspace is currently processing and
/// a list of paths to match against.
///
/// If the file path matches, than `true` is returned and it should be considered ignored.
fn is_file_ignored(&self, rome_path: &RomePath, feature: &FeatureName) -> bool {
let settings = self.settings();
let is_ignored_by_file_config = settings
.as_ref()
.files
.ignored_files
.matches_path(rome_path.as_path());
match feature {
FeatureName::Format => {
settings
.as_ref()
.formatter
.ignored_files
.matches_path(rome_path.as_path())
|| is_ignored_by_file_config
}
FeatureName::Lint => {
settings
.as_ref()
.linter
.ignored_files
.matches_path(rome_path.as_path())
|| is_ignored_by_file_config
}
}
}
}

impl Workspace for WorkspaceServer {
Expand All @@ -219,7 +193,10 @@ impl Workspace for WorkspaceServer {
) -> Result<SupportsFeatureResult, WorkspaceError> {
let capabilities = self.get_capabilities(&params.path);
let settings = self.settings.read().unwrap();
let is_ignored = self.is_file_ignored(&params.path, &params.feature);
let is_ignored = self.is_path_ignored(IsPathIgnoredParams {
rome_path: params.path,
feature: params.feature.clone(),
})?;
let result = match params.feature {
FeatureName::Format => {
if is_ignored {
Expand Down Expand Up @@ -247,6 +224,34 @@ impl Workspace for WorkspaceServer {
Ok(result)
}

fn is_path_ignored(&self, params: IsPathIgnoredParams) -> Result<bool, WorkspaceError> {
let settings = self.settings();
let is_ignored_by_file_config = settings
.as_ref()
.files
.ignored_files
.matches_path(params.rome_path.as_path());

Ok(match params.feature {
FeatureName::Format => {
settings
.as_ref()
.formatter
.ignored_files
.matches_path(params.rome_path.as_path())
|| is_ignored_by_file_config
}
FeatureName::Lint => {
settings
.as_ref()
.linter
.ignored_files
.matches_path(params.rome_path.as_path())
|| is_ignored_by_file_config
}
})
}

/// Update the global settings for this workspace
///
/// ## Panics
Expand Down
2 changes: 1 addition & 1 deletion rome.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"$schema": "./npm/rome/configuration_schema.json",
"files": {
"ignore": [
"website/build",
"build",
"dist",
"target"
]
Expand Down
2 changes: 1 addition & 1 deletion website/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"start": "astro dev",
"format": "cargo rome-cli-dev format --write .",
"tsc": "tsc --skipLibCheck",
"check": "cargo rome-cli-dev check ./src",
"check": "cargo rome-cli-dev check ./",
"start:playground": "pnpm build:wasm-dev && pnpm start",
"build": "pnpm build:wasm && pnpm build:js",
"build:js": "astro build",
Expand Down