From be03cb04c13cbd74467acdb4a1501b620db16975 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 26 Feb 2025 10:17:11 +0530 Subject: [PATCH] Expand `ruff.configuration` to allow inline config (#16296) ## Summary [Internal design document](https://www.notion.so/astral-sh/In-editor-settings-19e48797e1ca807fa8c2c91b689d9070?pvs=4) This PR expands `ruff.configuration` to allow inline configuration directly in the editor. For example: ```json { "ruff.configuration": { "line-length": 100, "lint": { "unfixable": ["F401"], "flake8-tidy-imports": { "banned-api": { "typing.TypedDict": { "msg": "Use `typing_extensions.TypedDict` instead" } } } }, "format": { "quote-style": "single" } } } ``` This means that now `ruff.configuration` accepts either a path to configuration file or the raw config itself. It's _mostly_ similar to `--config` with one difference that's highlighted in the following section. So, it can be said that the format of `ruff.configuration` when provided the config map is same as the one on the [playground] [^1]. ## Limitations
Casing (kebab-case v/s/ camelCase)

The config keys needs to be in `kebab-case` instead of `camelCase` which is being used for other settings in the editor. This could be a bit confusing. For example, the `line-length` option can be set directly via an editor setting or can be configured via `ruff.configuration`: ```json { "ruff.configuration": { "line-length": 100 }, "ruff.lineLength": 120 } ``` #### Possible solution We could use feature flag with [conditional compilation](https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute) to indicate that when used in `ruff_server`, we need the `Options` fields to be renamed as `camelCase` while for other crates it needs to be renamed as `kebab-case`. But, this might not work very easily because it will require wrapping the `Options` struct and create two structs in which we'll have to add `#[cfg_attr(...)]` because otherwise `serde` will complain: ``` error: duplicate serde attribute `rename_all` --> crates/ruff_workspace/src/options.rs:43:38 | 43 | #[cfg_attr(feature = "editor", serde(rename_all = "camelCase"))] | ^^^^^^^^^^ ```

Nesting (flat v/s nested keys)

This is the major difference between `--config` flag on the command-line v/s `ruff.configuration` and it makes it such that `ruff.configuration` has same value format as [playground] [^1]. The config keys needs to be split up into keys which can result in nested structure instead of flat structure: So, the following **won't work**: ```json { "ruff.configuration": { "format.quote-style": "single", "lint.flake8-tidy-imports.banned-api.\"typing.TypedDict\".msg": "Use `typing_extensions.TypedDict` instead" } } ``` But, instead it would need to be split up like the following: ```json { "ruff.configuration": { "format": { "quote-style": "single" }, "lint": { "flake8-tidy-imports": { "banned-api": { "typing.TypedDict": { "msg": "Use `typing_extensions.TypedDict` instead" } } } } } } ``` #### Possible solution (1) The way we could solve this and make it same as `--config` would be to add a manual logic of converting the JSON map into an equivalent TOML string which would be then parsed into `Options`. So, the following JSON map: ```json { "lint.flake8-tidy-imports": { "banned-api": {"\"typing.TypedDict\".msg": "Use typing_extensions.TypedDict instead"}}} ``` would need to be converted into the following TOML string: ```toml lint.flake8-tidy-imports = { banned-api = { "typing.TypedDict".msg = "Use typing_extensions.TypedDict instead" } } ``` by recursively convering `"key": value` into `key = value` which is to remove the quotes from key and replacing `:` with `=`. #### Possible solution (2) Another would be to just accept `Map` strictly and convert it into `key = value` and then parse it as a TOML string. This would also match `--config` but quotes might become a nuisance because JSON only allows double quotes and so it'll require escaping any inner quotes or use single quotes.

## Test Plan ### VS Code **Requires https://github.com/astral-sh/ruff-vscode/pull/702** **`settings.json`**: ```json { "ruff.lint.extendSelect": ["TID"], "ruff.configuration": { "line-length": 50, "format": { "quote-style": "single" }, "lint": { "unfixable": ["F401"], "flake8-tidy-imports": { "banned-api": { "typing.TypedDict": { "msg": "Use `typing_extensions.TypedDict` instead" } } } } } } ``` Following video showcases me doing the following: 1. Check diagnostics that it includes `TID` 2. Run `Ruff: Fix all auto-fixable problems` to test `unfixable` 3. Run `Format: Document` to test `line-length` and `quote-style` https://github.com/user-attachments/assets/0a38176f-3fb0-4960-a213-73b2ea5b1180 ### Neovim **`init.lua`**: ```lua require('lspconfig').ruff.setup { init_options = { settings = { lint = { extendSelect = { 'TID' }, }, configuration = { ['line-length'] = 50, format = { ['quote-style'] = 'single', }, lint = { unfixable = { 'F401' }, ['flake8-tidy-imports'] = { ['banned-api'] = { ['typing.TypedDict'] = { msg = 'Use typing_extensions.TypedDict instead', }, }, }, }, }, }, }, } ``` Same steps as in the VS Code test: https://github.com/user-attachments/assets/cfe49a9b-9a89-43d7-94f2-7f565d6e3c9d ## Documentation Preview https://github.com/user-attachments/assets/e0062f58-6ec8-4e01-889d-fac76fd8b3c7 [playground]: https://play.ruff.rs [^1]: This has one advantage that the value can be copy-pasted directly into the playground --- Cargo.lock | 1 + crates/ruff/src/args.rs | 2 +- crates/ruff_server/Cargo.toml | 1 + .../settings/inline_configuration.json | 16 +++ .../src/session/index/ruff_settings.rs | 94 +++++++++++-- crates/ruff_server/src/session/settings.rs | 129 ++++++++++++++++-- docs/editors/settings.md | 126 +++++++++++++++-- 7 files changed, 336 insertions(+), 33 deletions(-) create mode 100644 crates/ruff_server/resources/test/fixtures/settings/inline_configuration.json diff --git a/Cargo.lock b/Cargo.lock index 4b5766f4fc..2ab1244d41 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3190,6 +3190,7 @@ dependencies = [ "serde_json", "shellexpand", "thiserror 2.0.11", + "toml", "tracing", "tracing-subscriber", ] diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 6c37948f32..57a437fd2f 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -830,7 +830,7 @@ enum InvalidConfigFlagReason { ValidTomlButInvalidRuffSchema(toml::de::Error), /// It was a valid ruff config file, but the user tried to pass a /// value for `extend` as part of the config override. - // `extend` is special, because it affects which config files we look at + /// `extend` is special, because it affects which config files we look at /// in the first place. We currently only parse --config overrides *after* /// we've combined them with all the arguments from the various config files /// that we found, so trying to override `extend` as part of a --config diff --git a/crates/ruff_server/Cargo.toml b/crates/ruff_server/Cargo.toml index c9678e1a12..7924b988be 100644 --- a/crates/ruff_server/Cargo.toml +++ b/crates/ruff_server/Cargo.toml @@ -38,6 +38,7 @@ serde = { workspace = true } serde_json = { workspace = true } shellexpand = { workspace = true } thiserror = { workspace = true } +toml = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true } diff --git a/crates/ruff_server/resources/test/fixtures/settings/inline_configuration.json b/crates/ruff_server/resources/test/fixtures/settings/inline_configuration.json new file mode 100644 index 0000000000..50460eb0b3 --- /dev/null +++ b/crates/ruff_server/resources/test/fixtures/settings/inline_configuration.json @@ -0,0 +1,16 @@ +{ + "settings": { + "configuration": { + "line-length": 100, + "lint": { + "extend-select": ["I001"] + }, + "format": { + "quote-style": "single" + } + }, + "lint": { + "extendSelect": ["RUF001"] + } + } +} diff --git a/crates/ruff_server/src/session/index/ruff_settings.rs b/crates/ruff_server/src/session/index/ruff_settings.rs index a9962c5fa7..92c5d74b3e 100644 --- a/crates/ruff_server/src/session/index/ruff_settings.rs +++ b/crates/ruff_server/src/session/index/ruff_settings.rs @@ -18,7 +18,9 @@ use ruff_workspace::{ resolver::{ConfigurationTransformer, Relativity}, }; -use crate::session::settings::{ConfigurationPreference, ResolvedEditorSettings}; +use crate::session::settings::{ + ConfigurationPreference, ResolvedConfiguration, ResolvedEditorSettings, +}; #[derive(Debug)] pub struct RuffSettings { @@ -363,21 +365,39 @@ impl ConfigurationTransformer for EditorConfigurationTransformer<'_> { ..Configuration::default() }; - // Merge in the editor-specified configuration file, if it exists. - let editor_configuration = if let Some(config_file_path) = configuration { - tracing::debug!( - "Combining settings from editor-specified configuration file at: {}", - config_file_path.display() - ); - match open_configuration_file(&config_file_path) { - Ok(config_from_file) => editor_configuration.combine(config_from_file), - err => { - tracing::error!( - "{:?}", - err.context("Unable to load editor-specified configuration file") - .unwrap_err() + // Merge in the editor-specified configuration. + let editor_configuration = if let Some(configuration) = configuration { + match configuration { + ResolvedConfiguration::FilePath(path) => { + tracing::debug!( + "Combining settings from editor-specified configuration file at: {}", + path.display() ); - editor_configuration + match open_configuration_file(&path) { + Ok(config_from_file) => editor_configuration.combine(config_from_file), + err => { + tracing::error!( + "{:?}", + err.context("Unable to load editor-specified configuration file") + .unwrap_err() + ); + editor_configuration + } + } + } + ResolvedConfiguration::Inline(options) => { + tracing::debug!( + "Combining settings from editor-specified inline configuration" + ); + match Configuration::from_options(options, None, project_root) { + Ok(configuration) => editor_configuration.combine(configuration), + Err(err) => { + tracing::error!( + "Unable to load editor-specified inline configuration: {err:?}", + ); + editor_configuration + } + } } } } else { @@ -411,3 +431,47 @@ impl ConfigurationTransformer for IdentityTransformer { config } } + +#[cfg(test)] +mod tests { + use ruff_linter::line_width::LineLength; + use ruff_workspace::options::Options; + + use super::*; + + /// This test ensures that the inline configuration is correctly applied to the configuration. + #[test] + fn inline_settings() { + let editor_settings = ResolvedEditorSettings { + configuration: Some(ResolvedConfiguration::Inline(Options { + line_length: Some(LineLength::try_from(120).unwrap()), + ..Default::default() + })), + ..Default::default() + }; + + let config = EditorConfigurationTransformer(&editor_settings, Path::new("/src/project")) + .transform(Configuration::default()); + + assert_eq!(config.line_length.unwrap().value(), 120); + } + + /// This test ensures that between the inline configuration and specific settings, the specific + /// settings is prioritized. + #[test] + fn inline_and_specific_settings_resolution_order() { + let editor_settings = ResolvedEditorSettings { + configuration: Some(ResolvedConfiguration::Inline(Options { + line_length: Some(LineLength::try_from(120).unwrap()), + ..Default::default() + })), + line_length: Some(LineLength::try_from(100).unwrap()), + ..Default::default() + }; + + let config = EditorConfigurationTransformer(&editor_settings, Path::new("/src/project")) + .transform(Configuration::default()); + + assert_eq!(config.line_length.unwrap().value(), 100); + } +} diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index 50bbd41307..f8fba08c4e 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -3,8 +3,11 @@ use std::{ops::Deref, path::PathBuf, str::FromStr}; use lsp_types::Url; use rustc_hash::FxHashMap; use serde::Deserialize; +use serde_json::{Map, Value}; +use thiserror::Error; use ruff_linter::{line_width::LineLength, RuleSelector}; +use ruff_workspace::options::Options; /// Maps a workspace URI to its associated client settings. Used during server initialization. pub(crate) type WorkspaceSettingsMap = FxHashMap; @@ -29,9 +32,9 @@ pub(crate) struct ResolvedClientSettings { /// 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(Clone, Debug)] -#[cfg_attr(test, derive(PartialEq, Eq))] +#[cfg_attr(test, derive(Default, PartialEq, Eq))] pub(crate) struct ResolvedEditorSettings { - pub(super) configuration: Option, + pub(super) configuration: Option, pub(super) lint_preview: Option, pub(super) format_preview: Option, pub(super) select: Option>, @@ -42,6 +45,48 @@ pub(crate) struct ResolvedEditorSettings { pub(super) configuration_preference: ConfigurationPreference, } +/// The resolved configuration from the client settings. +#[derive(Clone, Debug)] +#[cfg_attr(test, derive(PartialEq, Eq))] +pub(crate) enum ResolvedConfiguration { + FilePath(PathBuf), + Inline(Options), +} + +impl TryFrom<&ClientConfiguration> for ResolvedConfiguration { + type Error = ResolvedConfigurationError; + + fn try_from(value: &ClientConfiguration) -> Result { + match value { + ClientConfiguration::String(path) => Ok(ResolvedConfiguration::FilePath( + PathBuf::from(shellexpand::full(path)?.as_ref()), + )), + ClientConfiguration::Object(map) => { + let options = toml::Table::try_from(map)?.try_into::()?; + if options.extend.is_some() { + Err(ResolvedConfigurationError::ExtendNotSupported) + } else { + Ok(ResolvedConfiguration::Inline(options)) + } + } + } + } +} + +/// An error that can occur when trying to resolve the `configuration` value from the client +/// settings. +#[derive(Debug, Error)] +pub(crate) enum ResolvedConfigurationError { + #[error(transparent)] + EnvVarLookupError(#[from] shellexpand::LookupError), + #[error("error serializing configuration to TOML: {0}")] + InvalidToml(#[from] toml::ser::Error), + #[error(transparent)] + InvalidRuffSchema(#[from] toml::de::Error), + #[error("using `extend` is unsupported for inline configuration")] + ExtendNotSupported, +} + /// Determines how multiple conflicting configurations should be resolved - in this /// case, the configuration from the client settings and configuration from local /// `.toml` files (aka 'workspace' configuration). @@ -57,12 +102,23 @@ pub(crate) enum ConfigurationPreference { EditorOnly, } +/// A direct representation of of `configuration` schema within the client settings. +#[derive(Debug, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(untagged)] +enum ClientConfiguration { + /// A path to a configuration file. + String(String), + /// An object containing the configuration options. + Object(Map), +} + /// This is a direct representation of the settings schema sent by the client. #[derive(Debug, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] pub struct ClientSettings { - configuration: Option, + configuration: Option, fix_all: Option, organize_imports: Option, lint: Option, @@ -306,11 +362,17 @@ impl ResolvedClientSettings { ), editor_settings: ResolvedEditorSettings { configuration: Self::resolve_optional(all_settings, |settings| { - settings - .configuration - .as_ref() - .and_then(|config_path| shellexpand::full(config_path).ok()) - .map(|config_path| PathBuf::from(config_path.as_ref())) + settings.configuration.as_ref().and_then(|configuration| { + match ResolvedConfiguration::try_from(configuration) { + Ok(configuration) => Some(configuration), + Err(err) => { + tracing::error!( + "Failed to load settings from `configuration`: {err}" + ); + None + } + } + }) }), lint_preview: Self::resolve_optional(all_settings, |settings| { settings.lint.as_ref()?.preview @@ -425,6 +487,10 @@ impl Default for InitializationOptions { #[cfg(test)] mod tests { use insta::assert_debug_snapshot; + use ruff_python_formatter::QuoteStyle; + use ruff_workspace::options::{ + FormatOptions as RuffFormatOptions, LintCommonOptions, LintOptions, + }; use serde::de::DeserializeOwned; #[cfg(not(windows))] @@ -445,6 +511,9 @@ mod tests { const EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE: &str = include_str!("../../resources/test/fixtures/settings/empty_multiple_workspace.json"); + const INLINE_CONFIGURATION_FIXTURE: &str = + include_str!("../../resources/test/fixtures/settings/inline_configuration.json"); + fn deserialize_fixture(content: &str) -> T { serde_json::from_str(content).expect("test fixture JSON should deserialize") } @@ -855,4 +924,48 @@ mod tests { all_settings.set_preview(true); assert_preview_all_settings(&all_settings, true); } + + #[test] + fn inline_configuration() { + let options: InitializationOptions = deserialize_fixture(INLINE_CONFIGURATION_FIXTURE); + + let AllSettings { + global_settings, + workspace_settings: None, + } = AllSettings::from_init_options(options) + else { + panic!("Expected global settings only"); + }; + + assert_eq!( + ResolvedClientSettings::global(&global_settings), + ResolvedClientSettings { + fix_all: true, + organize_imports: true, + lint_enable: true, + disable_rule_comment_enable: true, + fix_violation_enable: true, + show_syntax_errors: true, + editor_settings: ResolvedEditorSettings { + configuration: Some(ResolvedConfiguration::Inline(Options { + line_length: Some(LineLength::try_from(100).unwrap()), + lint: Some(LintOptions { + common: LintCommonOptions { + extend_select: Some(vec![RuleSelector::from_str("I001").unwrap()]), + ..Default::default() + }, + ..Default::default() + }), + format: Some(RuffFormatOptions { + quote_style: Some(QuoteStyle::Single), + ..Default::default() + }), + ..Default::default() + })), + extend_select: Some(vec![RuleSelector::from_str("RUF001").unwrap()]), + ..Default::default() + } + } + ); + } } diff --git a/docs/editors/settings.md b/docs/editors/settings.md index e6ac9ed177..1eb36af3a6 100644 --- a/docs/editors/settings.md +++ b/docs/editors/settings.md @@ -11,10 +11,39 @@ as per the editor. ### `configuration` -Path to a `ruff.toml` or `pyproject.toml` file to use for configuration. +The `configuration` setting allows you to configure editor-specific Ruff behavior. This can be done +in one of the following ways: -By default, Ruff will discover configuration for each project from the filesystem, mirroring the -behavior of the Ruff CLI. +1. **Configuration file path:** Specify the path to a `ruff.toml` or `pyproject.toml` file that + contains the configuration. User home directory and environment variables will be expanded. +1. **Inline JSON configuration:** Directly provide the configuration as a JSON object. + +!!! note "Added in Ruff `0.9.8`" + + The **Inline JSON configuration** option was introduced in Ruff `0.9.8`. + +The default behavior, if `configuration` is unset, is to load the settings from the project's +configuration (a `ruff.toml` or `pyproject.toml` in the project's directory), consistent with when +running Ruff on the command-line. + +The [`configurationPreference`](#configurationpreference) setting controls the precedence if both an +editor-provided configuration (`configuration`) and a project level configuration file are present. + +#### Resolution order {: #configuration_resolution_order } + +In an editor, Ruff supports three sources of configuration, prioritized as follows (from highest to +lowest): + +1. **Specific settings:** Individual settings like [`lineLength`](#linelength) or + [`lint.select`](#select) defined in the editor +1. [**`ruff.configuration`**](#configuration): Settings provided via the + [`configuration`](#configuration) field (either a path to a configuration file or an inline + configuration object) +1. **Configuration file:** Settings defined in a `ruff.toml` or `pyproject.toml` file in the + project's directory (if present) + +For example, if the line length is specified in all three sources, Ruff will use the value from the +[`lineLength`](#linelength) setting. **Default value**: `null` @@ -22,6 +51,8 @@ behavior of the Ruff CLI. **Example usage**: +_Using configuration file path:_ + === "VS Code" ```json @@ -35,9 +66,7 @@ behavior of the Ruff CLI. ```lua require('lspconfig').ruff.setup { init_options = { - settings = { - configuration = "~/path/to/ruff.toml" - } + configuration = "~/path/to/ruff.toml" } } ``` @@ -58,6 +87,87 @@ behavior of the Ruff CLI. } ``` +_Using inline configuration:_ + +=== "VS Code" + + ```json + { + "ruff.configuration": { + "lint": { + "unfixable": ["F401"], + "extend-select": ["TID251"], + "flake8-tidy-imports": { + "banned-api": { + "typing.TypedDict": { + "msg": "Use `typing_extensions.TypedDict` instead", + } + } + } + }, + "format": { + "quote-style": "single" + } + } + } + ``` + +=== "Neovim" + + ```lua + require('lspconfig').ruff.setup { + init_options = { + configuration = { + lint = { + unfixable = {"F401"}, + ["extend-select"] = {"TID251"}, + ["flake8-tidy-imports"] = { + ["banned-api"] = { + ["typing.TypedDict"] = { + msg = "Use `typing_extensions.TypedDict` instead" + } + } + } + }, + format = { + ["quote-style"] = "single" + } + } + } + } + ``` + +=== "Zed" + + ```json + { + "lsp": { + "ruff": { + "initialization_options": { + "settings": { + "configuration": { + "lint": { + "unfixable": ["F401"], + "extend-select": ["TID251"], + "flake8-tidy-imports": { + "banned-api": { + "typing.TypedDict": { + "msg": "Use `typing_extensions.TypedDict` instead" + } + } + } + }, + "format": { + "quote-style": "single" + } + } + } + } + } + } + } + ``` + ### `configurationPreference` The strategy to use when resolving settings across VS Code and the filesystem. By default, editor @@ -594,9 +704,7 @@ Whether to enable linting. Set to `false` to use Ruff exclusively as a formatter "initialization_options": { "settings": { "lint": { - "enable" = { - "enable": false - } + "enable": false } } }