Skip to content

Commit d36d227

Browse files
committed
fix(language_server): don't lint file on code action when it is already ignored (#13976)
part of #13778 ## before: Even if the file is ignored or not supported by both linters, the code actions would re trigger a lint task. ## after: respect the `None` result which indicates the ignore status
1 parent d985aeb commit d36d227

File tree

3 files changed

+59
-11
lines changed

3 files changed

+59
-11
lines changed

crates/oxc_language_server/src/linter/error_with_position.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use oxc_diagnostics::Severity;
99

1010
use crate::LSP_MAX_INT;
1111

12-
#[derive(Debug, Clone)]
12+
#[derive(Debug, Clone, Default)]
1313
pub struct DiagnosticReport {
1414
pub diagnostic: lsp_types::Diagnostic,
1515
pub fixed_content: PossibleFixContent,
@@ -23,8 +23,9 @@ pub struct FixedContent {
2323
pub range: Range,
2424
}
2525

26-
#[derive(Debug, Clone)]
26+
#[derive(Debug, Clone, Default)]
2727
pub enum PossibleFixContent {
28+
#[default]
2829
None,
2930
Single(FixedContent),
3031
Multiple(Vec<FixedContent>),

crates/oxc_language_server/src/linter/server_linter.rs

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,17 @@ impl ServerLinterDiagnostics {
5252
pub fn get_diagnostics(&self, path: &str) -> Option<Vec<DiagnosticReport>> {
5353
let mut reports = Vec::new();
5454
let mut found = false;
55-
if let Some(Some(diagnostics)) = self.isolated_linter.pin().get(path) {
56-
reports.extend(diagnostics.clone());
55+
if let Some(entry) = self.isolated_linter.pin().get(path) {
5756
found = true;
57+
if let Some(diagnostics) = entry {
58+
reports.extend(diagnostics.clone());
59+
}
5860
}
59-
if let Some(Some(diagnostics)) = self.tsgo_linter.pin().get(path) {
60-
reports.extend(diagnostics.clone());
61+
if let Some(entry) = self.tsgo_linter.pin().get(path) {
6162
found = true;
63+
if let Some(diagnostics) = entry {
64+
reports.extend(diagnostics.clone());
65+
}
6266
}
6367
if found { Some(reports) } else { None }
6468
}
@@ -69,9 +73,10 @@ impl ServerLinterDiagnostics {
6973
}
7074

7175
pub fn get_cached_files_of_diagnostics(&self) -> Vec<String> {
72-
let mut files = Vec::new();
7376
let isolated_files = self.isolated_linter.pin().keys().cloned().collect::<Vec<_>>();
7477
let tsgo_files = self.tsgo_linter.pin().keys().cloned().collect::<Vec<_>>();
78+
79+
let mut files = Vec::with_capacity(isolated_files.len() + tsgo_files.len());
7580
files.extend(isolated_files);
7681
files.extend(tsgo_files);
7782
files.dedup();
@@ -407,9 +412,11 @@ mod test {
407412
use std::path::{Path, PathBuf};
408413

409414
use crate::{
415+
ConcurrentHashMap,
410416
linter::{
417+
error_with_position::DiagnosticReport,
411418
options::{LintOptions, Run, UnusedDisableDirectives},
412-
server_linter::{ServerLinter, normalize_path},
419+
server_linter::{ServerLinter, ServerLinterDiagnostics, normalize_path},
413420
},
414421
tester::{Tester, get_file_path},
415422
};
@@ -457,6 +464,45 @@ mod test {
457464
assert!(configs_dirs[0].ends_with("init_nested_configs"));
458465
}
459466

467+
#[test]
468+
fn test_get_diagnostics_found_and_none_entries() {
469+
let key = "file:///test.js".to_string();
470+
471+
// Case 1: Both entries present, Some diagnostics
472+
let diag = DiagnosticReport::default();
473+
let diag_map = ConcurrentHashMap::default();
474+
diag_map.pin().insert(key.clone(), Some(vec![diag.clone()]));
475+
let tsgo_map = ConcurrentHashMap::default();
476+
tsgo_map.pin().insert(key.clone(), Some(vec![diag]));
477+
478+
let server_diag = super::ServerLinterDiagnostics {
479+
isolated_linter: std::sync::Arc::new(diag_map),
480+
tsgo_linter: std::sync::Arc::new(tsgo_map),
481+
};
482+
let result = server_diag.get_diagnostics(&key);
483+
assert!(result.is_some());
484+
assert_eq!(result.unwrap().len(), 2);
485+
486+
// Case 2: Entry present, but value is None
487+
let diag_map_none = ConcurrentHashMap::default();
488+
diag_map_none.pin().insert(key.clone(), None);
489+
let tsgo_map_none = ConcurrentHashMap::default();
490+
tsgo_map_none.pin().insert(key.clone(), None);
491+
492+
let server_diag_none = ServerLinterDiagnostics {
493+
isolated_linter: std::sync::Arc::new(diag_map_none),
494+
tsgo_linter: std::sync::Arc::new(tsgo_map_none),
495+
};
496+
let result_none = server_diag_none.get_diagnostics(&key);
497+
assert!(result_none.is_some());
498+
assert_eq!(result_none.unwrap().len(), 0);
499+
500+
// Case 3: No entry at all
501+
let server_diag_empty = ServerLinterDiagnostics::default();
502+
let result_empty = server_diag_empty.get_diagnostics(&key);
503+
assert!(result_empty.is_none());
504+
}
505+
460506
#[test]
461507
#[cfg(not(target_endian = "big"))]
462508
fn test_lint_on_run_on_type_on_type() {

editors/vscode/tests/code_actions.spec.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ import {
1616
activateExtension,
1717
fixturesWorkspaceUri,
1818
loadFixture,
19-
sleep
19+
sleep,
20+
testSingleFolderMode
2021
} from './test-helpers';
2122
import assert = require('assert');
2223

@@ -35,11 +36,11 @@ teardown(async () => {
3536
});
3637

3738
suite('code actions', () => {
38-
test('listed code actions', async () => {
39+
// flaky test for multi workspace mode
40+
testSingleFolderMode('listed code actions', async () => {
3941
await loadFixture('debugger');
4042
const fileUri = Uri.joinPath(fixturesWorkspaceUri(), 'fixtures', 'debugger.js');
4143
// await window.showTextDocument(fileUri); -- should also work without opening the file
42-
await sleep(500); // ¯\_(ツ)_/¯ -- test it again when adding/removing tests
4344

4445
const codeActions: ProviderResult<Array<CodeAction>> = await commands.executeCommand(
4546
'vscode.executeCodeActionProvider',

0 commit comments

Comments
 (0)