mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-29 13:24:57 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Consider virtual path for various server actions (#18910)

Browse source 
## Summary

Ref:
https://github.com/astral-sh/ruff/issues/14820#issuecomment-2996690681

This PR fixes a bug where virtual paths or any paths that doesn't exists
on the file system weren't being considered for checking inclusion /
exclusion. This was because the logic used `file_path` which returns
`None` for those path. This PR fixes that by using the
`virtual_file_path` method that returns a `Path` corresponding to the
actual file on disk or any kind of virtual path.

This should ideally just fix the above linked issue by way of excluding
the documents representing the interactive window because they aren't in
the inclusion set. It failed only on Windows previously because the file
path construction would fail and then Ruff would default to including
all the files.

## Test Plan

On my machine, the `.interactive` paths are always excluded so I'm using
the inclusion set instead:

```json
{
  "ruff.nativeServer": "on",
  "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"],
  "ruff.configuration": {
    "extend-include": ["*.interactive"]
  }
}
```

The diagnostics are shown for both the file paths and the interactive
window:

<img width="1727" alt="Screenshot 2025-06-24 at 14 56 40"
src="https://github.com/user-attachments/assets/d36af96a-777e-4367-8acf-4d9c9014d025"
/>

And, the logs:

```
2025-06-24 14:56:26.478275000 DEBUG notification{method="notebookDocument/didChange"}: Included path via `extend-include`: /Interactive-1.interactive
```

And, when using `ruff.exclude` via:

```json
{
	"ruff.exclude": ["*.interactive"]
}
```

With logs:

```
2025-06-24 14:58:41.117743000 DEBUG notification{method="notebookDocument/didChange"}: Ignored path via `exclude`: /Interactive-1.interactive
```
This commit is contained in:
Dhruv Manilawala 2025-06-24 17:54:28 +05:30 • committed by GitHub
parent 237a5821ba
commit 66fc7c8fc0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 58 additions and 69 deletions
Download patch file Download diff file Expand all files Collapse all files

25
crates/ruff_server/src/fix.rs
View file

@ -28,21 +28,22 @@ pub(crate) fn fix_all(
) -> crate::Result<Fixes> { ) -> crate::Result<Fixes> {
let source_kind = query.make_source_kind(); let source_kind = query.make_source_kind();
let settings = query.settings(); let settings = query.settings();
let document_path = query.file_path(); let document_path = query.virtual_file_path();
// If the document is excluded, return an empty list of fixes. // If the document is excluded, return an empty list of fixes.
let package = if let Some(document_path) = document_path.as_ref() { if is_document_excluded_for_linting(
if is_document_excluded_for_linting( &document_path,
document_path, &settings.file_resolver,
&settings.file_resolver, linter_settings,
linter_settings, query.text_document_language_id(),
query.text_document_language_id(), ) {
) { return Ok(Fixes::default());
return Ok(Fixes::default()); }
}
let file_path = query.file_path();
let package = if let Some(file_path) = &file_path {
detect_package_root( detect_package_root(
document_path file_path
.parent() .parent()
.expect("a path to a document should have a parent path"), .expect("a path to a document should have a parent path"),
&linter_settings.namespace_packages, &linter_settings.namespace_packages,
@ -65,7 +66,7 @@ pub(crate) fn fix_all(
result, result,
.. ..
} = ruff_linter::linter::lint_fix( } = ruff_linter::linter::lint_fix(
&query.virtual_file_path(), &document_path,
package, package,
flags::Noqa::Enabled, flags::Noqa::Enabled,
settings.unsafe_fixes, settings.unsafe_fixes,

16
crates/ruff_server/src/format.rs
View file

@ -12,10 +12,10 @@ pub(crate) fn format(
document: &TextDocument, document: &TextDocument,
source_type: PySourceType, source_type: PySourceType,
formatter_settings: &FormatterSettings, formatter_settings: &FormatterSettings,
path: Option<&Path>, path: &Path,
) -> crate::Result<Option<String>> { ) -> crate::Result<Option<String>> {
let format_options = let format_options =
formatter_settings.to_format_options(source_type, document.contents(), path); formatter_settings.to_format_options(source_type, document.contents(), Some(path));
match format_module_source(document.contents(), format_options) { match format_module_source(document.contents(), format_options) {
Ok(formatted) => { Ok(formatted) => {
let formatted = formatted.into_code(); let formatted = formatted.into_code();
@ -40,10 +40,10 @@ pub(crate) fn format_range(
source_type: PySourceType, source_type: PySourceType,
formatter_settings: &FormatterSettings, formatter_settings: &FormatterSettings,
range: TextRange, range: TextRange,
path: Option<&Path>, path: &Path,
) -> crate::Result<Option<PrintedRange>> { ) -> crate::Result<Option<PrintedRange>> {
let format_options = let format_options =
formatter_settings.to_format_options(source_type, document.contents(), path); formatter_settings.to_format_options(source_type, document.contents(), Some(path));
match ruff_python_formatter::format_range(document.contents(), range, format_options) { match ruff_python_formatter::format_range(document.contents(), range, format_options) {
Ok(formatted) => { Ok(formatted) => {
@ -97,7 +97,7 @@ with open("a_really_long_foo") as foo, open("a_really_long_bar") as bar, open("a
per_file_target_version, per_file_target_version,
..Default::default() ..Default::default()
}, },
Some(Path::new("test.py")), Path::new("test.py"),
) )
.expect("Expected no errors when formatting") .expect("Expected no errors when formatting")
.expect("Expected formatting changes"); .expect("Expected formatting changes");
@ -119,7 +119,7 @@ with open("a_really_long_foo") as foo, open("a_really_long_bar") as bar, open("a
unresolved_target_version: PythonVersion::PY38, unresolved_target_version: PythonVersion::PY38,
..Default::default() ..Default::default()
}, },
Some(Path::new("test.py")), Path::new("test.py"),
) )
.expect("Expected no errors when formatting") .expect("Expected no errors when formatting")
.expect("Expected formatting changes"); .expect("Expected formatting changes");
@ -167,7 +167,7 @@ sys.exit(
..Default::default() ..Default::default()
}, },
range, range,
Some(Path::new("test.py")), Path::new("test.py"),
) )
.expect("Expected no errors when formatting") .expect("Expected no errors when formatting")
.expect("Expected formatting changes"); .expect("Expected formatting changes");
@ -190,7 +190,7 @@ sys.exit(
..Default::default() ..Default::default()
}, },
range, range,
Some(Path::new("test.py")), Path::new("test.py"),
) )
.expect("Expected no errors when formatting") .expect("Expected no errors when formatting")
.expect("Expected formatting changes"); .expect("Expected formatting changes");

46
crates/ruff_server/src/lint.rs
View file

@ -69,37 +69,29 @@ pub(crate) fn check(
) -> DiagnosticsMap { ) -> DiagnosticsMap {
let source_kind = query.make_source_kind(); let source_kind = query.make_source_kind();
let settings = query.settings(); let settings = query.settings();
let document_path = query.file_path(); let document_path = query.virtual_file_path();
// If the document is excluded, return an empty list of diagnostics. // If the document is excluded, return an empty list of diagnostics.
let package = if let Some(document_path) = document_path.as_ref() { if is_document_excluded_for_linting(
if is_document_excluded_for_linting( &document_path,
document_path, &settings.file_resolver,
&settings.file_resolver, &settings.linter,
&settings.linter, query.text_document_language_id(),
query.text_document_language_id(), ) {
) { return DiagnosticsMap::default();
return DiagnosticsMap::default(); }
}
detect_package_root( let package = detect_package_root(
document_path document_path
.parent() .parent()
.expect("a path to a document should have a parent path"), .expect("a path to a document should have a parent path"),
&settings.linter.namespace_packages, &settings.linter.namespace_packages,
) )
.map(PackageRoot::root) .map(PackageRoot::root);
} else {
None
};
let source_type = query.source_type(); let source_type = query.source_type();
let target_version = if let Some(path) = &document_path { let target_version = settings.linter.resolve_target_version(&document_path);
settings.linter.resolve_target_version(path)
} else {
settings.linter.unresolved_target_version
};
let parse_options = let parse_options =
ParseOptions::from(source_type).with_target_version(target_version.parser_version()); ParseOptions::from(source_type).with_target_version(target_version.parser_version());
@ -123,7 +115,7 @@ pub(crate) fn check(
// Generate checks. // Generate checks.
let diagnostics = check_path( let diagnostics = check_path(
&query.virtual_file_path(), &document_path,
package, package,
&locator, &locator,
&stylist, &stylist,
@ -138,7 +130,7 @@ pub(crate) fn check(
); );
let noqa_edits = generate_noqa_edits( let noqa_edits = generate_noqa_edits(
&query.virtual_file_path(), &document_path,
&diagnostics, &diagnostics,
&locator, &locator,
indexer.comment_ranges(), indexer.comment_ranges(),

20
crates/ruff_server/src/server/api/requests/format.rs
View file

@ -83,18 +83,16 @@ fn format_text_document(
is_notebook: bool, is_notebook: bool,
) -> Result<super::FormatResponse> { ) -> Result<super::FormatResponse> {
let settings = query.settings(); let settings = query.settings();
let file_path = query.virtual_file_path();
// If the document is excluded, return early. // If the document is excluded, return early.
let file_path = query.file_path(); if is_document_excluded_for_formatting(
if let Some(file_path) = &file_path { &file_path,
if is_document_excluded_for_formatting( &settings.file_resolver,
file_path, &settings.formatter,
&settings.file_resolver, text_document.language_id(),
&settings.formatter, ) {
text_document.language_id(), return Ok(None);
) {
return Ok(None);
}
} }
let source = text_document.contents(); let source = text_document.contents();
@ -102,7 +100,7 @@ fn format_text_document(
text_document, text_document,
query.source_type(), query.source_type(),
&settings.formatter, &settings.formatter,
file_path.as_deref(), &file_path,
) )
.with_failure_code(lsp_server::ErrorCode::InternalError)?; .with_failure_code(lsp_server::ErrorCode::InternalError)?;
let Some(mut formatted) = formatted else { let Some(mut formatted) = formatted else {

20
crates/ruff_server/src/server/api/requests/format_range.rs
View file

@ -47,18 +47,16 @@ fn format_text_document_range(
encoding: PositionEncoding, encoding: PositionEncoding,
) -> Result<super::FormatResponse> { ) -> Result<super::FormatResponse> {
let settings = query.settings(); let settings = query.settings();
let file_path = query.virtual_file_path();
// If the document is excluded, return early. // If the document is excluded, return early.
let file_path = query.file_path(); if is_document_excluded_for_formatting(
if let Some(file_path) = &file_path { &file_path,
if is_document_excluded_for_formatting( &settings.file_resolver,
file_path, &settings.formatter,
&settings.file_resolver, text_document.language_id(),
&settings.formatter, ) {
text_document.language_id(), return Ok(None);
) {
return Ok(None);
}
} }
let text = text_document.contents(); let text = text_document.contents();
@ -69,7 +67,7 @@ fn format_text_document_range(
query.source_type(), query.source_type(),
&settings.formatter, &settings.formatter,
range, range,
file_path.as_deref(), &file_path,
) )
.with_failure_code(lsp_server::ErrorCode::InternalError)?; .with_failure_code(lsp_server::ErrorCode::InternalError)?;