Skip to content

Commit 3ef37da

Browse files
committed
fix(oxlint/lsp): error on unknown command (#20841)
> ## Pull request overview > > Adjusts LSP workspace command execution so unknown/unsupported commands surface as errors (instead of silently returning `null`), by removing the pre-dispatch “responsible tool” check and updating tests accordingly.
1 parent dade989 commit 3ef37da

4 files changed

Lines changed: 15 additions & 34 deletions

File tree

apps/oxlint/src/lsp/server_linter.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -542,13 +542,8 @@ impl Tool for ServerLinter {
542542
}
543543
}
544544

545-
/// Check if the linter should know about the given command
546-
fn is_responsible_for_command(&self, command: &str) -> bool {
547-
command == FIX_ALL_COMMAND_ID
548-
}
549-
550545
/// Tries to execute the given command with the provided arguments.
551-
/// If the command is not recognized, returns `Ok(None)`.
546+
/// If the command is not recognized, returns `Err(ErrorCode)`.
552547
/// If the command is recognized and executed it can return:
553548
/// - `Ok(Some(WorkspaceEdit))` if the command was executed successfully and produced a workspace edit.
554549
/// - `Ok(None)` if the command was executed successfully but did not produce any workspace edit.
@@ -561,7 +556,7 @@ impl Tool for ServerLinter {
561556
arguments: Vec<serde_json::Value>,
562557
) -> Result<Option<WorkspaceEdit>, ErrorCode> {
563558
if command != FIX_ALL_COMMAND_ID {
564-
return Ok(None);
559+
return Err(ErrorCode::InvalidParams);
565560
}
566561

567562
let args = FixAllCommandArgs::try_from(arguments).map_err(|_| ErrorCode::InvalidParams)?;

crates/oxc_language_server/src/tests.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,13 @@ impl Tool for FakeTool {
5959
"FakeTool"
6060
}
6161

62-
fn is_responsible_for_command(&self, command: &str) -> bool {
63-
command == FAKE_COMMAND
64-
}
65-
6662
fn execute_command(
6763
&self,
6864
command: &str,
6965
arguments: Vec<serde_json::Value>,
7066
) -> Result<Option<WorkspaceEdit>, ErrorCode> {
7167
if command != FAKE_COMMAND {
72-
return Err(ErrorCode::MethodNotFound);
68+
return Err(ErrorCode::InvalidParams);
7369
}
7470

7571
if !arguments.is_empty() {
@@ -973,11 +969,11 @@ mod test_suite {
973969
let execute_command_request = execute_command_request("invalid.command", &[], 3);
974970
server.send_request(execute_command_request).await;
975971

976-
// Should not return an error, but a null result
972+
// Should return an error
977973
let execute_command_response = server.recv_response().await;
978-
assert!(execute_command_response.is_ok());
974+
assert!(execute_command_response.is_error());
979975
assert_eq!(execute_command_response.id(), &Id::Number(3));
980-
assert_eq!(execute_command_response.result().unwrap(), &json!(null));
976+
assert_eq!(execute_command_response.error().unwrap().code, ErrorCode::InvalidParams);
981977

982978
server.shutdown(4).await;
983979
}

crates/oxc_language_server/src/tool.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,8 @@ pub trait Tool: Send + Sync {
5757
options: serde_json::Value,
5858
) -> ToolRestartChanges;
5959

60-
/// Check if this tool is responsible for handling the given command.
61-
/// TODO: this is not needed anymore, we have only one tool per server,
62-
/// we can remove this method and directly call execute_command on the tool.
63-
fn is_responsible_for_command(&self, _command: &str) -> bool {
64-
false
65-
}
66-
6760
/// Tries to execute the given command with the provided arguments.
68-
/// If the command is not recognized, returns `Ok(None)`.
61+
/// If the command is not recognized, returns `Err(ErrorCode)`.
6962
/// If the command is recognized and executed it can return:
7063
/// - `Ok(Some(WorkspaceEdit))` if the command was executed successfully and produced a workspace edit.
7164
/// - `Ok(None)` if the command was executed successfully but did not produce any workspace edit.
@@ -77,7 +70,7 @@ pub trait Tool: Send + Sync {
7770
_command: &str,
7871
_arguments: Vec<serde_json::Value>,
7972
) -> Result<Option<WorkspaceEdit>, ErrorCode> {
80-
Ok(None)
73+
Err(ErrorCode::InvalidParams)
8174
}
8275

8376
/// Get code actions or commands provided by this tool for the given URI and range.

crates/oxc_language_server/src/worker.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -374,10 +374,9 @@ impl WorkspaceWorker {
374374
}
375375

376376
/// Execute a command for the workspace.
377-
/// Currently, only the `oxc.fixAll` command is supported.
378377
///
379378
/// # Errors
380-
/// Returns `ErrorCode` when the command is found but could not be executed.
379+
/// Returns `ErrorCode` when the command is not found or could not be executed.
381380
pub async fn execute_command(
382381
&self,
383382
command: &str,
@@ -387,10 +386,7 @@ impl WorkspaceWorker {
387386
let Some(tool) = tool_guard.as_ref() else {
388387
return Ok(None);
389388
};
390-
if tool.is_responsible_for_command(command) {
391-
return tool.execute_command(command, arguments);
392-
}
393-
Ok(None)
389+
tool.execute_command(command, arguments)
394390
}
395391
}
396392

@@ -427,8 +423,9 @@ mod tests {
427423
use std::str::FromStr;
428424

429425
use std::sync::Arc;
430-
use tower_lsp_server::ls_types::{
431-
CodeActionContext, CodeActionOrCommand, FileChangeType, FileEvent, Range, Uri,
426+
use tower_lsp_server::{
427+
jsonrpc::ErrorCode,
428+
ls_types::{CodeActionContext, CodeActionOrCommand, FileChangeType, FileEvent, Range, Uri},
432429
};
433430

434431
use crate::{
@@ -501,8 +498,8 @@ mod tests {
501498

502499
// Test command not found
503500
let result = worker.execute_command("unknown.command", vec![]).await;
504-
assert!(result.is_ok());
505-
assert!(result.ok().unwrap().is_none());
501+
assert!(result.is_err());
502+
assert_eq!(result.err().unwrap(), ErrorCode::InvalidParams);
506503

507504
// Test command found but no arguments
508505
let result = worker.execute_command(FAKE_COMMAND, vec![]).await;

0 commit comments

Comments
 (0)