From 1d86eecd82c2b8e8562050ebf30a19039f807f0e Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 8 May 2023 16:12:27 +0100 Subject: [PATCH] fix(rome_lsp): don't throw error for ignored files in code actions (#4443) --- CHANGELOG.md | 10 +- Cargo.lock | 28 ++-- Cargo.toml | 2 + crates/rome_analyze/Cargo.toml | 2 +- crates/rome_analyze/src/categories.rs | 1 + crates/rome_cli/src/execute/diagnostics.rs | 6 +- crates/rome_cli/src/execute/process_file.rs | 135 ++++++++++++------ crates/rome_cli/src/execute/std_in.rs | 57 +++----- crates/rome_cli/src/execute/traverse.rs | 83 ++++------- crates/rome_formatter_test/src/spec.rs | 32 ++--- crates/rome_js_parser/Cargo.toml | 2 +- crates/rome_js_parser/src/lexer/mod.rs | 1 + crates/rome_js_parser/src/state.rs | 10 +- crates/rome_js_parser/src/syntax/class.rs | 26 ++-- crates/rome_js_parser/src/syntax/expr.rs | 1 + .../src/syntax/typescript/types.rs | 2 +- crates/rome_lsp/src/handlers/analysis.rs | 40 ++++-- crates/rome_lsp/src/server.rs | 2 +- crates/rome_lsp/src/session.rs | 26 +++- crates/rome_lsp/tests/server.rs | 84 +++++++++++ .../src/file_handlers/javascript.rs | 32 +++-- crates/rome_service/src/file_handlers/mod.rs | 8 +- crates/rome_service/src/workspace.rs | 134 ++++++++++++++--- crates/rome_service/src/workspace/client.rs | 10 +- crates/rome_service/src/workspace/server.rs | 69 ++++----- crates/rome_service/src/workspace_types.rs | 2 +- crates/rome_wasm/src/lib.rs | 6 +- npm/backend-jsonrpc/src/workspace.ts | 15 +- 28 files changed, 520 insertions(+), 306 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d993d1c102c..d1cd7250c7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,9 +85,13 @@ when there are breaking changes. ### Configuration ### Editors - - Fix an issue where the VSCode extension duplicates text when using VSCode git utilities [#4338] - - Remove code assists from being added to the code actions when apply fixes; - - +#### Other changes + +- Fix an issue where the VSCode extension duplicates text when using VSCode git utilities [#4338](https://github.com/rome/tools/issues/4338) +- Remove code assists from being added to the code actions when apply fixes; +- When requesting code actions, ignored files should not throw errors. Fixes [#4434](https://github.com/rome/tools/issues/4434) + + ### Formatter - Fix an issue where formatting of JSX string literals property values were using incorrect quotes [#4054](https://github.com/rome/tools/issues/4054) diff --git a/Cargo.lock b/Cargo.lock index e7527eb085f..6b8bd3c2f79 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -144,6 +144,12 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" +[[package]] +name = "bitflags" +version = "2.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24a6904aef64d73cf10ab17ebace7befb918b82164785cb89907993be7f83813" + [[package]] name = "bpaf" version = "0.7.10" @@ -269,7 +275,7 @@ version = "3.2.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71655c45cb9845d3270c9d6df84ebe72b4dad3c2ba3f7023ad47c144e4e473a5" dependencies = [ - "bitflags", + "bitflags 1.3.2", "clap_lex", "indexmap", "textwrap", @@ -759,7 +765,7 @@ version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b7905cdfe33d31a88bb2e8419ddd054451f5432d1da9eaf2ac7804ee1ea12d5" dependencies = [ - "bitflags", + "bitflags 1.3.2", "libc", "libgit2-sys", "log", @@ -791,7 +797,7 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "93e3af942408868f6934a7b85134a3230832b9977cf66125df2f9edcfce4ddcc" dependencies = [ - "bitflags", + "bitflags 1.3.2", "ignore", "walkdir", ] @@ -1079,7 +1085,7 @@ version = "0.94.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b63735a13a1f9cd4f4835223d828ed9c2e35c8c5e61837774399f558b6a1237" dependencies = [ - "bitflags", + "bitflags 1.3.2", "serde", "serde_json", "serde_repr", @@ -1384,7 +1390,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "29f1b898011ce9595050a68e60f90bad083ff2987a695a42357134c8381fba70" dependencies = [ "bit-set", - "bitflags", + "bitflags 1.3.2", "byteorder", "lazy_static", "num-traits", @@ -1404,7 +1410,7 @@ version = "0.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d9cc634bc78768157b5cbfe988ffcd1dcba95cd2b2f03a88316c08c6d00ed63" dependencies = [ - "bitflags", + "bitflags 1.3.2", "memchr", "unicase", ] @@ -1570,7 +1576,7 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fb5a58c1855b4b6819d59012155603f0b22ad30cad752600aadfcb695265519a" dependencies = [ - "bitflags", + "bitflags 1.3.2", ] [[package]] @@ -1644,7 +1650,7 @@ dependencies = [ name = "rome_analyze" version = "0.0.0" dependencies = [ - "bitflags", + "bitflags 2.2.1", "rome_console", "rome_deserialize", "rome_diagnostics", @@ -1770,7 +1776,7 @@ name = "rome_diagnostics" version = "0.0.1" dependencies = [ "backtrace", - "bitflags", + "bitflags 1.3.2", "bpaf", "pico-args", "rome_console", @@ -1939,7 +1945,7 @@ dependencies = [ name = "rome_js_parser" version = "0.0.2" dependencies = [ - "bitflags", + "bitflags 2.2.1", "cfg-if", "drop_bomb", "expect-test", @@ -2217,7 +2223,7 @@ version = "0.36.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fd5c6ff11fecd55b40746d1995a02f2eb375bf8c00d192d521ee09f42bef37bc" dependencies = [ - "bitflags", + "bitflags 1.3.2", "errno", "io-lifetimes", "libc", diff --git a/Cargo.toml b/Cargo.toml index 4985d07d85b..1094b534541 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,4 +34,6 @@ insta = "1.21.2" quote = { version = "1.0.21" } lazy_static = "1.4.0" bpaf = { version = "0.7.10", features = ["derive"] } +bitflags = "2.2.1" + diff --git a/crates/rome_analyze/Cargo.toml b/crates/rome_analyze/Cargo.toml index 33e87aa02fc..130a52e0821 100644 --- a/crates/rome_analyze/Cargo.toml +++ b/crates/rome_analyze/Cargo.toml @@ -14,7 +14,7 @@ rome_console = { path = "../rome_console" } rome_diagnostics = { path = "../rome_diagnostics" } rome_json_parser = { path = "../rome_json_parser" } rome_deserialize = { path = "../rome_deserialize"} -bitflags = "1.3.2" +bitflags.workspace = true rustc-hash = { workspace = true } serde = { version = "1.0.136", features = ["derive"] } schemars = { version = "0.8.10", optional = true } diff --git a/crates/rome_analyze/src/categories.rs b/crates/rome_analyze/src/categories.rs index 82f6c8484ed..cfd1f964b38 100644 --- a/crates/rome_analyze/src/categories.rs +++ b/crates/rome_analyze/src/categories.rs @@ -165,6 +165,7 @@ pub enum SourceActionKind { } bitflags! { + #[derive(Debug, Copy, Clone, Eq, PartialEq)] pub struct RuleCategories: u8 { const SYNTAX = 1 << RuleCategory::Syntax as u8; const LINT = 1 << RuleCategory::Lint as u8; diff --git a/crates/rome_cli/src/execute/diagnostics.rs b/crates/rome_cli/src/execute/diagnostics.rs index 4221d4cbcb6..dc90de71f9e 100644 --- a/crates/rome_cli/src/execute/diagnostics.rs +++ b/crates/rome_cli/src/execute/diagnostics.rs @@ -101,7 +101,11 @@ pub(crate) struct PanicDiagnostic { } #[derive(Debug, Diagnostic)] -#[diagnostic(category = "files/missingHandler", message = "unhandled file type")] +#[diagnostic( + category = "files/missingHandler", + message = "Rome doesn't know how to process this file", + severity = Warning +)] pub(crate) struct UnhandledDiagnostic; #[derive(Debug, Diagnostic)] diff --git a/crates/rome_cli/src/execute/process_file.rs b/crates/rome_cli/src/execute/process_file.rs index 743a0fd7974..6c36031e776 100644 --- a/crates/rome_cli/src/execute/process_file.rs +++ b/crates/rome_cli/src/execute/process_file.rs @@ -2,14 +2,16 @@ use crate::execute::diagnostics::{ResultExt, ResultIoExt, SkippedDiagnostic, Unh use crate::execute::traverse::TraversalOptions; use crate::execute::TraversalMode; use crate::{CliDiagnostic, FormatterReportFileDetail}; -use rome_diagnostics::{category, DiagnosticExt, Error}; +use rome_diagnostics::{category, Context, DiagnosticExt, Error}; use rome_fs::{OpenOptions, RomePath}; use rome_service::workspace::{ - FileGuard, Language, OpenFileParams, RuleCategories, UnsupportedReason, + FeatureName, FeaturesBuilder, FileGuard, Language, OpenFileParams, RuleCategories, SupportKind, + SupportsFeatureParams, }; use std::path::Path; use std::sync::atomic::Ordering; +#[derive(Debug)] pub(crate) enum FileStatus { Success, Message(Message), @@ -17,6 +19,7 @@ pub(crate) enum FileStatus { } /// Wrapper type for messages that can be printed during the traversal process +#[derive(Debug)] pub(crate) enum Message { SkippedFixes { /// Suggested fixes skipped during the lint traversal @@ -38,6 +41,7 @@ pub(crate) enum Message { }, } +#[derive(Debug)] pub(crate) enum DiffKind { Format, OrganizeImports, @@ -46,6 +50,7 @@ pub(crate) enum DiffKind { impl From for Message where Error: From, + D: std::fmt::Debug, { fn from(err: D) -> Self { Self::Error(Error::from(err)) @@ -72,45 +77,88 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { tracing::trace_span!("process_file", path = ?path).in_scope(move || { let rome_path = RomePath::new(path); - let supported_format = ctx.can_format(&rome_path).with_file_path_and_code( - path.display().to_string(), - category!("files/missingHandler"), - )?; - - let supported_lint = ctx.can_lint(&rome_path).with_file_path_and_code( - path.display().to_string(), - category!("files/missingHandler"), - )?; - - let supported_organize_imports = ctx - .can_organize_imports(&rome_path) + let file_features = ctx + .workspace + .file_features(SupportsFeatureParams { + path: rome_path.clone(), + feature: FeaturesBuilder::new() + .with_formatter() + .with_linter() + .with_organize_imports() + .build(), + }) .with_file_path_and_code( path.display().to_string(), category!("files/missingHandler"), )?; let unsupported_reason = match ctx.execution.traversal_mode() { - TraversalMode::Check { .. } => supported_lint - .reason - .as_ref() - .and(supported_organize_imports.reason.as_ref()), - TraversalMode::CI { .. } => supported_lint - .reason - .as_ref() - .and(supported_format.reason.as_ref()) - .and(supported_organize_imports.reason.as_ref()), - TraversalMode::Format { .. } => supported_format.reason.as_ref(), + TraversalMode::Check { .. } => file_features + .support_kind_for(&FeatureName::Lint) + .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) + } else { + None + } + }), + ), + TraversalMode::CI { .. } => file_features + .support_kind_for(&FeatureName::Lint) + .and_then(|support_kind| { + if support_kind.is_not_enabled() { + Some(support_kind) + } else { + None + } + }) + .and( + file_features + .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) + } else { + None + } + }), + ), + TraversalMode::Format { .. } => file_features.support_kind_for(&FeatureName::Format), TraversalMode::Migrate { .. } => None, }; if let Some(reason) = unsupported_reason { - return match reason { - UnsupportedReason::FileNotSupported => Err(Message::from( - UnhandledDiagnostic.with_file_path(path.display().to_string()), - )), - UnsupportedReason::FeatureNotEnabled | UnsupportedReason::Ignored => { - Ok(FileStatus::Ignored) + match reason { + SupportKind::FileNotSupported => { + return Err(Message::from( + UnhandledDiagnostic.with_file_path(path.display().to_string()), + )) + } + SupportKind::FeatureNotEnabled | SupportKind::Ignored => { + return Ok(FileStatus::Ignored) } + SupportKind::Supported => {} }; } @@ -156,11 +204,10 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { } errors = fixed.errors; } - - if supported_organize_imports.is_supported() && ctx.execution.is_check() { + if file_features.supports_for(&FeatureName::OrganizeImports) && ctx.execution.is_check() { let sorted = file_guard.organize_imports().with_file_path_and_code( path.display().to_string(), - category!("organizeImports"), + category!("internalError/fs"), )?; if sorted.code != input { @@ -191,11 +238,12 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { return Ok(FileStatus::Success); } - let categories = if ctx.execution.is_format() || !supported_lint.is_supported() { - RuleCategories::SYNTAX - } else { - RuleCategories::SYNTAX | RuleCategories::LINT - }; + let categories = + if ctx.execution.is_format() || !file_features.supports_for(&FeatureName::Lint) { + RuleCategories::SYNTAX + } else { + RuleCategories::SYNTAX | RuleCategories::LINT + }; let max_diagnostics = ctx.remaining_diagnostics.load(Ordering::Relaxed); let result = file_guard @@ -246,15 +294,14 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { return Ok(result); } - if supported_organize_imports.is_supported() + if file_features.supports_for(&FeatureName::OrganizeImports) // we want to print a diff only if we are in CI // or we are running "check" or "check --apply" && (ctx.execution.is_ci() || !ctx.execution.is_check_apply_unsafe()) { - let sorted = file_guard.organize_imports().with_file_path_and_code( - path.display().to_string(), - category!("organizeImports"), - )?; + let sorted = file_guard + .organize_imports() + .with_file_path(path.display().to_string())?; if sorted.code != input { ctx.messages @@ -268,7 +315,7 @@ pub(crate) fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult { } } - if supported_format.is_supported() { + if file_features.supports_for(&FeatureName::Format) { let should_write = match ctx.execution.traversal_mode() { // In check mode do not run the formatter and return the result immediately, // but only if the argument `--apply` is not passed. diff --git a/crates/rome_cli/src/execute/std_in.rs b/crates/rome_cli/src/execute/std_in.rs index 4219f1e69bb..53f38611fc9 100644 --- a/crates/rome_cli/src/execute/std_in.rs +++ b/crates/rome_cli/src/execute/std_in.rs @@ -4,9 +4,10 @@ use crate::execute::Execution; use crate::{CliDiagnostic, CliSession}; use rome_console::{markup, ConsoleExt}; use rome_fs::RomePath; + use rome_service::workspace::{ - ChangeFileParams, FeatureName, FixFileParams, FormatFileParams, Language, OpenFileParams, - OrganizeImportsParams, SupportsFeatureParams, + ChangeFileParams, FeatureName, FeaturesBuilder, FixFileParams, FormatFileParams, Language, + OpenFileParams, OrganizeImportsParams, SupportsFeatureParams, }; use std::borrow::Cow; @@ -21,13 +22,11 @@ pub(crate) fn run<'a>( let mut version = 0; if mode.is_format() { - let unsupported_format_reason = workspace - .supports_feature(SupportsFeatureParams { - path: rome_path.clone(), - feature: FeatureName::Format, - })? - .reason; - if unsupported_format_reason.is_none() { + let file_features = workspace.file_features(SupportsFeatureParams { + path: rome_path.clone(), + feature: FeaturesBuilder::new().with_formatter().build(), + })?; + if file_features.supports_for(&FeatureName::Format) { workspace.open_file(OpenFileParams { path: rome_path.clone(), version: 0, @@ -56,17 +55,17 @@ pub(crate) fn run<'a>( content: content.into(), language_hint: Language::default(), })?; - + // apply fix file of the linter + let file_features = workspace.file_features(SupportsFeatureParams { + path: rome_path.clone(), + feature: FeaturesBuilder::new() + .with_linter() + .with_organize_imports() + .with_formatter() + .build(), + })?; if let Some(fix_file_mode) = mode.as_fix_file_mode() { - // apply fix file of the linter - let unsupported_lint_reason = workspace - .supports_feature(SupportsFeatureParams { - path: rome_path.clone(), - feature: FeatureName::Lint, - })? - .reason; - - if unsupported_lint_reason.is_none() { + if file_features.supports_for(&FeatureName::Lint) { let fix_file_result = workspace.fix_file(FixFileParams { fix_file_mode: *fix_file_mode, path: rome_path.clone(), @@ -82,15 +81,7 @@ pub(crate) fn run<'a>( } } - // apply organize imports - let unsupported_organize_imports_reason = workspace - .supports_feature(SupportsFeatureParams { - path: rome_path.clone(), - feature: FeatureName::OrganizeImports, - })? - .reason; - - if unsupported_organize_imports_reason.is_none() { + if file_features.supports_for(&FeatureName::OrganizeImports) { let result = workspace.organize_imports(OrganizeImportsParams { path: rome_path.clone(), })?; @@ -105,15 +96,7 @@ pub(crate) fn run<'a>( } } } - - let unsupported_format_reason = workspace - .supports_feature(SupportsFeatureParams { - path: rome_path.clone(), - feature: FeatureName::Format, - })? - .reason; - - if unsupported_format_reason.is_none() { + if file_features.supports_for(&FeatureName::Format) { let printed = workspace.format_file(FormatFileParams { path: rome_path })?; if printed.as_code() != new_content { new_content = Cow::Owned(printed.into_code()); diff --git a/crates/rome_cli/src/execute/traverse.rs b/crates/rome_cli/src/execute/traverse.rs index 9f109a6c5dc..c2ecb160499 100644 --- a/crates/rome_cli/src/execute/traverse.rs +++ b/crates/rome_cli/src/execute/traverse.rs @@ -19,7 +19,7 @@ use rome_diagnostics::{ }; use rome_fs::{FileSystem, PathInterner, RomePath}; use rome_fs::{TraversalContext, TraversalScope}; -use rome_service::workspace::{IsPathIgnoredParams, SupportsFeatureResult}; +use rome_service::workspace::{FeaturesBuilder, IsPathIgnoredParams}; use rome_service::{ workspace::{FeatureName, SupportsFeatureParams}, Workspace, WorkspaceError, @@ -608,42 +608,12 @@ impl<'ctx, 'app> TraversalOptions<'ctx, 'app> { self.messages.send(msg.into()).ok(); } - pub(crate) fn can_format( - &self, - rome_path: &RomePath, - ) -> Result { - self.workspace.supports_feature(SupportsFeatureParams { - path: rome_path.clone(), - feature: FeatureName::Format, - }) - } - pub(crate) fn push_format_stat(&self, path: String, stat: FormatterReportFileDetail) { self.sender_reports .send(ReportKind::Formatter(path, stat)) .ok(); } - pub(crate) fn can_lint( - &self, - rome_path: &RomePath, - ) -> Result { - self.workspace.supports_feature(SupportsFeatureParams { - path: rome_path.clone(), - feature: FeatureName::Lint, - }) - } - - pub(crate) fn can_organize_imports( - &self, - rome_path: &RomePath, - ) -> Result { - self.workspace.supports_feature(SupportsFeatureParams { - path: rome_path.clone(), - feature: FeatureName::OrganizeImports, - }) - } - pub(crate) fn miss_handler_err(&self, err: WorkspaceError, rome_path: &RomePath) { self.push_diagnostic( StdError::from(err) @@ -677,35 +647,32 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> { return can_handle; } - let can_lint = self.can_lint(rome_path); - let can_format = self.can_format(rome_path); - let can_organize_imports = self.can_organize_imports(rome_path); + let file_features = self.workspace.file_features(SupportsFeatureParams { + path: rome_path.clone(), + feature: FeaturesBuilder::new() + .with_linter() + .with_formatter() + .with_organize_imports() + .build(), + }); + + let file_features = match file_features { + Ok(file_features) => file_features, + Err(err) => { + self.miss_handler_err(err, rome_path); + + return false; + } + }; match self.execution.traversal_mode() { - TraversalMode::Check { .. } => can_lint - .map(|result| result.reason.is_none()) - .unwrap_or_else(|err| { - self.miss_handler_err(err, rome_path); - false - }), - TraversalMode::CI { .. } => match (can_format, can_lint, can_organize_imports) { - // the result of the error is the same, rome can't handle the file - (Err(err), _, _) | (_, Err(err), _) | (_, _, Err(err)) => { - self.miss_handler_err(err, rome_path); - false - } - (Ok(can_format), Ok(can_lint), Ok(can_organize_imports)) => { - can_lint.reason.is_none() - || can_format.reason.is_none() - || can_organize_imports.reason.is_none() - } - }, - TraversalMode::Format { .. } => can_format - .map(|result| result.reason.is_none()) - .unwrap_or_else(|err| { - self.miss_handler_err(err, rome_path); - false - }), + TraversalMode::Check { .. } => file_features.supports_for(&FeatureName::Lint), + TraversalMode::CI { .. } => { + file_features.supports_for(&FeatureName::Lint) + || file_features.supports_for(&FeatureName::Format) + || file_features.supports_for(&FeatureName::OrganizeImports) + } + TraversalMode::Format { .. } => file_features.supports_for(&FeatureName::Format), // Imagine if Rome can't handle its own configuration file... TraversalMode::Migrate { .. } => true, } diff --git a/crates/rome_formatter_test/src/spec.rs b/crates/rome_formatter_test/src/spec.rs index 5140723886e..eaa8c8a378f 100644 --- a/crates/rome_formatter_test/src/spec.rs +++ b/crates/rome_formatter_test/src/spec.rs @@ -7,7 +7,7 @@ use rome_formatter::{FormatOptions, Printed}; use rome_fs::RomePath; use rome_parser::AnyParse; use rome_rowan::{TextRange, TextSize}; -use rome_service::workspace::{FeatureName, SupportsFeatureParams}; +use rome_service::workspace::{FeatureName, FeaturesBuilder, SupportsFeatureParams}; use rome_service::App; use std::ops::Range; use std::path::{Path, PathBuf}; @@ -39,30 +39,28 @@ impl<'a> SpecTestFile<'a> { let mut input_file = RomePath::new(file_path); let can_format = app .workspace - .supports_feature(SupportsFeatureParams { + .file_features(SupportsFeatureParams { path: input_file.clone(), - feature: FeatureName::Format, + feature: FeaturesBuilder::new().with_formatter().build(), }) .unwrap(); - match can_format.reason { - None => { - let mut input_code = input_file.get_buffer_from_file(); + if can_format.supports_for(&FeatureName::Format) { + let mut input_code = input_file.get_buffer_from_file(); - let (_, range_start_index, range_end_index) = - strip_rome_placeholders(&mut input_code); + let (_, range_start_index, range_end_index) = strip_rome_placeholders(&mut input_code); - Some(SpecTestFile { - input_file, - root_path, + Some(SpecTestFile { + input_file, + root_path, - input_code, + input_code, - range_start_index, - range_end_index, - }) - } - Some(_) => None, + range_start_index, + range_end_index, + }) + } else { + None } } diff --git a/crates/rome_js_parser/Cargo.toml b/crates/rome_js_parser/Cargo.toml index 5dc838eed3a..ce73acce384 100644 --- a/crates/rome_js_parser/Cargo.toml +++ b/crates/rome_js_parser/Cargo.toml @@ -18,7 +18,7 @@ rome_js_unicode_table = { version = "0.0.1", path = "../rome_js_unicode_table" } rome_rowan = { version = "0.0.1", path = "../rome_rowan" } rome_parser = { version = "0.0.1", path = "../rome_parser" } drop_bomb = "0.1.5" -bitflags = "1.3.2" +bitflags.workspace = true indexmap = { workspace = true } cfg-if = "1.0.0" smallvec = { version = "1.8.0", features = ["union", "const_new"] } diff --git a/crates/rome_js_parser/src/lexer/mod.rs b/crates/rome_js_parser/src/lexer/mod.rs index 698b794bc4b..bb39d9cc6b4 100644 --- a/crates/rome_js_parser/src/lexer/mod.rs +++ b/crates/rome_js_parser/src/lexer/mod.rs @@ -111,6 +111,7 @@ pub enum ReLexContext { bitflags! { /// Flags for a lexed token. + #[derive(Debug, Copy, Clone, Eq, PartialEq)] pub(crate) struct TokenFlags: u8 { /// Indicates that there has been a line break between the last non-trivia token const PRECEDING_LINE_BREAK = 1 << 0; diff --git a/crates/rome_js_parser/src/state.rs b/crates/rome_js_parser/src/state.rs index cb916a17f36..1c4e59983e9 100644 --- a/crates/rome_js_parser/src/state.rs +++ b/crates/rome_js_parser/src/state.rs @@ -330,6 +330,8 @@ impl ChangeParserState for EnableStrictMode { } bitflags! { + #[derive(Debug, Default, Copy, Clone, Eq, PartialEq)] + /// Flags describing the context of a function. pub(crate) struct SignatureFlags: u8 { /// Is the function in an async context @@ -369,7 +371,7 @@ bitflags! { /// * It's easier to snapshot the previous state. Individual boolean fields would require that a change /// snapshots each individual boolean field to allow restoring the previous state. With bitflags, all that /// is needed is to copy away the flags field and restore it after. - #[derive(Default)] + #[derive(Debug, Copy, Default, Clone, Eq, PartialEq)] pub(crate) struct ParsingContextFlags: u8 { /// Whether the parser is in a generator function like `function* a() {}` /// Matches the `Yield` parameter in the ECMA spec @@ -396,13 +398,13 @@ bitflags! { /// Whatever the parser is in a TypeScript ambient context const AMBIENT_CONTEXT = 1 << 7; - const LOOP = Self::BREAK_ALLOWED.bits | Self::CONTINUE_ALLOWED.bits; + const LOOP = Self::BREAK_ALLOWED.bits() | Self::CONTINUE_ALLOWED.bits(); /// Bitmask of all the flags that must be reset (shouldn't be inherited) when the parser enters a function - const FUNCTION_RESET_MASK = Self::BREAK_ALLOWED.bits | Self::CONTINUE_ALLOWED.bits | Self::IN_CONSTRUCTOR.bits | Self::IN_ASYNC.bits | Self::IN_GENERATOR.bits | Self::TOP_LEVEL.bits; + const FUNCTION_RESET_MASK = Self::BREAK_ALLOWED.bits() | Self::CONTINUE_ALLOWED.bits() | Self::IN_CONSTRUCTOR.bits() | Self::IN_ASYNC.bits() | Self::IN_GENERATOR.bits() | Self::TOP_LEVEL.bits(); /// Bitmask of all the flags that must be reset (shouldn't be inherited) when entering parameters. - const PARAMETER_RESET_MASK = Self::IN_CONSTRUCTOR.bits | Self::IN_FUNCTION.bits | Self::TOP_LEVEL.bits | Self::IN_GENERATOR.bits | Self::IN_ASYNC.bits; + const PARAMETER_RESET_MASK = Self::IN_CONSTRUCTOR.bits() | Self::IN_FUNCTION.bits() | Self::TOP_LEVEL.bits() | Self::IN_GENERATOR.bits() | Self::IN_ASYNC.bits(); } } diff --git a/crates/rome_js_parser/src/syntax/class.rs b/crates/rome_js_parser/src/syntax/class.rs index 052ce9e14b2..1ff1a2e06be 100644 --- a/crates/rome_js_parser/src/syntax/class.rs +++ b/crates/rome_js_parser/src/syntax/class.rs @@ -1793,7 +1793,7 @@ fn parse_modifier(p: &mut JsParser, constructor_parameter: bool) -> Option). - #[derive(Default)] + #[derive(Debug, Default, Copy, Clone, Eq, PartialEq)] struct ModifierFlags: u16 { const DECLARE = 1 << 0; const PRIVATE = 1 << 1; @@ -1807,18 +1807,18 @@ bitflags! { const ACCESSOR = 1 << 9; const DECORATOR = 1 << 10; - const ACCESSIBILITY = ModifierFlags::PRIVATE.bits | ModifierFlags::PROTECTED.bits | ModifierFlags::PUBLIC.bits; - - const ALL_MODIFIERS_EXCEPT_DECORATOR = ModifierFlags::DECLARE.bits - | ModifierFlags::PRIVATE.bits - | ModifierFlags::PROTECTED.bits - | ModifierFlags::PUBLIC.bits - | ModifierFlags::STATIC.bits - | ModifierFlags::READONLY.bits - | ModifierFlags::ABSTRACT.bits - | ModifierFlags::OVERRIDE.bits - | ModifierFlags::PRIVATE_NAME.bits - | ModifierFlags::ACCESSOR.bits; + const ACCESSIBILITY = ModifierFlags::PRIVATE.bits() | ModifierFlags::PROTECTED.bits() | ModifierFlags::PUBLIC.bits(); + + const ALL_MODIFIERS_EXCEPT_DECORATOR = ModifierFlags::DECLARE.bits() + | ModifierFlags::PRIVATE.bits() + | ModifierFlags::PROTECTED.bits() + | ModifierFlags::PUBLIC.bits() + | ModifierFlags::STATIC.bits() + | ModifierFlags::READONLY.bits() + | ModifierFlags::ABSTRACT.bits() + | ModifierFlags::OVERRIDE.bits() + | ModifierFlags::PRIVATE_NAME.bits() + | ModifierFlags::ACCESSOR.bits(); } } diff --git a/crates/rome_js_parser/src/syntax/expr.rs b/crates/rome_js_parser/src/syntax/expr.rs index 6418acd82ef..6bf41b18638 100644 --- a/crates/rome_js_parser/src/syntax/expr.rs +++ b/crates/rome_js_parser/src/syntax/expr.rs @@ -43,6 +43,7 @@ pub const EXPR_RECOVERY_SET: TokenSet = pub(crate) struct ExpressionContext(ExpressionContextFlags); bitflags! { + #[derive(Debug, Copy, Clone, Eq, PartialEq)] struct ExpressionContextFlags: u8 { /// Whether `in` should be counted in a binary expression. /// This is for `for...in` statements to prevent ambiguity. diff --git a/crates/rome_js_parser/src/syntax/typescript/types.rs b/crates/rome_js_parser/src/syntax/typescript/types.rs index d02eb07564a..2ece4ed59fb 100644 --- a/crates/rome_js_parser/src/syntax/typescript/types.rs +++ b/crates/rome_js_parser/src/syntax/typescript/types.rs @@ -40,7 +40,7 @@ use super::{expect_ts_index_signature_member, is_at_ts_index_signature_member, M bitflags! { /// Context tracking state that applies to the parsing of all types - #[derive(Default)] + #[derive(Debug, Default, Copy, Clone, Eq, PartialEq)] pub(crate) struct TypeContext: u8 { /// Whether conditional types `extends string ? string : number` are allowed in the current context. /// diff --git a/crates/rome_lsp/src/handlers/analysis.rs b/crates/rome_lsp/src/handlers/analysis.rs index 2f198f32981..6a35ab0ec99 100644 --- a/crates/rome_lsp/src/handlers/analysis.rs +++ b/crates/rome_lsp/src/handlers/analysis.rs @@ -6,7 +6,8 @@ use anyhow::{Context, Result}; use rome_analyze::{ActionCategory, SourceActionKind}; use rome_fs::RomePath; use rome_service::workspace::{ - FeatureName, FixFileMode, FixFileParams, PullActionsParams, SupportsFeatureParams, + FeatureName, FeaturesBuilder, FixFileMode, FixFileParams, PullActionsParams, + SupportsFeatureParams, }; use rome_service::WorkspaceError; use std::borrow::Cow; @@ -35,18 +36,18 @@ pub(crate) fn code_actions( let url = params.text_document.uri.clone(); let rome_path = session.file_path(&url)?; - let unsupported_lint = &session.workspace.supports_feature(SupportsFeatureParams { - path: rome_path.clone(), - feature: FeatureName::Lint, + let file_features = &session.workspace.file_features(SupportsFeatureParams { + path: rome_path, + feature: FeaturesBuilder::new() + .with_linter() + .with_organize_imports() + .build(), })?; - let unsupported_organize_imports = - &session.workspace.supports_feature(SupportsFeatureParams { - path: rome_path, - feature: FeatureName::OrganizeImports, - })?; - - if unsupported_lint.is_not_supported() && unsupported_organize_imports.is_not_supported() { + if !file_features.supports_for(&FeatureName::Lint) + && !file_features.supports_for(&FeatureName::OrganizeImports) + { + debug!("Linter and organize imports are both disabled"); return Ok(Some(Vec::new())); } @@ -78,10 +79,21 @@ pub(crate) fn code_actions( ) })?; - let result = session.workspace.pull_actions(PullActionsParams { + let result = match session.workspace.pull_actions(PullActionsParams { path: rome_path.clone(), range: cursor_range, - })?; + }) { + Ok(result) => result, + Err(err) => { + return if matches!(err, WorkspaceError::FileIgnored(_)) { + Ok(Some(Vec::new())) + } else { + Err(err.into()) + } + } + }; + + debug!("Pull actions result: {:?}", result); // Generate an additional code action to apply all safe fixes on the // document if the action category "source.fixAll" was explicitly requested @@ -98,7 +110,7 @@ pub(crate) fn code_actions( .into_iter() .filter_map(|action| { if action.category.matches("source.organizeImports.rome") - && unsupported_organize_imports.is_not_supported() + && !file_features.supports_for(&FeatureName::OrganizeImports) { return None; } diff --git a/crates/rome_lsp/src/server.rs b/crates/rome_lsp/src/server.rs index a4856bc43a2..df5075a0bc4 100644 --- a/crates/rome_lsp/src/server.rs +++ b/crates/rome_lsp/src/server.rs @@ -513,7 +513,7 @@ impl ServerFactory { builder = builder.custom_method("rome/rage", LSPServer::rage); - workspace_method!(builder, supports_feature); + workspace_method!(builder, file_features); workspace_method!(builder, is_path_ignored); workspace_method!(builder, update_settings); workspace_method!(builder, open_file); diff --git a/crates/rome_lsp/src/session.rs b/crates/rome_lsp/src/session.rs index 46f9fcb8b4a..4d4203fc11e 100644 --- a/crates/rome_lsp/src/session.rs +++ b/crates/rome_lsp/src/session.rs @@ -9,7 +9,9 @@ use futures::StreamExt; use rome_analyze::RuleCategories; use rome_console::markup; use rome_fs::{FileSystem, OsFileSystem, RomePath}; -use rome_service::workspace::{FeatureName, PullDiagnosticsParams, SupportsFeatureParams}; +use rome_service::workspace::{ + FeatureName, FeaturesBuilder, PullDiagnosticsParams, SupportsFeatureParams, +}; use rome_service::workspace::{RageEntry, RageParams, RageResult, UpdateSettingsParams}; use rome_service::{load_config, ConfigurationBasePath, Workspace}; use rome_service::{DynRef, WorkspaceError}; @@ -269,22 +271,34 @@ impl Session { pub(crate) async fn update_diagnostics(&self, url: lsp_types::Url) -> Result<()> { let rome_path = self.file_path(&url)?; let doc = self.document(&url)?; - let unsupported_lint = self.workspace.supports_feature(SupportsFeatureParams { - feature: FeatureName::Lint, + let file_features = self.workspace.file_features(SupportsFeatureParams { + feature: FeaturesBuilder::new() + .with_linter() + .with_organize_imports() + .build(), path: rome_path.clone(), })?; let diagnostics = if self.is_linting_and_formatting_disabled() { tracing::trace!("Linting disabled because Rome configuration is missing and `requireConfiguration` is true."); vec![] - } else if let Some(reason) = unsupported_lint.reason { - tracing::trace!("linting not supported: {reason:?}"); + } else if !file_features.supports_for(&FeatureName::Lint) + && !file_features.supports_for(&FeatureName::OrganizeImports) + { + tracing::trace!("linting and import sorting are not supported: {file_features:?}"); // Sending empty vector clears published diagnostics vec![] } else { + let mut categories = RuleCategories::SYNTAX; + if file_features.supports_for(&FeatureName::Lint) { + categories |= RuleCategories::LINT + } + if file_features.supports_for(&FeatureName::OrganizeImports) { + categories |= RuleCategories::ACTION + } let result = self.workspace.pull_diagnostics(PullDiagnosticsParams { path: rome_path, - categories: RuleCategories::SYNTAX | RuleCategories::LINT, + categories, max_diagnostics: u64::MAX, })?; diff --git a/crates/rome_lsp/tests/server.rs b/crates/rome_lsp/tests/server.rs index 5fae4ee1d7d..64f0b511622 100644 --- a/crates/rome_lsp/tests/server.rs +++ b/crates/rome_lsp/tests/server.rs @@ -843,6 +843,90 @@ async fn pull_diagnostics_for_rome_json() -> Result<()> { Ok(()) } +#[tokio::test] +async fn no_code_actions_for_ignored_json_files() -> Result<()> { + let factory = ServerFactory::default(); + let (service, client) = factory.create().into_inner(); + let (stream, sink) = client.split(); + let mut server = Server::new(service); + + let (sender, mut receiver) = channel(CHANNEL_BUFFER_SIZE); + let reader = tokio::spawn(client_handler(stream, sink, sender)); + + server.initialize().await?; + server.initialized().await?; + + let incorrect_config = r#"{ + "name": "test" + }"#; + server + .open_named_document( + incorrect_config, + url!("./node_modules/preact/package.json"), + "json", + ) + .await?; + + let notification = tokio::select! { + msg = receiver.next() => msg, + _ = sleep(Duration::from_secs(1)) => { + panic!("timed out waiting for the server to send diagnostics") + } + }; + + assert_eq!( + notification, + Some(ServerNotification::PublishDiagnostics( + PublishDiagnosticsParams { + uri: url!("./node_modules/preact/package.json"), + version: Some(0), + diagnostics: vec![], + } + )) + ); + let res: lsp::CodeActionResponse = server + .request( + "textDocument/codeAction", + "pull_code_actions", + lsp::CodeActionParams { + text_document: lsp::TextDocumentIdentifier { + uri: url!("./node_modules/preact/package.json"), + }, + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 7, + }, + end: lsp::Position { + line: 0, + character: 7, + }, + }, + context: lsp::CodeActionContext { + diagnostics: vec![], + ..Default::default() + }, + work_done_progress_params: lsp::WorkDoneProgressParams { + work_done_token: None, + }, + partial_result_params: lsp::PartialResultParams { + partial_result_token: None, + }, + }, + ) + .await? + .context("codeAction returned None")?; + + assert_eq!(res, vec![]); + + server.close_document().await?; + + server.shutdown().await?; + reader.abort(); + + Ok(()) +} + #[tokio::test] async fn pull_refactors() -> Result<()> { let factory = ServerFactory::default(); diff --git a/crates/rome_service/src/file_handlers/javascript.rs b/crates/rome_service/src/file_handlers/javascript.rs index 6d7e41a1a97..9e4b9e77963 100644 --- a/crates/rome_service/src/file_handlers/javascript.rs +++ b/crates/rome_service/src/file_handlers/javascript.rs @@ -38,7 +38,7 @@ use rome_rowan::{AstNode, BatchMutationExt, Direction, NodeCache}; use std::borrow::Cow; use std::fmt::Debug; use std::path::PathBuf; -use tracing::debug; +use tracing::{debug, trace}; #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] @@ -314,6 +314,7 @@ impl RegistryVisitor for ActionsVisitor<'_> { } } +#[tracing::instrument(level = "debug", skip(parse))] fn code_actions( parse: AnyParse, range: TextRange, @@ -325,27 +326,36 @@ fn code_actions( let mut actions = Vec::new(); - let enabled_rules: Option> = if let Some(rules) = rules { - let enabled_rules = rules.as_enabled_rules().into_iter().collect(); + let mut enabled_rules = vec![]; + if settings.as_ref().organize_imports.enabled { + enabled_rules.push(RuleFilter::Rule("correctness", "organizeImports")); + } + if let Some(rules) = rules { + let rules = rules.as_enabled_rules().into_iter().collect(); // The rules in the assist category do not have configuration entries, // always add them all to the enabled rules list - let mut visitor = ActionsVisitor { enabled_rules }; + let mut visitor = ActionsVisitor { + enabled_rules: rules, + }; visit_registry(&mut visitor); - Some(visitor.enabled_rules) - } else { - None - }; + enabled_rules.extend(visitor.enabled_rules); + } - let mut filter = match &enabled_rules { - Some(rules) => AnalysisFilter::from_enabled_rules(Some(rules.as_slice())), - _ => AnalysisFilter::default(), + let mut filter = if !enabled_rules.is_empty() { + AnalysisFilter::from_enabled_rules(Some(enabled_rules.as_slice())) + } else { + AnalysisFilter::default() }; filter.categories = RuleCategories::SYNTAX | RuleCategories::LINT; + if settings.as_ref().organize_imports.enabled { + filter.categories |= RuleCategories::ACTION; + } filter.range = Some(range); + trace!("Filter applied for code actions: {:?}", &filter); let analyzer_options = compute_analyzer_options(&settings, PathBuf::from(path.as_path())); analyze( diff --git a/crates/rome_service/src/file_handlers/mod.rs b/crates/rome_service/src/file_handlers/mod.rs index f91a998fa37..82882f82687 100644 --- a/crates/rome_service/src/file_handlers/mod.rs +++ b/crates/rome_service/src/file_handlers/mod.rs @@ -152,7 +152,7 @@ pub struct FixAllParams<'a> { #[derive(Default)] /// The list of capabilities that are available for a language -pub(crate) struct Capabilities { +pub struct Capabilities { pub(crate) parser: ParserCapabilities, pub(crate) debug: DebugCapabilities, pub(crate) analyzer: AnalyzerCapabilities, @@ -162,7 +162,7 @@ pub(crate) struct Capabilities { type Parse = fn(&RomePath, Language, &str, &mut NodeCache) -> AnyParse; #[derive(Default)] -pub(crate) struct ParserCapabilities { +pub struct ParserCapabilities { /// Parse a file pub(crate) parse: Option, } @@ -172,7 +172,7 @@ type DebugControlFlow = fn(AnyParse, TextSize) -> String; type DebugFormatterIR = fn(&RomePath, AnyParse, SettingsHandle) -> Result; #[derive(Default)] -pub(crate) struct DebugCapabilities { +pub struct DebugCapabilities { /// Prints the syntax tree pub(crate) debug_syntax_tree: Option, /// Prints the control flow graph @@ -204,7 +204,7 @@ type Rename = fn(&RomePath, AnyParse, TextSize, String) -> Result Result; #[derive(Default)] -pub(crate) struct AnalyzerCapabilities { +pub struct AnalyzerCapabilities { /// It lints a file pub(crate) lint: Option, /// It extracts code actions for a file diff --git a/crates/rome_service/src/workspace.rs b/crates/rome_service/src/workspace.rs index b2eda532968..17f3de4ca2f 100644 --- a/crates/rome_service/src/workspace.rs +++ b/crates/rome_service/src/workspace.rs @@ -51,6 +51,7 @@ //! document does not implement the required capability: for instance trying to //! format a file with a language that does not have a formatter +use crate::file_handlers::Capabilities; use crate::{Configuration, Deserialize, Serialize, WorkspaceError}; use rome_analyze::ActionCategory; pub use rome_analyze::RuleCategories; @@ -60,10 +61,12 @@ use rome_formatter::Printed; use rome_fs::RomePath; use rome_js_syntax::{TextRange, TextSize}; use rome_text_edit::TextEdit; +use std::collections::HashMap; use std::{borrow::Cow, panic::RefUnwindSafe, sync::Arc}; pub use self::client::{TransportRequest, WorkspaceClient, WorkspaceTransport}; pub use crate::file_handlers::Language; +use crate::settings::WorkspaceSettings; mod client; mod server; @@ -72,37 +75,98 @@ mod server; #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub struct SupportsFeatureParams { pub path: RomePath, - pub feature: FeatureName, + pub feature: Vec, } #[derive(Debug, serde::Serialize, serde::Deserialize, Default)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub struct SupportsFeatureResult { - pub reason: Option, + pub reason: Option, } -impl SupportsFeatureResult { - const fn ignored() -> Self { +#[derive(Debug, serde::Serialize, serde::Deserialize, Default)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct FileFeaturesResult { + pub features_supported: HashMap, +} + +impl FileFeaturesResult { + /// By default, all features are not supported by a file. + const WORKSPACE_FEATURES: [(FeatureName, SupportKind); 3] = [ + (FeatureName::Lint, SupportKind::FileNotSupported), + (FeatureName::Format, SupportKind::FileNotSupported), + (FeatureName::OrganizeImports, SupportKind::FileNotSupported), + ]; + + pub fn new() -> Self { Self { - reason: Some(UnsupportedReason::Ignored), + features_supported: HashMap::from(FileFeaturesResult::WORKSPACE_FEATURES), } } - const fn disabled() -> Self { - Self { - reason: Some(UnsupportedReason::FeatureNotEnabled), + pub fn with_capabilities(mut self, capabilities: &Capabilities) -> Self { + if capabilities.formatter.format.is_some() { + self.features_supported + .insert(FeatureName::Format, SupportKind::Supported); + } + if capabilities.analyzer.lint.is_some() { + self.features_supported + .insert(FeatureName::Lint, SupportKind::Supported); + } + if capabilities.analyzer.organize_imports.is_some() { + self.features_supported + .insert(FeatureName::OrganizeImports, SupportKind::Supported); } + + self } - const fn file_not_supported() -> Self { - Self { - reason: Some(UnsupportedReason::FileNotSupported), + pub fn with_settings(mut self, settings: &WorkspaceSettings) -> Self { + if !settings.formatter().enabled { + self.features_supported + .insert(FeatureName::Format, SupportKind::FeatureNotEnabled); + } + if !settings.linter().enabled { + self.features_supported + .insert(FeatureName::Lint, SupportKind::FeatureNotEnabled); } + if !settings.organize_imports().enabled { + self.features_supported + .insert(FeatureName::OrganizeImports, SupportKind::FeatureNotEnabled); + } + + self + } + + pub fn ignored(&mut self, feature: FeatureName) { + self.features_supported + .insert(feature, SupportKind::Ignored); + } + + /// Checks whether the file support the given `feature` + pub fn supports_for(&self, feature: &FeatureName) -> bool { + self.features_supported + .get(feature) + .map(|support_kind| matches!(support_kind, SupportKind::Supported)) + .unwrap_or_default() } + pub fn is_ignored_for(&self, feature: &FeatureName) -> bool { + self.features_supported + .get(feature) + .map(|support_kind| matches!(support_kind, SupportKind::Ignored)) + .unwrap_or_default() + } + + pub fn support_kind_for(&self, feature: &FeatureName) -> Option<&SupportKind> { + self.features_supported.get(feature) + } +} + +impl SupportsFeatureResult { /// Whether the feature is intentionally disabled pub const fn is_not_enabled(&self) -> bool { - matches!(self.reason, Some(UnsupportedReason::FeatureNotEnabled)) + matches!(self.reason, Some(SupportKind::FeatureNotEnabled)) } /// Whether the feature is supported @@ -118,19 +182,27 @@ impl SupportsFeatureResult { #[derive(Debug, serde::Serialize, serde::Deserialize, Eq, PartialEq)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] -pub enum UnsupportedReason { +pub enum SupportKind { + /// The feature is enabled for the file + Supported, + /// The file is ignored (configuration) Ignored, + /// The feature is not enabled (configuration or the file doesn't need it) FeatureNotEnabled, + /// The file is not capable of having this feature FileNotSupported, } -impl UnsupportedReason { +impl SupportKind { + pub const fn is_supported(&self) -> bool { + matches!(self, SupportKind::Supported) + } pub const fn is_not_enabled(&self) -> bool { - matches!(self, UnsupportedReason::FeatureNotEnabled) + matches!(self, SupportKind::FeatureNotEnabled) } } -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, Hash, serde::Serialize, serde::Deserialize, Eq, PartialEq)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub enum FeatureName { Format, @@ -138,6 +210,32 @@ pub enum FeatureName { OrganizeImports, } +#[derive(Debug, Default)] +pub struct FeaturesBuilder(Vec); + +impl FeaturesBuilder { + pub fn new() -> Self { + Self::default() + } + + pub fn with_formatter(mut self) -> Self { + self.0.push(FeatureName::Format); + self + } + pub fn with_linter(mut self) -> Self { + self.0.push(FeatureName::Lint); + self + } + pub fn with_organize_imports(mut self) -> Self { + self.0.push(FeatureName::OrganizeImports); + self + } + + pub fn build(self) -> Vec { + self.0 + } +} + #[derive(Debug, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub struct UpdateSettingsParams { @@ -390,10 +488,10 @@ pub trait Workspace: Send + Sync + RefUnwindSafe { /// - Rome doesn't recognize a file, so it can't provide the feature; /// - the feature is disabled inside the configuration; /// - the file is ignored - fn supports_feature( + fn file_features( &self, params: SupportsFeatureParams, - ) -> Result; + ) -> Result; /// Checks if the current path is ignored by the workspace, against a particular feature. /// diff --git a/crates/rome_service/src/workspace/client.rs b/crates/rome_service/src/workspace/client.rs index f6f55e73dd7..48347a9b10d 100644 --- a/crates/rome_service/src/workspace/client.rs +++ b/crates/rome_service/src/workspace/client.rs @@ -1,6 +1,6 @@ use crate::workspace::{ - GetFileContentParams, IsPathIgnoredParams, OrganizeImportsParams, OrganizeImportsResult, - RageParams, RageResult, ServerInfo, SupportsFeatureResult, + FileFeaturesResult, GetFileContentParams, IsPathIgnoredParams, OrganizeImportsParams, + OrganizeImportsResult, RageParams, RageResult, ServerInfo, }; use crate::{TransportError, Workspace, WorkspaceError}; use rome_formatter::Printed; @@ -100,11 +100,11 @@ impl Workspace for WorkspaceClient where T: WorkspaceTransport + RefUnwindSafe + Send + Sync, { - fn supports_feature( + fn file_features( &self, params: SupportsFeatureParams, - ) -> Result { - self.request("rome/supports_feature", params) + ) -> Result { + self.request("rome/file_features", params) } fn is_path_ignored(&self, params: IsPathIgnoredParams) -> Result { diff --git a/crates/rome_service/src/workspace/server.rs b/crates/rome_service/src/workspace/server.rs index c358067cad0..6e3443fae4c 100644 --- a/crates/rome_service/src/workspace/server.rs +++ b/crates/rome_service/src/workspace/server.rs @@ -7,8 +7,8 @@ use super::{ }; use crate::file_handlers::{Capabilities, FixAllParams, Language, LintParams}; use crate::workspace::{ - GetFileContentParams, IsPathIgnoredParams, OrganizeImportsParams, OrganizeImportsResult, - RageEntry, RageParams, RageResult, ServerInfo, SupportsFeatureResult, + FileFeaturesResult, GetFileContentParams, IsPathIgnoredParams, OrganizeImportsParams, + OrganizeImportsResult, RageEntry, RageParams, RageResult, ServerInfo, }; use crate::{ file_handlers::Features, @@ -25,6 +25,7 @@ use rome_parser::AnyParse; use rome_rowan::NodeCache; use std::ffi::OsStr; use std::{panic::RefUnwindSafe, sync::RwLock}; +use tracing::trace; pub(super) struct WorkspaceServer { /// features available throughout the application @@ -189,52 +190,27 @@ impl WorkspaceServer { } impl Workspace for WorkspaceServer { - fn supports_feature( + fn file_features( &self, params: SupportsFeatureParams, - ) -> Result { + ) -> Result { let capabilities = self.get_capabilities(¶ms.path); let settings = self.settings.read().unwrap(); - 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 { - SupportsFeatureResult::ignored() - } else if capabilities.formatter.format.is_none() { - SupportsFeatureResult::file_not_supported() - } else if !settings.formatter().enabled { - SupportsFeatureResult::disabled() - } else { - SupportsFeatureResult { reason: None } - } + let mut file_features = FileFeaturesResult::new() + .with_capabilities(&capabilities) + .with_settings(&settings); + + for feature in params.feature { + let is_ignored = self.is_path_ignored(IsPathIgnoredParams { + rome_path: params.path.clone(), + feature: feature.clone(), + })?; + + if is_ignored { + file_features.ignored(feature); } - FeatureName::Lint => { - if is_ignored { - SupportsFeatureResult::ignored() - } else if capabilities.analyzer.lint.is_none() { - SupportsFeatureResult::file_not_supported() - } else if !settings.linter().enabled { - SupportsFeatureResult::disabled() - } else { - SupportsFeatureResult { reason: None } - } - } - FeatureName::OrganizeImports => { - if is_ignored { - SupportsFeatureResult::ignored() - } else if capabilities.analyzer.organize_imports.is_none() { - SupportsFeatureResult::file_not_supported() - } else if !settings.organize_imports().enabled { - SupportsFeatureResult::disabled() - } else { - SupportsFeatureResult { reason: None } - } - } - }; - Ok(result) + } + Ok(file_features) } fn is_path_ignored(&self, params: IsPathIgnoredParams) -> Result { @@ -400,10 +376,15 @@ impl Workspace for WorkspaceServer { self.get_capabilities(¶ms.path).analyzer.lint { let rules = settings.linter().rules.as_ref(); - let rule_filter_list = self.build_rule_filter_list(rules); + let mut rule_filter_list = self.build_rule_filter_list(rules); + if settings.organize_imports.enabled { + rule_filter_list.push(RuleFilter::Rule("correctness", "organizeImports")); + } let mut filter = AnalysisFilter::from_enabled_rules(Some(rule_filter_list.as_slice())); filter.categories = params.categories; + trace!("Analyzer filter to apply to lint: {:?}", &filter); + let results = lint(LintParams { parse, filter, diff --git a/crates/rome_service/src/workspace_types.rs b/crates/rome_service/src/workspace_types.rs index cc1c5434cd5..0a0032e425d 100644 --- a/crates/rome_service/src/workspace_types.rs +++ b/crates/rome_service/src/workspace_types.rs @@ -453,7 +453,7 @@ macro_rules! workspace_method { /// Returns a list of signature for all the methods in the [Workspace] trait pub fn methods() -> [WorkspaceMethod; 16] { [ - WorkspaceMethod::of::("supports_feature"), + WorkspaceMethod::of::("file_features"), workspace_method!(update_settings), workspace_method!(open_file), workspace_method!(change_file), diff --git a/crates/rome_wasm/src/lib.rs b/crates/rome_wasm/src/lib.rs index 0e1a5d44b72..b2ad6d416e0 100644 --- a/crates/rome_wasm/src/lib.rs +++ b/crates/rome_wasm/src/lib.rs @@ -36,14 +36,14 @@ impl Workspace { } } - #[wasm_bindgen(js_name = supportsFeature)] - pub fn supports_feature( + #[wasm_bindgen(js_name = fileFeatures)] + pub fn file_features( &self, params: ISupportsFeatureParams, ) -> Result { let params: SupportsFeatureParams = serde_wasm_bindgen::from_value(params.into()).map_err(into_error)?; - let result = self.inner.supports_feature(params).map_err(into_error)?; + let result = self.inner.file_features(params).map_err(into_error)?; to_value(&result) .map(ISupportsFeatureResult::from) .map_err(into_error) diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index 1165e8dbb44..aa35df353c5 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -1,7 +1,7 @@ // Generated file, do not edit by hand, see `xtask/codegen` import type { Transport } from "./transport"; export interface SupportsFeatureParams { - feature: FeatureName; + feature: FeatureName[]; path: RomePath; } export type FeatureName = "Format" | "Lint" | "OrganizeImports"; @@ -9,9 +9,10 @@ export interface RomePath { path: string; } export interface SupportsFeatureResult { - reason?: UnsupportedReason; + reason?: SupportKind; } -export type UnsupportedReason = +export type SupportKind = + | "Supported" | "Ignored" | "FeatureNotEnabled" | "FileNotSupported"; @@ -1333,9 +1334,7 @@ export interface RenameResult { range: TextRange; } export interface Workspace { - supportsFeature( - params: SupportsFeatureParams, - ): Promise; + fileFeatures(params: SupportsFeatureParams): Promise; updateSettings(params: UpdateSettingsParams): Promise; openFile(params: OpenFileParams): Promise; changeFile(params: ChangeFileParams): Promise; @@ -1357,8 +1356,8 @@ export interface Workspace { } export function createWorkspace(transport: Transport): Workspace { return { - supportsFeature(params) { - return transport.request("rome/supports_feature", params); + fileFeatures(params) { + return transport.request("rome/file_features", params); }, updateSettings(params) { return transport.request("rome/update_settings", params);