Skip to content

Commit 8338db6

Browse files
snowsignalzanieb
andauthored
ruff server: Formatting a document with syntax problems no longer spams a visible error popup (#11745)
## Summary Fixes astral-sh/ruff-vscode#482. I've made adjustments to `format` and `format_range` that handle parsing errors before they become server errors. We'll still log this as a problem, but there will no longer be a visible popup. ## Test Plan Instead of seeing a visible error when formatting a document with syntax issues, you should see this warning in the LSP logs: <img width="991" alt="Screenshot 2024-06-04 at 3 38 23 PM" src="https://github.com/astral-sh/ruff/assets/19577865/9d68947d-6462-4ca6-ab5a-65e573c91db6"> Similarly, if you try to format a range with syntax issues, you should see this warning in the LSP logs instead of a visible error popup: <img width="1010" alt="Screenshot 2024-06-04 at 3 39 10 PM" src="https://github.com/astral-sh/ruff/assets/19577865/99fff098-798d-406a-976e-81ead0da0352"> --------- Co-authored-by: Zanie Blue <[email protected]>
1 parent d056d09 commit 8338db6

File tree

3 files changed

+48
-22
lines changed

3 files changed

+48
-22
lines changed

crates/ruff_server/src/format.rs

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use ruff_formatter::PrintedRange;
22
use ruff_python_ast::PySourceType;
3-
use ruff_python_formatter::format_module_source;
3+
use ruff_python_formatter::{format_module_source, FormatModuleError};
44
use ruff_text_size::TextRange;
55
use ruff_workspace::FormatterSettings;
66

@@ -10,23 +10,49 @@ pub(crate) fn format(
1010
document: &TextDocument,
1111
source_type: PySourceType,
1212
formatter_settings: &FormatterSettings,
13-
) -> crate::Result<String> {
13+
) -> crate::Result<Option<String>> {
1414
let format_options = formatter_settings.to_format_options(source_type, document.contents());
15-
let formatted = format_module_source(document.contents(), format_options)?;
16-
Ok(formatted.into_code())
15+
match format_module_source(document.contents(), format_options) {
16+
Ok(formatted) => {
17+
let formatted = formatted.into_code();
18+
if formatted == document.contents() {
19+
Ok(None)
20+
} else {
21+
Ok(Some(formatted))
22+
}
23+
}
24+
// Special case - syntax/parse errors are handled here instead of
25+
// being propagated as visible server errors.
26+
Err(FormatModuleError::ParseError(error)) => {
27+
tracing::warn!("Unable to format document: {error}");
28+
Ok(None)
29+
}
30+
Err(err) => Err(err.into()),
31+
}
1732
}
1833

1934
pub(crate) fn format_range(
2035
document: &TextDocument,
2136
source_type: PySourceType,
2237
formatter_settings: &FormatterSettings,
2338
range: TextRange,
24-
) -> crate::Result<PrintedRange> {
39+
) -> crate::Result<Option<PrintedRange>> {
2540
let format_options = formatter_settings.to_format_options(source_type, document.contents());
2641

27-
Ok(ruff_python_formatter::format_range(
28-
document.contents(),
29-
range,
30-
format_options,
31-
)?)
42+
match ruff_python_formatter::format_range(document.contents(), range, format_options) {
43+
Ok(formatted) => {
44+
if formatted.as_code() == document.contents() {
45+
Ok(None)
46+
} else {
47+
Ok(Some(formatted))
48+
}
49+
}
50+
// Special case - syntax/parse errors are handled here instead of
51+
// being propagated as visible server errors.
52+
Err(FormatModuleError::ParseError(error)) => {
53+
tracing::warn!("Unable to format document range: {error}");
54+
Ok(None)
55+
}
56+
Err(err) => Err(err.into()),
57+
}
3258
}

crates/ruff_server/src/server/api/requests/format.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,11 @@ fn format_text_document(
9898
}
9999

100100
let source = text_document.contents();
101-
let mut formatted =
102-
crate::format::format(text_document, query.source_type(), formatter_settings)
103-
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
104-
// fast path - if the code is the same, return early
105-
if formatted == source {
101+
let formatted = crate::format::format(text_document, query.source_type(), formatter_settings)
102+
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
103+
let Some(mut formatted) = formatted else {
106104
return Ok(None);
107-
}
105+
};
108106

109107
// special case - avoid adding a newline to a notebook cell if it didn't already exist
110108
if is_notebook {

crates/ruff_server/src/server/api/requests/format_range.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,12 @@ fn format_text_document_range(
7373
)
7474
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
7575

76-
Ok(Some(vec![types::TextEdit {
77-
range: formatted_range
78-
.source_range()
79-
.to_range(text, index, encoding),
80-
new_text: formatted_range.into_code(),
81-
}]))
76+
Ok(formatted_range.map(|formatted_range| {
77+
vec![types::TextEdit {
78+
range: formatted_range
79+
.source_range()
80+
.to_range(text, index, encoding),
81+
new_text: formatted_range.into_code(),
82+
}]
83+
}))
8284
}

0 commit comments

Comments
 (0)