Skip to content

Commit e010485

Browse files
SysixSysix
committed
fix(oxlint/lsp): report diagnostics referencing another file
1 parent fc96ee0 commit e010485

File tree

13 files changed

+303
-153
lines changed

13 files changed

+303
-153
lines changed

‎crates/oxc_language_server/fixtures/linter/deprecated_tsconfig/.oxlintrc.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"rules": {
3+
"no-debugger": "off"
4+
}
5+
}

‎crates/oxc_language_server/fixtures/linter/deprecated_tsconfig/debugger.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
debugger;

‎crates/oxc_language_server/fixtures/linter/deprecated_tsconfig/tsconfig.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"compilerOptions": {
3+
"baseUrl": ".",
4+
}
5+
}

‎crates/oxc_language_server/src/linter/error_with_position.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,26 @@ pub fn message_to_lsp_diagnostic(
130130
}
131131
}
132132

133+
// Ignore fixes are not applicable for JSON files
134+
let is_json = uri
135+
.to_file_path()
136+
.and_then(|path| {
137+
path.extension()
138+
.map(|e| e.eq_ignore_ascii_case("json") || e.eq_ignore_ascii_case("jsonc"))
139+
})
140+
.unwrap_or(false);
141+
142+
if is_json {
143+
return DiagnosticReport {
144+
diagnostic,
145+
code_action: if fixed_content.is_empty() {
146+
None
147+
} else {
148+
Some(LinterCodeAction { range, fixed_content })
149+
},
150+
};
151+
}
152+
133153
// Add ignore fixes
134154
let error_offset = message.span.start;
135155
let section_offset = message.section_offset;

‎crates/oxc_language_server/src/linter/isolated_lint_handler.rs‎

Lines changed: 98 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ use std::{
33
sync::{Arc, OnceLock},
44
};
55

6-
use log::{debug, warn};
6+
use log::{debug, error, warn};
77
use oxc_data_structures::rope::Rope;
8-
use rustc_hash::{FxHashMap, FxHashSet};
8+
use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};
99
use tower_lsp_server::ls_types::Uri;
1010

1111
use oxc_allocator::Allocator;
@@ -101,47 +101,118 @@ impl IsolatedLintHandler {
101101
Self { runner, unused_directives_severity: lint_options.report_unused_directive }
102102
}
103103

104-
pub fn run_single(&self, uri: &Uri, content: Option<&str>) -> Option<Vec<DiagnosticReport>> {
105-
let path = uri.to_file_path()?;
104+
pub fn run_single(
105+
&self,
106+
uri: &Uri,
107+
content: Option<&str>,
108+
) -> FxHashMap<Uri, Vec<DiagnosticReport>> {
109+
let Some(path) = uri.to_file_path() else {
110+
error!("Failed to convert URI to path: {}", uri.as_str());
111+
return FxHashMap::default();
112+
};
106113

107114
if !Self::should_lint_path(&path) {
108-
return None;
115+
return FxHashMap::default();
109116
}
110117

111-
let source_text =
112-
if let Some(content) = content { content } else { &read_to_string(&path).ok()? };
118+
let owned_source_text;
119+
let source_text = if let Some(content) = content {
120+
content
121+
} else {
122+
match read_to_string(&path) {
123+
Ok(text) => {
124+
owned_source_text = text;
125+
&owned_source_text
126+
}
127+
Err(e) => {
128+
error!("Failed to read file {}: {}", path.display(), e);
129+
return FxHashMap::default();
130+
}
131+
}
132+
};
113133

114-
let mut diagnostics = self.lint_path(&path, uri, source_text);
115-
diagnostics.append(&mut generate_inverted_diagnostics(&diagnostics, uri));
116-
Some(diagnostics)
134+
self.lint_path(&path, source_text)
117135
}
118136

119-
fn lint_path(&self, path: &Path, uri: &Uri, source_text: &str) -> Vec<DiagnosticReport> {
137+
fn lint_path(&self, path: &Path, source_text: &str) -> FxHashMap<Uri, Vec<DiagnosticReport>> {
120138
debug!("lint {}", path.display());
121139
let rope = &Rope::from_str(source_text);
122140

123141
let mut fs = IsolatedLintHandlerFileSystem::default();
124142
fs.add_file(path.to_path_buf(), Arc::from(source_text));
125143

126-
let mut messages: Vec<DiagnosticReport> = self
127-
.runner
128-
.run_source(&[Arc::from(path.as_os_str())], &fs)
129-
.into_iter()
130-
.map(|message| message_to_lsp_diagnostic(message, uri, source_text, rope))
131-
.collect();
144+
let lint_result = self.runner.run_source(&[Arc::from(path.as_os_str())], &fs);
132145

133-
// Add unused directives if configured
134-
if let Some(severity) = self.unused_directives_severity
135-
&& let Some(directives) = self.runner.directives_coordinator().get(path)
136-
{
137-
messages.extend(
138-
create_unused_directives_messages(&directives, severity, source_text)
139-
.into_iter()
140-
.map(|message| message_to_lsp_diagnostic(message, uri, source_text, rope)),
141-
);
146+
let mut result: FxHashMap<Uri, Vec<DiagnosticReport>> =
147+
FxHashMap::with_capacity_and_hasher(lint_result.len(), FxBuildHasher);
148+
149+
let mut has_directives_collected = false;
150+
151+
for (msg_path_os, msgs) in lint_result {
152+
// Convert Arc<OsStr> to &Path for coordinator lookup
153+
let msg_path = Path::new(msg_path_os.as_ref());
154+
let Some(uri) = Uri::from_file_path(msg_path) else {
155+
error!("Failed to convert path to URI: {}", msg_path.display());
156+
continue;
157+
};
158+
159+
// Ensure the borrowed source_text outlives usage by keeping an owned String in scope.
160+
let owned_source_text: Option<String> = if msg_path == path {
161+
None
162+
} else {
163+
match read_to_string(msg_path) {
164+
Ok(text) => Some(text),
165+
Err(e) => {
166+
error!("Failed to read file {}: {}", msg_path.display(), e);
167+
continue;
168+
}
169+
}
170+
};
171+
let source_text_ref: &str = owned_source_text.as_deref().unwrap_or(source_text);
172+
let rope = if msg_path == path { rope } else { &Rope::from_str(source_text_ref) };
173+
174+
// Map rule diagnostics to LSP diagnostics
175+
let mut reports: Vec<DiagnosticReport> = msgs
176+
.into_iter()
177+
.map(|message| message_to_lsp_diagnostic(message, &uri, source_text_ref, rope))
178+
.collect();
179+
reports.append(&mut generate_inverted_diagnostics(&reports, &uri));
180+
result.entry(uri.clone()).or_default().extend(reports);
181+
182+
// Add unused directives for this file if configured
183+
if let Some(severity) = self.unused_directives_severity
184+
&& let Some(directives) = self.runner.directives_coordinator().get(msg_path)
185+
{
186+
result.entry(uri.clone()).or_default().extend(
187+
create_unused_directives_messages(&directives, severity, source_text_ref)
188+
.into_iter()
189+
.map(|message| {
190+
message_to_lsp_diagnostic(message, &uri, source_text_ref, rope)
191+
}),
192+
);
193+
}
194+
195+
// Track if we have already collected unused directives for this path
196+
if msg_path == path {
197+
has_directives_collected = true;
198+
}
199+
}
200+
201+
if !has_directives_collected {
202+
// If there were no lint messages for this file, we still need to check for unused directives
203+
if let Some(severity) = self.unused_directives_severity
204+
&& let Some(directives) = self.runner.directives_coordinator().get(path)
205+
{
206+
let uri = Uri::from_file_path(path).unwrap();
207+
result.entry(uri.clone()).or_default().extend(
208+
create_unused_directives_messages(&directives, severity, source_text)
209+
.into_iter()
210+
.map(|message| message_to_lsp_diagnostic(message, &uri, source_text, rope)),
211+
);
212+
}
142213
}
143214

144-
messages
215+
result
145216
}
146217

147218
fn should_lint_path(path: &Path) -> bool {

‎crates/oxc_language_server/src/linter/server_linter.rs‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use tower_lsp_server::{
88
jsonrpc::ErrorCode,
99
ls_types::{
1010
CodeActionKind, CodeActionOptions, CodeActionOrCommand, CodeActionProviderCapability,
11-
Diagnostic, ExecuteCommandOptions, Pattern, Range, ServerCapabilities, Uri,
12-
WorkDoneProgressOptions, WorkspaceEdit,
11+
ExecuteCommandOptions, Pattern, Range, ServerCapabilities, Uri, WorkDoneProgressOptions,
12+
WorkspaceEdit,
1313
},
1414
};
1515

@@ -499,10 +499,7 @@ impl Tool for ServerLinter {
499499
/// Lint a file with the current linter
500500
/// - If the file is not lintable or ignored, an empty vector is returned
501501
fn run_diagnostic(&self, uri: &Uri, content: Option<&str>) -> DiagnosticResult {
502-
let Some(diagnostics) = self.run_file(uri, content) else {
503-
return vec![];
504-
};
505-
vec![(uri.clone(), diagnostics)]
502+
self.run_file(uri, content)
506503
}
507504

508505
/// Lint a file with the current linter
@@ -588,28 +585,31 @@ impl ServerLinter {
588585
}
589586

590587
/// Lint a single file, return `None` if the file is ignored.
591-
fn run_file(&self, uri: &Uri, content: Option<&str>) -> Option<Vec<Diagnostic>> {
588+
fn run_file(&self, uri: &Uri, content: Option<&str>) -> DiagnosticResult {
592589
if self.is_ignored(uri) {
593-
return None;
590+
return Vec::new();
594591
}
595592

596593
let mut diagnostics = vec![];
597-
let mut code_actions = vec![];
598594

599595
let reports = self.isolated_linter.run_single(uri, content);
600-
if let Some(reports) = reports {
596+
597+
for (report_uri, reports) in reports {
598+
let mut uri_diagnostics = Vec::new();
599+
let mut uri_actions = Vec::new();
601600
for report in reports {
602-
diagnostics.push(report.diagnostic);
601+
uri_diagnostics.push(report.diagnostic);
603602

604603
if let Some(code_action) = report.code_action {
605-
code_actions.push(code_action);
604+
uri_actions.push(code_action);
606605
}
607606
}
608-
}
609607

610-
self.code_actions.pin().insert(uri.clone(), Some(code_actions));
608+
diagnostics.push((report_uri.clone(), uri_diagnostics));
609+
self.code_actions.pin().insert(report_uri, Some(uri_actions));
610+
}
611611

612-
Some(diagnostics)
612+
diagnostics
613613
}
614614

615615
fn needs_restart(old_options: &LSPLintOptions, new_options: &LSPLintOptions) -> bool {
@@ -1110,4 +1110,16 @@ mod test {
11101110
);
11111111
tester.test_and_snapshot_single_file("foo-bar.astro");
11121112
}
1113+
1114+
#[test]
1115+
#[cfg(not(target_endian = "big"))]
1116+
fn test_deprecated_tsconfig() {
1117+
let tester = Tester::new(
1118+
"fixtures/linter/deprecated_tsconfig",
1119+
json!({
1120+
"typeAware": true
1121+
}),
1122+
);
1123+
tester.test_and_snapshot_single_file("debugger.ts");
1124+
}
11131125
}

‎crates/oxc_language_server/src/linter/snapshots/[email protected]_folder__folder-dep-a.ts.snap‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,7 @@ source: crates/oxc_language_server/src/linter/tester.rs
44
##########
55
Linted file: fixtures/linter/cross_module_nested_config/dep-a.ts
66
----------
7-
########## Diagnostic Reports
8-
File URI: file://<variable>/fixtures/linter/cross_module_nested_config/dep-a.ts
9-
10-
########### Code Actions/Commands
11-
7+
No diagnostics / Files are ignored
128
##########
139
Linted file: fixtures/linter/cross_module_nested_config/folder/folder-dep-a.ts
1410
----------

‎crates/oxc_language_server/src/linter/snapshots/[email protected]‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
source: crates/oxc_language_server/src/linter/tester.rs
3+
---
4+
##########
5+
Linted file: fixtures/linter/deprecated_tsconfig/debugger.ts
6+
----------
7+
########## Diagnostic Reports
8+
File URI: file://<variable>/fixtures/linter/deprecated_tsconfig/tsconfig.json
9+
10+
code: "typescript(tsconfig-error)"
11+
code_description.href: "None"
12+
message: "Invalid tsconfig\nhelp: Option 'baseUrl' has been removed. Please remove it from your configuration.\nSee https://github.com/oxc-project/tsgolint/issues/351 for more information."
13+
range: Range { start: Position { line: 2, character: 4 }, end: Position { line: 2, character: 13 } }
14+
related_information[0].message: ""
15+
related_information[0].location.uri: "file://<variable>/fixtures/linter/deprecated_tsconfig/tsconfig.json"
16+
related_information[0].location.range: Range { start: Position { line: 2, character: 4 }, end: Position { line: 2, character: 13 } }
17+
severity: Some(Error)
18+
source: Some("oxc")
19+
tags: None
20+
21+
########### Code Actions/Commands

‎crates/oxc_language_server/src/linter/snapshots/fixtures_linter_no_errors@hello_world.js.snap‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,4 @@ source: crates/oxc_language_server/src/linter/tester.rs
44
##########
55
Linted file: fixtures/linter/no_errors/hello_world.js
66
----------
7-
########## Diagnostic Reports
8-
File URI: file://<variable>/fixtures/linter/no_errors/hello_world.js
9-
10-
########### Code Actions/Commands
7+
No diagnostics / Files are ignored

‎crates/oxc_linter/src/lint_runner.rs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,9 @@ impl LintRunner {
237237
&self,
238238
files: &[Arc<OsStr>],
239239
file_system: &(dyn crate::RuntimeFileSystem + Sync + Send),
240-
) -> Vec<Message> {
240+
) -> Vec<(Arc<OsStr>, Vec<crate::Message>)> {
241+
debug_assert!(!files.is_empty());
242+
241243
let mut messages = self.lint_service.run_source(file_system, files.to_owned());
242244

243245
if let Some(type_aware_linter) = &self.type_aware_linter {
@@ -248,9 +250,15 @@ impl LintRunner {
248250
) {
249251
Ok(msgs) => msgs,
250252
Err(err) => {
251-
vec![Message::new(
252-
OxcDiagnostic::warn(format!("Failed to run type-aware linting: `{err}`",)),
253-
PossibleFixes::None,
253+
vec![(
254+
// Report error for the first file only
255+
Arc::clone(&files[0]),
256+
vec![Message::new(
257+
OxcDiagnostic::warn(format!(
258+
"Failed to run type-aware linting: `{err}`",
259+
)),
260+
PossibleFixes::None,
261+
)],
254262
)]
255263
}
256264
};

0 commit comments

Comments
 (0)