diff --git a/crates/oxc_language_server/README.md b/crates/oxc_language_server/README.md index f26e54c0be5cd..3cc7a7825b5eb 100644 --- a/crates/oxc_language_server/README.md +++ b/crates/oxc_language_server/README.md @@ -25,6 +25,7 @@ These options can be passed with [initialize](#initialize), [workspace/didChange | `configPath` | `` \| `null` | `null` | Path to a oxlint configuration file, passing a string will disable nested configuration | | `tsConfigPath` | `` \| `null` | `null` | Path to a TypeScript configuration file. If your `tsconfig.json` is not at the root, alias paths will not be resolve correctly for the `import` plugin | | `unusedDisableDirectives` | `"allow" \| "warn"` \| "deny"` | `"allow"` | Define how directive comments like `// oxlint-disable-line` should be reported, when no errors would have been reported on that line anyway | +| `typeAware` | `true` \| `false` | `false` | Enables type-aware linting | | `flags` | `Map` | `` | Special oxc language server flags, currently only one flag key is supported: `disable_nested_config` | ## Supported LSP Specifications from Server @@ -43,6 +44,7 @@ The client can pass the workspace options like following: "configPath": null, "tsConfigPath": null, "unusedDisableDirectives": "allow", + "typeAware": false, "flags": {} } }] @@ -78,6 +80,7 @@ The client can pass the workspace options like following: "configPath": null, "tsConfigPath": null, "unusedDisableDirectives": "allow", + "typeAware": false, "flags": {} } }] @@ -166,6 +169,7 @@ The client can return a response like: "configPath": null, "tsConfigPath": null, "unusedDisableDirectives": "allow", + "typeAware": false, "flags": {} }] ``` diff --git a/crates/oxc_language_server/fixtures/linter/tsgolint/no-floating-promises/index.ts b/crates/oxc_language_server/fixtures/linter/tsgolint/no-floating-promises/index.ts new file mode 100644 index 0000000000000..d72280a5103e4 --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/tsgolint/no-floating-promises/index.ts @@ -0,0 +1,14 @@ +const promise = new Promise((resolve, _reject) => resolve("value")); +promise; + +async function returnsPromise() { + return "value"; +} + +returnsPromise().then(() => {}); + +Promise.reject("value").catch(); + +Promise.reject("value").finally(); + +[1, 2, 3].map(async (x) => x + 1); \ No newline at end of file diff --git a/crates/oxc_language_server/src/linter/mod.rs b/crates/oxc_language_server/src/linter/mod.rs index 448ee7ad9d20e..26b904384637a 100644 --- a/crates/oxc_language_server/src/linter/mod.rs +++ b/crates/oxc_language_server/src/linter/mod.rs @@ -2,3 +2,4 @@ pub mod config_walker; pub mod error_with_position; pub mod isolated_lint_handler; pub mod server_linter; +pub mod tsgo_linter; diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index e875589720c1d..5e2ca8cdf8b20 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -16,6 +16,7 @@ use tower_lsp_server::UriExt; use crate::linter::{ error_with_position::DiagnosticReport, isolated_lint_handler::{IsolatedLintHandler, IsolatedLintHandlerOptions}, + tsgo_linter::TsgoLinter, }; use crate::options::UnusedDisableDirectives; use crate::{ConcurrentHashMap, OXC_CONFIG_FILE, Options}; @@ -24,6 +25,7 @@ use super::config_walker::ConfigWalker; pub struct ServerLinter { isolated_linter: Arc>, + tsgo_linter: Arc>, gitignore_glob: Vec, pub extended_paths: Vec, } @@ -98,7 +100,7 @@ impl ServerLinter { let isolated_linter = IsolatedLintHandler::new( lint_options, - config_store, + config_store.clone(), // clone because tsgo linter needs it &IsolatedLintHandlerOptions { use_cross_module, root_path: root_path.to_path_buf(), @@ -113,6 +115,11 @@ impl ServerLinter { isolated_linter: Arc::new(Mutex::new(isolated_linter)), gitignore_glob: Self::create_ignore_glob(&root_path), extended_paths, + tsgo_linter: if options.type_aware { + Arc::new(Some(TsgoLinter::new(&root_path, config_store))) + } else { + Arc::new(None) + }, } } @@ -221,7 +228,19 @@ impl ServerLinter { return None; } - self.isolated_linter.lock().await.run_single(uri, content) + // 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 Some(tsgo_linter) = &*self.tsgo_linter else { + return Some(reports); + }; + + if let Some(tsgo_reports) = tsgo_linter.lint_file(uri, content) { + reports.extend(tsgo_reports); + } + + Some(reports) } } @@ -411,4 +430,13 @@ mod test { ) .test_and_snapshot_single_file("deep/src/dep-a.ts"); } + + #[test] + fn test_tsgo_lint() { + let tester = Tester::new( + "fixtures/linter/tsgolint", + Some(Options { type_aware: true, ..Default::default() }), + ); + tester.test_and_snapshot_single_file("no-floating-promises/index.ts"); + } } diff --git a/crates/oxc_language_server/src/linter/tsgo_linter.rs b/crates/oxc_language_server/src/linter/tsgo_linter.rs new file mode 100644 index 0000000000000..72a9b6ef69f92 --- /dev/null +++ b/crates/oxc_language_server/src/linter/tsgo_linter.rs @@ -0,0 +1,60 @@ +use std::{ + path::Path, + sync::{Arc, OnceLock}, +}; + +use oxc_linter::{ + ConfigStore, LINTABLE_EXTENSIONS, TsGoLintState, loader::LINT_PARTIAL_LOADER_EXTENSIONS, + read_to_string, +}; +use rustc_hash::FxHashSet; +use tower_lsp_server::{UriExt, lsp_types::Uri}; + +use crate::linter::error_with_position::{ + DiagnosticReport, message_with_position_to_lsp_diagnostic_report, +}; + +pub struct TsgoLinter { + state: TsGoLintState, +} + +impl TsgoLinter { + pub fn new(root_uri: &Path, config_store: ConfigStore) -> Self { + let state = TsGoLintState::new(root_uri, config_store); + Self { state } + } + + pub fn lint_file(&self, uri: &Uri, content: Option) -> Option> { + let path = uri.to_file_path()?; + + if !Self::should_lint_path(&path) { + return None; + } + + let source_text = content.or_else(|| read_to_string(&path).ok())?; + + let messages = self.state.lint_source(&Arc::from(path.as_os_str()), source_text).ok()?; + + Some( + messages + .iter() + .map(|e| message_with_position_to_lsp_diagnostic_report(e, uri)) + .collect(), + ) + } + + fn should_lint_path(path: &Path) -> bool { + static WANTED_EXTENSIONS: OnceLock> = OnceLock::new(); + let wanted_exts = WANTED_EXTENSIONS.get_or_init(|| { + LINTABLE_EXTENSIONS + .iter() + .filter(|ext| !LINT_PARTIAL_LOADER_EXTENSIONS.contains(ext)) + .copied() + .collect() + }); + + path.extension() + .and_then(std::ffi::OsStr::to_str) + .is_some_and(|ext| wanted_exts.contains(ext)) + } +} diff --git a/crates/oxc_language_server/src/options.rs b/crates/oxc_language_server/src/options.rs index d2bde02f65f40..0ed0961c7cd02 100644 --- a/crates/oxc_language_server/src/options.rs +++ b/crates/oxc_language_server/src/options.rs @@ -29,6 +29,7 @@ pub struct Options { pub config_path: Option, pub ts_config_path: Option, pub unused_disable_directives: UnusedDisableDirectives, + pub type_aware: bool, pub flags: FxHashMap, } @@ -103,6 +104,9 @@ impl TryFrom for Options { ts_config_path: object .get("tsConfigPath") .and_then(|config_path| serde_json::from_value::(config_path.clone()).ok()), + type_aware: object + .get("typeAware") + .is_some_and(|key| serde_json::from_value::(key.clone()).unwrap_or_default()), flags, }) } @@ -128,6 +132,7 @@ mod test { "run": "onSave", "configPath": "./custom.json", "unusedDisableDirectives": "warn", + "typeAware": true, "flags": { "disable_nested_config": "true", "fix_kind": "dangerous_fix" @@ -138,6 +143,7 @@ mod test { assert_eq!(options.run, Run::OnSave); assert_eq!(options.config_path, Some("./custom.json".into())); assert_eq!(options.unused_disable_directives, UnusedDisableDirectives::Warn); + assert!(options.type_aware); assert_eq!(options.flags.get("disable_nested_config"), Some(&"true".to_string())); assert_eq!(options.flags.get("fix_kind"), Some(&"dangerous_fix".to_string())); } @@ -150,6 +156,7 @@ mod test { assert_eq!(options.run, Run::OnType); assert_eq!(options.config_path, None); assert_eq!(options.unused_disable_directives, UnusedDisableDirectives::Allow); + assert!(!options.type_aware); assert!(options.flags.is_empty()); } diff --git a/crates/oxc_language_server/src/snapshots/fixtures_linter_tsgolint@no-floating-promises_index.ts.snap b/crates/oxc_language_server/src/snapshots/fixtures_linter_tsgolint@no-floating-promises_index.ts.snap new file mode 100644 index 0000000000000..9ac2c99d3d88c --- /dev/null +++ b/crates/oxc_language_server/src/snapshots/fixtures_linter_tsgolint@no-floating-promises_index.ts.snap @@ -0,0 +1,80 @@ +--- +source: crates/oxc_language_server/src/tester.rs +input_file: crates/oxc_language_server/fixtures/linter/tsgolint/no-floating-promises/index.ts +--- +code: "typescript-eslint(no-confusing-void-expression)" +code_description.href: "None" +message: "Returning a void expression from an arrow function shorthand is forbidden. Please add braces to the arrow function." +range: Range { start: Position { line: 0, character: 50 }, end: Position { line: 0, character: 66 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/tsgolint/no-floating-promises/index.ts" +related_information[0].location.range: Range { start: Position { line: 0, character: 50 }, end: Position { line: 0, character: 66 } } +severity: Some(Warning) +source: Some("oxc") +tags: None +fixed: Single(FixedContent { message: None, code: "{ resolve(\"value\"); }", range: Range { start: Position { line: 0, character: 49 }, end: Position { line: 0, character: 66 } } }) + + +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: 1, character: 0 }, end: Position { line: 1, character: 8 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/tsgolint/no-floating-promises/index.ts" +related_information[0].location.range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 8 } } +severity: Some(Warning) +source: Some("oxc") +tags: None +fixed: Multiple([FixedContent { message: Some("Promises must be awaited."), code: "void ", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 0 } } }, FixedContent { message: Some("Promises must be awaited."), code: "await ", range: Range { start: Position { line: 1, character: 0 }, end: Position { line: 1, character: 0 } } }]) + + +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:///fixtures/linter/tsgolint/no-floating-promises/index.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 } } }]) + + +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: 9, character: 0 }, end: Position { line: 9, character: 32 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/tsgolint/no-floating-promises/index.ts" +related_information[0].location.range: Range { start: Position { line: 9, character: 0 }, end: Position { line: 9, 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: 9, character: 0 }, end: Position { line: 9, character: 0 } } }, FixedContent { message: Some("Promises must be awaited."), code: "await ", range: Range { start: Position { line: 9, character: 0 }, end: Position { line: 9, character: 0 } } }]) + + +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: 11, character: 0 }, end: Position { line: 11, character: 34 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/tsgolint/no-floating-promises/index.ts" +related_information[0].location.range: Range { start: Position { line: 11, character: 0 }, end: Position { line: 11, character: 34 } } +severity: Some(Warning) +source: Some("oxc") +tags: None +fixed: Multiple([FixedContent { message: Some("Promises must be awaited."), code: "void ", range: Range { start: Position { line: 11, character: 0 }, end: Position { line: 11, character: 0 } } }, FixedContent { message: Some("Promises must be awaited."), code: "await ", range: Range { start: Position { line: 11, character: 0 }, end: Position { line: 11, character: 0 } } }]) + + +code: "typescript-eslint(no-floating-promises)" +code_description.href: "None" +message: "An array of Promises may be unintentional.\nhelp: Consider handling the promises' fulfillment or rejection with Promise.all or similar, or explicitly marking the expression as ignored with the `void` operator." +range: Range { start: Position { line: 13, character: 0 }, end: Position { line: 13, character: 34 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/tsgolint/no-floating-promises/index.ts" +related_information[0].location.range: Range { start: Position { line: 13, character: 0 }, end: Position { line: 13, character: 34 } } +severity: Some(Warning) +source: Some("oxc") +tags: None +fixed: None diff --git a/crates/oxc_language_server/src/worker.rs b/crates/oxc_language_server/src/worker.rs index b9383f2d41319..0d683f1afa0d2 100644 --- a/crates/oxc_language_server/src/worker.rs +++ b/crates/oxc_language_server/src/worker.rs @@ -1,6 +1,6 @@ use std::{str::FromStr, sync::Arc, vec}; -use log::debug; +use log::{debug, warn}; use rustc_hash::FxBuildHasher; use tokio::sync::{Mutex, RwLock}; use tower_lsp_server::{ @@ -129,12 +129,23 @@ impl WorkspaceWorker { || old_options.use_nested_configs() != new_options.use_nested_configs() || old_options.fix_kind() != new_options.fix_kind() || old_options.unused_disable_directives != new_options.unused_disable_directives + // TODO: only the TsgoLinter needs to be dropped or created + || old_options.type_aware != new_options.type_aware } pub async fn should_lint_on_run_type(&self, current_run: Run) -> bool { - let run_level = { self.options.lock().await.run }; + let options = self.options.lock().await; + // `tsgolint` only supported the os file system. We can not run it on memory file system. + if options.type_aware { + if options.run == Run::OnType { + warn!( + "Linting with type aware is only supported with the OS file system. Change your settings to use onSave." + ); + } + return current_run == Run::OnSave; + } - run_level == current_run + options.run == current_run } pub async fn lint_file( diff --git a/crates/oxc_linter/src/tsgolint.rs b/crates/oxc_linter/src/tsgolint.rs index 226ba5005f248..72f64242045dc 100644 --- a/crates/oxc_linter/src/tsgolint.rs +++ b/crates/oxc_linter/src/tsgolint.rs @@ -11,12 +11,13 @@ use serde::{Deserialize, Serialize}; use oxc_diagnostics::{DiagnosticSender, DiagnosticService, OxcDiagnostic, Severity}; use oxc_span::{SourceType, Span}; -use crate::fixer::Message; -#[cfg(test)] -use crate::fixer::{CompositeFix, PossibleFixes}; +use crate::fixer::{CompositeFix, Message, PossibleFixes}; use super::{AllowWarnDeny, ConfigStore, ResolvedLinterState, read_to_string}; +#[cfg(feature = "language_server")] +use crate::{MessageWithPosition, fixer::message_to_message_with_position}; + /// State required to initialize the `tsgolint` linter. #[derive(Debug, Clone)] pub struct TsGoLintState { @@ -162,13 +163,14 @@ impl TsGoLintState { } }, ); - - let oxc_diagnostic: OxcDiagnostic = - OxcDiagnostic::from(tsgolint_diagnostic); let Some(severity) = severity else { // If the severity is not found, we should not report the diagnostic continue; }; + + let oxc_diagnostic: OxcDiagnostic = + OxcDiagnostic::from(tsgolint_diagnostic); + let oxc_diagnostic = oxc_diagnostic.with_severity( if severity == AllowWarnDeny::Deny { Severity::Error @@ -258,6 +260,181 @@ impl TsGoLintState { } } + /// # Panics + /// - when `stdin` of subprocess cannot be opened + /// - when `stdout` of subprocess cannot be opened + /// - when `tsgolint` process cannot be awaited + /// + /// # Errors + /// A human-readable error message indicating why the linting failed. + #[cfg(feature = "language_server")] + pub fn lint_source( + &self, + path: &Arc, + source_text: String, + ) -> Result>, String> { + use oxc_data_structures::rope::Rope; + + let mut resolved_configs: FxHashMap = FxHashMap::default(); + + let json_input = self.json_input(std::slice::from_ref(path), &mut resolved_configs); + let executable_path = self.executable_path.clone(); + + let handler = std::thread::spawn(move || { + let child = std::process::Command::new(&executable_path) + .arg("headless") + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .spawn(); + + let mut child = match child { + Ok(c) => c, + Err(e) => { + return Err(format!( + "Failed to spawn tsgolint from path `{}`, with error: {e}", + executable_path.display() + )); + } + }; + + let mut stdin = child.stdin.take().expect("Failed to open tsgolint stdin"); + + // Write the input synchronously and handle BrokenPipe gracefully in case the child + // exits early and closes its stdin. + let json = serde_json::to_string(&json_input).expect("Failed to serialize JSON"); + if let Err(e) = stdin.write_all(json.as_bytes()) { + // If the child closed stdin early, avoid crashing on SIGPIPE/BrokenPipe. + if e.kind() != ErrorKind::BrokenPipe { + return Err(format!("Failed to write to tsgolint stdin: {e}")); + } + } + // Explicitly drop stdin to send EOF to the child. + drop(stdin); + + // Stream diagnostics as they are emitted, rather than waiting for all output + let mut stdout = child.stdout.take().expect("Failed to open tsgolint stdout"); + + let stdout_handler = + std::thread::spawn(move || -> Result>, String> { + let mut buffer = Vec::with_capacity(8192); + let mut read_buf = [0u8; 8192]; + + let mut result = vec![]; + + loop { + match stdout.read(&mut read_buf) { + Ok(0) => break, // EOF + Ok(n) => { + buffer.extend_from_slice(&read_buf[..n]); + + // Try to parse complete messages from buffer + let mut cursor = std::io::Cursor::new(buffer.as_slice()); + let mut processed_up_to: u64 = 0; + + while cursor.position() < buffer.len() as u64 { + let start_pos = cursor.position(); + match parse_single_message(&mut cursor) { + Ok(Some(tsgolint_diagnostic)) => { + processed_up_to = cursor.position(); + + // For now, ignore any `tsgolint` errors. + if tsgolint_diagnostic.r#type == MessageType::Error { + continue; + } + + let path = tsgolint_diagnostic.file_path.clone(); + let Some(resolved_config) = resolved_configs.get(&path) + else { + // If we don't have a resolved config for this path, skip it. We should always + // have a resolved config though, since we processed them already above. + continue; + }; + + let severity = resolved_config.rules.iter().find_map( + |(rule, status)| { + if rule.name() == tsgolint_diagnostic.rule { + Some(*status) + } else { + None + } + }, + ); + let Some(severity) = severity else { + // If the severity is not found, we should not report the diagnostic + continue; + }; + + let mut message_with_position: MessageWithPosition<'_> = + message_to_message_with_position( + &Message::from_tsgo_lint_diagnostic( + tsgolint_diagnostic, + &source_text, + ), + &source_text, + &Rope::from_str(&source_text), + ); + + message_with_position.severity = + if severity == AllowWarnDeny::Deny { + Severity::Error + } else { + Severity::Warning + }; + + result.push(message_with_position); + } + Ok(None) => { + // Successfully parsed but no diagnostic to add + processed_up_to = cursor.position(); + } + Err(_) => { + // Could not parse a complete message, break and keep remaining data + cursor.set_position(start_pos); + break; + } + } + } + + // Keep unprocessed data for next iteration + if processed_up_to > 0 { + #[expect(clippy::cast_possible_truncation)] + buffer.drain(..processed_up_to as usize); + } + } + Err(e) => { + return Err(format!("Failed to read from tsgolint stdout: {e}")); + } + } + } + + Ok(result) + }); + + // Wait for process to complete and stdout processing to finish + let exit_status = child.wait().expect("Failed to wait for tsgolint process"); + let stdout_result = stdout_handler.join(); + + if !exit_status.success() { + return Err(format!("tsgolint process exited with status: {exit_status}")); + } + + match stdout_result { + Ok(Ok(diagnostics)) => Ok(diagnostics), + Ok(Err(err)) => Err(err), + Err(_) => Err("Failed to join stdout processing thread".to_string()), + } + }); + + match handler.join() { + Ok(Ok(diagnostics)) => { + // Successfully ran tsgolint + Ok(diagnostics) + } + Ok(Err(err)) => Err(format!("Error running tsgolint: {err:?}")), + Err(err) => Err(format!("Error running tsgolint: {err:?}")), + } + } + /// Create a JSON input for STDIN of tsgolint in this format: /// /// ```json @@ -370,7 +547,6 @@ impl From for OxcDiagnostic { impl Message<'_> { /// Converts a `TsGoLintDiagnostic` into a `Message` with possible fixes. - #[cfg(test)] fn from_tsgo_lint_diagnostic(val: TsGoLintDiagnostic, source_text: &str) -> Self { let mut fixes = Vec::with_capacity(usize::from(!val.fixes.is_empty()) + val.suggestions.len());