diff --git a/Cargo.lock b/Cargo.lock index 4d9a14aab8029..4f6ed70b1329c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1780,6 +1780,7 @@ dependencies = [ "futures", "globset", "ignore", + "insta", "log", "oxc_allocator", "oxc_data_structures", diff --git a/crates/oxc_language_server/Cargo.toml b/crates/oxc_language_server/Cargo.toml index 5b3da430c52eb..1ee1560634bd7 100644 --- a/crates/oxc_language_server/Cargo.toml +++ b/crates/oxc_language_server/Cargo.toml @@ -42,3 +42,6 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } tokio = { workspace = true, features = ["rt-multi-thread", "io-std", "macros"] } tower-lsp = { workspace = true, features = ["proposed"] } + +[dev-dependencies] +insta = { workspace = true } diff --git a/crates/oxc_language_server/fixtures/linter/hello_world.js b/crates/oxc_language_server/fixtures/linter/hello_world.js new file mode 100644 index 0000000000000..a8141d3b18d34 --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/hello_world.js @@ -0,0 +1 @@ +console.log("Hello, world!"); diff --git a/crates/oxc_language_server/fixtures/linter/issue_9958/.oxlintrc.json b/crates/oxc_language_server/fixtures/linter/issue_9958/.oxlintrc.json new file mode 100644 index 0000000000000..339d43432db5e --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/issue_9958/.oxlintrc.json @@ -0,0 +1,48 @@ +{ + "env": { + "browser": true, + "node": true, + "es2022": true + }, + "plugins": [ + "node", + "eslint", + "oxc", + "unicorn", + // "import", + "typescript", + "react", + "react-perf", + "jsx-a11y", + "vitest", + "nextjs" + ], + + "rules": { + "eqeqeq": "error", + "react/rules-of-hooks": "error", + "react/exhaustive-deps": "error", + "eslint/no-unused-vars": [ + "error", + { + "varsIgnorePattern": "^_", + "argsIgnorePattern": "^_", + "caughtErrorsIgnorePattern": "^_", + "ignoreRestSiblings": true + } + ] + }, + "categories": { + "correctness": "error" + }, + "overrides": [ + { + "files": ["**/*.spec.{ts,tsx,js,jsx}"], + "rules": { + "typescript/no-non-null-assertion": "off", + "typescript/no-non-null-asserted-optional-chain": "off" + } + } + ], + "ignorePatterns": ["e2e/**/*", "codemods/**/*", "scripts/**/*"] +} diff --git a/crates/oxc_language_server/fixtures/linter/issue_9958/issue.ts b/crates/oxc_language_server/fixtures/linter/issue_9958/issue.ts new file mode 100644 index 0000000000000..a86a11d5c7806 --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/issue_9958/issue.ts @@ -0,0 +1,13 @@ +// Test case from https://github.com/oxc-project/oxc/issues/9958 +export function test() { + let x = true; + console.log(!!x ? 10 : 20); + + let a: { + b?: { + c: number; + }; + } = {}; + + console.log(a?.b?.c!); +} diff --git a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs index bd36efdf1ef62..049928aaef139 100644 --- a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs +++ b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs @@ -65,6 +65,11 @@ impl IsolatedLintHandler { if r.location.range == d.diagnostic.range { continue; } + // If there is no message content for this span, then don't produce an additional diagnostic + // which also has no content. This prevents issues where editors expect diagnostics to have messages. + if r.message.is_empty() { + continue; + } inverted_diagnostics.push(DiagnosticReport { diagnostic: lsp_types::Diagnostic { range: r.location.range, diff --git a/crates/oxc_language_server/src/linter/mod.rs b/crates/oxc_language_server/src/linter/mod.rs index 0251b0b6115f2..9c47a4f089087 100644 --- a/crates/oxc_language_server/src/linter/mod.rs +++ b/crates/oxc_language_server/src/linter/mod.rs @@ -5,6 +5,9 @@ pub mod error_with_position; mod isolated_lint_handler; pub mod server_linter; +#[cfg(test)] +mod tester; + #[expect(clippy::cast_possible_truncation)] pub fn offset_to_position(offset: usize, source_text: &str) -> Position { // TODO(perf): share a single instance of `Rope` diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index 3195ce0d71e36..650745e06d6a6 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -28,3 +28,49 @@ impl ServerLinter { .run_single(&uri.to_file_path().unwrap(), content) } } + +#[cfg(test)] +mod test { + use std::path::PathBuf; + + use super::*; + use crate::linter::tester::Tester; + use oxc_linter::{LintFilter, LintFilterKind, Oxlintrc}; + + #[test] + fn test_no_errors() { + Tester::new() + .with_snapshot_suffix("no_errors") + .test_and_snapshot_single_file("fixtures/linter/hello_world.js"); + } + + #[test] + fn test_no_console() { + let config_store = ConfigStoreBuilder::default() + .with_filter(LintFilter::deny(LintFilterKind::parse("no-console".into()).unwrap())) + .build() + .unwrap(); + let linter = Linter::new(LintOptions::default(), config_store).with_fix(FixKind::SafeFix); + + Tester::new_with_linter(linter) + .with_snapshot_suffix("deny_no_console") + .test_and_snapshot_single_file("fixtures/linter/hello_world.js"); + } + + // Test case for https://github.com/oxc-project/oxc/issues/9958 + #[test] + fn test_issue_9958() { + let config_store = ConfigStoreBuilder::from_oxlintrc( + true, + Oxlintrc::from_file(&PathBuf::from("fixtures/linter/issue_9958/.oxlintrc.json")) + .unwrap(), + ) + .unwrap() + .build() + .unwrap(); + let linter = Linter::new(LintOptions::default(), config_store).with_fix(FixKind::SafeFix); + + Tester::new_with_linter(linter) + .test_and_snapshot_single_file("fixtures/linter/issue_9958/issue.ts"); + } +} diff --git a/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_hello_world.js@deny_no_console.snap b/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_hello_world.js@deny_no_console.snap new file mode 100644 index 0000000000000..f213d3797bd9d --- /dev/null +++ b/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_hello_world.js@deny_no_console.snap @@ -0,0 +1,13 @@ +--- +source: crates/oxc_language_server/src/linter/tester.rs +--- +code: "eslint(no-console)" +code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-console" +message: "eslint(no-console): Unexpected console statement.\nhelp: Delete this console statement." +range: Range { start: Position { line: 0, character: 0 }, end: Position { line: 0, character: 11 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/hello_world.js" +related_information[0].location.range: Range { start: Position { line: 0, character: 0 }, end: Position { line: 0, character: 11 } } +severity: Some(Error) +source: Some("oxc") +tags: None diff --git a/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_hello_world.js@no_errors.snap b/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_hello_world.js@no_errors.snap new file mode 100644 index 0000000000000..9292d49673ab2 --- /dev/null +++ b/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_hello_world.js@no_errors.snap @@ -0,0 +1,4 @@ +--- +source: crates/oxc_language_server/src/linter/tester.rs +--- +No diagnostic reports diff --git a/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_issue_9958_issue.ts.snap b/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_issue_9958_issue.ts.snap new file mode 100644 index 0000000000000..3650af5e16f2d --- /dev/null +++ b/crates/oxc_language_server/src/linter/snapshots/fixtures_linter_issue_9958_issue.ts.snap @@ -0,0 +1,28 @@ +--- +source: crates/oxc_language_server/src/linter/tester.rs +--- +code: "eslint(no-extra-boolean-cast)" +code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-extra-boolean-cast" +message: "Redundant double negation\nhelp: Remove the double negation as it will already be coerced to a boolean" +range: Range { start: Position { line: 3, character: 14 }, end: Position { line: 3, character: 17 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/issue_9958/issue.ts" +related_information[0].location.range: Range { start: Position { line: 3, character: 14 }, end: Position { line: 3, character: 17 } } +severity: Some(Error) +source: Some("oxc") +tags: None + + +code: "typescript-eslint(no-non-null-asserted-optional-chain)" +code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/typescript_eslint/no-non-null-asserted-optional-chain" +message: "non-null assertions after an optional chain expression\nhelp: Optional chain expressions can return undefined by design - using a non-null assertion is unsafe and wrong. You should remove the non-null assertion." +range: Range { start: Position { line: 11, character: 18 }, end: Position { line: 11, character: 18 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/issue_9958/issue.ts" +related_information[0].location.range: Range { start: Position { line: 11, character: 18 }, end: Position { line: 11, character: 18 } } +related_information[1].message: "" +related_information[1].location.uri: "file:///fixtures/linter/issue_9958/issue.ts" +related_information[1].location.range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 21 } } +severity: Some(Error) +source: Some("oxc") +tags: None diff --git a/crates/oxc_language_server/src/linter/tester.rs b/crates/oxc_language_server/src/linter/tester.rs new file mode 100644 index 0000000000000..f9d5531c6f331 --- /dev/null +++ b/crates/oxc_language_server/src/linter/tester.rs @@ -0,0 +1,125 @@ +use oxc_linter::Linter; +use tower_lsp::lsp_types::{CodeDescription, NumberOrString, Url}; + +use super::{error_with_position::DiagnosticReport, server_linter::ServerLinter}; + +/// Given a file path relative to the crate root directory, return the URI of the file. +pub fn get_file_uri(relative_file_path: &str) -> Url { + let absolute_file_path = + std::env::current_dir().expect("could not get current dir").join(relative_file_path); + Url::from_file_path(absolute_file_path).expect("failed to convert file path to URL") +} + +fn get_snapshot_from_report(report: &DiagnosticReport) -> String { + let code = match &report.diagnostic.code { + Some(NumberOrString::Number(code)) => code.to_string(), + Some(NumberOrString::String(code)) => code.to_string(), + None => "None".to_string(), + }; + let code_description_href = match &report.diagnostic.code_description { + Some(CodeDescription { href }) => href.to_string(), + None => "None".to_string(), + }; + let message = report.diagnostic.message.clone(); + let range = report.diagnostic.range; + let related_information = match &report.diagnostic.related_information { + Some(infos) => { + infos + .iter() + .enumerate() + .map(|(i, info)| { + let mut result = String::new(); + result.push_str(&format!( + "related_information[{}].message: {:?}", + i, info.message + )); + // replace everything between `file://` and `oxc_language_server` with ``, to avoid + // the absolute path causing snapshot test failures in different environments + let mut location = info.location.uri.to_string(); + let start = + location.find("file://").expect("file:// protocol not found in URI"); + let end = location + .find("oxc_language_server") + .expect("oxc_language_server not found in URI"); + location.replace_range( + start + "file://".len()..end + "oxc_language_server".len(), + "", + ); + + result.push_str(&format!( + "\nrelated_information[{i}].location.uri: {location:?}", + )); + result.push_str(&format!( + "\nrelated_information[{}].location.range: {:?}", + i, info.location.range + )); + result + }) + .collect::>() + .join("\n") + } + None => "related_information: None".to_string(), + }; + let severity = report.diagnostic.severity; + let source = report.diagnostic.source.clone(); + let tags = report.diagnostic.tags.clone(); + format!( + r" +code: {code:?} +code_description.href: {code_description_href:?} +message: {message:?} +range: {range:?} +{related_information} +severity: {severity:?} +source: {source:?} +tags: {tags:?} + " + ) +} + +/// Testing struct for the [linter server][crate::linter::server_linter::ServerLinter]. +pub struct Tester<'t> { + server_linter: ServerLinter, + snapshot_suffix: Option<&'t str>, +} + +impl Tester<'_> { + pub fn new() -> Self { + Self { snapshot_suffix: None, server_linter: ServerLinter::new() } + } + + pub fn new_with_linter(linter: Linter) -> Self { + Self { snapshot_suffix: None, server_linter: ServerLinter::new_with_linter(linter) } + } + + pub fn with_snapshot_suffix(mut self, suffix: &'static str) -> Self { + self.snapshot_suffix = Some(suffix); + self + } + + /// Given a relative file path (relative to `oxc_language_server` crate root), run the linter + /// and return the resulting diagnostics in a custom snapshot format. + #[expect(clippy::disallowed_methods)] + pub fn test_and_snapshot_single_file(&self, relative_file_path: &str) { + let uri = get_file_uri(relative_file_path); + let content = std::fs::read_to_string(uri.to_file_path().unwrap()) + .expect("could not read fixture file"); + let reports = self.server_linter.run_single(&uri, Some(content)).unwrap(); + let snapshot = if reports.is_empty() { + "No diagnostic reports".to_string() + } else { + reports.iter().map(get_snapshot_from_report).collect::>().join("\n") + }; + + let snapshot_name = relative_file_path.replace('/', "_"); + let mut settings = insta::Settings::clone_current(); + settings.set_prepend_module_to_snapshot(false); + settings.set_omit_expression(true); + if let Some(suffix) = self.snapshot_suffix { + settings.set_snapshot_suffix(suffix); + } + settings.bind(|| { + insta::assert_snapshot!(snapshot_name, snapshot); + }); + } +}