From 4433262627dfb030ece7160514f969ae74807f1b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 10 Feb 2024 22:25:49 -0500 Subject: [PATCH 1/2] Merge maps when combining options --- crates/ruff_macros/src/combine_options.rs | 50 ++++++++++++++++++++--- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/crates/ruff_macros/src/combine_options.rs b/crates/ruff_macros/src/combine_options.rs index 05b2395e072bf2..f8886e93607603 100644 --- a/crates/ruff_macros/src/combine_options.rs +++ b/crates/ruff_macros/src/combine_options.rs @@ -48,13 +48,53 @@ fn handle_field(field: &Field) -> syn::Result { .. }) => match segments.first() { Some(PathSegment { - ident: type_ident, .. - }) if type_ident == "Option" => Ok(quote_spanned!( - ident.span() => #ident: self.#ident.or(other.#ident) - )), + ident: type_ident, + arguments, + }) if type_ident == "Option" => { + // Given `Option>`, combine the maps by merging. In TOML, a hash map is + // represented as a table, so merging the maps is the correct behavior. + if let syn::PathArguments::AngleBracketed(args) = arguments { + let inner_type_ident = args + .args + .first() + .and_then(|arg| match arg { + syn::GenericArgument::Type(Type::Path(TypePath { + path: Path { segments, .. }, + .. + })) => segments.first().map(|seg| &seg.ident), + _ => None, + }) + .ok_or_else(|| { + syn::Error::new( + ident.span(), + "Expected `Option<_>` with a single type argument.", + ) + })?; + if inner_type_ident == "HashMap" + || inner_type_ident == "BTreeMap" + || inner_type_ident == "FxHashMap" + { + return Ok(quote_spanned!( + ident.span() => #ident: match (self.#ident, other.#ident) { + (Some(mut m1), Some(m2)) => { + m1.extend(m2); + Some(m1) + }, + (None, Some(m)) | (Some(m), None) => Some(m), + (None, None) => None, + } + )); + } + } + + Ok(quote_spanned!( + ident.span() => #ident: self.#ident.or(other.#ident) + )) + } + _ => Err(syn::Error::new( ident.span(), - "Expected `Option<_>` or `Vec<_>` as type.", + "Expected `Option<_>` as type.", )), }, _ => Err(syn::Error::new(ident.span(), "Expected type.")), From 1d2d0c684fcababcfd3c494b2452ae289534f8ac Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 14 Feb 2024 15:36:06 +0100 Subject: [PATCH 2/2] Simplify macro to simply forward to `CombineOptions` trait --- crates/ruff_macros/src/combine_options.rs | 84 ++++--------------- crates/ruff_macros/src/lib.rs | 4 + crates/ruff_workspace/src/configuration.rs | 98 +++++++++++++++++----- 3 files changed, 95 insertions(+), 91 deletions(-) diff --git a/crates/ruff_macros/src/combine_options.rs b/crates/ruff_macros/src/combine_options.rs index f8886e93607603..936cf752e31e15 100644 --- a/crates/ruff_macros/src/combine_options.rs +++ b/crates/ruff_macros/src/combine_options.rs @@ -1,5 +1,5 @@ use quote::{quote, quote_spanned}; -use syn::{Data, DataStruct, DeriveInput, Field, Fields, Path, PathSegment, Type, TypePath}; +use syn::{Data, DataStruct, DeriveInput, Fields}; pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result { let DeriveInput { ident, data, .. } = input; @@ -9,15 +9,24 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result { - let output = fields + let output: Vec<_> = fields .named .iter() - .map(handle_field) - .collect::, _>>()?; + .map(|field| { + let ident = field + .ident + .as_ref() + .expect("Expected to handle named fields"); + + quote_spanned!( + ident.span() => #ident: crate::configuration::CombineOptions::combine(self.#ident, other.#ident) + ) + }) + .collect(); Ok(quote! { #[automatically_derived] - impl crate::configuration::CombinePluginOptions for #ident { + impl crate::configuration::CombineOptions for #ident { fn combine(self, other: Self) -> Self { #[allow(deprecated)] Self { @@ -35,68 +44,3 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result syn::Result { - let ident = field - .ident - .as_ref() - .expect("Expected to handle named fields"); - - match &field.ty { - Type::Path(TypePath { - path: Path { segments, .. }, - .. - }) => match segments.first() { - Some(PathSegment { - ident: type_ident, - arguments, - }) if type_ident == "Option" => { - // Given `Option>`, combine the maps by merging. In TOML, a hash map is - // represented as a table, so merging the maps is the correct behavior. - if let syn::PathArguments::AngleBracketed(args) = arguments { - let inner_type_ident = args - .args - .first() - .and_then(|arg| match arg { - syn::GenericArgument::Type(Type::Path(TypePath { - path: Path { segments, .. }, - .. - })) => segments.first().map(|seg| &seg.ident), - _ => None, - }) - .ok_or_else(|| { - syn::Error::new( - ident.span(), - "Expected `Option<_>` with a single type argument.", - ) - })?; - if inner_type_ident == "HashMap" - || inner_type_ident == "BTreeMap" - || inner_type_ident == "FxHashMap" - { - return Ok(quote_spanned!( - ident.span() => #ident: match (self.#ident, other.#ident) { - (Some(mut m1), Some(m2)) => { - m1.extend(m2); - Some(m1) - }, - (None, Some(m)) | (Some(m), None) => Some(m), - (None, None) => None, - } - )); - } - } - - Ok(quote_spanned!( - ident.span() => #ident: self.#ident.or(other.#ident) - )) - } - - _ => Err(syn::Error::new( - ident.span(), - "Expected `Option<_>` as type.", - )), - }, - _ => Err(syn::Error::new(ident.span(), "Expected type.")), - } -} diff --git a/crates/ruff_macros/src/lib.rs b/crates/ruff_macros/src/lib.rs index 04b91099829593..1563106fd8da01 100644 --- a/crates/ruff_macros/src/lib.rs +++ b/crates/ruff_macros/src/lib.rs @@ -24,6 +24,10 @@ pub fn derive_options_metadata(input: TokenStream) -> TokenStream { .into() } +/// Automatically derives a `ruff_workspace::configuration::CombineOptions` implementation for the attributed type +/// that calls `ruff_workspace::configuration::CombineOptions::combine` for each field. +/// +/// The derive macro can only be used on structs with named fields. #[proc_macro_derive(CombineOptions)] pub fn derive_combine_options(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index a278f77307a5fd..15344f54194132 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -23,7 +23,15 @@ use ruff_linter::line_width::{IndentWidth, LineLength}; use ruff_linter::registry::RuleNamespace; use ruff_linter::registry::{Rule, RuleSet, INCOMPATIBLE_CODES}; use ruff_linter::rule_selector::{PreviewOptions, Specificity}; +use ruff_linter::rules::flake8_pytest_style::types::{ + ParametrizeNameType, ParametrizeValuesRowType, ParametrizeValuesType, +}; +use ruff_linter::rules::flake8_quotes::settings::Quote; +use ruff_linter::rules::flake8_tidy_imports::settings::Strictness; +use ruff_linter::rules::isort::settings::RelativeImportsOrder; +use ruff_linter::rules::isort::ImportSection; use ruff_linter::rules::pycodestyle; +use ruff_linter::rules::pydocstyle::settings::Convention; use ruff_linter::settings::fix_safety_table::FixSafetyTable; use ruff_linter::settings::rule_table::RuleTable; use ruff_linter::settings::types::{ @@ -35,6 +43,7 @@ use ruff_linter::{ fs, warn_user_once, warn_user_once_by_id, warn_user_once_by_message, RuleSelector, RUFF_PKG_VERSION, }; +use ruff_macros::CombineOptions; use ruff_python_formatter::{ DocstringCode, DocstringCodeLineWidth, MagicTrailingComma, QuoteStyle, }; @@ -1151,7 +1160,7 @@ impl LintConfiguration { } } -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, CombineOptions)] pub struct FormatConfiguration { pub exclude: Option>, pub preview: Option, @@ -1202,31 +1211,37 @@ impl FormatConfiguration { docstring_code_line_width: options.docstring_code_line_length, }) } - - #[must_use] - #[allow(clippy::needless_pass_by_value)] - pub fn combine(self, config: Self) -> Self { - Self { - exclude: self.exclude.or(config.exclude), - preview: self.preview.or(config.preview), - extension: self.extension.or(config.extension), - indent_style: self.indent_style.or(config.indent_style), - quote_style: self.quote_style.or(config.quote_style), - magic_trailing_comma: self.magic_trailing_comma.or(config.magic_trailing_comma), - line_ending: self.line_ending.or(config.line_ending), - docstring_code_format: self.docstring_code_format.or(config.docstring_code_format), - docstring_code_line_width: self - .docstring_code_line_width - .or(config.docstring_code_line_width), - } - } } -pub(crate) trait CombinePluginOptions { +pub(crate) trait CombineOptions { #[must_use] fn combine(self, other: Self) -> Self; } -impl CombinePluginOptions for Option { +macro_rules! or_combine_options_impl { + ($ty:ident) => { + impl CombineOptions for Option<$ty> { + #[inline] + fn combine(self, other: Self) -> Self { + self.or(other) + } + } + }; +} + +or_combine_options_impl!(bool); +or_combine_options_impl!(u8); +or_combine_options_impl!(u16); +or_combine_options_impl!(u32); +or_combine_options_impl!(u64); +or_combine_options_impl!(usize); +or_combine_options_impl!(i8); +or_combine_options_impl!(i16); +or_combine_options_impl!(i32); +or_combine_options_impl!(i64); +or_combine_options_impl!(isize); +or_combine_options_impl!(String); + +impl CombineOptions for Option { fn combine(self, other: Self) -> Self { match (self, other) { (Some(base), Some(other)) => Some(base.combine(other)), @@ -1237,6 +1252,47 @@ impl CombinePluginOptions for Option { } } +impl CombineOptions for Option> { + fn combine(self, other: Self) -> Self { + self.or(other) + } +} + +impl CombineOptions for Option> { + fn combine(self, other: Self) -> Self { + self.or(other) + } +} + +impl CombineOptions for std::collections::hash_map::HashMap +where + K: Eq + std::hash::Hash, + S: std::hash::BuildHasher, +{ + fn combine(mut self, other: Self) -> Self { + self.extend(other); + self + } +} + +or_combine_options_impl!(ParametrizeNameType); +or_combine_options_impl!(ParametrizeValuesType); +or_combine_options_impl!(ParametrizeValuesRowType); +or_combine_options_impl!(Quote); +or_combine_options_impl!(Strictness); +or_combine_options_impl!(RelativeImportsOrder); +or_combine_options_impl!(LineLength); +or_combine_options_impl!(Convention); +or_combine_options_impl!(IndentStyle); +or_combine_options_impl!(QuoteStyle); +or_combine_options_impl!(LineEnding); +or_combine_options_impl!(DocstringCodeLineWidth); +or_combine_options_impl!(ExtensionMapping); +or_combine_options_impl!(MagicTrailingComma); +or_combine_options_impl!(DocstringCode); +or_combine_options_impl!(PreviewMode); +or_combine_options_impl!(ImportSection); + /// Given a list of source paths, which could include glob patterns, resolve the /// matching paths. pub fn resolve_src(src: &[String], project_root: &Path) -> Result> {