Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect #(deprecated) attribute in configuration options #8035

Merged
merged 1 commit into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions crates/ruff_dev/src/generate_docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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")
Expand All @@ -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();

Expand All @@ -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"));
Expand Down Expand Up @@ -138,6 +152,7 @@ Something [`else`][other].

[other]: http://example.com.",
&mut output,
"example",
);
assert_eq!(
output,
Expand Down
18 changes: 18 additions & 0 deletions crates/ruff_dev/src/generate_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_macros/src/combine_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result<proc_macro2::TokenS
Ok(quote! {
impl crate::configuration::CombinePluginOptions for #ident {
fn combine(self, other: Self) -> Self {
#[allow(deprecated)]
Self {
#(
#output
Expand Down
157 changes: 124 additions & 33 deletions crates/ruff_macros/src/config.rs
Original file line number Diff line number Diff line change
@@ -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<proc_macro2::TokenStream> {
pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result<TokenStream> {
let DeriveInput {
ident,
data,
Expand Down Expand Up @@ -190,16 +189,38 @@ fn handle_option(field: &Field, attr: &Attribute) -> syn::Result<proc_macro2::To
default,
value_type,
example,
} = attr.parse_args::<FieldAttributes>()?;
} = 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<String>) -> 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{
doc: &#doc,
default: &#default,
value_type: &#value_type,
example: &#example,
deprecated: #deprecated
})
}
))
Expand All @@ -212,39 +233,109 @@ struct FieldAttributes {
example: String,
}

impl Parse for FieldAttributes {
fn parse(input: ParseStream) -> syn::Result<Self> {
let default = _parse_key_value(input, "default")?;
input.parse::<Comma>()?;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old logic relied on the specific order. The new logic allows arbitrary ordering and is in line with that serde and other crates use for parsing attribute fields.

let value_type = _parse_key_value(input, "value_type")?;
input.parse::<Comma>()?;
let example = _parse_key_value(input, "example")?;
if !input.is_empty() {
input.parse::<Comma>()?;
fn parse_field_attributes(attribute: &Attribute) -> syn::Result<FieldAttributes> {
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<String> {
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<DeprecatedAttribute> {
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<syn::LitStr> {
let expr: syn::Expr = meta.value()?.parse()?;

let mut value = &expr;
while let syn::Expr::Group(e) = value {
value = &e.expr;
}

input.parse::<Token![=]>()?;
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<String>,
note: Option<String>,
}
32 changes: 18 additions & 14 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,22 @@ pub struct LintConfiguration {

impl LintConfiguration {
fn from_options(options: LintOptions, project_root: &Path) -> Result<Self> {
#[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
Expand All @@ -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(),
Expand Down
14 changes: 6 additions & 8 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]",
Expand All @@ -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<Vec<RuleSelector>>,

/// A list of rule codes or prefixes to enable, in addition to those
Expand Down Expand Up @@ -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<Vec<RuleSelector>>,

/// A list of rule codes that are unsupported by Ruff, but should be
Expand Down
Loading
Loading