Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ruff server: Formatting a document with syntax problems no longer spams a visible error popup #11745

Merged
merged 3 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 36 additions & 10 deletions crates/ruff_server/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ruff_formatter::PrintedRange;
use ruff_python_ast::PySourceType;
use ruff_python_formatter::format_module_source;
use ruff_python_formatter::{format_module_source, FormatModuleError};
use ruff_text_size::TextRange;
use ruff_workspace::FormatterSettings;

Expand All @@ -10,23 +10,49 @@ pub(crate) fn format(
document: &TextDocument,
source_type: PySourceType,
formatter_settings: &FormatterSettings,
) -> crate::Result<String> {
) -> crate::Result<Option<String>> {
let format_options = formatter_settings.to_format_options(source_type, document.contents());
let formatted = format_module_source(document.contents(), format_options)?;
Ok(formatted.into_code())
match format_module_source(document.contents(), format_options) {
Ok(formatted) => {
let formatted = formatted.into_code();
if formatted == document.contents() {
Ok(None)
} else {
Ok(Some(formatted))
}
}
// Special case - syntax/parse errors are handled here instead of
// being propagated as visible server errors.
Err(FormatModuleError::ParseError(error)) => {
tracing::warn!("Unable to format document: {error}");
Ok(None)
}
Err(err) => Err(err.into()),
}
}

pub(crate) fn format_range(
document: &TextDocument,
source_type: PySourceType,
formatter_settings: &FormatterSettings,
range: TextRange,
) -> crate::Result<PrintedRange> {
) -> crate::Result<Option<PrintedRange>> {
let format_options = formatter_settings.to_format_options(source_type, document.contents());

Ok(ruff_python_formatter::format_range(
document.contents(),
range,
format_options,
)?)
match ruff_python_formatter::format_range(document.contents(), range, format_options) {
Ok(formatted) => {
if formatted.as_code() == document.contents() {
Ok(None)
} else {
Ok(Some(formatted))
}
}
// Special case - syntax/parse errors are handled here instead of
// being propagated as visible server errors.
Err(FormatModuleError::ParseError(error)) => {
tracing::warn!("Unable to format document range: {error}");
Ok(None)
}
Err(err) => Err(err.into()),
}
}
10 changes: 4 additions & 6 deletions crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,11 @@ fn format_text_document(
}

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

// special case - avoid adding a newline to a notebook cell if it didn't already exist
if is_notebook {
Expand Down
14 changes: 8 additions & 6 deletions crates/ruff_server/src/server/api/requests/format_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,12 @@ fn format_text_document_range(
)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;

Ok(Some(vec![types::TextEdit {
range: formatted_range
.source_range()
.to_range(text, index, encoding),
new_text: formatted_range.into_code(),
}]))
Ok(formatted_range.map(|formatted_range| {
vec![types::TextEdit {
range: formatted_range
.source_range()
.to_range(text, index, encoding),
new_text: formatted_range.into_code(),
}]
}))
}
Loading