Skip to content

Commit

Permalink
Ignore non-file workspace URL (#12725)
Browse files Browse the repository at this point in the history
## Summary

This PR updates the server to ignore non-file workspace URL.

This is to avoid crashing the server if the URL scheme is not "file".
We'd still raise an error if the URL to file path conversion fails.

Also, as per the docs of
[`to_file_path`](https://docs.rs/url/2.5.2/url/struct.Url.html#method.to_file_path):

> Note: This does not actually check the URL’s scheme, and may give
nonsensical results for other schemes. It is the user’s responsibility
to check the URL’s scheme before calling this.

resolves: #12660

## Test Plan

I'm not sure how to test this locally but the change is small enough to
validate on its own.
  • Loading branch information
dhruvmanila authored Aug 7, 2024
1 parent 50ff5c7 commit 7fcfedd
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 9 deletions.
12 changes: 10 additions & 2 deletions crates/ruff_server/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,18 @@ pub(super) fn try_show_message(
Ok(())
}

/// Sends an error to the client with a formatted message. The error is sent in a
/// `window/showMessage` notification.
/// Sends a request to display an error to the client with a formatted message. The error is sent
/// in a `window/showMessage` notification.
macro_rules! show_err_msg {
($msg:expr$(, $($arg:tt),*)?) => {
crate::message::show_message(::core::format_args!($msg, $($($arg),*)?).to_string(), lsp_types::MessageType::ERROR)
};
}

/// Sends a request to display a warning to the client with a formatted message. The warning is
/// sent in a `window/showMessage` notification.
macro_rules! show_warn_msg {
($msg:expr$(, $($arg:tt),*)?) => {
crate::message::show_message(::core::format_args!($msg, $($($arg),*)?).to_string(), lsp_types::MessageType::WARNING)
};
}
19 changes: 12 additions & 7 deletions crates/ruff_server/src/session/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,21 @@ impl Index {
workspace_settings: Option<ClientSettings>,
global_settings: &ClientSettings,
) -> crate::Result<()> {
if workspace_url.scheme() != "file" {
tracing::warn!("Ignoring non-file workspace: {workspace_url}");
show_warn_msg!("Ruff does not support non-file workspaces; Ignoring {workspace_url}");
return Ok(());
}
let workspace_path = workspace_url.to_file_path().map_err(|()| {
anyhow!("Failed to convert workspace URL to file path: {workspace_url}")
})?;

let client_settings = if let Some(workspace_settings) = workspace_settings {
ResolvedClientSettings::with_workspace(&workspace_settings, global_settings)
} else {
ResolvedClientSettings::global(global_settings)
};

let workspace_path = workspace_url
.to_file_path()
.map_err(|()| anyhow!("workspace URL was not a file path!"))?;

let workspace_settings_index = ruff_settings::RuffSettingsIndex::new(
&workspace_path,
client_settings.editor_settings(),
Expand All @@ -227,9 +232,9 @@ impl Index {
}

pub(super) fn close_workspace_folder(&mut self, workspace_url: &Url) -> crate::Result<()> {
let workspace_path = workspace_url
.to_file_path()
.map_err(|()| anyhow!("workspace URL was not a file path!"))?;
let workspace_path = workspace_url.to_file_path().map_err(|()| {
anyhow!("Failed to convert workspace URL to file path: {workspace_url}")
})?;

self.settings.remove(&workspace_path).ok_or_else(|| {
anyhow!(
Expand Down

0 comments on commit 7fcfedd

Please sign in to comment.