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

Commit

Permalink
feat(rome_service): apply formatter to code actions
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Apr 4, 2023
1 parent 0fd72a3 commit b0a7927
Show file tree
Hide file tree
Showing 15 changed files with 83 additions and 17 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,17 @@
- Fix an issue where formatting of JSX string literals property values were using incorrect quotes [#4054](https://github.com/rome/tools/issues/4054)

### Linter

#### Other changes
- Code action are formatted using Rome's formatter. If the formatter is disabled,
the code action is not formatted.

#### New rules
- [`noConfusingArrow`](https://docs.rome.tools/lint/rules/noConfusingArrow/)
- [`noRedundantRoles`](https://docs.rome.tools/lint/rules/noRedundantRoles/)
- [`noNoninteractiveTabindex`](https://docs.rome.tools/lint/rules/noNoninteractiveTabindex/)
- [`noAriaUnsupportedElements`](https://docs.rome.tools/lint/rules/noAriaUnsupportedElements/)

### Parser

- Allow module syntax in `cts` files
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions crates/rome_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,5 @@ tikv-jemallocator = "0.5.0"
insta = { workspace = true }
tokio = { workspace = true, features = ["io-util"] }
rome_json_formatter = { path = "../rome_json_formatter" }
rome_js_formatter = { path = "../rome_js_formatter" }
rome_json_parser = { path = "../rome_json_parser" }
11 changes: 5 additions & 6 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ const NEW_SYMBOL: &str = "new Symbol(\"\");";
const FIX_BEFORE: &str = "
(1 >= -0)
";
const FIX_AFTER: &str = "
(1 >= 0)
const FIX_AFTER: &str = "1 >= 0;
";

const APPLY_SUGGESTED_BEFORE: &str = "let a = 4;
Expand All @@ -49,8 +48,8 @@ console.log(a);

const APPLY_SUGGESTED_AFTER: &str = "const a = 4;\nconsole.log(a);\n";

const NO_DEBUGGER_BEFORE: &str = "debugger;";
const NO_DEBUGGER_AFTER: &str = "debugger;";
const NO_DEBUGGER_BEFORE: &str = "debugger;\n";
const NO_DEBUGGER_AFTER: &str = "debugger;\n";

const UPGRADE_SEVERITY_CODE: &str = r#"if(!cond) { exprA(); } else { exprB() }"#;

Expand Down Expand Up @@ -332,7 +331,7 @@ function f() { arguments; }

let expected = "const a = 4;
console.log(a);
function f() { arguments; }
function f() {\n\targuments;\n}
";

let test1 = Path::new("test1.js");
Expand Down Expand Up @@ -533,7 +532,7 @@ fn should_disable_a_rule_group() {
.read_to_string(&mut buffer)
.unwrap();

assert_eq!(buffer, FIX_BEFORE);
assert_eq!(buffer, "1 >= -0;\n");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ expression: content
## `fix.js`

```js

(1 >= 0)
1 >= 0;

```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ expression: content
## `fix.js`

```js

(1 >= 0)
1 >= 0;
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ expression: content
```js
const a = 4;
console.log(a);
function f() { arguments; }
function f() {
arguments;
}

```

Expand All @@ -16,7 +18,9 @@ function f() { arguments; }
```js
const a = 4;
console.log(a);
function f() { arguments; }
function f() {
arguments;
}

```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ expression: content

```js
debugger;

```

# Emitted Messages
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ expression: content
## `fix.js`

```js

(1 >= -0)
1 >= -0;

```

Expand Down
24 changes: 24 additions & 0 deletions crates/rome_formatter/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::prelude::TagKind;
use rome_console::fmt::Formatter;
use rome_console::markup;
use rome_diagnostics::{category, Category, Diagnostic, DiagnosticTags, Location, Severity};
use rome_rowan::{SyntaxError, TextRange};
use std::error::Error;
Expand Down Expand Up @@ -204,6 +206,7 @@ impl std::fmt::Display for InvalidDocumentError {
}

#[derive(Debug, Clone, Eq, PartialEq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum PrintError {
InvalidDocument(InvalidDocumentError),
}
Expand All @@ -220,6 +223,27 @@ impl std::fmt::Display for PrintError {
}
}

impl Diagnostic for PrintError {
fn category(&self) -> Option<&'static Category> {
Some(category!("format"))
}

fn severity(&self) -> Severity {
Severity::Error
}

fn message(&self, fmt: &mut Formatter<'_>) -> std::io::Result<()> {
match self {
PrintError::InvalidDocument(inner) => {
let inner = format!("{}", inner);
fmt.write_markup(markup! {
"Invalid document: "{{inner}}
})
}
}
}
}

#[cfg(test)]
mod test {
use crate::diagnostics::{ActualStart, InvalidDocumentError};
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_lsp/tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,7 @@ async fn pull_fix_all() -> Result<()> {
character: 0,
},
},
new_text: String::from("if(a === 0) {}\nif(a === 0) {}\nif(a === 0) {}"),
new_text: String::from("if (a === 0) {\n}\nif (a === 0) {\n}\nif (a === 0) {\n}\n"),
}],
);

Expand Down
17 changes: 16 additions & 1 deletion crates/rome_service/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::ConfigurationDiagnostic;
use rome_console::fmt::Bytes;
use rome_console::markup;
use rome_diagnostics::{category, Category, Diagnostic, DiagnosticTags, Location, Severity};
use rome_formatter::FormatError;
use rome_formatter::{FormatError, PrintError};
use rome_js_analyze::utils::rename::RenameError;
use rome_js_analyze::RuleError;
use serde::{Deserialize, Serialize};
Expand All @@ -26,6 +26,8 @@ pub enum WorkspaceError {
SourceFileNotSupported(SourceFileNotSupported),
/// The formatter encountered an error while formatting the file
FormatError(FormatError),
/// The formatter encountered an error while formatting the file
PrintError(PrintError),
/// The file could not be formatted since it has syntax errors and `format_with_errors` is disabled
FormatWithErrorsDisabled(FormatWithErrorsDisabled),
/// The file could not be analyzed because a rule caused an error.
Expand Down Expand Up @@ -111,6 +113,7 @@ impl Diagnostic for WorkspaceError {
match self {
WorkspaceError::FormatWithErrorsDisabled(error) => error.category(),
WorkspaceError::FormatError(err) => err.category(),
WorkspaceError::PrintError(err) => err.category(),
WorkspaceError::RuleError(error) => error.category(),
WorkspaceError::Configuration(error) => error.category(),
WorkspaceError::RenameError(error) => error.category(),
Expand All @@ -130,6 +133,7 @@ impl Diagnostic for WorkspaceError {
match self {
WorkspaceError::FormatWithErrorsDisabled(error) => error.description(fmt),
WorkspaceError::FormatError(error) => Diagnostic::description(error, fmt),
WorkspaceError::PrintError(error) => Diagnostic::description(error, fmt),
WorkspaceError::RuleError(error) => Diagnostic::description(error, fmt),
WorkspaceError::Configuration(error) => error.description(fmt),
WorkspaceError::RenameError(error) => error.description(fmt),
Expand All @@ -149,6 +153,7 @@ impl Diagnostic for WorkspaceError {
match self {
WorkspaceError::FormatWithErrorsDisabled(error) => error.message(fmt),
WorkspaceError::FormatError(err) => err.message(fmt),
WorkspaceError::PrintError(err) => err.message(fmt),
WorkspaceError::RuleError(error) => error.message(fmt),
WorkspaceError::Configuration(error) => error.message(fmt),
WorkspaceError::RenameError(error) => error.message(fmt),
Expand All @@ -167,6 +172,7 @@ impl Diagnostic for WorkspaceError {
fn severity(&self) -> Severity {
match self {
WorkspaceError::FormatError(err) => err.severity(),
WorkspaceError::PrintError(err) => err.severity(),
WorkspaceError::RuleError(error) => error.severity(),
WorkspaceError::Configuration(error) => error.severity(),
WorkspaceError::RenameError(error) => error.severity(),
Expand All @@ -186,6 +192,7 @@ impl Diagnostic for WorkspaceError {
fn tags(&self) -> DiagnosticTags {
match self {
WorkspaceError::FormatError(err) => err.tags(),
WorkspaceError::PrintError(err) => err.tags(),
WorkspaceError::RuleError(error) => error.tags(),
WorkspaceError::Configuration(error) => error.tags(),
WorkspaceError::RenameError(error) => error.tags(),
Expand All @@ -205,6 +212,7 @@ impl Diagnostic for WorkspaceError {
fn location(&self) -> Location<'_> {
match self {
WorkspaceError::FormatError(err) => err.location(),
WorkspaceError::PrintError(err) => err.location(),
WorkspaceError::RuleError(error) => error.location(),
WorkspaceError::Configuration(error) => error.location(),
WorkspaceError::RenameError(error) => error.location(),
Expand All @@ -224,6 +232,7 @@ impl Diagnostic for WorkspaceError {
fn source(&self) -> Option<&dyn Diagnostic> {
match self {
WorkspaceError::FormatError(error) => Diagnostic::source(error),
WorkspaceError::PrintError(error) => Diagnostic::source(error),
WorkspaceError::RuleError(error) => Diagnostic::source(error),
WorkspaceError::Configuration(error) => Diagnostic::source(error),
WorkspaceError::RenameError(error) => Diagnostic::source(error),
Expand Down Expand Up @@ -253,6 +262,12 @@ impl From<TransportError> for WorkspaceError {
}
}

impl From<PrintError> for WorkspaceError {
fn from(err: PrintError) -> Self {
Self::PrintError(err)
}
}

#[derive(Debug, Serialize, Deserialize, Diagnostic)]
#[diagnostic(
category = "internalError/fs",
Expand Down
14 changes: 13 additions & 1 deletion crates/rome_service/src/file_handlers/javascript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
rules,
fix_file_mode,
settings,
should_format,
rome_path,
} = params;

let mut tree: AnyJsRoot = parse.tree();
Expand Down Expand Up @@ -451,8 +453,18 @@ fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
}
}
None => {
let code = if should_format {
format_node(
settings.format_options::<JsLanguage>(rome_path),
tree.syntax(),
)?
.print()?
.into_code()
} else {
tree.syntax().to_string()
};
return Ok(FixFileResult {
code: tree.syntax().to_string(),
code,
skipped_suggested_fixes,
actions,
errors: errors.into(),
Expand Down
3 changes: 3 additions & 0 deletions crates/rome_service/src/file_handlers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ pub struct FixAllParams<'a> {
pub(crate) rules: Option<&'a Rules>,
pub(crate) fix_file_mode: FixFileMode,
pub(crate) settings: SettingsHandle<'a>,
/// Whether it should format the code action
pub(crate) should_format: bool,
pub(crate) rome_path: &'a RomePath,
}

#[derive(Default)]
Expand Down
3 changes: 3 additions & 0 deletions crates/rome_service/src/workspace/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,11 +516,14 @@ impl Workspace for WorkspaceServer {
let parse = self.get_parse(params.path.clone(), Some(FeatureName::Lint))?;

let rules = settings.linter().rules.as_ref();
let should_format = settings.formatter().enabled;
fix_all(FixAllParams {
parse,
rules,
fix_file_mode: params.fix_file_mode,
settings: self.settings(),
should_format,
rome_path: &params.path,
})
}

Expand Down

0 comments on commit b0a7927

Please sign in to comment.