Skip to content

Commit 66fc7c8

Browse files
authored
Consider virtual path for various server actions (#18910)
## Summary Ref: #14820 (comment) 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 ```
1 parent 237a582 commit 66fc7c8

File tree

5 files changed

+58
-69
lines changed

5 files changed

+58
-69
lines changed

crates/ruff_server/src/fix.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,22 @@ pub(crate) fn fix_all(
2828
) -> crate::Result<Fixes> {
2929
let source_kind = query.make_source_kind();
3030
let settings = query.settings();
31-
let document_path = query.file_path();
31+
let document_path = query.virtual_file_path();
3232

3333
// If the document is excluded, return an empty list of fixes.
34-
let package = if let Some(document_path) = document_path.as_ref() {
35-
if is_document_excluded_for_linting(
36-
document_path,
37-
&settings.file_resolver,
38-
linter_settings,
39-
query.text_document_language_id(),
40-
) {
41-
return Ok(Fixes::default());
42-
}
34+
if is_document_excluded_for_linting(
35+
&document_path,
36+
&settings.file_resolver,
37+
linter_settings,
38+
query.text_document_language_id(),
39+
) {
40+
return Ok(Fixes::default());
41+
}
4342

43+
let file_path = query.file_path();
44+
let package = if let Some(file_path) = &file_path {
4445
detect_package_root(
45-
document_path
46+
file_path
4647
.parent()
4748
.expect("a path to a document should have a parent path"),
4849
&linter_settings.namespace_packages,
@@ -65,7 +66,7 @@ pub(crate) fn fix_all(
6566
result,
6667
..
6768
} = ruff_linter::linter::lint_fix(
68-
&query.virtual_file_path(),
69+
&document_path,
6970
package,
7071
flags::Noqa::Enabled,
7172
settings.unsafe_fixes,

crates/ruff_server/src/format.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ pub(crate) fn format(
1212
document: &TextDocument,
1313
source_type: PySourceType,
1414
formatter_settings: &FormatterSettings,
15-
path: Option<&Path>,
15+
path: &Path,
1616
) -> crate::Result<Option<String>> {
1717
let format_options =
18-
formatter_settings.to_format_options(source_type, document.contents(), path);
18+
formatter_settings.to_format_options(source_type, document.contents(), Some(path));
1919
match format_module_source(document.contents(), format_options) {
2020
Ok(formatted) => {
2121
let formatted = formatted.into_code();
@@ -40,10 +40,10 @@ pub(crate) fn format_range(
4040
source_type: PySourceType,
4141
formatter_settings: &FormatterSettings,
4242
range: TextRange,
43-
path: Option<&Path>,
43+
path: &Path,
4444
) -> crate::Result<Option<PrintedRange>> {
4545
let format_options =
46-
formatter_settings.to_format_options(source_type, document.contents(), path);
46+
formatter_settings.to_format_options(source_type, document.contents(), Some(path));
4747

4848
match ruff_python_formatter::format_range(document.contents(), range, format_options) {
4949
Ok(formatted) => {
@@ -97,7 +97,7 @@ with open("a_really_long_foo") as foo, open("a_really_long_bar") as bar, open("a
9797
per_file_target_version,
9898
..Default::default()
9999
},
100-
Some(Path::new("test.py")),
100+
Path::new("test.py"),
101101
)
102102
.expect("Expected no errors when formatting")
103103
.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
119119
unresolved_target_version: PythonVersion::PY38,
120120
..Default::default()
121121
},
122-
Some(Path::new("test.py")),
122+
Path::new("test.py"),
123123
)
124124
.expect("Expected no errors when formatting")
125125
.expect("Expected formatting changes");
@@ -167,7 +167,7 @@ sys.exit(
167167
..Default::default()
168168
},
169169
range,
170-
Some(Path::new("test.py")),
170+
Path::new("test.py"),
171171
)
172172
.expect("Expected no errors when formatting")
173173
.expect("Expected formatting changes");
@@ -190,7 +190,7 @@ sys.exit(
190190
..Default::default()
191191
},
192192
range,
193-
Some(Path::new("test.py")),
193+
Path::new("test.py"),
194194
)
195195
.expect("Expected no errors when formatting")
196196
.expect("Expected formatting changes");

crates/ruff_server/src/lint.rs

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -69,37 +69,29 @@ pub(crate) fn check(
6969
) -> DiagnosticsMap {
7070
let source_kind = query.make_source_kind();
7171
let settings = query.settings();
72-
let document_path = query.file_path();
72+
let document_path = query.virtual_file_path();
7373

7474
// If the document is excluded, return an empty list of diagnostics.
75-
let package = if let Some(document_path) = document_path.as_ref() {
76-
if is_document_excluded_for_linting(
77-
document_path,
78-
&settings.file_resolver,
79-
&settings.linter,
80-
query.text_document_language_id(),
81-
) {
82-
return DiagnosticsMap::default();
83-
}
75+
if is_document_excluded_for_linting(
76+
&document_path,
77+
&settings.file_resolver,
78+
&settings.linter,
79+
query.text_document_language_id(),
80+
) {
81+
return DiagnosticsMap::default();
82+
}
8483

85-
detect_package_root(
86-
document_path
87-
.parent()
88-
.expect("a path to a document should have a parent path"),
89-
&settings.linter.namespace_packages,
90-
)
91-
.map(PackageRoot::root)
92-
} else {
93-
None
94-
};
84+
let package = detect_package_root(
85+
document_path
86+
.parent()
87+
.expect("a path to a document should have a parent path"),
88+
&settings.linter.namespace_packages,
89+
)
90+
.map(PackageRoot::root);
9591

9692
let source_type = query.source_type();
9793

98-
let target_version = if let Some(path) = &document_path {
99-
settings.linter.resolve_target_version(path)
100-
} else {
101-
settings.linter.unresolved_target_version
102-
};
94+
let target_version = settings.linter.resolve_target_version(&document_path);
10395

10496
let parse_options =
10597
ParseOptions::from(source_type).with_target_version(target_version.parser_version());
@@ -123,7 +115,7 @@ pub(crate) fn check(
123115

124116
// Generate checks.
125117
let diagnostics = check_path(
126-
&query.virtual_file_path(),
118+
&document_path,
127119
package,
128120
&locator,
129121
&stylist,
@@ -138,7 +130,7 @@ pub(crate) fn check(
138130
);
139131

140132
let noqa_edits = generate_noqa_edits(
141-
&query.virtual_file_path(),
133+
&document_path,
142134
&diagnostics,
143135
&locator,
144136
indexer.comment_ranges(),

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,26 +83,24 @@ fn format_text_document(
8383
is_notebook: bool,
8484
) -> Result<super::FormatResponse> {
8585
let settings = query.settings();
86+
let file_path = query.virtual_file_path();
8687

8788
// If the document is excluded, return early.
88-
let file_path = query.file_path();
89-
if let Some(file_path) = &file_path {
90-
if is_document_excluded_for_formatting(
91-
file_path,
92-
&settings.file_resolver,
93-
&settings.formatter,
94-
text_document.language_id(),
95-
) {
96-
return Ok(None);
97-
}
89+
if is_document_excluded_for_formatting(
90+
&file_path,
91+
&settings.file_resolver,
92+
&settings.formatter,
93+
text_document.language_id(),
94+
) {
95+
return Ok(None);
9896
}
9997

10098
let source = text_document.contents();
10199
let formatted = crate::format::format(
102100
text_document,
103101
query.source_type(),
104102
&settings.formatter,
105-
file_path.as_deref(),
103+
&file_path,
106104
)
107105
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
108106
let Some(mut formatted) = formatted else {

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,16 @@ fn format_text_document_range(
4747
encoding: PositionEncoding,
4848
) -> Result<super::FormatResponse> {
4949
let settings = query.settings();
50+
let file_path = query.virtual_file_path();
5051

5152
// If the document is excluded, return early.
52-
let file_path = query.file_path();
53-
if let Some(file_path) = &file_path {
54-
if is_document_excluded_for_formatting(
55-
file_path,
56-
&settings.file_resolver,
57-
&settings.formatter,
58-
text_document.language_id(),
59-
) {
60-
return Ok(None);
61-
}
53+
if is_document_excluded_for_formatting(
54+
&file_path,
55+
&settings.file_resolver,
56+
&settings.formatter,
57+
text_document.language_id(),
58+
) {
59+
return Ok(None);
6260
}
6361

6462
let text = text_document.contents();
@@ -69,7 +67,7 @@ fn format_text_document_range(
6967
query.source_type(),
7068
&settings.formatter,
7169
range,
72-
file_path.as_deref(),
70+
&file_path,
7371
)
7472
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
7573

0 commit comments

Comments
 (0)