From f5ddc5a5f4932b14289376f078d545a2316b0779 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 | 18 +++ crates/ruff_macros/src/combine_options.rs | 1 + crates/ruff_macros/src/config.rs | 157 ++++++++++++++++----- crates/ruff_workspace/src/configuration.rs | 32 +++-- crates/ruff_workspace/src/options.rs | 14 +- crates/ruff_workspace/src/options_base.rs | 40 +++++- docs/configuration.md | 2 +- ruff.schema.json | 44 ++++++ 9 files changed, 269 insertions(+), 62 deletions(-) diff --git a/crates/ruff_dev/src/generate_docs.rs b/crates/ruff_dev/src/generate_docs.rs index 4cddb678ffa8f..1b557b68bcccb 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 3e73b74f43010..38737fb439d69 100644 --- a/crates/ruff_dev/src/generate_options.rs +++ b/crates/ruff_dev/src/generate_options.rs @@ -101,6 +101,24 @@ fn emit_field(output: &mut String, name: &str, field: &OptionField, parent_set: output.push_str(&format!("{header_level} [`{name}`](#{name})\n")); } output.push('\n'); + + if let Some(deprecated) = &field.deprecated { + output.push_str("!!! warning \"Deprecated\"\n"); + output.push_str(" This option has been deprecated"); + + if let Some(since) = deprecated.since { + write!(output, " in {since}").unwrap(); + } + + output.push('.'); + + if let Some(message) = deprecated.message { + writeln!(output, " {message}").unwrap(); + } + + output.push('\n'); + } + output.push_str(field.doc); output.push_str("\n\n"); output.push_str(&format!("**Default value**: `{}`\n", field.default)); diff --git a/crates/ruff_macros/src/combine_options.rs b/crates/ruff_macros/src/combine_options.rs index b6a940b72d1b8..ea63a1b91467f 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 ac7851384e9b4..50227ae8b01c2 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 86b1d756dc528..8f208da039139 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -567,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 @@ -581,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 356414322d1d6..19e53af7ac389 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 = "The `extend-ignore` option is now interchangeable with `ignore`. Please update your configuration to use the `ignore` option instead." + )] 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 = "The `extend-unfixable` option is now interchangeable with `unfixable`. Please update your configuration to use the `unfixable` option instead." + )] 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 8bf095598976e..d13f5545dbf36 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/docs/configuration.md b/docs/configuration.md index 4f91a90563d42..935ddf960c23e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -391,7 +391,7 @@ whether a rule supports fixing, see [_Rules_](rules.md). Ruff labels fixes as "safe" and "unsafe". The meaning and intent of your code will be retained when applying safe fixes, but the meaning could be changed when applying unsafe fixes. -For example, [`unnecessary-iterable-allocation-for-first-element`](../rules/unnecessary-iterable-allocation-for-first-element) (`RUF015`) is a rule which checks for potentially unperformant use of `list(...)[0]`. The fix replaces this pattern with `next(iter(...))` which can result in a drastic speedup: +For example, [`unnecessary-iterable-allocation-for-first-element`](rules/unnecessary-iterable-allocation-for-first-element.md) (`RUF015`) is a rule which checks for potentially unperformant use of `list(...)[0]`. The fix replaces this pattern with `next(iter(...))` which can result in a drastic speedup: ```shell $ python -m timeit "head = list(range(99999999))[0]" diff --git a/ruff.schema.json b/ruff.schema.json index 81f943756265a..e4ad0ae65cd86 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": [