mirror of
https://github.com/astral-sh/ruff.git
synced 2025-07-24 13:33:50 +00:00
ruff server
: Ruff configuration from client settings overrides project configuration (#11062)
## Summary This is a follow-up to https://github.com/astral-sh/ruff/pull/10984 that implements configuration resolution for editor configuration. By 'editor configuration', I'm referring to the client settings that correspond to Ruff configuration/options, like `preview`, `select`, and so on. These will be combined with 'project configuration' (configuration taken from project files such as `pyproject.toml`) to generate the final linter and formatter settings used by `RuffSettings`. Editor configuration takes priority over project configuration. In a follow-up pull request, I'll implement a new client setting that allows project configuration to override editor configuration, as per [this issue](https://github.com/astral-sh/ruff-vscode/issues/425). ## Review guide The first commit, e38966d8843becc7234fa7d46009c16af4ba41e9, is just doing re-arrangement so that we can pass the right things to `RuffSettings::resolve`. The actual resolution logic is in the second commit, 0eec9ee75c10e5ec423bd9f5ce1764f4d7a5ad86. It might help to look at these comments individually since the diff is rather messy. ## Test Plan For the settings to show up in VS Code, you'll need to checkout this branch: https://github.com/astral-sh/ruff-vscode/pull/456. To test that the resolution for a specific setting works as expected, run through the following scenarios, setting it in project and editor configuration as needed: | Set in project configuration? | Set in editor configuration? | Expected Outcome | |-------------------------------|--------------------------------------------------|------------------------------------------------------------------------------------------| | No | No | The editor should behave as if the setting was set to its default value. | | Yes | No | The editor should behave as if the setting was set to the value in project configuration. | | No | Yes | The editor should behave as if the setting was set to the value in editor configuration. | | Yes | Yes (but distinctive from project configuration) | The editor should behave as if the setting was set to the value in editor configuration. | An exception to this is `extendSelect`, which does not have an analog in TOML configuration. Instead, you should verify that `extendSelect` amends the `select` setting. If `select` is set in both editor and project configuration, `extendSelect` will only append to the `select` value in editor configuration, so make sure to un-set it there if you're testing `extendSelect` with `select` in project configuration.
This commit is contained in:
parent
f5c7a62aa6
commit
35ca887e02
10 changed files with 127 additions and 61 deletions
|
@ -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 {
|
||||
|
|
|
@ -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(),
|
||||
)?),
|
||||
|
@ -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(),
|
||||
)?),
|
||||
|
|
|
@ -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(),
|
||||
)
|
||||
|
@ -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(),
|
||||
)
|
||||
|
|
|
@ -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)?;
|
||||
|
@ -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)?;
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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)?,
|
||||
})
|
||||
}
|
||||
|
||||
|
@ -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(())
|
||||
}
|
||||
|
||||
|
|
|
@ -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};
|
||||
|
@ -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 {
|
||||
|
@ -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.
|
||||
|
@ -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),
|
||||
},
|
||||
|
@ -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 {
|
||||
|
@ -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())
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
|
||||
|
@ -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 {
|
||||
|
@ -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(())
|
||||
}
|
||||
|
@ -117,12 +130,7 @@ impl Workspaces {
|
|||
);
|
||||
settings::ResolvedClientSettings::global(global_settings)
|
||||
},
|
||||
|workspace| {
|
||||
settings::ResolvedClientSettings::with_workspace(
|
||||
&workspace.settings,
|
||||
global_settings,
|
||||
)
|
||||
},
|
||||
|workspace| workspace.settings.clone(),
|
||||
)
|
||||
}
|
||||
|
||||
|
@ -152,13 +160,19 @@ 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,
|
||||
};
|
||||
|
||||
|
@ -166,15 +180,16 @@ impl 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),
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -1,5 +1,9 @@
|
|||
use ruff_linter::display_settings;
|
||||
use ruff_linter::{
|
||||
display_settings, fs::normalize_path_to, settings::types::FilePattern,
|
||||
settings::types::PreviewMode,
|
||||
};
|
||||
use ruff_workspace::{
|
||||
configuration::{Configuration, FormatConfiguration, LintConfiguration, RuleSelection},
|
||||
pyproject::settings_toml,
|
||||
resolver::{ConfigurationTransformer, Relativity},
|
||||
};
|
||||
|
@ -10,12 +14,14 @@ use std::{
|
|||
};
|
||||
use walkdir::{DirEntry, WalkDir};
|
||||
|
||||
use crate::session::settings::ResolvedEditorSettings;
|
||||
|
||||
#[derive(Default)]
|
||||
pub(crate) struct RuffSettings {
|
||||
// settings to pass into the ruff linter
|
||||
pub(crate) linter: ruff_linter::settings::LinterSettings,
|
||||
linter: ruff_linter::settings::LinterSettings,
|
||||
// settings to pass into the ruff formatter
|
||||
pub(crate) formatter: ruff_workspace::FormatterSettings,
|
||||
formatter: ruff_workspace::FormatterSettings,
|
||||
}
|
||||
|
||||
pub(super) struct RuffSettingsIndex {
|
||||
|
@ -36,8 +42,18 @@ impl std::fmt::Display for RuffSettings {
|
|||
}
|
||||
}
|
||||
|
||||
impl RuffSettings {
|
||||
pub(crate) fn linter(&self) -> &ruff_linter::settings::LinterSettings {
|
||||
&self.linter
|
||||
}
|
||||
|
||||
pub(crate) fn formatter(&self) -> &ruff_workspace::FormatterSettings {
|
||||
&self.formatter
|
||||
}
|
||||
}
|
||||
|
||||
impl RuffSettingsIndex {
|
||||
pub(super) fn new(root: &Path) -> Self {
|
||||
pub(super) fn new(root: &Path, editor_settings: &ResolvedEditorSettings) -> Self {
|
||||
let mut index = BTreeMap::default();
|
||||
|
||||
for directory in WalkDir::new(root)
|
||||
|
@ -50,7 +66,7 @@ impl RuffSettingsIndex {
|
|||
let Ok(settings) = ruff_workspace::resolver::resolve_root_settings(
|
||||
&pyproject,
|
||||
Relativity::Parent,
|
||||
&LSPConfigTransformer,
|
||||
&EditorConfigurationTransformer(editor_settings, root),
|
||||
) else {
|
||||
continue;
|
||||
};
|
||||
|
@ -86,13 +102,53 @@ impl RuffSettingsIndex {
|
|||
}
|
||||
}
|
||||
|
||||
struct LSPConfigTransformer;
|
||||
struct EditorConfigurationTransformer<'a>(&'a ResolvedEditorSettings, &'a Path);
|
||||
|
||||
impl ConfigurationTransformer for LSPConfigTransformer {
|
||||
impl<'a> ConfigurationTransformer for EditorConfigurationTransformer<'a> {
|
||||
fn transform(
|
||||
&self,
|
||||
config: ruff_workspace::configuration::Configuration,
|
||||
project_configuration: ruff_workspace::configuration::Configuration,
|
||||
) -> ruff_workspace::configuration::Configuration {
|
||||
config
|
||||
let ResolvedEditorSettings {
|
||||
format_preview,
|
||||
lint_preview,
|
||||
select,
|
||||
extend_select,
|
||||
ignore,
|
||||
exclude,
|
||||
line_length,
|
||||
} = self.0.clone();
|
||||
|
||||
let project_root = self.1;
|
||||
|
||||
let editor_configuration = Configuration {
|
||||
lint: LintConfiguration {
|
||||
preview: lint_preview.map(PreviewMode::from),
|
||||
rule_selections: vec![RuleSelection {
|
||||
select,
|
||||
extend_select: extend_select.unwrap_or_default(),
|
||||
ignore: ignore.unwrap_or_default(),
|
||||
..Default::default()
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
format: FormatConfiguration {
|
||||
preview: format_preview.map(PreviewMode::from),
|
||||
..Default::default()
|
||||
},
|
||||
exclude: exclude.map(|exclude| {
|
||||
exclude
|
||||
.into_iter()
|
||||
.map(|pattern| {
|
||||
let absolute = normalize_path_to(&pattern, project_root);
|
||||
FilePattern::User(pattern, absolute)
|
||||
})
|
||||
.collect()
|
||||
}),
|
||||
line_length,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
editor_configuration.combine(project_configuration)
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue