From 2cc487eb22970036f5621758e34996a8ece1e194 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Thu, 18 Apr 2024 00:53:48 -0700 Subject: [PATCH] `ruff server`: Introduce settings for directly configuring the linter and formatter (#10984) ## Summary The following client settings have been introduced to the language server: * `lint.preview` * `format.preview` * `lint.select` * `lint.extendSelect` * `lint.ignore` * `exclude` * `lineLength` `exclude` and `lineLength` apply to both the linter and formatter. This does not actually use the settings yet, but makes them available for future use. ## Test Plan Snapshot tests have been updated. --- .../test/fixtures/settings/global_only.json | 5 +- .../vs_code_initialization_options.json | 3 + crates/ruff_server/src/session/settings.rs | 243 ++++++++++++++++-- 3 files changed, 222 insertions(+), 29 deletions(-) diff --git a/crates/ruff_server/resources/test/fixtures/settings/global_only.json b/crates/ruff_server/resources/test/fixtures/settings/global_only.json index 674c756a2a575..0ed3bf16d5526 100644 --- a/crates/ruff_server/resources/test/fixtures/settings/global_only.json +++ b/crates/ruff_server/resources/test/fixtures/settings/global_only.json @@ -6,9 +6,12 @@ } }, "lint": { + "ignore": ["RUF001"], "run": "onSave" }, "fixAll": false, - "logLevel": "warn" + "logLevel": "warn", + "lineLength": 80, + "exclude": ["third_party"] } } diff --git a/crates/ruff_server/resources/test/fixtures/settings/vs_code_initialization_options.json b/crates/ruff_server/resources/test/fixtures/settings/vs_code_initialization_options.json index d7e7c1c7b7efc..7ce732add22b2 100644 --- a/crates/ruff_server/resources/test/fixtures/settings/vs_code_initialization_options.json +++ b/crates/ruff_server/resources/test/fixtures/settings/vs_code_initialization_options.json @@ -53,6 +53,7 @@ }, "lint": { "enable": true, + "preview": false, "run": "onType", "args": [ "--preview" @@ -85,6 +86,8 @@ }, "lint": { "enable": true, + "preview": true, + "select": ["F", "I"], "run": "onType", "args": [ "--preview" diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index de30b922d75cf..84e841ad72ab6 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -1,6 +1,7 @@ -use std::ops::Deref; +use std::{ffi::OsString, ops::Deref, path::PathBuf, str::FromStr}; use lsp_types::Url; +use ruff_linter::{line_width::LineLength, RuleSelector}; use rustc_hash::FxHashMap; use serde::Deserialize; @@ -21,6 +22,25 @@ 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)] +#[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, + format_preview: Option, + select: Option>, + extend_select: Option>, + ignore: Option>, + exclude: Option>, + line_length: Option, } /// This is a direct representation of the settings schema sent by the client. @@ -30,8 +50,11 @@ pub(crate) struct ResolvedClientSettings { pub(crate) struct ClientSettings { fix_all: Option, organize_imports: Option, - lint: Option, - code_action: Option, + lint: Option, + format: Option, + code_action: Option, + exclude: Option>, + line_length: Option, } /// This is a direct representation of the workspace settings schema, @@ -49,22 +72,33 @@ struct WorkspaceSettings { #[derive(Debug, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] -struct Lint { +struct LintOptions { enable: Option, + preview: Option, + select: Option>, + extend_select: Option>, + ignore: Option>, +} + +#[derive(Debug, Default, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +struct FormatOptions { + preview: Option, } #[derive(Debug, Default, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] -struct CodeAction { - disable_rule_comment: Option, - fix_violation: Option, +struct CodeActionOptions { + disable_rule_comment: Option, + fix_violation: Option, } #[derive(Debug, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] -struct CodeActionSettings { +struct CodeActionParameters { enable: Option, } @@ -179,22 +213,80 @@ impl ResolvedClientSettings { }, true, ), + editor_settings: ResolvedEditorSettings { + lint_preview: Self::resolve_optional(all_settings, |settings| { + settings.lint.as_ref()?.preview + }), + format_preview: Self::resolve_optional(all_settings, |settings| { + settings.format.as_ref()?.preview + }), + select: Self::resolve_optional(all_settings, |settings| { + settings + .lint + .as_ref()? + .select + .as_ref()? + .iter() + .map(|rule| RuleSelector::from_str(rule).ok()) + .collect() + }), + extend_select: Self::resolve_optional(all_settings, |settings| { + settings + .lint + .as_ref()? + .extend_select + .as_ref()? + .iter() + .map(|rule| RuleSelector::from_str(rule).ok()) + .collect() + }), + ignore: Self::resolve_optional(all_settings, |settings| { + settings + .lint + .as_ref()? + .ignore + .as_ref()? + .iter() + .map(|rule| RuleSelector::from_str(rule).ok()) + .collect() + }), + exclude: Self::resolve_optional(all_settings, |settings| { + Some( + settings + .exclude + .as_ref()? + .iter() + .map(|path| PathBuf::from(OsString::from(path))) + .collect(), + ) + }), + line_length: Self::resolve_optional(all_settings, |settings| settings.line_length), + }, } } + /// Attempts to resolve a setting using a list of available client settings as sources. + /// Client settings that come earlier in the list take priority. This function is for fields + /// that do not have a default value and should be left unset. + /// Use [`ResolvedClientSettings::resolve_or`] for settings that should have default values. + fn resolve_optional( + all_settings: &[&ClientSettings], + get: impl Fn(&ClientSettings) -> Option, + ) -> Option { + all_settings.iter().map(Deref::deref).find_map(get) + } + /// Attempts to resolve a setting using a list of available client settings as sources. /// Client settings that come earlier in the list take priority. `default` will be returned /// if none of the settings specify the requested setting. + /// Use [`ResolvedClientSettings::resolve_optional`] if the setting should be optional instead + /// of having a default value. fn resolve_or( all_settings: &[&ClientSettings], get: impl Fn(&ClientSettings) -> Option, default: T, ) -> T { - all_settings - .iter() - .map(Deref::deref) - .find_map(get) - .unwrap_or(default) + Self::resolve_optional(all_settings, get).unwrap_or(default) } } @@ -225,6 +317,7 @@ impl Default for InitializationOptions { #[cfg(test)] mod tests { use insta::assert_debug_snapshot; + use ruff_linter::registry::Linter; use serde::de::DeserializeOwned; use super::*; @@ -254,23 +347,39 @@ mod tests { true, ), lint: Some( - Lint { + LintOptions { enable: Some( true, ), + preview: Some( + true, + ), + select: Some( + [ + "F", + "I", + ], + ), + extend_select: None, + ignore: None, + }, + ), + format: Some( + FormatOptions { + preview: None, }, ), code_action: Some( - CodeAction { + CodeActionOptions { disable_rule_comment: Some( - CodeActionSettings { + CodeActionParameters { enable: Some( false, ), }, ), fix_violation: Some( - CodeActionSettings { + CodeActionParameters { enable: Some( false, ), @@ -278,6 +387,8 @@ mod tests { ), }, ), + exclude: None, + line_length: None, }, workspace_settings: [ WorkspaceSettings { @@ -289,23 +400,32 @@ mod tests { true, ), lint: Some( - Lint { + LintOptions { enable: Some( true, ), + preview: None, + select: None, + extend_select: None, + ignore: None, + }, + ), + format: Some( + FormatOptions { + preview: None, }, ), code_action: Some( - CodeAction { + CodeActionOptions { disable_rule_comment: Some( - CodeActionSettings { + CodeActionParameters { enable: Some( false, ), }, ), fix_violation: Some( - CodeActionSettings { + CodeActionParameters { enable: Some( false, ), @@ -313,6 +433,8 @@ mod tests { ), }, ), + exclude: None, + line_length: None, }, workspace: Url { scheme: "file", @@ -335,23 +457,34 @@ mod tests { true, ), lint: Some( - Lint { + LintOptions { enable: Some( true, ), + preview: Some( + false, + ), + select: None, + extend_select: None, + ignore: None, + }, + ), + format: Some( + FormatOptions { + preview: None, }, ), code_action: Some( - CodeAction { + CodeActionOptions { disable_rule_comment: Some( - CodeActionSettings { + CodeActionParameters { enable: Some( true, ), }, ), fix_violation: Some( - CodeActionSettings { + CodeActionParameters { enable: Some( false, ), @@ -359,6 +492,8 @@ mod tests { ), }, ), + exclude: None, + line_length: None, }, workspace: Url { scheme: "file", @@ -399,6 +534,18 @@ mod tests { lint_enable: true, disable_rule_comment_enable: false, fix_violation_enable: false, + editor_settings: ResolvedEditorSettings { + lint_preview: Some(true), + format_preview: None, + select: Some(vec![ + RuleSelector::Linter(Linter::Pyflakes), + RuleSelector::Linter(Linter::Isort) + ]), + extend_select: None, + ignore: None, + exclude: None, + line_length: None + } } ); let url = Url::parse("file:///Users/test/projects/scipy").expect("url should parse"); @@ -415,6 +562,18 @@ mod tests { lint_enable: true, disable_rule_comment_enable: true, fix_violation_enable: false, + editor_settings: ResolvedEditorSettings { + lint_preview: Some(false), + format_preview: None, + select: Some(vec![ + RuleSelector::Linter(Linter::Pyflakes), + RuleSelector::Linter(Linter::Isort) + ]), + extend_select: None, + ignore: None, + exclude: None, + line_length: None + } } ); } @@ -432,14 +591,23 @@ mod tests { ), organize_imports: None, lint: Some( - Lint { + LintOptions { enable: None, + preview: None, + select: None, + extend_select: None, + ignore: Some( + [ + "RUF001", + ], + ), }, ), + format: None, code_action: Some( - CodeAction { + CodeActionOptions { disable_rule_comment: Some( - CodeActionSettings { + CodeActionParameters { enable: Some( false, ), @@ -448,6 +616,16 @@ mod tests { fix_violation: None, }, ), + exclude: Some( + [ + "third_party", + ], + ), + line_length: Some( + LineLength( + 80, + ), + ), }, ), } @@ -469,6 +647,15 @@ mod tests { lint_enable: true, disable_rule_comment_enable: false, fix_violation_enable: true, + editor_settings: ResolvedEditorSettings { + lint_preview: None, + format_preview: None, + select: None, + extend_select: None, + ignore: Some(vec![RuleSelector::from_str("RUF001").unwrap()]), + exclude: Some(vec![PathBuf::from_str("third_party").unwrap()]), + line_length: Some(LineLength::try_from(80).unwrap()) + } } ); }