mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-03 18:28:24 +00:00
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 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
This commit is contained in:
Dhruv Manilawala
GitHub
parent
78806361fd
commit
be03cb04c1
7 changed files with 336 additions and 33 deletions
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Url, ClientSettings>;
|
||||
|
|
@ -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<PathBuf>,
|
||||
pub(super) configuration: Option<ResolvedConfiguration>,
|
||||
pub(super) lint_preview: Option<bool>,
|
||||
pub(super) format_preview: Option<bool>,
|
||||
pub(super) select: Option<Vec<RuleSelector>>,
|
||||
|
|
@ -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<Self, Self::Error> {
|
||||
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::<Options>()?;
|
||||
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<std::env::VarError>),
|
||||
#[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<String, Value>),
|
||||
}
|
||||
|
||||
/// 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<String>,
|
||||
configuration: Option<ClientConfiguration>,
|
||||
fix_all: Option<bool>,
|
||||
organize_imports: Option<bool>,
|
||||
lint: Option<LintOptions>,
|
||||
|
|
@ -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<T: DeserializeOwned>(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()
|
||||
}
|
||||
}
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue