mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
feat(oxfmt/lsp): show root config errors to LSP client (#26014)
## Summary
This PR improves the oxfmt LSP server's error reporting by propagating root configuration resolution errors to the language server client as notification messages. Previously, configuration errors were only logged via `warn!` macros, making them invisible to users in their editors.
## Problem
When the oxfmt LSP server encountered configuration errors (e.g., invalid `.oxfmtrc.json`, missing required files), these errors were silently logged on the server side. Users had no visibility into why formatting wasn't working or configuration wasn't being applied, resulting in poor debugging experience.
## Solution
The PR modifies the configuration loading pipeline to:
1. Capture configuration errors as `ClientMessage` structs containing the error message and message type
2. Propagate these messages through the `ServerFormatterBuilder` and `ServerFormatter::new()` initialization flow
3. Return collected messages alongside the formatter instance
4. Display these messages to the client via LSP `ShowMessage` notifications during initialization and on config changes
_PR description generated by AI 🤖_
This commit is contained in:
1 parent
5dae56f577
commit
77f8156f08
6 files changed
+107
-38
No files matched your search
@@ -5,7 +5,7 @@ use std::{
|
||||
|
||||
use ignore::gitignore::Gitignore;
|
||||
use tower_lsp_server::gen_lsp_types::{
|
||||
DocumentFormattingProvider, Pattern, Range, ServerCapabilities, TextEdit, Uri,
|
||||
DocumentFormattingProvider, MessageType, Pattern, Range, ServerCapabilities, TextEdit, Uri,
|
||||
};
|
||||
use tracing::{debug, error, warn};
|
||||
|
||||
@@ -77,16 +77,15 @@ impl ServerFormatterBuilder {
|
||||
let source_formatter = SourceFormatter::new(num_of_threads)
|
||||
.with_external_services(Some(self.external_services.clone()));
|
||||
|
||||
(
|
||||
ServerFormatter::new(
|
||||
root_path.to_path_buf(),
|
||||
source_formatter,
|
||||
JsConfigLoaderCb::clone(&self.js_config_loader),
|
||||
explicit_config_path,
|
||||
use_nested_config,
|
||||
),
|
||||
Vec::new(),
|
||||
)
|
||||
let (formatter, client_message) = ServerFormatter::new(
|
||||
root_path.to_path_buf(),
|
||||
source_formatter,
|
||||
JsConfigLoaderCb::clone(&self.js_config_loader),
|
||||
explicit_config_path,
|
||||
use_nested_config,
|
||||
);
|
||||
|
||||
(formatter, client_message.into_iter().collect())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -200,7 +199,7 @@ impl Tool for ServerFormatter {
|
||||
// and only the root resolver, `.editorconfig` and `.prettierignore` do eager file IO.
|
||||
// The trade-off is over-eviction, a config change in one nested dir also drops cached probes elsewhere.
|
||||
// But format requests are sporadic enough that lazy re-population costs nothing observable.
|
||||
let new_state = Self::build_state(
|
||||
let (new_state, client_message) = Self::build_state(
|
||||
&self.root_path,
|
||||
self.explicit_config_path.as_deref(),
|
||||
self.use_nested_config,
|
||||
@@ -208,7 +207,11 @@ impl Tool for ServerFormatter {
|
||||
);
|
||||
*self.state.write().expect("state rwlock poisoned") = Arc::new(new_state);
|
||||
|
||||
ToolRestartChanges { tool: None, watch_patterns: None, client_messages: Vec::new() }
|
||||
ToolRestartChanges {
|
||||
tool: None,
|
||||
watch_patterns: None,
|
||||
client_messages: client_message.into_iter().collect(),
|
||||
}
|
||||
}
|
||||
|
||||
fn run_format(&self, document: TextDocument) -> Result<Vec<TextEdit>, String> {
|
||||
@@ -277,21 +280,24 @@ impl ServerFormatter {
|
||||
js_config_loader: JsConfigLoaderCb,
|
||||
explicit_config_path: Option<PathBuf>,
|
||||
use_nested_config: bool,
|
||||
) -> Self {
|
||||
let state = Self::build_state(
|
||||
) -> (Self, Vec<ClientMessage>) {
|
||||
let (state, client_message) = Self::build_state(
|
||||
&root_path,
|
||||
explicit_config_path.as_deref(),
|
||||
use_nested_config,
|
||||
&js_config_loader,
|
||||
);
|
||||
Self {
|
||||
root_path,
|
||||
source_formatter,
|
||||
js_config_loader,
|
||||
explicit_config_path,
|
||||
use_nested_config,
|
||||
state: RwLock::new(Arc::new(state)),
|
||||
}
|
||||
(
|
||||
Self {
|
||||
root_path,
|
||||
source_formatter,
|
||||
js_config_loader,
|
||||
explicit_config_path,
|
||||
use_nested_config,
|
||||
state: RwLock::new(Arc::new(state)),
|
||||
},
|
||||
client_message,
|
||||
)
|
||||
}
|
||||
|
||||
/// Build a fresh [`FormatterState`] from scratch.
|
||||
@@ -308,29 +314,45 @@ impl ServerFormatter {
|
||||
explicit_config_path: Option<&Path>,
|
||||
use_nested_config: bool,
|
||||
js_config_loader: &JsConfigLoaderCb,
|
||||
) -> FormatterState {
|
||||
let scopes = ConfigScopes::load(
|
||||
) -> (FormatterState, Vec<ClientMessage>) {
|
||||
let mut client_message = vec![];
|
||||
let scopes = match ConfigScopes::load(
|
||||
root_path,
|
||||
explicit_config_path,
|
||||
use_nested_config,
|
||||
Some(js_config_loader),
|
||||
)
|
||||
.unwrap_or_else(|err| {
|
||||
warn!("{err}\nFalling back to default config for {}", root_path.display());
|
||||
ConfigScopes::with_default_root(root_path, use_nested_config, Some(js_config_loader))
|
||||
});
|
||||
) {
|
||||
Ok(scopes) => scopes,
|
||||
Err(err) => {
|
||||
client_message.push(ClientMessage {
|
||||
message: format!("{}: {err}", root_path.display()),
|
||||
r#type: MessageType::Error,
|
||||
});
|
||||
ConfigScopes::with_default_root(
|
||||
root_path,
|
||||
use_nested_config,
|
||||
Some(js_config_loader),
|
||||
)
|
||||
}
|
||||
};
|
||||
|
||||
// NOTE: `.gitignore` is intentionally NOT included here.
|
||||
// An LSP document is explicitly formatted by the user.
|
||||
// (CLI also formats explicitly specified git ignored files.)
|
||||
let ignore_matchers = resolve_ignore_paths(root_path, &[])
|
||||
let ignore_matchers = match resolve_ignore_paths(root_path, &[])
|
||||
.and_then(|paths| build_global_ignore_matchers(root_path, &[], &paths))
|
||||
.unwrap_or_else(|err| {
|
||||
warn!("Failed to load .prettierignore: {err}, proceeding without ignore globs");
|
||||
{
|
||||
Ok(matchers) => matchers,
|
||||
Err(err) => {
|
||||
client_message.push(ClientMessage {
|
||||
message: format!("Failed to load .prettierignore: {err}"),
|
||||
r#type: MessageType::Error,
|
||||
});
|
||||
vec![]
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
FormatterState { scopes, ignore_matchers }
|
||||
(FormatterState { scopes, ignore_matchers }, client_message)
|
||||
}
|
||||
|
||||
/// Snapshot the current state.
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
|
||||
|
||||
exports[`LSP initialization > should show a client message when the workspace config is invalid 1`] = `
|
||||
"--- Show Messages ---------
|
||||
[0] type=1
|
||||
<cwd>/test/lsp/init/fixtures/invalid-config: Failed to parse configuration.
|
||||
Invalid tabWidth: The indent width should be between 0 and 24
|
||||
"
|
||||
`;
|
||||
@@ -0,0 +1,3 @@
|
||||
{
|
||||
"tabWidth": 250
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
import { join } from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { createLspConnection } from "../utils";
|
||||
import { createLspConnection, snapshotShowMessages } from "../utils";
|
||||
import { WatchKind } from "vscode-languageserver-protocol/node";
|
||||
|
||||
describe("LSP initialization", () => {
|
||||
@@ -33,6 +34,18 @@ describe("LSP initialization", () => {
|
||||
expect(initResult.serverInfo?.version).toContain(`(VP: ${vpVersion})`);
|
||||
});
|
||||
|
||||
it("should show a client message when the workspace config is invalid", async () => {
|
||||
const dirUri = pathToFileURL(join(import.meta.dirname, "fixtures", "invalid-config")).href;
|
||||
await using client = createLspConnection();
|
||||
|
||||
const showMessagePromise = client.getShowMessage();
|
||||
await client.initialize([{ uri: dirUri, name: "test" }], {}, [
|
||||
{ workspaceUri: dirUri, options: null },
|
||||
]);
|
||||
const showMessage = await showMessagePromise;
|
||||
expect(snapshotShowMessages([showMessage])).toMatchSnapshot();
|
||||
});
|
||||
|
||||
it.each([
|
||||
[
|
||||
undefined,
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
InitializedNotification,
|
||||
InitializeRequest,
|
||||
RegistrationRequest,
|
||||
ShowMessageNotification,
|
||||
ShutdownRequest,
|
||||
StreamMessageReader,
|
||||
StreamMessageWriter,
|
||||
@@ -100,6 +101,15 @@ export function createLspConnection(env: Record<string, string> = {}) {
|
||||
});
|
||||
},
|
||||
|
||||
getShowMessage(): Promise<{ type: number; message: string }> {
|
||||
return new Promise((resolve) => {
|
||||
const disposer = connection.onNotification(ShowMessageNotification.type, (params) => {
|
||||
resolve(params);
|
||||
disposer.dispose();
|
||||
});
|
||||
});
|
||||
},
|
||||
|
||||
async [Symbol.asyncDispose]() {
|
||||
await connection.sendRequest(ShutdownRequest.type);
|
||||
await connection.sendNotification(ExitNotification.type);
|
||||
@@ -320,6 +330,19 @@ function uriSnapshotHeader(fileUri: string, fixtureDir: string): string {
|
||||
${safeUri}`;
|
||||
}
|
||||
|
||||
export 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${sanitizeMessage(message)}`,
|
||||
),
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
function sanitizeMessage(message: string): string {
|
||||
return message.replaceAll(process.cwd(), "<cwd>");
|
||||
}
|
||||
@@ -4,8 +4,7 @@
|
||||
/// To be aligned with the CLI integration, for files the extension should be used.
|
||||
/// If the document is not a file (protocol not `file://`), or the extension is not known,
|
||||
/// the language ID is used to select the parser strategy.
|
||||
/// For non file protocols (e.g. `untitled://`, `vscode-notebook-cell://`, etc.),
|
||||
/// the language ID is always used to select the parser strategy.
|
||||
/// For non file protocols (e.g. `untitled://`, `vscode-notebook-cell://`, etc.), the language ID is always used to select the parser strategy.
|
||||
///
|
||||
/// For a starting list of known language identifiers, see:
|
||||
/// <https://code.visualstudio.com/docs/languages/identifiers#_known-language-identifiers>
|
||||
|
||||
Reference in new issue
Block a user