fix(language_server): split run logic for oxlint and tsgolint

Author: Sysix

crates/oxc_language_server/fixtures/linter/lint_on_run/.oxlintrc.json

@@ -0,0 +1 @@
+{}

crates/oxc_language_server/fixtures/linter/lint_on_run/on_save/on-save.ts

@@ -0,0 +1,8 @@
+
+debugger;
+
+async function returnsPromise() {
+  return "value";
+}
+
+returnsPromise().then(() => {});

crates/oxc_language_server/fixtures/linter/lint_on_run/on_save/on-type.ts

@@ -0,0 +1,8 @@
+
+debugger;
+
+async function returnsPromise() {
+  return "value";
+}
+
+returnsPromise().then(() => {});

crates/oxc_language_server/fixtures/linter/lint_on_run/on_type/on-save.ts

@@ -0,0 +1,8 @@
+
+debugger;
+
+async function returnsPromise() {
+  return "value";
+}
+
+returnsPromise().then(() => {});

crates/oxc_language_server/fixtures/linter/lint_on_run/on_type/on-type.ts

@@ -0,0 +1,8 @@
+
+debugger;
+
+async function returnsPromise() {
+  return "value";
+}
+
+returnsPromise().then(() => {});

crates/oxc_language_server/src/linter/server_linter.rs

@@ -19,16 +19,24 @@ use crate::linter::{
     isolated_lint_handler::{IsolatedLintHandler, IsolatedLintHandlerOptions},
     tsgo_linter::TsgoLinter,
 };
-use crate::options::UnusedDisableDirectives;
+use crate::options::{Run, UnusedDisableDirectives};
 use crate::{ConcurrentHashMap, OXC_CONFIG_FILE, Options};
 
 use super::config_walker::ConfigWalker;
 
+#[derive(Debug, PartialEq, Eq)]
+pub enum ServerLinterRun {
+    OnType,
+    OnSave,
+    Always,
+}
+
 pub struct ServerLinter {
     isolated_linter: Arc<Mutex<IsolatedLintHandler>>,
     tsgo_linter: Arc<Option<TsgoLinter>>,
     ignore_matcher: LintIgnoreMatcher,
     gitignore_glob: Vec<Gitignore>,
+    lint_on_run: Run,
     pub extended_paths: Vec<PathBuf>,
 }
 
@@ -125,6 +133,7 @@ impl ServerLinter {
             ),
             gitignore_glob: Self::create_ignore_glob(&root_path),
             extended_paths,
+            lint_on_run: options.run,
             tsgo_linter: if options.type_aware {
                 Arc::new(Some(TsgoLinter::new(&root_path, config_store)))
             } else {
@@ -243,14 +252,42 @@ impl ServerLinter {
         &self,
         uri: &Uri,
         content: Option<String>,
+        run_type: ServerLinterRun,
     ) -> Option<Vec<DiagnosticReport>> {
+        let (oxlint, tsgolint) = match (run_type, self.lint_on_run) {
+            // run everything on save, or when it is forced
+            (ServerLinterRun::Always, _) | (ServerLinterRun::OnSave, Run::OnSave) => (true, true),
+            // run only oxlint on type
+            // tsgolint does not support memory source_text
+            (ServerLinterRun::OnType, Run::OnType) => (true, false),
+            // it does not match, run nothing
+            (ServerLinterRun::OnType, Run::OnSave) => (false, false),
+            // run only tsglint on save, even if the user wants it with type
+            // tsgolint only supports the OS file system.
+            (ServerLinterRun::OnSave, Run::OnType) => (false, true),
+        };
+
+        // return `None` when both tools doe not want to be used
+        if !oxlint && !tsgolint {
+            return None;
+        }
+
         if self.is_ignored(uri) {
             return None;
         }
 
-        // when `IsolatedLintHandler` returns `None`, it means it does not want to lint.
-        // Do not try `tsgolint` because it could be ignored or is not supported.
-        let mut reports = self.isolated_linter.lock().await.run_single(uri, content.clone())?;
+        let mut reports = Vec::with_capacity(0);
+
+        if oxlint
+            && let Some(oxlint_reports) =
+                self.isolated_linter.lock().await.run_single(uri, content.clone())
+        {
+            reports.extend(oxlint_reports);
+        }
+
+        if !tsgolint {
+            return Some(reports);
+        }
 
         let Some(tsgo_linter) = &*self.tsgo_linter else {
             return Some(reports);
@@ -296,6 +333,7 @@ mod test {
     use crate::{
         Options,
         linter::server_linter::{ServerLinter, normalize_path},
+        options::Run,
         tester::{Tester, get_file_path},
     };
     use rustc_hash::FxHashMap;
@@ -342,6 +380,42 @@ mod test {
         assert!(configs_dirs[0].ends_with("init_nested_configs"));
     }
 
+    #[test]
+    fn test_lint_on_run_on_type_on_type() {
+        Tester::new(
+            "fixtures/linter/lint_on_run/on_type",
+            Some(Options { type_aware: true, run: Run::OnType, ..Default::default() }),
+        )
+        .test_and_snapshot_single_file_with_run_type("on-type.ts", Run::OnType);
+    }
+
+    #[test]
+    fn test_lint_on_run_on_type_on_save() {
+        Tester::new(
+            "fixtures/linter/lint_on_run/on_save",
+            Some(Options { type_aware: true, run: Run::OnType, ..Default::default() }),
+        )
+        .test_and_snapshot_single_file_with_run_type("on-save.ts", Run::OnSave);
+    }
+
+    #[test]
+    fn test_lint_on_run_on_save_on_type() {
+        Tester::new(
+            "fixtures/linter/lint_on_run/on_save",
+            Some(Options { type_aware: true, run: Run::OnSave, ..Default::default() }),
+        )
+        .test_and_snapshot_single_file_with_run_type("on-type.ts", Run::OnType);
+    }
+
+    #[test]
+    fn test_lint_on_run_on_save_on_save() {
+        Tester::new(
+            "fixtures/linter/lint_on_run/on_type",
+            Some(Options { type_aware: true, run: Run::OnSave, ..Default::default() }),
+        )
+        .test_and_snapshot_single_file_with_run_type("on-save.ts", Run::OnSave);
+    }
+
     #[test]
     fn test_no_errors() {
         Tester::new("fixtures/linter/no_errors", None)
@@ -460,7 +534,7 @@ mod test {
     fn test_tsgo_lint() {
         let tester = Tester::new(
             "fixtures/linter/tsgolint",
-            Some(Options { type_aware: true, ..Default::default() }),
+            Some(Options { type_aware: true, run: Run::OnSave, ..Default::default() }),
         );
         tester.test_and_snapshot_single_file("no-floating-promises/index.ts");
     }

crates/oxc_language_server/src/main.rs

@@ -30,7 +30,8 @@ mod worker;
 use capabilities::Capabilities;
 use code_actions::CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC;
 use commands::{FIX_ALL_COMMAND_ID, FixAllCommandArgs};
-use options::{Options, Run, WorkspaceOption};
+use linter::server_linter::ServerLinterRun;
+use options::{Options, WorkspaceOption};
 use worker::WorkspaceWorker;
 
 type ConcurrentHashMap<K, V> = papaya::HashMap<K, V, FxBuildHasher>;
@@ -441,10 +442,7 @@ impl LanguageServer for Backend {
         let Some(worker) = workers.iter().find(|worker| worker.is_responsible_for_uri(uri)) else {
             return;
         };
-        if !worker.should_lint_on_run_type(Run::OnSave).await {
-            return;
-        }
-        if let Some(diagnostics) = worker.lint_file(uri, None).await {
+        if let Some(diagnostics) = worker.lint_file(uri, None, ServerLinterRun::OnSave).await {
             self.client
                 .publish_diagnostics(
                     uri.clone(),
@@ -463,11 +461,8 @@ impl LanguageServer for Backend {
         let Some(worker) = workers.iter().find(|worker| worker.is_responsible_for_uri(uri)) else {
             return;
         };
-        if !worker.should_lint_on_run_type(Run::OnType).await {
-            return;
-        }
         let content = params.content_changes.first().map(|c| c.text.clone());
-        if let Some(diagnostics) = worker.lint_file(uri, content).await {
+        if let Some(diagnostics) = worker.lint_file(uri, content, ServerLinterRun::OnType).await {
             self.client
                 .publish_diagnostics(
                     uri.clone(),
@@ -486,7 +481,9 @@ impl LanguageServer for Backend {
         };
 
         let content = params.text_document.text;
-        if let Some(diagnostics) = worker.lint_file(uri, Some(content)).await {
+        if let Some(diagnostics) =
+            worker.lint_file(uri, Some(content), ServerLinterRun::Always).await
+        {
             self.client
                 .publish_diagnostics(
                     uri.clone(),

crates/oxc_language_server/src/snapshots/fixtures_linter_lint_on_run_on_save@on-save.ts.snap

@@ -0,0 +1,15 @@
+---
+source: crates/oxc_language_server/src/tester.rs
+input_file: crates/oxc_language_server/fixtures/linter/lint_on_run/on_save/on-save.ts
+---
+code: "typescript-eslint(no-floating-promises)"
+code_description.href: "None"
+message: "Promises must be awaited.\nhelp: The promise must end with a call to .catch, or end with a call to .then with a rejection handler, or be explicitly marked as ignored with the `void` operator."
+range: Range { start: Position { line: 7, character: 0 }, end: Position { line: 7, character: 32 } }
+related_information[0].message: ""
+related_information[0].location.uri: "file://<variable>/fixtures/linter/lint_on_run/on_save/on-save.ts"
+related_information[0].location.range: Range { start: Position { line: 7, character: 0 }, end: Position { line: 7, character: 32 } }
+severity: Some(Warning)
+source: Some("oxc")
+tags: None
+fixed: Multiple([FixedContent { message: Some("Promises must be awaited."), code: "void ", range: Range { start: Position { line: 7, character: 0 }, end: Position { line: 7, character: 0 } } }, FixedContent { message: Some("Promises must be awaited."), code: "await ", range: Range { start: Position { line: 7, character: 0 }, end: Position { line: 7, character: 0 } } }])

crates/oxc_language_server/src/snapshots/fixtures_linter_lint_on_run_on_save@on-type.ts.snap

@@ -0,0 +1,5 @@
+---
+source: crates/oxc_language_server/src/tester.rs
+input_file: crates/oxc_language_server/fixtures/linter/lint_on_run/on_save/on-type.ts
+---
+File is ignored

crates/oxc_language_server/src/snapshots/fixtures_linter_lint_on_run_on_type@on-save.ts.snap

@@ -0,0 +1,28 @@
+---
+source: crates/oxc_language_server/src/tester.rs
+input_file: crates/oxc_language_server/fixtures/linter/lint_on_run/on_type/on-save.ts
+---
+code: "eslint(no-debugger)"
+code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html"
+message: "`debugger` statement is not allowed\nhelp: Remove the debugger statement"
+range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 9 } }
+related_information[0].message: ""
+related_information[0].location.uri: "file://<variable>/fixtures/linter/lint_on_run/on_type/on-save.ts"
+related_information[0].location.range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 9 } }
+severity: Some(Warning)
+source: Some("oxc")
+tags: None
+fixed: Single(FixedContent { message: Some("Remove the debugger statement"), code: "", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 9 } } })
+
+
+code: "typescript-eslint(no-floating-promises)"
+code_description.href: "None"
+message: "Promises must be awaited.\nhelp: The promise must end with a call to .catch, or end with a call to .then with a rejection handler, or be explicitly marked as ignored with the `void` operator."
+range: Range { start: Position { line: 7, character: 0 }, end: Position { line: 7, character: 32 } }
+related_information[0].message: ""
+related_information[0].location.uri: "file://<variable>/fixtures/linter/lint_on_run/on_type/on-save.ts"
+related_information[0].location.range: Range { start: Position { line: 7, character: 0 }, end: Position { line: 7, character: 32 } }
+severity: Some(Warning)
+source: Some("oxc")
+tags: None
+fixed: Multiple([FixedContent { message: Some("Promises must be awaited."), code: "void ", range: Range { start: Position { line: 7, character: 0 }, end: Position { line: 7, character: 0 } } }, FixedContent { message: Some("Promises must be awaited."), code: "await ", range: Range { start: Position { line: 7, character: 0 }, end: Position { line: 7, character: 0 } } }])