From bdcf7c1e6215cf31b3b353dc8d1220deb21e2a6e Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 18 Oct 2023 12:10:04 +0900 Subject: [PATCH] Respect `deprecated` attribute in configuration options --- crates/ruff_dev/src/generate_docs.rs | 23 ++- crates/ruff_dev/src/generate_options.rs | 15 ++ crates/ruff_macros/src/combine_options.rs | 1 + crates/ruff_macros/src/config.rs | 157 ++++++++++++++++----- crates/ruff_workspace/src/configuration.rs | 36 +++-- crates/ruff_workspace/src/options.rs | 14 +- crates/ruff_workspace/src/options_base.rs | 40 +++++- ruff.schema.json | 44 ++++++ 8 files changed, 268 insertions(+), 62 deletions(-) diff --git a/crates/ruff_dev/src/generate_docs.rs b/crates/ruff_dev/src/generate_docs.rs index 4cddb678ffa8f8..1b557b68bcccb6 100644 --- a/crates/ruff_dev/src/generate_docs.rs +++ b/crates/ruff_dev/src/generate_docs.rs @@ -11,7 +11,7 @@ use strum::IntoEnumIterator; use ruff_diagnostics::FixAvailability; use ruff_linter::registry::{Linter, Rule, RuleNamespace}; use ruff_workspace::options::Options; -use ruff_workspace::options_base::OptionsMetadata; +use ruff_workspace::options_base::{OptionEntry, OptionsMetadata}; use crate::ROOT_DIR; @@ -55,7 +55,11 @@ pub(crate) fn main(args: &Args) -> Result<()> { output.push('\n'); } - process_documentation(explanation.trim(), &mut output); + process_documentation( + explanation.trim(), + &mut output, + &rule.noqa_code().to_string(), + ); let filename = PathBuf::from(ROOT_DIR) .join("docs") @@ -74,7 +78,7 @@ pub(crate) fn main(args: &Args) -> Result<()> { Ok(()) } -fn process_documentation(documentation: &str, out: &mut String) { +fn process_documentation(documentation: &str, out: &mut String, rule_name: &str) { let mut in_options = false; let mut after = String::new(); @@ -100,7 +104,17 @@ fn process_documentation(documentation: &str, out: &mut String) { if let Some(rest) = line.strip_prefix("- `") { let option = rest.trim_end().trim_end_matches('`'); - assert!(Options::metadata().has(option), "unknown option {option}"); + match Options::metadata().find(option) { + Some(OptionEntry::Field(field)) => { + if field.deprecated.is_some() { + eprintln!("Rule {rule_name} references deprecated option {option}."); + } + } + Some(_) => {} + None => { + panic!("Unknown option {option} referenced by rule {rule_name}"); + } + } let anchor = option.replace('.', "-"); out.push_str(&format!("- [`{option}`][{option}]\n")); @@ -138,6 +152,7 @@ Something [`else`][other]. [other]: http://example.com.", &mut output, + "example", ); assert_eq!( output, diff --git a/crates/ruff_dev/src/generate_options.rs b/crates/ruff_dev/src/generate_options.rs index 3e73b74f430100..84334588316023 100644 --- a/crates/ruff_dev/src/generate_options.rs +++ b/crates/ruff_dev/src/generate_options.rs @@ -117,6 +117,21 @@ fn emit_field(output: &mut String, name: &str, field: &OptionField, parent_set: field.example )); output.push('\n'); + + if let Some(deprecated) = &field.deprecated { + output.push_str("> **Warning**\n"); + output.push_str("> This option is deprecated"); + + if let Some(since) = deprecated.since { + write!(output, " (since {since})").unwrap(); + } + + if let Some(message) = deprecated.message { + writeln!(output, ": {message}").unwrap(); + } else { + output.push('.'); + } + } } #[derive(Default)] diff --git a/crates/ruff_macros/src/combine_options.rs b/crates/ruff_macros/src/combine_options.rs index b6a940b72d1b8e..ea63a1b91467f2 100644 --- a/crates/ruff_macros/src/combine_options.rs +++ b/crates/ruff_macros/src/combine_options.rs @@ -18,6 +18,7 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result Self { + #[allow(deprecated)] Self { #( #output diff --git a/crates/ruff_macros/src/config.rs b/crates/ruff_macros/src/config.rs index ac7851384e9b4e..50227ae8b01c21 100644 --- a/crates/ruff_macros/src/config.rs +++ b/crates/ruff_macros/src/config.rs @@ -1,16 +1,15 @@ -use proc_macro2::TokenTree; +use proc_macro2::{TokenStream, TokenTree}; use quote::{quote, quote_spanned}; -use syn::parse::{Parse, ParseStream}; +use syn::meta::ParseNestedMeta; use syn::spanned::Spanned; -use syn::token::Comma; use syn::{ AngleBracketedGenericArguments, Attribute, Data, DataStruct, DeriveInput, ExprLit, Field, - Fields, Lit, LitStr, Meta, Path, PathArguments, PathSegment, Token, Type, TypePath, + Fields, Lit, LitStr, Meta, Path, PathArguments, PathSegment, Type, TypePath, }; use ruff_python_trivia::textwrap::dedent; -pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result { +pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result { let DeriveInput { ident, data, @@ -190,9 +189,30 @@ fn handle_option(field: &Field, attr: &Attribute) -> syn::Result()?; + } = parse_field_attributes(attr)?; let kebab_name = LitStr::new(&ident.to_string().replace('_', "-"), ident.span()); + let deprecated = if let Some(deprecated) = field + .attrs + .iter() + .find(|attr| attr.path().is_ident("deprecated")) + { + fn quote_option(option: Option) -> TokenStream { + match option { + None => quote!(None), + Some(value) => quote!(Some(#value)), + } + } + + let deprecated = parse_deprecated_attribute(deprecated)?; + let note = quote_option(deprecated.note); + let since = quote_option(deprecated.since); + + quote!(Some(crate::options_base::Deprecated { since: #since, message: #note })) + } else { + quote!(None) + }; + Ok(quote_spanned!( ident.span() => { visit.record_field(#kebab_name, crate::options_base::OptionField{ @@ -200,6 +220,7 @@ fn handle_option(field: &Field, attr: &Attribute) -> syn::Result syn::Result { - let default = _parse_key_value(input, "default")?; - input.parse::()?; - let value_type = _parse_key_value(input, "value_type")?; - input.parse::()?; - let example = _parse_key_value(input, "example")?; - if !input.is_empty() { - input.parse::()?; +fn parse_field_attributes(attribute: &Attribute) -> syn::Result { + let mut default = None; + let mut value_type = None; + let mut example = None; + + attribute.parse_nested_meta(|meta| { + if meta.path.is_ident("default") { + default = Some(get_string_literal(&meta, "default", "option")?.value()); + } else if meta.path.is_ident("value_type") { + value_type = Some(get_string_literal(&meta, "value_type", "option")?.value()); + } else if meta.path.is_ident("example") { + let example_text = get_string_literal(&meta, "value_type", "option")?.value(); + example = Some(dedent(&example_text).trim_matches('\n').to_string()); + } else { + return Err(syn::Error::new( + meta.path.span(), + format!( + "Deprecated meta {:?} is not supported by ruff's option macro.", + meta.path.get_ident() + ), + )); } - Ok(Self { - default, - value_type, - example: dedent(&example).trim_matches('\n').to_string(), - }) - } + Ok(()) + })?; + + let Some(default) = default else { + return Err(syn::Error::new(attribute.span(), "Mandatory `default` field is missing in `#[option]` attribute. Specify the default using `#[option(default=\"..\")]`.")); + }; + + let Some(value_type) = value_type else { + return Err(syn::Error::new(attribute.span(), "Mandatory `value_type` field is missing in `#[option]` attribute. Specify the value type using `#[option(value_type=\"..\")]`.")); + }; + + let Some(example) = example else { + return Err(syn::Error::new(attribute.span(), "Mandatory `example` field is missing in `#[option]` attribute. Add an example using `#[option(example=\"..\")]`.")); + }; + + Ok(FieldAttributes { + default, + value_type, + example, + }) } -fn _parse_key_value(input: ParseStream, name: &str) -> syn::Result { - let ident: proc_macro2::Ident = input.parse()?; - if ident != name { - return Err(syn::Error::new( - ident.span(), - format!("Expected `{name}` name"), - )); +fn parse_deprecated_attribute(attribute: &Attribute) -> syn::Result { + let mut deprecated = DeprecatedAttribute::default(); + attribute.parse_nested_meta(|meta| { + if meta.path.is_ident("note") { + deprecated.note = Some(get_string_literal(&meta, "note", "deprecated")?.value()); + } else if meta.path.is_ident("since") { + deprecated.since = Some(get_string_literal(&meta, "since", "deprecated")?.value()); + } else { + return Err(syn::Error::new( + meta.path.span(), + format!( + "Deprecated meta {:?} is not supported by ruff's option macro.", + meta.path.get_ident() + ), + )); + } + + Ok(()) + })?; + + Ok(deprecated) +} + +fn get_string_literal( + meta: &ParseNestedMeta, + meta_name: &str, + attribute_name: &str, +) -> syn::Result { + let expr: syn::Expr = meta.value()?.parse()?; + + let mut value = &expr; + while let syn::Expr::Group(e) = value { + value = &e.expr; } - input.parse::()?; - let value: Lit = input.parse()?; + if let syn::Expr::Lit(ExprLit { + lit: Lit::Str(lit), .. + }) = value + { + let suffix = lit.suffix(); + if !suffix.is_empty() { + return Err(syn::Error::new( + lit.span(), + format!("unexpected suffix `{suffix}` on string literal"), + )); + } - match &value { - Lit::Str(v) => Ok(v.value()), - _ => Err(syn::Error::new(value.span(), "Expected literal string")), + Ok(lit.clone()) + } else { + Err(syn::Error::new( + expr.span(), + format!("expected {attribute_name} attribute to be a string: `{meta_name} = \"...\"`"), + )) } } + +#[derive(Default, Debug)] +struct DeprecatedAttribute { + since: Option, + note: Option, +} diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 39874a42d47e83..8f208da039139c 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -171,7 +171,9 @@ impl Configuration { indent_style: format.indent_style.unwrap_or(format_defaults.indent_style), indent_width: self .tab_size - .map_or(format_defaults.indent_width, |tab_size| IndentWidth::from(NonZeroU8::from(tab_size))), + .map_or(format_defaults.indent_width, |tab_size| { + IndentWidth::from(NonZeroU8::from(tab_size)) + }), quote_style: format.quote_style.unwrap_or(format_defaults.quote_style), magic_trailing_comma: format .magic_trailing_comma @@ -565,6 +567,22 @@ pub struct LintConfiguration { impl LintConfiguration { fn from_options(options: LintOptions, project_root: &Path) -> Result { + #[allow(deprecated)] + let ignore = options + .common + .ignore + .into_iter() + .flatten() + .chain(options.common.extend_ignore.into_iter().flatten()) + .collect(); + #[allow(deprecated)] + let unfixable = options + .common + .unfixable + .into_iter() + .flatten() + .chain(options.common.extend_unfixable.into_iter().flatten()) + .collect(); Ok(LintConfiguration { exclude: options.exclude.map(|paths| { paths @@ -579,22 +597,10 @@ impl LintConfiguration { rule_selections: vec![RuleSelection { select: options.common.select, - ignore: options - .common - .ignore - .into_iter() - .flatten() - .chain(options.common.extend_ignore.into_iter().flatten()) - .collect(), + ignore, extend_select: options.common.extend_select.unwrap_or_default(), fixable: options.common.fixable, - unfixable: options - .common - .unfixable - .into_iter() - .flatten() - .chain(options.common.extend_unfixable.into_iter().flatten()) - .collect(), + unfixable, extend_fixable: options.common.extend_fixable.unwrap_or_default(), }], extend_safe_fixes: options.common.extend_safe_fixes.unwrap_or_default(), diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 356414322d1d68..d268d110de9440 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -471,9 +471,6 @@ pub struct LintCommonOptions { /// A list of rule codes or prefixes to ignore, in addition to those /// specified by `ignore`. - /// - /// This option has been **deprecated** in favor of `ignore` - /// since its usage is now interchangeable with `ignore`. #[option( default = "[]", value_type = "list[RuleSelector]", @@ -482,7 +479,9 @@ pub struct LintCommonOptions { extend-ignore = ["F841"] "# )] - #[cfg_attr(feature = "schemars", schemars(skip))] + #[deprecated( + note = "This option has been **deprecated** in favor of `ignore` since its usage is now interchangeable with `ignore`." + )] pub extend_ignore: Option>, /// A list of rule codes or prefixes to enable, in addition to those @@ -511,10 +510,9 @@ pub struct LintCommonOptions { /// A list of rule codes or prefixes to consider non-auto-fixable, in addition to those /// specified by `unfixable`. - /// - /// This option has been **deprecated** in favor of `unfixable` since its usage is now - /// interchangeable with `unfixable`. - #[cfg_attr(feature = "schemars", schemars(skip))] + #[deprecated( + note = "This option has been **deprecated** in favor of `unfixable` since its usage is now interchangeable with `unfixable`." + )] pub extend_unfixable: Option>, /// A list of rule codes that are unsupported by Ruff, but should be diff --git a/crates/ruff_workspace/src/options_base.rs b/crates/ruff_workspace/src/options_base.rs index 8bf095598976ea..d13f5545dbf369 100644 --- a/crates/ruff_workspace/src/options_base.rs +++ b/crates/ruff_workspace/src/options_base.rs @@ -100,6 +100,7 @@ impl OptionSet { /// default: "false", /// value_type: "bool", /// example: "", + /// deprecated: None, /// }); /// } /// } @@ -121,6 +122,7 @@ impl OptionSet { /// default: "false", /// value_type: "bool", /// example: "", + /// deprecated: None /// }); /// /// visit.record_set("format", Nested::metadata()); @@ -136,6 +138,7 @@ impl OptionSet { /// default: "false", /// value_type: "bool", /// example: "", + /// deprecated: None /// }); /// } /// } @@ -166,6 +169,7 @@ impl OptionSet { /// default: "false", /// value_type: "bool", /// example: "", + /// deprecated: None /// }; /// /// impl OptionsMetadata for WithOptions { @@ -187,6 +191,7 @@ impl OptionSet { /// default: "false", /// value_type: "bool", /// example: "", + /// deprecated: None /// }; /// /// struct Root; @@ -198,6 +203,7 @@ impl OptionSet { /// default: "false", /// value_type: "bool", /// example: "", + /// deprecated: None /// }); /// /// visit.record_set("format", Nested::metadata()); @@ -283,8 +289,16 @@ impl Visit for DisplayVisitor<'_, '_> { self.result = self.result.and_then(|_| writeln!(self.f, "{name}")); } - fn record_field(&mut self, name: &str, _: OptionField) { - self.result = self.result.and_then(|_| writeln!(self.f, "{name}")); + fn record_field(&mut self, name: &str, field: OptionField) { + self.result = self.result.and_then(|_| { + write!(self.f, "{name}")?; + + if field.deprecated.is_some() { + write!(self.f, " (deprecated)")?; + } + + writeln!(self.f) + }); } } @@ -308,6 +322,13 @@ pub struct OptionField { pub default: &'static str, pub value_type: &'static str, pub example: &'static str, + pub deprecated: Option, +} + +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct Deprecated { + pub since: Option<&'static str>, + pub message: Option<&'static str>, } impl Display for OptionField { @@ -316,6 +337,21 @@ impl Display for OptionField { writeln!(f)?; writeln!(f, "Default value: {}", self.default)?; writeln!(f, "Type: {}", self.value_type)?; + + if let Some(deprecated) = &self.deprecated { + write!(f, "Deprecated")?; + + if let Some(since) = deprecated.since { + write!(f, " (since {since})")?; + } + + if let Some(message) = deprecated.message { + write!(f, ": {message}")?; + } + + writeln!(f)?; + } + writeln!(f, "Example usage:\n```toml\n{}\n```", self.example) } } diff --git a/ruff.schema.json b/ruff.schema.json index 81f943756265a6..e4ad0ae65cd86d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -84,6 +84,17 @@ "$ref": "#/definitions/RuleSelector" } }, + "extend-ignore": { + "description": "A list of rule codes or prefixes to ignore, in addition to those specified by `ignore`.", + "deprecated": true, + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/definitions/RuleSelector" + } + }, "extend-include": { "description": "A list of file patterns to include when linting, in addition to those specified by `include`.\n\nInclusion are based on globs, and should be single-path patterns, like `*.pyw`, to include any file with the `.pyw` extension.\n\nFor more information on the glob syntax, refer to the [`globset` documentation](https://docs.rs/globset/latest/globset/#syntax).", "type": [ @@ -127,6 +138,17 @@ "$ref": "#/definitions/RuleSelector" } }, + "extend-unfixable": { + "description": "A list of rule codes or prefixes to consider non-auto-fixable, in addition to those specified by `unfixable`.", + "deprecated": true, + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/definitions/RuleSelector" + } + }, "extend-unsafe-fixes": { "description": "A list of rule codes or prefixes for which safe fixes should be considered unsafe.", "type": [ @@ -1629,6 +1651,17 @@ "$ref": "#/definitions/RuleSelector" } }, + "extend-ignore": { + "description": "A list of rule codes or prefixes to ignore, in addition to those specified by `ignore`.", + "deprecated": true, + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/definitions/RuleSelector" + } + }, "extend-per-file-ignores": { "description": "A list of mappings from file pattern to rule codes or prefixes to exclude, in addition to any rules excluded by `per-file-ignores`.", "type": [ @@ -1662,6 +1695,17 @@ "$ref": "#/definitions/RuleSelector" } }, + "extend-unfixable": { + "description": "A list of rule codes or prefixes to consider non-auto-fixable, in addition to those specified by `unfixable`.", + "deprecated": true, + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/definitions/RuleSelector" + } + }, "extend-unsafe-fixes": { "description": "A list of rule codes or prefixes for which safe fixes should be considered unsafe.", "type": [