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,
+    Force,
+}
+
 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 force
+            (ServerLinterRun::Force, _) | (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,70 +380,110 @@ 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("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("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("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("on-save.ts", Run::OnSave);
+    }
+
     #[test]
     fn test_no_errors() {
         Tester::new("fixtures/linter/no_errors", None)
-            .test_and_snapshot_single_file("hello_world.js");
+            .test_and_snapshot_single_file("hello_world.js", Run::default());
     }
 
     #[test]
     fn test_no_console() {
         Tester::new("fixtures/linter/deny_no_console", None)
-            .test_and_snapshot_single_file("hello_world.js");
+            .test_and_snapshot_single_file("hello_world.js", Run::default());
     }
 
     // Test case for https://github.com/oxc-project/oxc/issues/9958
     #[test]
     fn test_issue_9958() {
-        Tester::new("fixtures/linter/issue_9958", None).test_and_snapshot_single_file("issue.ts");
+        Tester::new("fixtures/linter/issue_9958", None)
+            .test_and_snapshot_single_file("issue.ts", Run::default());
     }
 
     // Test case for https://github.com/oxc-project/oxc/issues/9957
     #[test]
     fn test_regexp() {
         Tester::new("fixtures/linter/regexp_feature", None)
-            .test_and_snapshot_single_file("index.ts");
+            .test_and_snapshot_single_file("index.ts", Run::default());
     }
 
     #[test]
     fn test_frameworks() {
-        Tester::new("fixtures/linter/astro", None).test_and_snapshot_single_file("debugger.astro");
-        Tester::new("fixtures/linter/vue", None).test_and_snapshot_single_file("debugger.vue");
+        Tester::new("fixtures/linter/astro", None)
+            .test_and_snapshot_single_file("debugger.astro", Run::default());
+        Tester::new("fixtures/linter/vue", None)
+            .test_and_snapshot_single_file("debugger.vue", Run::default());
         Tester::new("fixtures/linter/svelte", None)
-            .test_and_snapshot_single_file("debugger.svelte");
+            .test_and_snapshot_single_file("debugger.svelte", Run::default());
         // ToDo: fix Tester to work only with Uris and do not access the file system
         // Tester::new("fixtures/linter/nextjs").test_and_snapshot_single_file("%5B%5B..rest%5D%5D/debugger.ts");
     }
 
     #[test]
     fn test_invalid_syntax_file() {
         Tester::new("fixtures/linter/invalid_syntax", None)
-            .test_and_snapshot_single_file("debugger.ts");
+            .test_and_snapshot_single_file("debugger.ts", Run::default());
     }
 
     #[test]
     fn test_cross_module_debugger() {
         Tester::new("fixtures/linter/cross_module", None)
-            .test_and_snapshot_single_file("debugger.ts");
+            .test_and_snapshot_single_file("debugger.ts", Run::default());
     }
 
     #[test]
     fn test_cross_module_no_cycle() {
-        Tester::new("fixtures/linter/cross_module", None).test_and_snapshot_single_file("dep-a.ts");
+        Tester::new("fixtures/linter/cross_module", None)
+            .test_and_snapshot_single_file("dep-a.ts", Run::default());
     }
 
     #[test]
     fn test_cross_module_no_cycle_nested_config() {
         Tester::new("fixtures/linter/cross_module_nested_config", None)
-            .test_and_snapshot_single_file("dep-a.ts");
+            .test_and_snapshot_single_file("dep-a.ts", Run::default());
         Tester::new("fixtures/linter/cross_module_nested_config", None)
-            .test_and_snapshot_single_file("folder/folder-dep-a.ts");
+            .test_and_snapshot_single_file("folder/folder-dep-a.ts", Run::default());
     }
 
     #[test]
     fn test_cross_module_no_cycle_extended_config() {
         Tester::new("fixtures/linter/cross_module_extended_config", None)
-            .test_and_snapshot_single_file("dep-a.ts");
+            .test_and_snapshot_single_file("dep-a.ts", Run::default());
     }
 
     #[test]
@@ -420,7 +498,7 @@ mod test {
                 ..Options::default()
             }),
         )
-        .test_and_snapshot_single_file("forward_ref.ts");
+        .test_and_snapshot_single_file("forward_ref.ts", Run::default());
     }
 
     #[test]
@@ -433,14 +511,14 @@ mod test {
                 ..Default::default()
             }),
         )
-        .test_and_snapshot_single_file("test.js");
+        .test_and_snapshot_single_file("test.js", Run::default());
     }
 
     #[test]
     fn test_root_ignore_patterns() {
         let tester = Tester::new("fixtures/linter/ignore_patterns", None);
-        tester.test_and_snapshot_single_file("ignored-file.ts");
-        tester.test_and_snapshot_single_file("another_config/not-ignored-file.ts");
+        tester.test_and_snapshot_single_file("ignored-file.ts", Run::default());
+        tester.test_and_snapshot_single_file("another_config/not-ignored-file.ts", Run::default());
     }
 
     #[test]
@@ -452,7 +530,7 @@ mod test {
                 ..Default::default()
             }),
         )
-        .test_and_snapshot_single_file("deep/src/dep-a.ts");
+        .test_and_snapshot_single_file("deep/src/dep-a.ts", Run::default());
     }
 
     #[test]
@@ -462,6 +540,6 @@ mod test {
             "fixtures/linter/tsgolint",
             Some(Options { type_aware: true, ..Default::default() }),
         );
-        tester.test_and_snapshot_single_file("no-floating-promises/index.ts");
+        tester.test_and_snapshot_single_file("no-floating-promises/index.ts", Run::OnSave);
     }
 }

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::Force).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 } } }])