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

Commit

Permalink
fix(rome_cli): prevent exploration of ignored directories
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Mar 7, 2023
1 parent 9cedc42 commit 4c49569
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 49 deletions.
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
20 changes: 19 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,24 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> {
}

fn can_handle(&self, rome_path: &RomePath) -> bool {
if rome_path.is_dir() {
if 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 false;
} else {
return true;
}
}

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 @@ -255,9 +254,11 @@ fn handle_dir_entry<'scope>(
}

if file_type.is_dir() {
scope.spawn(move |scope| {
handle_dir(scope, ctx, &path);
});
if ctx.can_handle(&RomePath::new(path.clone())) {
scope.spawn(move |scope| {
handle_dir(scope, ctx, &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

0 comments on commit 4c49569

Please sign in to comment.