Skip to content

Commit

Permalink
derive-msg-formats 4/5: Implement #[derive_message_formats]
Browse files Browse the repository at this point in the history
The idea is nice and simple we replace:

    fn placeholder() -> Self;

with

    fn message_formats() -> &'static [&'static str];

So e.g. if a Violation implementation defines:

    fn message(&self) -> String {
        format!("Local variable `{name}` is assigned to but never used")
    }

it would also have to define:

   fn message_formats() -> &'static [&'static str] {
       &["Local variable `{name}` is assigned to but never used"]
   }

Since we however obviously do not want to duplicate all of our format
strings we simply introduce a new procedural macro attribute
#[derive_message_formats] that can be added to the message method
declaration in order to automatically derive the message_formats
implementation.

This commit implements the macro. The following and final commit
updates violations.rs to use the macro. (The changes have been separated
because the next commit is autogenerated via a Python script.)
  • Loading branch information
not-my-profile committed Jan 19, 2023
1 parent 72ffa07 commit 707da7c
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 20 deletions.
4 changes: 2 additions & 2 deletions ruff_cli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ pub fn explain(rule: &Rule, format: SerializationFormat) -> Result<()> {
"{} ({}): {}",
rule.code(),
rule.origin().name(),
rule.kind().body()
rule.message_formats()[0]
);
}
SerializationFormat::Json => {
Expand All @@ -306,7 +306,7 @@ pub fn explain(rule: &Rule, format: SerializationFormat) -> Result<()> {
serde_json::to_string_pretty(&Explanation {
code: rule.code(),
origin: rule.origin().name(),
summary: &rule.kind().body(),
summary: rule.message_formats()[0],
})?
);
}
Expand Down
3 changes: 1 addition & 2 deletions ruff_dev/src/generate_rules_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ fn generate_table(table_out: &mut String, prefix: &RuleCodePrefix) {
table_out.push_str("| ---- | ---- | ------- | --- |");
table_out.push('\n');
for rule in prefix.codes() {
let kind = rule.kind();
let fix_token = match rule.autofixable() {
Autofixability::Never => "",
Autofixability::Sometimes => "🛠 (sometimes)",
Expand All @@ -38,7 +37,7 @@ fn generate_table(table_out: &mut String, prefix: &RuleCodePrefix) {
"| {} | {} | {} | {} |",
rule.code(),
rule.as_ref(),
kind.body().replace('|', r"\|"),
rule.message_formats()[0].replace('|', r"\|"),
fix_token
));
table_out.push('\n');
Expand Down
13 changes: 6 additions & 7 deletions ruff_macros/src/define_rule_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use syn::{Ident, LitStr, Path, Token};
pub fn define_rule_mapping(mapping: &Mapping) -> proc_macro2::TokenStream {
let mut rule_variants = quote!();
let mut diagkind_variants = quote!();
let mut rule_kind_match_arms = quote!();
let mut rule_message_formats_match_arms = quote!();
let mut rule_autofixable_match_arms = quote!();
let mut rule_origin_match_arms = quote!();
let mut rule_code_match_arms = quote!();
Expand All @@ -26,9 +26,8 @@ pub fn define_rule_mapping(mapping: &Mapping) -> proc_macro2::TokenStream {
#name,
});
diagkind_variants.extend(quote! {#name(#path),});
rule_kind_match_arms.extend(
quote! {Self::#name => DiagnosticKind::#name(<#path as Violation>::placeholder()),},
);
rule_message_formats_match_arms
.extend(quote! {Self::#name => <#path as Violation>::message_formats(),});
rule_autofixable_match_arms
.extend(quote! {Self::#name => <#path as Violation>::AUTOFIXABLE,});
let origin = get_origin(code);
Expand Down Expand Up @@ -87,9 +86,9 @@ pub fn define_rule_mapping(mapping: &Mapping) -> proc_macro2::TokenStream {
}

impl Rule {
/// A placeholder representation of the `DiagnosticKind` for the diagnostic.
pub fn kind(&self) -> DiagnosticKind {
match self { #rule_kind_match_arms }
/// Returns the format strings used to report violations of this rule.
pub fn message_formats(&self) -> &'static [&'static str] {
match self { #rule_message_formats_match_arms }
}

pub fn autofixable(&self) -> crate::violation::Autofixability {
Expand Down
55 changes: 55 additions & 0 deletions ruff_macros/src/derive_message_formats.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
use proc_macro2::TokenStream;
use quote::{quote, quote_spanned, ToTokens};
use syn::spanned::Spanned;
use syn::{Block, Expr, ItemFn, Stmt};

pub fn derive_message_formats(func: &ItemFn) -> proc_macro2::TokenStream {
let mut strings = quote!();

if let Err(err) = parse_block(&func.block, &mut strings) {
return err;
}

quote! {
#func
fn message_formats() -> &'static [&'static str] {
&[#strings]
}
}
}

fn parse_block(block: &Block, strings: &mut TokenStream) -> Result<(), TokenStream> {
let Some(Stmt::Expr(last)) = block.stmts.last() else {panic!("expected last statement in block to be an expression")};
parse_expr(last, strings)?;
Ok(())
}

fn parse_expr(expr: &Expr, strings: &mut TokenStream) -> Result<(), TokenStream> {
match expr {
Expr::Macro(mac) if mac.mac.path.is_ident("format") => {
let Some(first_token) = mac.mac.tokens.to_token_stream().into_iter().next() else {
return Err(quote_spanned!(expr.span() => compile_error!("expected format! to have an argument")))
};
strings.extend(quote! {#first_token,});
Ok(())
}
Expr::Block(block) => parse_block(&block.block, strings),
Expr::If(expr) => {
parse_block(&expr.then_branch, strings)?;
if let Some((_, then)) = &expr.else_branch {
parse_expr(then, strings)?;
}
Ok(())
}
Expr::Match(block) => {
for arm in &block.arms {
parse_expr(&arm.body, strings)?;
}
Ok(())
}
_ => Err(quote_spanned!(
expr.span() =>
compile_error!("expected last expression to be a format! macro or a match block")
)),
}
}
10 changes: 9 additions & 1 deletion ruff_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@
)]
#![forbid(unsafe_code)]

use syn::{parse_macro_input, DeriveInput};
use proc_macro::TokenStream;
use syn::{parse_macro_input, DeriveInput, ItemFn};

mod config;
mod define_rule_mapping;
mod derive_message_formats;
mod prefixes;
mod rule_code_prefix;

Expand All @@ -34,3 +36,9 @@ pub fn define_rule_mapping(item: proc_macro::TokenStream) -> proc_macro::TokenSt
let mapping = parse_macro_input!(item as define_rule_mapping::Mapping);
define_rule_mapping::define_rule_mapping(&mapping).into()
}

#[proc_macro_attribute]
pub fn derive_message_formats(_attr: TokenStream, item: TokenStream) -> TokenStream {
let func = parse_macro_input!(item as ItemFn);
derive_message_formats::derive_message_formats(&func).into()
}
1 change: 1 addition & 0 deletions src/rules/flake8_tidy_imports/banned_api.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use ruff_macros::derive_message_formats;
use rustc_hash::FxHashMap;
use rustpython_ast::{Alias, Expr, Located};
use schemars::JsonSchema;
Expand Down
1 change: 1 addition & 0 deletions src/rules/flake8_tidy_imports/relative_imports.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use ruff_macros::derive_message_formats;
use rustpython_ast::Stmt;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand Down
1 change: 1 addition & 0 deletions src/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::useless_format)]
pub mod eradicate;
pub mod flake8_2020;
pub mod flake8_annotations;
Expand Down
13 changes: 7 additions & 6 deletions src/violation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ pub trait Violation: Debug + PartialEq + Eq + Serialize + DeserializeOwned {
None
}

/// A placeholder instance of the violation.
fn placeholder() -> Self;
/// Returns the format strings used by [`message`](Violation::message).
fn message_formats() -> &'static [&'static str];
}

/// This trait exists just to make implementing the [`Violation`] trait more
Expand All @@ -37,8 +37,9 @@ pub trait AlwaysAutofixableViolation:
/// The title displayed for the available autofix.
fn autofix_title(&self) -> String;

/// A placeholder instance of the violation.
fn placeholder() -> Self;
/// Returns the format strings used by
/// [`message`](AlwaysAutofixableViolation::message).
fn message_formats() -> &'static [&'static str];
}

/// A blanket implementation.
Expand All @@ -53,8 +54,8 @@ impl<VA: AlwaysAutofixableViolation> Violation for VA {
Some(Self::autofix_title)
}

fn placeholder() -> Self {
<Self as AlwaysAutofixableViolation>::placeholder()
fn message_formats() -> &'static [&'static str] {
<Self as AlwaysAutofixableViolation>::message_formats()
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/violations.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![allow(clippy::useless_format)]
use std::fmt;

use itertools::Itertools;
use ruff_macros::derive_message_formats;
use rustpython_ast::Cmpop;
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -255,7 +257,7 @@ define_violation!(
impl Violation for IOError {
fn message(&self) -> String {
let IOError(message) = self;
message.clone()
format!("{message}")
}

fn placeholder() -> Self {
Expand Down Expand Up @@ -4282,7 +4284,7 @@ define_violation!(
);
impl Violation for NoThisPrefix {
fn message(&self) -> String {
"First word of the docstring should not be \"This\"".to_string()
r#"First word of the docstring should not be "This""#.to_string()
}

fn placeholder() -> Self {
Expand Down

0 comments on commit 707da7c

Please sign in to comment.