mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
feat(oxlint/lsp): show config errors to LSP client (#25486)
Surface Oxlint configuration failures to LSP clients as `window/showMessage` notifications during workspace initialization. Report root, nested, and custom-config-path loading/build failures to the client while preserving fallback linting behavior. VSCode only show a max of 5 messages from a short time, but with #25962 we already reduce the communication to a maximum of 5 messages. Here is a screenshot of the `oxc` project where no `oxc.configPath` is defined in the VSCode setting (loading all our fixtures configs) <img width="459" height="581" alt="grafik" src="https://github.com/user-attachments/assets/9a39e5b7-b049-414b-a487-16a505eb278b" />
This commit is contained in:
1 parent
add04438cd
commit
bde9f699e7
14 files changed
+238
-57
No files matched your search
@@ -219,16 +219,6 @@ pub enum ConfigLoadError {
|
||||
Diagnostic(OxcDiagnostic),
|
||||
}
|
||||
|
||||
impl ConfigLoadError {
|
||||
/// Get the path of the config file that failed
|
||||
pub fn path(&self) -> Option<&Path> {
|
||||
match self {
|
||||
ConfigLoadError::Parse { path, .. } | ConfigLoadError::Build { path, .. } => Some(path),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// High-level errors that can occur when loading CLI configurations.
|
||||
///
|
||||
/// This groups together failures related to the root configuration file
|
||||
|
||||
@@ -14,7 +14,7 @@ use oxc_linter::{
|
||||
|
||||
use crate::lsp::{
|
||||
options::{RuleCustomizationSeverity, RulesCustomization},
|
||||
utils::get_full_rule_name,
|
||||
utils::{get_full_rule_name, get_oxc_diagnostic_full_message},
|
||||
};
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
@@ -130,22 +130,7 @@ pub fn message_to_lsp_diagnostic(
|
||||
.and_then(|url| url.parse().ok())
|
||||
.map(|href| CodeDescription { href });
|
||||
|
||||
let mut diagnostic_message = String::with_capacity(
|
||||
message.error.message.len()
|
||||
+ message.error.help.as_ref().map_or(0, |h| h.len() + 7) // "help: " prefix
|
||||
+ message.error.note.as_ref().map_or(0, |n| n.len() + 7), // "note: " prefix
|
||||
);
|
||||
|
||||
diagnostic_message.push_str(&message.error.message);
|
||||
if let Some(help) = &message.error.help {
|
||||
diagnostic_message.push_str("\nhelp: ");
|
||||
diagnostic_message.push_str(help);
|
||||
}
|
||||
|
||||
if let Some(note) = &message.error.note {
|
||||
diagnostic_message.push_str("\nnote: ");
|
||||
diagnostic_message.push_str(note);
|
||||
}
|
||||
let diagnostic_message = get_oxc_diagnostic_full_message(&message.error);
|
||||
|
||||
let diagnostic = Diagnostic {
|
||||
range,
|
||||
|
||||
@@ -3,15 +3,13 @@ use std::str::FromStr;
|
||||
use std::sync::{Arc, OnceLock};
|
||||
|
||||
use ignore::gitignore::Gitignore;
|
||||
use oxc_language_server::uri_utils::uri_to_file_path;
|
||||
use oxc_language_server::{ClientMessage, ToolBuildResult};
|
||||
use rustc_hash::{FxHashMap, FxHashSet};
|
||||
use tower_lsp_server::gen_lsp_types::{CodeActionProvider, CodeActionResponse, DiagnosticProvider};
|
||||
use tower_lsp_server::{
|
||||
gen_lsp_types::{
|
||||
CodeActionKind, CodeActionOptions, CodeActionTriggerKind, Diagnostic, DiagnosticOptions,
|
||||
ExecuteCommandOptions, Pattern, ServerCapabilities, Uri, WorkDoneProgressOptions,
|
||||
WorkspaceEdit,
|
||||
CodeActionKind, CodeActionOptions, CodeActionProvider, CodeActionResponse,
|
||||
CodeActionTriggerKind, Diagnostic, DiagnosticOptions, DiagnosticProvider,
|
||||
ExecuteCommandOptions, MessageType, Pattern, ServerCapabilities, Uri,
|
||||
WorkDoneProgressOptions, WorkspaceEdit,
|
||||
},
|
||||
jsonrpc::ErrorCode,
|
||||
};
|
||||
@@ -24,11 +22,12 @@ use oxc_linter::{
|
||||
};
|
||||
|
||||
use oxc_language_server::{
|
||||
Capabilities, CodeActionParams, ConcurrentHashMap, DiagnosticMode, DiagnosticResult,
|
||||
TextDocument, Tool, ToolBuilder, ToolRestartChanges,
|
||||
utils::normalize_user_config_path_to_watch_pattern,
|
||||
Capabilities, ClientMessage, CodeActionParams, ConcurrentHashMap, DiagnosticMode,
|
||||
DiagnosticResult, TextDocument, Tool, ToolBuildResult, ToolBuilder, ToolRestartChanges,
|
||||
uri_utils::uri_to_file_path, utils::normalize_user_config_path_to_watch_pattern,
|
||||
};
|
||||
|
||||
use crate::lsp::utils::{config_loader_error_to_message, oxc_diagnostic_to_client_message};
|
||||
use crate::{
|
||||
config_loader::{
|
||||
ConfigLoader, build_nested_configs, config_file_names, discover_configs_in_tree,
|
||||
@@ -96,13 +95,18 @@ impl ServerLinterBuilder {
|
||||
let root_path = uri_to_file_path(root_uri).unwrap();
|
||||
let mut external_linter = self.external_linter.as_ref();
|
||||
let mut external_plugin_store = ExternalPluginStore::new(external_linter.is_some());
|
||||
// create a set for config errors, because if the main config fails to load,
|
||||
// the same error might occur again.
|
||||
let mut client_messages = FxHashSet::default();
|
||||
|
||||
// Setup JS workspace. This must be done before loading any configs
|
||||
if let Some(external_linter) = external_linter {
|
||||
let res = (external_linter.create_workspace)(root_uri.to_string());
|
||||
|
||||
if let Err(err) = res {
|
||||
error!("Failed to setup JS workspace:\n{err}\n");
|
||||
let message = format!("Failed to setup JS workspace:\n{err}\n");
|
||||
error!(message);
|
||||
client_messages.insert(ClientMessage { message, r#type: MessageType::Error });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -119,7 +123,9 @@ impl ServerLinterBuilder {
|
||||
let mut oxlintrc = match loader.load_root_config(&root_path, config_path.as_ref()) {
|
||||
Ok(config) => config,
|
||||
Err(e) => {
|
||||
warn!("Failed to load config: {e}");
|
||||
let message = oxc_diagnostic_to_client_message(&e);
|
||||
error!(message.message);
|
||||
client_messages.insert(message);
|
||||
Oxlintrc::default()
|
||||
}
|
||||
};
|
||||
@@ -127,7 +133,7 @@ impl ServerLinterBuilder {
|
||||
|
||||
let mut nested_ignore_patterns = Vec::new();
|
||||
let mut extended_paths = FxHashSet::default();
|
||||
let nested_configs = if options.use_nested_configs() {
|
||||
let (nested_configs, config_client_message) = if options.use_nested_configs() {
|
||||
self.create_nested_configs(
|
||||
&root_path,
|
||||
&oxlintrc.path,
|
||||
@@ -137,9 +143,11 @@ impl ServerLinterBuilder {
|
||||
Some(root_uri.as_ref()),
|
||||
)
|
||||
} else {
|
||||
FxHashMap::default()
|
||||
(FxHashMap::default(), Vec::new())
|
||||
};
|
||||
|
||||
client_messages.extend(config_client_message);
|
||||
|
||||
let base_patterns = oxlintrc.ignore_patterns.clone();
|
||||
// Without a config file there are no patterns and the root is never consulted,
|
||||
// so the `root_path` fallback is an arbitrary placeholder.
|
||||
@@ -154,7 +162,11 @@ impl ServerLinterBuilder {
|
||||
) {
|
||||
Ok(builder) => builder,
|
||||
Err(e) => {
|
||||
warn!("Failed to build config from oxlintrc: {e}");
|
||||
let message = format!("Failed to build config from oxlintrc:\n{e}");
|
||||
// show message in LSP stderr
|
||||
error!(message);
|
||||
|
||||
client_messages.insert(ClientMessage { message, r#type: MessageType::Error });
|
||||
ConfigStoreBuilder::default()
|
||||
}
|
||||
};
|
||||
@@ -169,7 +181,9 @@ impl ServerLinterBuilder {
|
||||
|
||||
extended_paths.extend(config_builder.extended_paths.clone());
|
||||
let base_config = config_builder.build(&mut external_plugin_store).unwrap_or_else(|err| {
|
||||
warn!("Failed to build config: {err}");
|
||||
let message = format!("Failed to build config:\n{err}");
|
||||
error!("{message}");
|
||||
client_messages.insert(ClientMessage { message, r#type: MessageType::Error });
|
||||
ConfigStoreBuilder::empty().build(&mut ExternalPluginStore::new(false)).unwrap()
|
||||
});
|
||||
|
||||
@@ -204,7 +218,9 @@ impl ServerLinterBuilder {
|
||||
external_linter,
|
||||
);
|
||||
if let Err(err) = res {
|
||||
error!("Failed to setup JS plugins config:\n{err}\n");
|
||||
let message = format!("Failed to setup JS plugins config:\n{err}");
|
||||
error!("{message}");
|
||||
client_messages.insert(ClientMessage { message, r#type: MessageType::Error });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -229,7 +245,11 @@ impl ServerLinterBuilder {
|
||||
{
|
||||
Ok(runner) => runner,
|
||||
Err(e) => {
|
||||
warn!("Failed to initialize type-aware linting: {e}");
|
||||
let message = format!("Failed to initialize type-aware linting:\n{e}");
|
||||
|
||||
error!(message);
|
||||
|
||||
client_messages.insert(ClientMessage { message, r#type: MessageType::Error });
|
||||
let linter =
|
||||
Linter::new(lint_options, config_store_clone, external_linter.cloned())
|
||||
.with_workspace_uri(Some(root_uri.as_ref()));
|
||||
@@ -254,7 +274,7 @@ impl ServerLinterBuilder {
|
||||
lint_options.report_unused_directive,
|
||||
options.rules_customization,
|
||||
),
|
||||
Vec::new(),
|
||||
client_messages.into_iter().collect(),
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -332,7 +352,7 @@ impl ServerLinterBuilder {
|
||||
nested_ignore_patterns: &mut Vec<(Vec<String>, PathBuf)>,
|
||||
extended_paths: &mut FxHashSet<PathBuf>,
|
||||
workspace_uri: Option<&str>,
|
||||
) -> FxHashMap<PathBuf, Config> {
|
||||
) -> (FxHashMap<PathBuf, Config>, Vec<ClientMessage>) {
|
||||
let config_paths = discover_configs_in_tree(root_path, base_config_path);
|
||||
|
||||
#[cfg_attr(not(feature = "napi"), allow(unused_mut))]
|
||||
@@ -350,15 +370,21 @@ impl ServerLinterBuilder {
|
||||
|
||||
let (configs, errors) = loader.load_discovered_with_root_dir(root_path, config_paths);
|
||||
|
||||
let mut client_messages = Vec::with_capacity(errors.len());
|
||||
for error in errors {
|
||||
if let Some(path) = error.path() {
|
||||
warn!("Skipping config file {}: {:?}", path.display(), error);
|
||||
} else {
|
||||
warn!("Skipping config file: {:?}", error);
|
||||
}
|
||||
let message = config_loader_error_to_message(error);
|
||||
|
||||
// send message to LSP stderr
|
||||
warn!(message);
|
||||
|
||||
// show message to the LSP client, if there is already a message, append to it
|
||||
client_messages.push(ClientMessage { message, r#type: MessageType::Error });
|
||||
}
|
||||
|
||||
build_nested_configs(configs, nested_ignore_patterns, Some(extended_paths))
|
||||
(
|
||||
build_nested_configs(configs, nested_ignore_patterns, Some(extended_paths)),
|
||||
client_messages,
|
||||
)
|
||||
}
|
||||
|
||||
#[expect(clippy::filetype_is_file)]
|
||||
@@ -1140,7 +1166,7 @@ mod test {
|
||||
let mut external_plugin_store = ExternalPluginStore::new(false);
|
||||
let mut extended_paths = FxHashSet::default();
|
||||
let base_config_path = get_file_path("fixtures/lsp/init_nested_configs/.oxlintrc.json");
|
||||
let configs = builder.create_nested_configs(
|
||||
let (configs, client_message) = builder.create_nested_configs(
|
||||
&get_file_path("fixtures/lsp/init_nested_configs"),
|
||||
&base_config_path,
|
||||
&mut external_plugin_store,
|
||||
@@ -1155,6 +1181,7 @@ mod test {
|
||||
assert_eq!(configs_dirs.len(), 2);
|
||||
assert!(configs_dirs[1].ends_with("deep2"));
|
||||
assert!(configs_dirs[0].ends_with("deep1"));
|
||||
assert_eq!(client_message, []);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
use cow_utils::CowUtils;
|
||||
use std::borrow::Cow;
|
||||
|
||||
use oxc_diagnostics::OxcCode;
|
||||
use tower_lsp_server::gen_lsp_types::Range;
|
||||
use oxc_diagnostics::{OxcCode, OxcDiagnostic, Severity};
|
||||
use oxc_language_server::ClientMessage;
|
||||
use tower_lsp_server::gen_lsp_types::{MessageType, Range};
|
||||
|
||||
use crate::config_loader::ConfigLoadError;
|
||||
|
||||
/// Returns `true` if LSP ranges `a` and `b` overlap or touch (share a boundary point).
|
||||
///
|
||||
@@ -29,6 +33,64 @@ pub fn get_full_rule_name(rule_code: &OxcCode) -> Option<Cow<'_, str>> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns a full diagnostic message string, including the main message, help text, and note if present.
|
||||
pub fn get_oxc_diagnostic_full_message(diagnostic: &OxcDiagnostic) -> String {
|
||||
let mut diagnostic_message = String::with_capacity(
|
||||
diagnostic.message.len()
|
||||
+ diagnostic.help.as_ref().map_or(0, |h| h.len() + 7) // "help: " prefix
|
||||
+ diagnostic.note.as_ref().map_or(0, |n| n.len() + 7), // "note: " prefix
|
||||
);
|
||||
|
||||
diagnostic_message.push_str(&diagnostic.message);
|
||||
if let Some(help) = &diagnostic.help {
|
||||
diagnostic_message.push_str("\nhelp: ");
|
||||
diagnostic_message.push_str(help);
|
||||
}
|
||||
|
||||
if let Some(note) = &diagnostic.note {
|
||||
diagnostic_message.push_str("\nnote: ");
|
||||
diagnostic_message.push_str(note);
|
||||
}
|
||||
diagnostic_message
|
||||
}
|
||||
|
||||
/// Converts an `OxcDiagnostic` to a `ClientMessage` for LSP communication.
|
||||
pub fn oxc_diagnostic_to_client_message(diagnostic: &OxcDiagnostic) -> ClientMessage {
|
||||
let message = get_oxc_diagnostic_full_message(diagnostic);
|
||||
let r#type = match diagnostic.severity {
|
||||
Severity::Error => MessageType::Error,
|
||||
Severity::Warning => MessageType::Warning,
|
||||
Severity::Advice => MessageType::Info,
|
||||
};
|
||||
ClientMessage { message, r#type }
|
||||
}
|
||||
|
||||
/// Converts a `ConfigLoadError` to a human-readable message string.
|
||||
pub fn config_loader_error_to_message(error: ConfigLoadError) -> String {
|
||||
match error {
|
||||
ConfigLoadError::Parse { path, error } => {
|
||||
format!(
|
||||
"Failed to parse config file {}: {}",
|
||||
path.to_string_lossy().cow_replace("\\", "/"),
|
||||
get_oxc_diagnostic_full_message(&error)
|
||||
)
|
||||
}
|
||||
ConfigLoadError::Build { path, error } => {
|
||||
format!(
|
||||
"Failed to build config from file {}: {}",
|
||||
path.to_string_lossy().cow_replace("\\", "/"),
|
||||
error
|
||||
)
|
||||
}
|
||||
ConfigLoadError::JsConfigFileFoundButJsRuntimeNotAvailable => {
|
||||
"JavaScript/TypeScript config file found but JS runtime not available.".to_string()
|
||||
}
|
||||
ConfigLoadError::Diagnostic(diag) => {
|
||||
format!("Failed to load config: {}", get_oxc_diagnostic_full_message(&diag))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
use tower_lsp_server::gen_lsp_types::{Position, Range};
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
|
||||
|
||||
exports[`LSP initialization > should show an error message when configPath points to an invalid config 1`] = `
|
||||
"--- Show Messages ---------
|
||||
[0] type=1
|
||||
Failed to parse config <cwd>/test/lsp/init/fixtures/invalid-custom-config-path/not-found.json with error Os { code: 2, kind: NotFound, message: "No such file or directory" }"
|
||||
`;
|
||||
|
||||
exports[`LSP initialization > should show an error message when the nested config is invalid 1`] = `
|
||||
"--- Show Messages ---------
|
||||
[0] type=1
|
||||
Failed to build config from file <cwd>/test/lsp/init/fixtures/invalid-config-nested/nested/.oxlintrc.json: Invalid configuration for rule \`no-return-assign\`:
|
||||
unknown variant \`foobar\`, expected \`always\` or \`except-parens\`
|
||||
received config: \`"foobar"\`"
|
||||
`;
|
||||
|
||||
exports[`LSP initialization > should show an error message when the root config is invalid 1`] = `
|
||||
"--- Show Messages ---------
|
||||
[0] type=1
|
||||
Failed to build config from oxlintrc:
|
||||
Invalid configuration for rule \`no-return-assign\`:
|
||||
unknown variant \`foobar\`, expected \`always\` or \`except-parens\`
|
||||
received config: \`"foobar"\`"
|
||||
`;
|
||||
@@ -0,0 +1 @@
|
||||
{}
|
||||
@@ -0,0 +1,5 @@
|
||||
{
|
||||
"rules": {
|
||||
"eslint/no-return-assign": ["error", "foobar"]
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
debugger;
|
||||
@@ -0,0 +1,5 @@
|
||||
{
|
||||
"rules": {
|
||||
"eslint/no-return-assign": ["error", "foobar"]
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
debugger;
|
||||
@@ -0,0 +1 @@
|
||||
debugger;
|
||||
@@ -1,8 +1,11 @@
|
||||
import { join } from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { createLspConnection } from "../utils";
|
||||
import { createLspConnection, initializationMessagesFixture } from "../utils";
|
||||
import { WatchKind } from "vscode-languageserver-protocol/node";
|
||||
|
||||
const FIXTURES_DIR = join(import.meta.dirname, "fixtures");
|
||||
|
||||
describe("LSP initialization", () => {
|
||||
it("should start LSP server and respond to initialize request", async () => {
|
||||
const dirPath = import.meta.dirname;
|
||||
@@ -115,4 +118,29 @@ describe("LSP initialization", () => {
|
||||
]);
|
||||
},
|
||||
);
|
||||
|
||||
it("should show an error message when the root config is invalid", async () => {
|
||||
expect(
|
||||
await initializationMessagesFixture(FIXTURES_DIR, "invalid-config-root/test.ts"),
|
||||
).toMatchSnapshot();
|
||||
});
|
||||
|
||||
it("should show an error message when the nested config is invalid", async () => {
|
||||
expect(
|
||||
await initializationMessagesFixture(FIXTURES_DIR, "invalid-config-nested/test.ts"),
|
||||
).toMatchSnapshot();
|
||||
});
|
||||
|
||||
it.skipIf(
|
||||
// skip this test on Windows due to different not found messages
|
||||
// on linux the message is "No such file or directory"
|
||||
// on windows the message is "The system cannot find the file specified."
|
||||
() => process.platform === "win32",
|
||||
)("should show an error message when configPath points to an invalid config", async () => {
|
||||
expect(
|
||||
await initializationMessagesFixture(FIXTURES_DIR, "invalid-custom-config-path/test.ts", {
|
||||
configPath: "./not-found.json",
|
||||
}),
|
||||
).toMatchSnapshot();
|
||||
});
|
||||
});
|
||||
@@ -84,6 +84,15 @@ export function createLspConnection(env: Record<string, string> = {}) {
|
||||
return result;
|
||||
},
|
||||
|
||||
getShowMessage(): Promise<{ type: number; message: string }> {
|
||||
return new Promise((resolve) => {
|
||||
const disposer = connection.onNotification("window/showMessage", (params) => {
|
||||
resolve(params);
|
||||
disposer.dispose();
|
||||
});
|
||||
});
|
||||
},
|
||||
|
||||
async didChangeConfiguration(settings: unknown) {
|
||||
await connection.sendNotification(DidChangeConfigurationNotification.type, { settings });
|
||||
},
|
||||
@@ -170,6 +179,25 @@ export async function lintFixture(
|
||||
);
|
||||
}
|
||||
|
||||
export async function initializationMessagesFixture(
|
||||
fixturesDir: string,
|
||||
fixturePath: string,
|
||||
initializationOptions?: OxlintLSPConfig,
|
||||
): Promise<string> {
|
||||
const workspaceUri = pathToFileURL(dirname(join(fixturesDir, fixturePath))).href;
|
||||
await using client = createLspConnection();
|
||||
const messagePromise = client.getShowMessage();
|
||||
await client.initialize(
|
||||
[{ uri: workspaceUri, name: "workspace-0" }],
|
||||
PULL_DIAGNOSTICS_CAPABILITY,
|
||||
[{ workspaceUri, options: initializationOptions ?? null }],
|
||||
);
|
||||
|
||||
const message = await messagePromise;
|
||||
|
||||
return snapshotShowMessages([message]);
|
||||
}
|
||||
|
||||
export async function lintSingleFileFixture(
|
||||
fixtureDir: string,
|
||||
fixturePath: string,
|
||||
@@ -430,6 +458,29 @@ type OxlintLSPConfig = {
|
||||
>;
|
||||
};
|
||||
|
||||
function snapshotShowMessages(messages: { type: number; message: string }[]): string {
|
||||
if (messages.length === 0) {
|
||||
return "--- Show Messages ---------\n(none)";
|
||||
}
|
||||
|
||||
return [
|
||||
"--- Show Messages ---------",
|
||||
...messages.map(
|
||||
({ type, message }, index) => `[${index}] type=${type}\n${sanitizeSnapshotText(message)}`,
|
||||
),
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
function sanitizeSnapshotText(text: string): string {
|
||||
return text.replaceAll(
|
||||
process
|
||||
.cwd()
|
||||
// replace current cwd with forward slashes, the `oxlint` backend output in test-mode uses forward slashes.
|
||||
.replaceAll("\\", "/"),
|
||||
"<cwd>",
|
||||
);
|
||||
}
|
||||
|
||||
async function getDiagnosticSnapshot(
|
||||
fixturePath: string,
|
||||
filePath: string,
|
||||
|
||||
@@ -139,7 +139,7 @@ pub trait Tool: Send + Sync {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
pub struct ClientMessage {
|
||||
/// The message to be sent to the client.
|
||||
pub message: String,
|
||||
|
||||
Reference in new issue
Block a user