Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ruff server: Ruff configuration from client settings overrides project configuration #11062

Merged
merged 4 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/ruff_server/src/server/api/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub(super) fn generate_diagnostics(snapshot: &DocumentSnapshot) -> Vec<lsp_types
if snapshot.client_settings().lint() {
crate::lint::check(
snapshot.document(),
&snapshot.settings().linter,
snapshot.settings().linter(),
snapshot.encoding(),
)
} else {
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_server/src/server/api/requests/code_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
document,
snapshot.resolved_client_capabilities(),
snapshot.url(),
&snapshot.settings().linter,
snapshot.settings().linter(),
snapshot.encoding(),
document.version(),
)?),
Expand Down Expand Up @@ -140,7 +140,7 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCo
document,
snapshot.resolved_client_capabilities(),
snapshot.url(),
&snapshot.settings().linter,
snapshot.settings().linter(),
snapshot.encoding(),
document.version(),
)?),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
document,
snapshot.resolved_client_capabilities(),
snapshot.url(),
&snapshot.settings().linter,
snapshot.settings().linter(),
snapshot.encoding(),
document.version(),
)
Expand All @@ -65,7 +65,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
document,
snapshot.resolved_client_capabilities(),
snapshot.url(),
&snapshot.settings().linter,
snapshot.settings().linter(),
snapshot.encoding(),
document.version(),
)
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_server/src/server/api/requests/execute_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl super::SyncRequestHandler for ExecuteCommand {
Command::FixAll => {
let edits = super::code_action_resolve::fix_all_edit(
snapshot.document(),
&snapshot.settings().linter,
snapshot.settings().linter(),
snapshot.encoding(),
)
.with_failure_code(ErrorCode::InternalError)?;
Expand All @@ -83,7 +83,7 @@ impl super::SyncRequestHandler for ExecuteCommand {
Command::OrganizeImports => {
let edits = super::code_action_resolve::organize_imports_edit(
snapshot.document(),
&snapshot.settings().linter,
snapshot.settings().linter(),
snapshot.encoding(),
)
.with_failure_code(ErrorCode::InternalError)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl super::BackgroundDocumentRequestHandler for Format {
pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::FormatResponse> {
let doc = snapshot.document();
let source = doc.contents();
let formatted = crate::format::format(doc, &snapshot.settings().formatter)
let formatted = crate::format::format(doc, snapshot.settings().formatter())
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
// fast path - if the code is the same, return early
if formatted == source {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_server/src/server/api/requests/format_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl super::BackgroundDocumentRequestHandler for FormatRange {
let index = document.index();
let range = params.range.to_text_range(text, index, snapshot.encoding());
let formatted_range =
crate::format::format_range(document, &snapshot.settings().formatter, range)
crate::format::format_range(document, snapshot.settings().formatter(), range)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
Ok(Some(vec![types::TextEdit {
range: formatted_range
Expand Down
5 changes: 3 additions & 2 deletions crates/ruff_server/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ impl Session {
) -> crate::Result<Self> {
Ok(Self {
position_encoding,
workspaces: workspace::Workspaces::new(workspaces, &global_settings)?,
global_settings,
resolved_client_capabilities: Arc::new(ResolvedClientCapabilities::new(
client_capabilities,
)),
workspaces: workspace::Workspaces::new(workspaces)?,
})
}

Expand Down Expand Up @@ -87,7 +87,8 @@ impl Session {
}

pub(crate) fn open_workspace_folder(&mut self, url: &Url) -> crate::Result<()> {
self.workspaces.open_workspace_folder(url)?;
self.workspaces
.open_workspace_folder(url, &self.global_settings)?;
Ok(())
}

Expand Down
38 changes: 16 additions & 22 deletions crates/ruff_server/src/session/settings.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{ffi::OsString, ops::Deref, path::PathBuf, str::FromStr};
use std::{ops::Deref, str::FromStr};

use lsp_types::Url;
use ruff_linter::{line_width::LineLength, RuleSelector};
Expand All @@ -11,7 +11,7 @@ pub(crate) type WorkspaceSettingsMap = FxHashMap<Url, ClientSettings>;
/// Resolved client settings for a specific document. These settings are meant to be
/// used directly by the server, and are *not* a 1:1 representation with how the client
/// sends them.
#[derive(Debug)]
#[derive(Clone, Debug)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[allow(clippy::struct_excessive_bools)]
pub(crate) struct ResolvedClientSettings {
Expand All @@ -22,25 +22,22 @@ pub(crate) struct ResolvedClientSettings {
#[allow(dead_code)]
disable_rule_comment_enable: bool,
fix_violation_enable: bool,
// TODO(jane): Remove once editor settings resolution is implemented
#[allow(dead_code)]
editor_settings: ResolvedEditorSettings,
}

/// Contains the resolved values of 'editor settings' - Ruff configuration for the linter/formatter that was passed in via
/// LSP client settings. These fields are optional because we don't want to override file-based linter/formatting settings
/// if these were un-set.
#[derive(Debug, Default)]
#[derive(Clone, Debug, Default)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[allow(dead_code)] // TODO(jane): Remove once editor settings resolution is implemented
pub(crate) struct ResolvedEditorSettings {
lint_preview: Option<bool>,
format_preview: Option<bool>,
select: Option<Vec<RuleSelector>>,
extend_select: Option<Vec<RuleSelector>>,
ignore: Option<Vec<RuleSelector>>,
exclude: Option<Vec<PathBuf>>,
line_length: Option<LineLength>,
pub(super) lint_preview: Option<bool>,
pub(super) format_preview: Option<bool>,
pub(super) select: Option<Vec<RuleSelector>>,
pub(super) extend_select: Option<Vec<RuleSelector>>,
pub(super) ignore: Option<Vec<RuleSelector>>,
pub(super) exclude: Option<Vec<String>>,
pub(super) line_length: Option<LineLength>,
}

/// This is a direct representation of the settings schema sent by the client.
Expand Down Expand Up @@ -251,14 +248,7 @@ impl ResolvedClientSettings {
.collect()
}),
exclude: Self::resolve_optional(all_settings, |settings| {
Some(
settings
.exclude
.as_ref()?
.iter()
.map(|path| PathBuf::from(OsString::from(path)))
.collect(),
)
Some(settings.exclude.as_ref()?.clone())
}),
line_length: Self::resolve_optional(all_settings, |settings| settings.line_length),
},
Expand Down Expand Up @@ -306,6 +296,10 @@ impl ResolvedClientSettings {
pub(crate) fn fix_violation(&self) -> bool {
self.fix_violation_enable
}

pub(crate) fn editor_settings(&self) -> &ResolvedEditorSettings {
&self.editor_settings
}
}

impl Default for InitializationOptions {
Expand Down Expand Up @@ -653,7 +647,7 @@ mod tests {
select: None,
extend_select: None,
ignore: Some(vec![RuleSelector::from_str("RUF001").unwrap()]),
exclude: Some(vec![PathBuf::from_str("third_party").unwrap()]),
exclude: Some(vec!["third_party".into()]),
line_length: Some(LineLength::try_from(80).unwrap())
}
}
Expand Down
53 changes: 34 additions & 19 deletions crates/ruff_server/src/session/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ use crate::{edit::DocumentVersion, Document};

use self::ruff_settings::RuffSettingsIndex;

use super::{settings, ClientSettings};
use super::{
settings::{self, ResolvedClientSettings, ResolvedEditorSettings},
ClientSettings,
};

mod ruff_settings;

Expand All @@ -23,7 +26,7 @@ pub(crate) struct Workspaces(BTreeMap<PathBuf, Workspace>);

pub(crate) struct Workspace {
open_documents: OpenDocuments,
settings: ClientSettings,
settings: ResolvedClientSettings,
}

pub(crate) struct OpenDocuments {
Expand All @@ -46,18 +49,28 @@ pub(crate) struct DocumentRef {
}

impl Workspaces {
pub(super) fn new(workspaces: Vec<(Url, ClientSettings)>) -> crate::Result<Self> {
pub(super) fn new(
workspaces: Vec<(Url, ClientSettings)>,
global_settings: &ClientSettings,
) -> crate::Result<Self> {
Ok(Self(
workspaces
.into_iter()
.map(|(url, settings)| Workspace::new(&url, settings))
.map(|(url, workspace_settings)| {
Workspace::new(&url, &workspace_settings, global_settings)
})
.collect::<crate::Result<_>>()?,
))
}

pub(super) fn open_workspace_folder(&mut self, folder_url: &Url) -> crate::Result<()> {
pub(super) fn open_workspace_folder(
&mut self,
folder_url: &Url,
global_settings: &ClientSettings,
) -> crate::Result<()> {
// TODO(jane): find a way to allow for workspace settings to be updated dynamically
let (path, workspace) = Workspace::new(folder_url, ClientSettings::default())?;
let (path, workspace) =
Workspace::new(folder_url, &ClientSettings::default(), global_settings)?;
self.0.insert(path, workspace);
Ok(())
}
Expand Down Expand Up @@ -117,12 +130,7 @@ impl Workspaces {
);
settings::ResolvedClientSettings::global(global_settings)
},
|workspace| {
settings::ResolvedClientSettings::with_workspace(
&workspace.settings,
global_settings,
)
},
|workspace| workspace.settings.clone(),
)
}

Expand Down Expand Up @@ -152,29 +160,36 @@ impl Workspaces {
}

impl Workspace {
pub(crate) fn new(root: &Url, settings: ClientSettings) -> crate::Result<(PathBuf, Self)> {
pub(crate) fn new(
root: &Url,
workspace_settings: &ClientSettings,
global_settings: &ClientSettings,
) -> crate::Result<(PathBuf, Self)> {
let path = root
.to_file_path()
.map_err(|()| anyhow!("workspace URL was not a file path!"))?;

let settings = ResolvedClientSettings::with_workspace(workspace_settings, global_settings);

let workspace = Self {
open_documents: OpenDocuments::new(&path),
open_documents: OpenDocuments::new(&path, settings.editor_settings()),
settings,
};

Ok((path, workspace))
}

fn reload_settings(&mut self, root: &Path) {
self.open_documents.reload_settings(root);
self.open_documents
.reload_settings(root, self.settings.editor_settings());
}
}

impl OpenDocuments {
fn new(path: &Path) -> Self {
fn new(path: &Path, editor_settings: &ResolvedEditorSettings) -> Self {
Self {
documents: FxHashMap::default(),
settings_index: RuffSettingsIndex::new(path),
settings_index: RuffSettingsIndex::new(path, editor_settings),
}
}

Expand Down Expand Up @@ -209,8 +224,8 @@ impl OpenDocuments {
Ok(())
}

fn reload_settings(&mut self, root: &Path) {
self.settings_index = RuffSettingsIndex::new(root);
fn reload_settings(&mut self, root: &Path, editor_settings: &ResolvedEditorSettings) {
self.settings_index = RuffSettingsIndex::new(root, editor_settings);
}
}

Expand Down