diff --git a/crates/ruff_server/src/fix.rs b/crates/ruff_server/src/fix.rs index 82dd1654cc..04b6bee64e 100644 --- a/crates/ruff_server/src/fix.rs +++ b/crates/ruff_server/src/fix.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; -use ruff_python_ast::SourceType; +use ruff_python_ast::{SourceType, TomlSourceType}; use rustc_hash::FxHashMap; use crate::{ @@ -14,6 +14,7 @@ use ruff_linter::{ linter::FixerResult, packaging::detect_package_root, settings::{LinterSettings, flags}, + toml::lint_fix_toml, }; use ruff_notebook::SourceValue; use ruff_source_file::LineIndex; @@ -30,11 +31,6 @@ pub(crate) fn fix_all( let settings = query.settings(); let document_path = query.virtual_file_path(); - let SourceType::Python(source_type) = query.source_type_for_lint() else { - return Ok(Fixes::default()); - }; - let source_kind = query.make_python_source_kind(source_type); - // If the document is excluded, return an empty list of fixes. if is_document_excluded_for_linting( &document_path, @@ -45,6 +41,15 @@ pub(crate) fn fix_all( return Ok(Fixes::default()); } + let source_type = match query.source_type_for_lint() { + SourceType::Python(source_type) => source_type, + SourceType::Toml(source_type @ (TomlSourceType::Pyproject | TomlSourceType::Ruff)) => { + return fix_toml(query, linter_settings, source_type, encoding); + } + SourceType::Toml(_) | SourceType::Markdown => return Ok(Fixes::default()), + }; + let source_kind = query.make_python_source_kind(source_type); + let file_path = query.file_path(); let package = if let Some(file_path) = &file_path { detect_package_root( @@ -132,28 +137,69 @@ pub(crate) fn fix_all( } Ok(fixes) } else { - let source_index = LineIndex::from_source_text(source_kind.source_code()); - - let modified = transformed.source_code(); - let modified_index = LineIndex::from_source_text(modified); - - let Replacement { - source_range, - modified_range, - } = Replacement::between( + Ok(text_document_fixes( + query, source_kind.source_code(), - source_index.line_starts(), - modified, - modified_index.line_starts(), - ); - Ok([( - query.make_key().into_uri(), - vec![lsp_types::TextEdit { - range: source_range.to_range(source_kind.source_code(), &source_index, encoding), - new_text: modified[modified_range].to_owned(), - }], - )] - .into_iter() - .collect()) + transformed.source_code(), + encoding, + )) } } + +fn fix_toml( + query: &DocumentQuery, + linter_settings: &LinterSettings, + source_type: TomlSourceType, + encoding: PositionEncoding, +) -> crate::Result { + let document = query.as_single_document()?; + let transformed = lint_fix_toml( + &query.virtual_file_path(), + document.contents(), + linter_settings, + source_type, + query.settings().unsafe_fixes, + ) + .transformed; + + if let Cow::Borrowed(_) = transformed { + return Ok(Fixes::default()); + } + + Ok(text_document_fixes( + query, + document.contents(), + transformed.as_ref(), + encoding, + )) +} + +fn text_document_fixes( + query: &DocumentQuery, + source: &str, + modified: &str, + encoding: PositionEncoding, +) -> Fixes { + let source_index = LineIndex::from_source_text(source); + let modified_index = LineIndex::from_source_text(modified); + + let Replacement { + source_range, + modified_range, + } = Replacement::between( + source, + source_index.line_starts(), + modified, + modified_index.line_starts(), + ); + + [( + query.make_key().into_uri(), + vec![lsp_types::TextEdit { + range: source_range.to_range(source, &source_index, encoding), + new_text: modified[modified_range].to_owned(), + }], + )] + .into_iter() + .collect() +} diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index 8579673c5c..0946eb6b83 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -3,13 +3,13 @@ use std::fmt::Write; use std::path::Path; -use ruff_python_ast::SourceType; +use ruff_python_ast::{SourceType, TomlSourceType}; use ruff_workspace::Settings; use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; use crate::{ - DIAGNOSTIC_NAME, PositionEncoding, + DIAGNOSTIC_NAME, PositionEncoding, TextDocument, edit::{NotebookDocument, NotebookRange, ToRangeExt}, resolve::is_document_excluded_for_linting, session::DocumentQuery, @@ -27,8 +27,9 @@ use ruff_linter::{ settings::flags, source_kind::SourceKind, suppression::Suppressions, + toml::lint_toml, }; -use ruff_notebook::Notebook; +use ruff_notebook::{Notebook, NotebookIndex}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; use ruff_source_file::LineIndex; @@ -77,13 +78,6 @@ pub(crate) fn check( let settings = query.settings(); let document_path = query.virtual_file_path(); - let SourceType::Python(source_type) = query.source_type_for_lint() else { - return DiagnosticsMap::default(); - }; - let source_kind = query.make_python_source_kind(source_type); - let document_uri = query.make_key().into_uri(); - let notebook = query.as_notebook(); - // If the document is excluded, return an empty list of diagnostics. if is_document_excluded_for_linting( &document_path, @@ -94,6 +88,88 @@ pub(crate) fn check( return DiagnosticsMap::default(); } + let result = match query.source_type_for_lint() { + SourceType::Python(source_type) => check_python(query, source_type), + SourceType::Toml(source_type @ (TomlSourceType::Pyproject | TomlSourceType::Ruff)) => { + let Ok(document) = query.as_single_document() else { + return DiagnosticsMap::default(); + }; + check_toml(query, document, source_type) + } + SourceType::Toml(_) | SourceType::Markdown => return DiagnosticsMap::default(), + }; + + let CheckResult { + diagnostics, + suppression_edits, + document, + } = result; + let document_uri = query.make_key().into_uri(); + let context = LspDiagnosticContext { + source: document.source(), + index: document.index(), + notebook_index: document.notebook_index(), + encoding, + document_path: &document_path, + document_uri: &document_uri, + notebook: query.as_notebook(), + supports_related_information, + settings, + }; + + let mut diagnostics_map = DiagnosticsMap::default(); + + // Populates all relevant URLs with an empty diagnostic list. + // This ensures that documents without diagnostics still get updated. + if let Some(notebook) = query.as_notebook() { + for uri in notebook.uris() { + diagnostics_map.entry(uri.clone()).or_default(); + } + } else { + diagnostics_map + .entry(query.make_key().into_uri()) + .or_default(); + } + + let mut suppression_edits = suppression_edits.into_iter(); + let lsp_diagnostics = diagnostics.into_iter().filter_map(|message| { + let suppression_edit = suppression_edits.next().flatten(); + if message.is_invalid_syntax() && !show_syntax_errors { + None + } else { + Some(to_lsp_diagnostic(&message, suppression_edit, &context)) + } + }); + + if let Some(notebook) = query.as_notebook() { + for (index, diagnostic) in lsp_diagnostics { + let Some(uri) = notebook.cell_uri_by_index(index) else { + tracing::warn!("Unable to find notebook cell at index {index}."); + continue; + }; + diagnostics_map + .entry(uri.clone()) + .or_default() + .push(diagnostic); + } + } else { + diagnostics_map + .entry(query.make_key().into_uri()) + .or_default() + .extend(lsp_diagnostics.map(|(_, diagnostic)| diagnostic)); + } + + diagnostics_map +} + +fn check_python( + query: &DocumentQuery, + source_type: ruff_python_ast::PySourceType, +) -> CheckResult<'_> { + let settings = query.settings(); + let document_path = query.virtual_file_path(); + let source_kind = query.make_python_source_kind(source_type); + let file_path = query.file_path(); let package = if let Some(file_path) = &file_path { detect_package_root( @@ -167,62 +243,45 @@ pub(crate) fn check( }, settings.linter.preview, ); - let context = LspDiagnosticContext { - source_kind: &source_kind, - index: locator.to_index(), - encoding, - document_path: document_path.as_ref(), - document_uri: &document_uri, - notebook, - supports_related_information, - settings, + let index = locator.to_index().clone(); + + CheckResult { + diagnostics, + suppression_edits, + document: CheckedDocument::Python { + source: source_kind, + index, + }, + } +} + +fn check_toml<'a>( + query: &DocumentQuery, + document: &'a TextDocument, + source_type: TomlSourceType, +) -> CheckResult<'a> { + let settings = query.settings(); + let diagnostics = if settings + .linter + .rules + .iter_enabled() + .any(|rule| rule.lint_source().is_toml()) + { + lint_toml( + &query.virtual_file_path(), + document.contents(), + &settings.linter, + source_type, + ) + } else { + Vec::new() }; - let mut diagnostics_map = DiagnosticsMap::default(); - - // Populates all relevant URLs with an empty diagnostic list. - // This ensures that documents without diagnostics still get updated. - if let Some(notebook) = query.as_notebook() { - for uri in notebook.uris() { - diagnostics_map.entry(uri.clone()).or_default(); - } - } else { - diagnostics_map - .entry(query.make_key().into_uri()) - .or_default(); + CheckResult { + diagnostics, + suppression_edits: Vec::new(), + document: CheckedDocument::Toml(document), } - - let lsp_diagnostics = - diagnostics - .into_iter() - .zip(suppression_edits) - .filter_map(|(message, noqa_edit)| { - if message.is_invalid_syntax() && !show_syntax_errors { - None - } else { - Some(to_lsp_diagnostic(&message, noqa_edit, &context)) - } - }); - - if let Some(notebook) = query.as_notebook() { - for (index, diagnostic) in lsp_diagnostics { - let Some(uri) = notebook.cell_uri_by_index(index) else { - tracing::warn!("Unable to find notebook cell at index {index}."); - continue; - }; - diagnostics_map - .entry(uri.clone()) - .or_default() - .push(diagnostic); - } - } else { - diagnostics_map - .entry(query.make_key().into_uri()) - .or_default() - .extend(lsp_diagnostics.map(|(_, diagnostic)| diagnostic)); - } - - diagnostics_map } /// Converts LSP diagnostics to a list of `DiagnosticFix`es by deserializing associated data on each diagnostic. @@ -253,9 +312,47 @@ pub(crate) fn fixes_for_diagnostics( .collect() } +enum CheckedDocument<'a> { + Python { + source: SourceKind, + index: LineIndex, + }, + Toml(&'a TextDocument), +} + +impl CheckedDocument<'_> { + fn source(&self) -> &str { + match self { + Self::Python { source, .. } => source.source_code(), + Self::Toml(document) => document.contents(), + } + } + + fn index(&self) -> &LineIndex { + match self { + Self::Python { index, .. } => index, + Self::Toml(document) => document.index(), + } + } + + fn notebook_index(&self) -> Option<&NotebookIndex> { + match self { + Self::Python { source, .. } => source.as_ipy_notebook().map(Notebook::index), + Self::Toml(_) => None, + } + } +} + +struct CheckResult<'a> { + diagnostics: Vec, + suppression_edits: Vec>, + document: CheckedDocument<'a>, +} + struct LspDiagnosticContext<'a> { - source_kind: &'a SourceKind, + source: &'a str, index: &'a LineIndex, + notebook_index: Option<&'a NotebookIndex>, encoding: PositionEncoding, document_path: &'a Path, document_uri: &'a lsp_types::Uri, @@ -305,22 +402,12 @@ fn to_lsp_diagnostic( .into_iter() .flat_map(Fix::edits) .map(|edit| lsp_types::TextEdit { - range: diagnostic_edit_range( - edit.range(), - context.source_kind, - context.index, - context.encoding, - ), + range: diagnostic_edit_range(edit.range(), context), new_text: edit.content().unwrap_or_default().to_string(), }) .collect(); let noqa_edit = noqa_edit.map(|noqa_edit| lsp_types::TextEdit { - range: diagnostic_edit_range( - noqa_edit.range(), - context.source_kind, - context.index, - context.encoding, - ), + range: diagnostic_edit_range(noqa_edit.range(), context), new_text: noqa_edit.into_content().unwrap_or_default().into_string(), }); serde_json::to_value(AssociatedDiagnosticData { @@ -336,20 +423,16 @@ fn to_lsp_diagnostic( let range: lsp_types::Range; let cell: usize; - if let Some(notebook_index) = context.source_kind.as_ipy_notebook().map(Notebook::index) { + if let Some(notebook_index) = context.notebook_index { NotebookRange { cell, range } = diagnostic_range.to_notebook_range( - context.source_kind.source_code(), + context.source, context.index, notebook_index, context.encoding, ); } else { cell = usize::default(); - range = diagnostic_range.to_range( - context.source_kind.source_code(), - context.index, - context.encoding, - ); + range = diagnostic_range.to_range(context.source, context.index, context.encoding); } let related_information = @@ -456,7 +539,7 @@ fn span_to_location(span: &Span, context: &LspDiagnosticContext) -> Option Option lsp_types::Range { - if let Some(notebook_index) = source_kind.as_ipy_notebook().map(Notebook::index) { +fn diagnostic_edit_range(range: TextRange, context: &LspDiagnosticContext) -> lsp_types::Range { + if let Some(notebook_index) = context.notebook_index { range - .to_notebook_range(source_kind.source_code(), index, notebook_index, encoding) + .to_notebook_range( + context.source, + context.index, + notebook_index, + context.encoding, + ) .range } else { - range.to_range(source_kind.source_code(), index, encoding) + range.to_range(context.source, context.index, context.encoding) } } @@ -521,6 +604,7 @@ fn tags(diagnostic: &Diagnostic) -> Option> { #[cfg(test)] mod tests { use ruff_db::diagnostic::{DiagnosticId, Severity, SubDiagnosticSeverity}; + use ruff_linter::source_kind::SourceKind; use ruff_source_file::SourceFileBuilder; use ruff_text_size::{TextRange, TextSize}; @@ -572,8 +656,9 @@ mod tests { let uri = lsp_types::Uri::parse("file:///test.py").expect("URI to be valid"); let settings = Settings::default(); let context = LspDiagnosticContext { - source_kind: &source_kind, + source: source_kind.source_code(), index: &index, + notebook_index: None, encoding: PositionEncoding::UTF8, document_path: Path::new("test.py"), document_uri: &uri, diff --git a/crates/ruff_server/src/server/api/requests/code_action.rs b/crates/ruff_server/src/server/api/requests/code_action.rs index 5f280b8629..fedf2239bd 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -1,6 +1,6 @@ use lsp_server::ErrorCode; use lsp_types::{self as types, CodeActionRequest, CodeActionResponse}; -use ruff_python_ast::SourceType; +use ruff_python_ast::{SourceType, TomlSourceType}; use rustc_hash::FxHashSet; use types::CodeActionKind; @@ -41,9 +41,10 @@ impl super::BackgroundDocumentRequestHandler for CodeActions { let query = snapshot.query(); - // Don't provide code actions for non-Python documents (e.g., markdown files). - let SourceType::Python(_) = query.source_type_for_lint() else { - return Ok(Some(response)); + let is_python = match query.source_type_for_lint() { + SourceType::Python(_) => true, + SourceType::Toml(TomlSourceType::Pyproject | TomlSourceType::Ruff) => false, + SourceType::Toml(_) | SourceType::Markdown => return Ok(Some(response)), }; let document_path = query.virtual_file_path(); @@ -70,7 +71,8 @@ impl super::BackgroundDocumentRequestHandler for CodeActions { .extend(quick_fix(&snapshot, &fixes).with_failure_code(ErrorCode::InternalError)?); } - if snapshot.client_settings().noqa_comments() + if is_python + && snapshot.client_settings().noqa_comments() && supported_code_actions.contains(&SupportedCodeAction::QuickFix) { response.extend(noqa_comments(&snapshot, &fixes)); @@ -93,7 +95,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActions { } } - if snapshot.client_settings().organize_imports() { + if is_python && snapshot.client_settings().organize_imports() { if supported_code_actions.contains(&SupportedCodeAction::SourceOrganizeImports) { if snapshot.is_notebook_cell() { // This is ignore here because the client requests this code action for each diff --git a/crates/ruff_server/tests/e2e/code_action.rs b/crates/ruff_server/tests/e2e/code_action.rs index 68c43ee24a..3952c248c2 100644 --- a/crates/ruff_server/tests/e2e/code_action.rs +++ b/crates/ruff_server/tests/e2e/code_action.rs @@ -81,6 +81,110 @@ fn code_actions_for_python() -> Result<()> { Ok(()) } +#[test] +fn code_actions_for_toml() -> Result<()> { + let source = r#" +[lint] +preview = true +select = ["rule-codes-in-selectors"] +extend-select = ["F401"] +"#; + let mut server = TestServerBuilder::new()? + .with_workspace(".")? + .with_file("ruff.toml", source)? + .build(); + + server.open_text_document_with_language_id("ruff.toml", "toml", source, 1); + + let diagnostics = match server.document_diagnostic_request("ruff.toml", None) { + DocumentDiagnosticReport::RelatedFullDocumentDiagnosticReport(report) => { + report.full_document_diagnostic_report.items + } + DocumentDiagnosticReport::RelatedUnchangedDocumentDiagnosticReport(_) => { + panic!("Expected a full diagnostic report"); + } + }; + let actions = server + .code_action_request("ruff.toml", diagnostics) + .expect("Expected code actions"); + + assert_json_snapshot!(actions, @r#" + [ + { + "title": "Ruff (rule-codes-in-selectors): Replace rule code with `unused-import`", + "kind": "quickfix", + "diagnostics": [ + { + "range": { + "start": { + "line": 4, + "character": 18 + }, + "end": { + "line": 4, + "character": 22 + } + }, + "severity": 2, + "code": "rule-codes-in-selectors", + "codeDescription": { + "href": "https://docs.astral.sh/ruff/rules/rule-codes-in-selectors" + }, + "source": "Ruff", + "message": "Rule code used instead of name in `lint.extend-select`\n\nhelp: Replace rule code with `unused-import`", + "tags": [] + } + ], + "edit": { + "changes": { + "file:///ruff.toml": [ + { + "range": { + "start": { + "line": 4, + "character": 18 + }, + "end": { + "line": 4, + "character": 22 + } + }, + "newText": "unused-import" + } + ] + } + }, + "data": "file:///ruff.toml" + }, + { + "title": "Ruff: Fix all auto-fixable problems", + "kind": "source.fixAll.ruff", + "edit": { + "changes": { + "file:///ruff.toml": [ + { + "range": { + "start": { + "line": 4, + "character": 0 + }, + "end": { + "line": 5, + "character": 0 + } + }, + "newText": "extend-select = [\"unused-import\"]\n" + } + ] + } + } + } + ] + "#); + + Ok(()) +} + #[test] fn human_readable_rule_names() -> Result<()> { let mut server = TestServerBuilder::new()? diff --git a/crates/ruff_server/tests/e2e/diagnostics.rs b/crates/ruff_server/tests/e2e/diagnostics.rs index 4fb75272c8..be6cf48ee6 100644 --- a/crates/ruff_server/tests/e2e/diagnostics.rs +++ b/crates/ruff_server/tests/e2e/diagnostics.rs @@ -81,3 +81,111 @@ fn uses_human_readable_names_in_preview() -> Result<()> { Ok(()) } + +#[test] +fn toml_diagnostics() -> Result<()> { + let source = r#" +[lint] +preview = true +select = ["rule-codes-in-selectors"] +extend-select = ["F401"] +"#; + let mut server = TestServerBuilder::new()? + .with_workspace(".")? + .with_file("ruff.toml", source)? + .build(); + + server.open_text_document_with_language_id("ruff.toml", "toml", source, 1); + + let diagnostics = server.document_diagnostic_request("ruff.toml", None); + + assert_json_snapshot!(diagnostics, @r#" + { + "items": [ + { + "range": { + "start": { + "line": 4, + "character": 18 + }, + "end": { + "line": 4, + "character": 22 + } + }, + "severity": 2, + "code": "rule-codes-in-selectors", + "codeDescription": { + "href": "https://docs.astral.sh/ruff/rules/rule-codes-in-selectors" + }, + "source": "Ruff", + "message": "Rule code used instead of name in `lint.extend-select`\n\nhelp: Replace rule code with `unused-import`", + "tags": [], + "data": { + "code": "rule-codes-in-selectors", + "edits": [ + { + "newText": "unused-import", + "range": { + "end": { + "character": 22, + "line": 4 + }, + "start": { + "character": 18, + "line": 4 + } + } + } + ], + "noqa_edit": null, + "title": "Replace rule code with `unused-import`" + } + } + ], + "kind": "full" + } + "#); + + Ok(()) +} + +#[test] +fn invalid_pyproject_toml_diagnostic() -> Result<()> { + let source = "[project]\nname = 1\n"; + let mut server = TestServerBuilder::new()?.with_workspace(".")?.build(); + + server.open_text_document_with_language_id("pyproject.toml", "toml", source, 1); + + let diagnostics = server.document_diagnostic_request("pyproject.toml", None); + + assert_json_snapshot!(diagnostics, @r#" + { + "items": [ + { + "range": { + "start": { + "line": 1, + "character": 7 + }, + "end": { + "line": 1, + "character": 8 + } + }, + "severity": 2, + "code": "RUF200", + "codeDescription": { + "href": "https://docs.astral.sh/ruff/rules/invalid-pyproject-toml" + }, + "source": "Ruff", + "message": "Failed to parse pyproject.toml: invalid type: integer `1`, expected a string", + "tags": [] + } + ], + "kind": "full" + } + "#); + + Ok(()) +}