Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-27 07:00:55 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #7299 - DevinR528:macro-brace, r=llogiq

Browse source 
Add macro_braces lint to check for irregular brace use in certain macros

The name is a bit long but this sounds good as `#[allow(unconventional_macro_braces)]` and it seems more clear that we are talking about the macro call not macro definitions, any feedback let me know. Thanks!
fixes #7278

changelog: Add ``[`unconventional_macro_braces`]`` lint that checks for uncommon brace usage with macros.
This commit is contained in:
bors 2021-06-19 17:56:56 +00:00
commit 3120b09151
12 changed files with 443 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -2704,6 +2704,7 @@ Released 2018-09-13
[`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions
[`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
[`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces
[`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
[`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref

6
clippy_lints/src/lib.rs
View file

@ -293,6 +293,7 @@ mod no_effect;
mod non_copy_const;
mod non_expressive_names;
mod non_octal_unix_permissions;
mod nonstandard_macro_braces;
mod open_options;
mod option_env_unwrap;
mod option_if_let_else;
@ -845,6 +846,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
non_expressive_names::MANY_SINGLE_CHAR_NAMES,
non_expressive_names::SIMILAR_NAMES,
non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS,
nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES,
open_options::NONSENSICAL_OPEN_OPTIONS,
option_env_unwrap::OPTION_ENV_UNWRAP,
option_if_let_else::OPTION_IF_LET_ELSE,
@ -1362,6 +1364,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES),
LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
@ -1537,6 +1540,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES),
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
LintId::of(ptr::CMP_NULL),
LintId::of(ptr::PTR_ARG),
LintId::of(ptr_eq::PTR_EQ),
@ -2042,6 +2046,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(move || box non_expressive_names::NonExpressiveNames {
single_char_binding_names_threshold,
});
let macro_matcher = conf.standard_macro_braces.iter().cloned().collect::<FxHashSet<_>>();
store.register_early_pass(move || box nonstandard_macro_braces::MacroBraces::new(&macro_matcher));
store.register_late_pass(|| box macro_use::MacroUseImports::default());
store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch);
store.register_late_pass(|| box stable_sort_primitive::StableSortPrimitive);

276
clippy_lints/src/nonstandard_macro_braces.rs Normal file
View file

@ -0,0 +1,276 @@
use std::{
fmt,
hash::{Hash, Hasher},
};
use clippy_utils::{diagnostics::span_lint_and_help, in_macro, is_direct_expn_of, source::snippet_opt};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::Span;
use serde::{de, Deserialize};
declare_clippy_lint! {
/// **What it does:** Checks that common macros are used with consistent bracing.
///
/// **Why is this bad?** This is mostly a consistency lint although using () or []
/// doesn't give you a semicolon in item position, which can be unexpected.
///
/// **Known problems:**
/// None
///
/// **Example:**
///
/// ```rust
/// vec!{1, 2, 3};
/// ```
/// Use instead:
/// ```rust
/// vec![1, 2, 3];
/// ```
pub NONSTANDARD_MACRO_BRACES,
style,
"check consistent use of braces in macro"
}
const BRACES: &[(&str, &str)] = &[("(", ")"), ("{", "}"), ("[", "]")];
/// The (name, (open brace, close brace), source snippet)
type MacroInfo<'a> = (&'a str, &'a (String, String), String);
#[derive(Clone, Debug, Default)]
pub struct MacroBraces {
macro_braces: FxHashMap<String, (String, String)>,
done: FxHashSet<Span>,
}
impl MacroBraces {
pub fn new(conf: &FxHashSet<MacroMatcher>) -> Self {
let macro_braces = macro_braces(conf.clone());
Self {
macro_braces,
done: FxHashSet::default(),
}
}
}
impl_lint_pass!(MacroBraces => [NONSTANDARD_MACRO_BRACES]);
impl EarlyLintPass for MacroBraces {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
if let Some((name, braces, snip)) = is_offending_macro(cx, item.span, self) {
let span = item.span.ctxt().outer_expn_data().call_site;
emit_help(cx, snip, braces, name, span);
self.done.insert(span);
}
}
fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) {
if let Some((name, braces, snip)) = is_offending_macro(cx, stmt.span, self) {
let span = stmt.span.ctxt().outer_expn_data().call_site;
emit_help(cx, snip, braces, name, span);
self.done.insert(span);
}
}
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
if let Some((name, braces, snip)) = is_offending_macro(cx, expr.span, self) {
let span = expr.span.ctxt().outer_expn_data().call_site;
emit_help(cx, snip, braces, name, span);
self.done.insert(span);
}
}
fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) {
if let Some((name, braces, snip)) = is_offending_macro(cx, ty.span, self) {
let span = ty.span.ctxt().outer_expn_data().call_site;
emit_help(cx, snip, braces, name, span);
self.done.insert(span);
}
}
}
fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, this: &'a MacroBraces) -> Option<MacroInfo<'a>> {
if_chain! {
if in_macro(span);
if let Some((name, braces)) = find_matching_macro(span, &this.macro_braces);
if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site);
let c = snip.replace(" ", ""); // make formatting consistent
if !c.starts_with(&format!("{}!{}", name, braces.0));
if !this.done.contains(&span.ctxt().outer_expn_data().call_site);
then {
Some((name, braces, snip))
} else {
None
}
}
}
fn emit_help(cx: &EarlyContext<'_>, snip: String, braces: &(String, String), name: &str, span: Span) {
let with_space = &format!("! {}", braces.0);
let without_space = &format!("!{}", braces.0);
let mut help = snip;
for b in BRACES.iter().filter(|b| b.0 != braces.0) {
help = help.replace(b.0, &braces.0).replace(b.1, &braces.1);
// Only `{` traditionally has space before the brace
if braces.0 != "{" && help.contains(with_space) {
help = help.replace(with_space, without_space);
} else if braces.0 == "{" && help.contains(without_space) {
help = help.replace(without_space, with_space);
}
}
span_lint_and_help(
cx,
NONSTANDARD_MACRO_BRACES,
span,
&format!("use of irregular braces for `{}!` macro", name),
Some(span),
&format!("consider writing `{}`", help),
);
}
fn find_matching_macro(
span: Span,
braces: &FxHashMap<String, (String, String)>,
) -> Option<(&String, &(String, String))> {
braces
.iter()
.find(|(macro_name, _)| is_direct_expn_of(span, macro_name).is_some())
}
fn macro_braces(conf: FxHashSet<MacroMatcher>) -> FxHashMap<String, (String, String)> {
let mut braces = vec![
macro_matcher!(
name: "print",
braces: ("(", ")"),
),
macro_matcher!(
name: "println",
braces: ("(", ")"),
),
macro_matcher!(
name: "eprint",
braces: ("(", ")"),
),
macro_matcher!(
name: "eprintln",
braces: ("(", ")"),
),
macro_matcher!(
name: "write",
braces: ("(", ")"),
),
macro_matcher!(
name: "writeln",
braces: ("(", ")"),
),
macro_matcher!(
name: "format",
braces: ("(", ")"),
),
macro_matcher!(
name: "format_args",
braces: ("(", ")"),
),
macro_matcher!(
name: "vec",
braces: ("[", "]"),
),
]
.into_iter()
.collect::<FxHashMap<_, _>>();
// We want users items to override any existing items
for it in conf {
braces.insert(it.name, it.braces);
}
braces
}
macro_rules! macro_matcher {
(name: $name:expr, braces: ($open:expr, $close:expr) $(,)?) => {
($name.to_owned(), ($open.to_owned(), $close.to_owned()))
};
}
pub(crate) use macro_matcher;
#[derive(Clone, Debug)]
pub struct MacroMatcher {
name: String,
braces: (String, String),
}
impl Hash for MacroMatcher {
fn hash<H: Hasher>(&self, state: &mut H) {
self.name.hash(state);
}
}
impl PartialEq for MacroMatcher {
fn eq(&self, other: &Self) -> bool {
self.name == other.name
}
}
impl Eq for MacroMatcher {}
impl<'de> Deserialize<'de> for MacroMatcher {
fn deserialize<D>(deser: D) -> Result<Self, D::Error>
where
D: de::Deserializer<'de>,
{
#[derive(Deserialize)]
#[serde(field_identifier, rename_all = "lowercase")]
enum Field {
Name,
Brace,
}
struct MacVisitor;
impl<'de> de::Visitor<'de> for MacVisitor {
type Value = MacroMatcher;
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("struct MacroMatcher")
}
fn visit_map<V>(self, mut map: V) -> Result<Self::Value, V::Error>
where
V: de::MapAccess<'de>,
{
let mut name = None;
let mut brace: Option<&str> = None;
while let Some(key) = map.next_key()? {
match key {
Field::Name => {
if name.is_some() {
return Err(de::Error::duplicate_field("name"));
}
name = Some(map.next_value()?);
},
Field::Brace => {
if brace.is_some() {
return Err(de::Error::duplicate_field("brace"));
}
brace = Some(map.next_value()?);
},
}
}
let name = name.ok_or_else(|| de::Error::missing_field("name"))?;
let brace = brace.ok_or_else(|| de::Error::missing_field("brace"))?;
Ok(MacroMatcher {
name,
braces: BRACES
.iter()
.find(|b| b.0 == brace)
.map(|(o, c)| ((*o).to_owned(), (*c).to_owned()))
.ok_or_else(|| {
de::Error::custom(&format!("expected one of `(`, `{{`, `[` found `{}`", brace))
})?,
})
}
}
const FIELDS: &[&str] = &["name", "brace"];
deser.deserialize_struct("MacroMatcher", FIELDS, MacVisitor)
}
}

16
clippy_lints/src/utils/conf.rs
View file

@ -26,13 +26,13 @@ impl TryConf {
macro_rules! define_Conf {
($(
#[doc = $doc:literal]
$(#[doc = $doc:literal])*
$(#[conf_deprecated($dep:literal)])?
($name:ident: $ty:ty = $default:expr),
)*) => {
/// Clippy lint configuration
pub struct Conf {
$(#[doc = $doc] pub $name: $ty,)*
$($(#[doc = $doc])* pub $name: $ty,)*
}
mod defaults {
@ -109,7 +109,7 @@ macro_rules! define_Conf {
stringify!($name),
stringify!($ty),
format!("{:?}", super::defaults::$name()),
$doc,
concat!($($doc,)*),
deprecation_reason,
)
},
@ -182,9 +182,9 @@ define_Conf! {
(vec_box_size_threshold: u64 = 4096),
/// Lint: TYPE_REPETITION_IN_BOUNDS. The maximum number of bounds a trait can have to be linted
(max_trait_bounds: u64 = 3),
/// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bools a struct can have
/// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bool fields a struct can have
(max_struct_bools: u64 = 3),
/// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have
/// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bool parameters a function can have
(max_fn_params_bools: u64 = 3),
/// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests).
(warn_on_all_wildcard_imports: bool = false),
@ -198,6 +198,12 @@ define_Conf! {
(upper_case_acronyms_aggressive: bool = false),
/// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest.
(cargo_ignore_publish: bool = false),
/// Lint: NONSTANDARD_MACRO_BRACES. Enforce the named macros always use the braces specified.
///
/// A `MacroMatcher` can be added like so `{ name = "macro_name", brace = "(" }`.
/// If the macro is could be used with a full path two `MacroMatcher`s have to be added one
/// with the full path `crate_name::macro_name` and one with just the macro name.
(standard_macro_braces: Vec<crate::nonstandard_macro_braces::MacroMatcher> = Vec::new()),
}
/// Search for the configuration file.

6
tests/ui-toml/nonstandard_macro_braces/clippy.toml Normal file
View file

@ -0,0 +1,6 @@
standard-macro-braces = [
{ name = "quote", brace = "{" },
{ name = "quote::quote", brace = "{" },
{ name = "eprint", brace = "[" },
{ name = "type_pos", brace = "[" },
]

44
tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs Normal file
View file

@ -0,0 +1,44 @@
// #![warn(clippy::nonstandard_macro_braces)]
extern crate quote;
use quote::quote;
#[rustfmt::skip]
macro_rules! test {
() => {
vec!{0, 0, 0}
};
}
#[rustfmt::skip]
macro_rules! test2 {
($($arg:tt)*) => {
format_args!($($arg)*)
};
}
macro_rules! type_pos {
($what:ty) => {
Vec<$what>
};
}
#[rustfmt::skip]
fn main() {
let _ = vec! {1, 2, 3};
let _ = format!["ugh {} stop being such a good compiler", "hello"];
let _ = quote!(let x = 1;);
let _ = quote::quote!(match match match);
let _ = test!();
let _ = vec![1,2,3];
let _ = quote::quote! {true || false};
let _ = vec! [0 ,0 ,0];
let _ = format!("fds{}fds", 10);
let _ = test2!["{}{}{}", 1, 2, 3];
let _: type_pos!(usize) = vec![];
eprint!("test if user config overrides defaults");
}

94
tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr Normal file
View file

@ -0,0 +1,94 @@
error: use of irregular braces for `vec!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:29:13
|
LL | let _ = vec! {1, 2, 3};
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::nonstandard-macro-braces` implied by `-D warnings`
help: consider writing `vec![1, 2, 3]`
--> $DIR/conf_nonstandard_macro_braces.rs:29:13
|
LL | let _ = vec! {1, 2, 3};
| ^^^^^^^^^^^^^^
error: use of irregular braces for `format!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:30:13
|
LL | let _ = format!["ugh {} stop being such a good compiler", "hello"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider writing `format!("ugh () stop being such a good compiler", "hello")`
--> $DIR/conf_nonstandard_macro_braces.rs:30:13
|
LL | let _ = format!["ugh {} stop being such a good compiler", "hello"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: use of irregular braces for `quote!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:31:13
|
LL | let _ = quote!(let x = 1;);
| ^^^^^^^^^^^^^^^^^^
|
help: consider writing `quote! {let x = 1;}`
--> $DIR/conf_nonstandard_macro_braces.rs:31:13
|
LL | let _ = quote!(let x = 1;);
| ^^^^^^^^^^^^^^^^^^
error: use of irregular braces for `quote::quote!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:32:13
|
LL | let _ = quote::quote!(match match match);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider writing `quote::quote! {match match match}`
--> $DIR/conf_nonstandard_macro_braces.rs:32:13
|
LL | let _ = quote::quote!(match match match);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: use of irregular braces for `vec!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:10:9
|
LL | vec!{0, 0, 0}
| ^^^^^^^^^^^^^
...
LL | let _ = test!();
| ------- in this macro invocation
|
help: consider writing `vec![0, 0, 0]`
--> $DIR/conf_nonstandard_macro_braces.rs:10:9
|
LL | vec!{0, 0, 0}
| ^^^^^^^^^^^^^
...
LL | let _ = test!();
| ------- in this macro invocation
= note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)
error: use of irregular braces for `type_pos!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:41:12
|
LL | let _: type_pos!(usize) = vec![];
| ^^^^^^^^^^^^^^^^
|
help: consider writing `type_pos![usize]`
--> $DIR/conf_nonstandard_macro_braces.rs:41:12
|
LL | let _: type_pos!(usize) = vec![];
| ^^^^^^^^^^^^^^^^
error: use of irregular braces for `eprint!` macro
--> $DIR/conf_nonstandard_macro_braces.rs:43:5
|
LL | eprint!("test if user config overrides defaults");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider writing `eprint!["test if user config overrides defaults"];`
--> $DIR/conf_nonstandard_macro_braces.rs:43:5
|
LL | eprint!("test if user config overrides defaults");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 7 previous errors

2
tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
View file

@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `third-party` at line 5 column 1
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `third-party` at line 5 column 1
error: aborting due to previous error

2
tests/ui/unnecessary_cast_fixable.fixed
View file

@ -1,7 +1,7 @@
// run-rustfix
#![warn(clippy::unnecessary_cast)]
#![allow(clippy::no_effect, clippy::unnecessary_operation)]
#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::nonstandard_macro_braces)]
fn main() {
// casting integer literal to float is unnecessary

2
tests/ui/unnecessary_cast_fixable.rs
View file

@ -1,7 +1,7 @@
// run-rustfix
#![warn(clippy::unnecessary_cast)]
#![allow(clippy::no_effect, clippy::unnecessary_operation)]
#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::nonstandard_macro_braces)]
fn main() {
// casting integer literal to float is unnecessary

2
tests/ui/vec.fixed
View file

@ -1,5 +1,5 @@
// run-rustfix
#![allow(clippy::nonstandard_macro_braces)]
#![warn(clippy::useless_vec)]
#[derive(Debug)]

2
tests/ui/vec.rs
View file

@ -1,5 +1,5 @@
// run-rustfix
#![allow(clippy::nonstandard_macro_braces)]
#![warn(clippy::useless_vec)]
#[derive(Debug)]