Skip to content

Commit be03cb0

Browse files
authored
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 <details><summary><b>Casing (<code>kebab-case</code> v/s/ <code>camelCase</code>)</b></summary> <p> 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"))] | ^^^^^^^^^^ ``` </p> </details> <details><summary><b>Nesting (flat v/s nested keys)</b></summary> <p> 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<String, String>` 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. </p> </details> ## Test Plan ### VS Code **Requires astral-sh/ruff-vscode#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
1 parent 7880636 commit be03cb0

7 files changed

Lines changed: 336 additions & 33 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ruff/src/args.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,7 @@ enum InvalidConfigFlagReason {
830830
ValidTomlButInvalidRuffSchema(toml::de::Error),
831831
/// It was a valid ruff config file, but the user tried to pass a
832832
/// value for `extend` as part of the config override.
833-
// `extend` is special, because it affects which config files we look at
833+
/// `extend` is special, because it affects which config files we look at
834834
/// in the first place. We currently only parse --config overrides *after*
835835
/// we've combined them with all the arguments from the various config files
836836
/// that we found, so trying to override `extend` as part of a --config

crates/ruff_server/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ serde = { workspace = true }
3838
serde_json = { workspace = true }
3939
shellexpand = { workspace = true }
4040
thiserror = { workspace = true }
41+
toml = { workspace = true }
4142
tracing = { workspace = true }
4243
tracing-subscriber = { workspace = true }
4344

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"settings": {
3+
"configuration": {
4+
"line-length": 100,
5+
"lint": {
6+
"extend-select": ["I001"]
7+
},
8+
"format": {
9+
"quote-style": "single"
10+
}
11+
},
12+
"lint": {
13+
"extendSelect": ["RUF001"]
14+
}
15+
}
16+
}

crates/ruff_server/src/session/index/ruff_settings.rs

Lines changed: 79 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ use ruff_workspace::{
1818
resolver::{ConfigurationTransformer, Relativity},
1919
};
2020

21-
use crate::session::settings::{ConfigurationPreference, ResolvedEditorSettings};
21+
use crate::session::settings::{
22+
ConfigurationPreference, ResolvedConfiguration, ResolvedEditorSettings,
23+
};
2224

2325
#[derive(Debug)]
2426
pub struct RuffSettings {
@@ -363,21 +365,39 @@ impl ConfigurationTransformer for EditorConfigurationTransformer<'_> {
363365
..Configuration::default()
364366
};
365367

366-
// Merge in the editor-specified configuration file, if it exists.
367-
let editor_configuration = if let Some(config_file_path) = configuration {
368-
tracing::debug!(
369-
"Combining settings from editor-specified configuration file at: {}",
370-
config_file_path.display()
371-
);
372-
match open_configuration_file(&config_file_path) {
373-
Ok(config_from_file) => editor_configuration.combine(config_from_file),
374-
err => {
375-
tracing::error!(
376-
"{:?}",
377-
err.context("Unable to load editor-specified configuration file")
378-
.unwrap_err()
368+
// Merge in the editor-specified configuration.
369+
let editor_configuration = if let Some(configuration) = configuration {
370+
match configuration {
371+
ResolvedConfiguration::FilePath(path) => {
372+
tracing::debug!(
373+
"Combining settings from editor-specified configuration file at: {}",
374+
path.display()
375+
);
376+
match open_configuration_file(&path) {
377+
Ok(config_from_file) => editor_configuration.combine(config_from_file),
378+
err => {
379+
tracing::error!(
380+
"{:?}",
381+
err.context("Unable to load editor-specified configuration file")
382+
.unwrap_err()
383+
);
384+
editor_configuration
385+
}
386+
}
387+
}
388+
ResolvedConfiguration::Inline(options) => {
389+
tracing::debug!(
390+
"Combining settings from editor-specified inline configuration"
379391
);
380-
editor_configuration
392+
match Configuration::from_options(options, None, project_root) {
393+
Ok(configuration) => editor_configuration.combine(configuration),
394+
Err(err) => {
395+
tracing::error!(
396+
"Unable to load editor-specified inline configuration: {err:?}",
397+
);
398+
editor_configuration
399+
}
400+
}
381401
}
382402
}
383403
} else {
@@ -411,3 +431,47 @@ impl ConfigurationTransformer for IdentityTransformer {
411431
config
412432
}
413433
}
434+
435+
#[cfg(test)]
436+
mod tests {
437+
use ruff_linter::line_width::LineLength;
438+
use ruff_workspace::options::Options;
439+
440+
use super::*;
441+
442+
/// This test ensures that the inline configuration is correctly applied to the configuration.
443+
#[test]
444+
fn inline_settings() {
445+
let editor_settings = ResolvedEditorSettings {
446+
configuration: Some(ResolvedConfiguration::Inline(Options {
447+
line_length: Some(LineLength::try_from(120).unwrap()),
448+
..Default::default()
449+
})),
450+
..Default::default()
451+
};
452+
453+
let config = EditorConfigurationTransformer(&editor_settings, Path::new("/src/project"))
454+
.transform(Configuration::default());
455+
456+
assert_eq!(config.line_length.unwrap().value(), 120);
457+
}
458+
459+
/// This test ensures that between the inline configuration and specific settings, the specific
460+
/// settings is prioritized.
461+
#[test]
462+
fn inline_and_specific_settings_resolution_order() {
463+
let editor_settings = ResolvedEditorSettings {
464+
configuration: Some(ResolvedConfiguration::Inline(Options {
465+
line_length: Some(LineLength::try_from(120).unwrap()),
466+
..Default::default()
467+
})),
468+
line_length: Some(LineLength::try_from(100).unwrap()),
469+
..Default::default()
470+
};
471+
472+
let config = EditorConfigurationTransformer(&editor_settings, Path::new("/src/project"))
473+
.transform(Configuration::default());
474+
475+
assert_eq!(config.line_length.unwrap().value(), 100);
476+
}
477+
}

crates/ruff_server/src/session/settings.rs

Lines changed: 121 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@ use std::{ops::Deref, path::PathBuf, str::FromStr};
33
use lsp_types::Url;
44
use rustc_hash::FxHashMap;
55
use serde::Deserialize;
6+
use serde_json::{Map, Value};
7+
use thiserror::Error;
68

79
use ruff_linter::{line_width::LineLength, RuleSelector};
10+
use ruff_workspace::options::Options;
811

912
/// Maps a workspace URI to its associated client settings. Used during server initialization.
1013
pub(crate) type WorkspaceSettingsMap = FxHashMap<Url, ClientSettings>;
@@ -29,9 +32,9 @@ pub(crate) struct ResolvedClientSettings {
2932
/// LSP client settings. These fields are optional because we don't want to override file-based linter/formatting settings
3033
/// if these were un-set.
3134
#[derive(Clone, Debug)]
32-
#[cfg_attr(test, derive(PartialEq, Eq))]
35+
#[cfg_attr(test, derive(Default, PartialEq, Eq))]
3336
pub(crate) struct ResolvedEditorSettings {
34-
pub(super) configuration: Option<PathBuf>,
37+
pub(super) configuration: Option<ResolvedConfiguration>,
3538
pub(super) lint_preview: Option<bool>,
3639
pub(super) format_preview: Option<bool>,
3740
pub(super) select: Option<Vec<RuleSelector>>,
@@ -42,6 +45,48 @@ pub(crate) struct ResolvedEditorSettings {
4245
pub(super) configuration_preference: ConfigurationPreference,
4346
}
4447

48+
/// The resolved configuration from the client settings.
49+
#[derive(Clone, Debug)]
50+
#[cfg_attr(test, derive(PartialEq, Eq))]
51+
pub(crate) enum ResolvedConfiguration {
52+
FilePath(PathBuf),
53+
Inline(Options),
54+
}
55+
56+
impl TryFrom<&ClientConfiguration> for ResolvedConfiguration {
57+
type Error = ResolvedConfigurationError;
58+
59+
fn try_from(value: &ClientConfiguration) -> Result<Self, Self::Error> {
60+
match value {
61+
ClientConfiguration::String(path) => Ok(ResolvedConfiguration::FilePath(
62+
PathBuf::from(shellexpand::full(path)?.as_ref()),
63+
)),
64+
ClientConfiguration::Object(map) => {
65+
let options = toml::Table::try_from(map)?.try_into::<Options>()?;
66+
if options.extend.is_some() {
67+
Err(ResolvedConfigurationError::ExtendNotSupported)
68+
} else {
69+
Ok(ResolvedConfiguration::Inline(options))
70+
}
71+
}
72+
}
73+
}
74+
}
75+
76+
/// An error that can occur when trying to resolve the `configuration` value from the client
77+
/// settings.
78+
#[derive(Debug, Error)]
79+
pub(crate) enum ResolvedConfigurationError {
80+
#[error(transparent)]
81+
EnvVarLookupError(#[from] shellexpand::LookupError<std::env::VarError>),
82+
#[error("error serializing configuration to TOML: {0}")]
83+
InvalidToml(#[from] toml::ser::Error),
84+
#[error(transparent)]
85+
InvalidRuffSchema(#[from] toml::de::Error),
86+
#[error("using `extend` is unsupported for inline configuration")]
87+
ExtendNotSupported,
88+
}
89+
4590
/// Determines how multiple conflicting configurations should be resolved - in this
4691
/// case, the configuration from the client settings and configuration from local
4792
/// `.toml` files (aka 'workspace' configuration).
@@ -57,12 +102,23 @@ pub(crate) enum ConfigurationPreference {
57102
EditorOnly,
58103
}
59104

105+
/// A direct representation of of `configuration` schema within the client settings.
106+
#[derive(Debug, Deserialize)]
107+
#[cfg_attr(test, derive(PartialEq, Eq))]
108+
#[serde(untagged)]
109+
enum ClientConfiguration {
110+
/// A path to a configuration file.
111+
String(String),
112+
/// An object containing the configuration options.
113+
Object(Map<String, Value>),
114+
}
115+
60116
/// This is a direct representation of the settings schema sent by the client.
61117
#[derive(Debug, Deserialize, Default)]
62118
#[cfg_attr(test, derive(PartialEq, Eq))]
63119
#[serde(rename_all = "camelCase")]
64120
pub struct ClientSettings {
65-
configuration: Option<String>,
121+
configuration: Option<ClientConfiguration>,
66122
fix_all: Option<bool>,
67123
organize_imports: Option<bool>,
68124
lint: Option<LintOptions>,
@@ -306,11 +362,17 @@ impl ResolvedClientSettings {
306362
),
307363
editor_settings: ResolvedEditorSettings {
308364
configuration: Self::resolve_optional(all_settings, |settings| {
309-
settings
310-
.configuration
311-
.as_ref()
312-
.and_then(|config_path| shellexpand::full(config_path).ok())
313-
.map(|config_path| PathBuf::from(config_path.as_ref()))
365+
settings.configuration.as_ref().and_then(|configuration| {
366+
match ResolvedConfiguration::try_from(configuration) {
367+
Ok(configuration) => Some(configuration),
368+
Err(err) => {
369+
tracing::error!(
370+
"Failed to load settings from `configuration`: {err}"
371+
);
372+
None
373+
}
374+
}
375+
})
314376
}),
315377
lint_preview: Self::resolve_optional(all_settings, |settings| {
316378
settings.lint.as_ref()?.preview
@@ -425,6 +487,10 @@ impl Default for InitializationOptions {
425487
#[cfg(test)]
426488
mod tests {
427489
use insta::assert_debug_snapshot;
490+
use ruff_python_formatter::QuoteStyle;
491+
use ruff_workspace::options::{
492+
FormatOptions as RuffFormatOptions, LintCommonOptions, LintOptions,
493+
};
428494
use serde::de::DeserializeOwned;
429495

430496
#[cfg(not(windows))]
@@ -445,6 +511,9 @@ mod tests {
445511
const EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE: &str =
446512
include_str!("../../resources/test/fixtures/settings/empty_multiple_workspace.json");
447513

514+
const INLINE_CONFIGURATION_FIXTURE: &str =
515+
include_str!("../../resources/test/fixtures/settings/inline_configuration.json");
516+
448517
fn deserialize_fixture<T: DeserializeOwned>(content: &str) -> T {
449518
serde_json::from_str(content).expect("test fixture JSON should deserialize")
450519
}
@@ -855,4 +924,48 @@ mod tests {
855924
all_settings.set_preview(true);
856925
assert_preview_all_settings(&all_settings, true);
857926
}
927+
928+
#[test]
929+
fn inline_configuration() {
930+
let options: InitializationOptions = deserialize_fixture(INLINE_CONFIGURATION_FIXTURE);
931+
932+
let AllSettings {
933+
global_settings,
934+
workspace_settings: None,
935+
} = AllSettings::from_init_options(options)
936+
else {
937+
panic!("Expected global settings only");
938+
};
939+
940+
assert_eq!(
941+
ResolvedClientSettings::global(&global_settings),
942+
ResolvedClientSettings {
943+
fix_all: true,
944+
organize_imports: true,
945+
lint_enable: true,
946+
disable_rule_comment_enable: true,
947+
fix_violation_enable: true,
948+
show_syntax_errors: true,
949+
editor_settings: ResolvedEditorSettings {
950+
configuration: Some(ResolvedConfiguration::Inline(Options {
951+
line_length: Some(LineLength::try_from(100).unwrap()),
952+
lint: Some(LintOptions {
953+
common: LintCommonOptions {
954+
extend_select: Some(vec![RuleSelector::from_str("I001").unwrap()]),
955+
..Default::default()
956+
},
957+
..Default::default()
958+
}),
959+
format: Some(RuffFormatOptions {
960+
quote_style: Some(QuoteStyle::Single),
961+
..Default::default()
962+
}),
963+
..Default::default()
964+
})),
965+
extend_select: Some(vec![RuleSelector::from_str("RUF001").unwrap()]),
966+
..Default::default()
967+
}
968+
}
969+
);
970+
}
858971
}

0 commit comments

Comments
 (0)