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

Commit

Permalink
fix(rome_lsp): don't throw error for ignored files in code actions (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored May 8, 2023
1 parent 06a7fdd commit 1d86eec
Show file tree
Hide file tree
Showing 28 changed files with 520 additions and 306 deletions.
10 changes: 7 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 17 additions & 11 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"


2 changes: 1 addition & 1 deletion crates/rome_analyze/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
1 change: 1 addition & 0 deletions crates/rome_analyze/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion crates/rome_cli/src/execute/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
135 changes: 91 additions & 44 deletions crates/rome_cli/src/execute/process_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,24 @@ 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),
Ignored,
}

/// 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
Expand All @@ -38,6 +41,7 @@ pub(crate) enum Message {
},
}

#[derive(Debug)]
pub(crate) enum DiffKind {
Format,
OrganizeImports,
Expand All @@ -46,6 +50,7 @@ pub(crate) enum DiffKind {
impl<D> From<D> for Message
where
Error: From<D>,
D: std::fmt::Debug,
{
fn from(err: D) -> Self {
Self::Error(Error::from(err))
Expand All @@ -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 => {}
};
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
Loading

0 comments on commit 1d86eec

Please sign in to comment.