Skip to content

Commit 723f515

Browse files
committed
Add macro_braces lint to check for irregular brace use in certain macros
Rename unconventional -> nonstandard, add config field Add standard_macro_braces fields so users can specify macro names and brace combinations to lint for in the clippy.toml file. Fix errors caused by nonstandard_macro_braces in other lint tests Fix users ability to override the default nonstandard macro braces Add type position macros impl `check_ty`
1 parent f1f5ccd commit 723f515

12 files changed

+443
-10
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2575,6 +2575,7 @@ Released 2018-09-13
25752575
[`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions
25762576
[`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
25772577
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
2578+
[`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces
25782579
[`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
25792580
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
25802581
[`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref

clippy_lints/src/lib.rs

+6
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ mod no_effect;
293293
mod non_copy_const;
294294
mod non_expressive_names;
295295
mod non_octal_unix_permissions;
296+
mod nonstandard_macro_braces;
296297
mod open_options;
297298
mod option_env_unwrap;
298299
mod option_if_let_else;
@@ -844,6 +845,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
844845
non_expressive_names::MANY_SINGLE_CHAR_NAMES,
845846
non_expressive_names::SIMILAR_NAMES,
846847
non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS,
848+
nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES,
847849
open_options::NONSENSICAL_OPEN_OPTIONS,
848850
option_env_unwrap::OPTION_ENV_UNWRAP,
849851
option_if_let_else::OPTION_IF_LET_ELSE,
@@ -1360,6 +1362,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13601362
LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
13611363
LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES),
13621364
LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
1365+
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
13631366
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
13641367
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
13651368
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
@@ -1536,6 +1539,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15361539
LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
15371540
LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
15381541
LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES),
1542+
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
15391543
LintId::of(ptr::CMP_NULL),
15401544
LintId::of(ptr::PTR_ARG),
15411545
LintId::of(ptr_eq::PTR_EQ),
@@ -2040,6 +2044,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
20402044
store.register_early_pass(move || box non_expressive_names::NonExpressiveNames {
20412045
single_char_binding_names_threshold,
20422046
});
2047+
let macro_matcher = conf.standard_macro_braces.iter().cloned().collect::<FxHashSet<_>>();
2048+
store.register_early_pass(move || box nonstandard_macro_braces::MacroBraces::new(&macro_matcher));
20432049
store.register_late_pass(|| box macro_use::MacroUseImports::default());
20442050
store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch);
20452051
store.register_late_pass(|| box stable_sort_primitive::StableSortPrimitive);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
use std::{
2+
fmt,
3+
hash::{Hash, Hasher},
4+
};
5+
6+
use clippy_utils::{diagnostics::span_lint_and_help, in_macro, is_direct_expn_of, source::snippet_opt};
7+
use if_chain::if_chain;
8+
use rustc_ast::ast;
9+
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
10+
use rustc_lint::{EarlyContext, EarlyLintPass};
11+
use rustc_session::{declare_tool_lint, impl_lint_pass};
12+
use rustc_span::Span;
13+
use serde::{de, Deserialize};
14+
15+
declare_clippy_lint! {
16+
/// **What it does:** Checks that common macros are used with consistent bracing.
17+
///
18+
/// **Why is this bad?** This is mostly a consistency lint although using () or []
19+
/// doesn't give you a semicolon in item position, which can be unexpected.
20+
///
21+
/// **Known problems:**
22+
/// None
23+
///
24+
/// **Example:**
25+
///
26+
/// ```rust
27+
/// vec!{1, 2, 3};
28+
/// ```
29+
/// Use instead:
30+
/// ```rust
31+
/// vec![1, 2, 3];
32+
/// ```
33+
pub NONSTANDARD_MACRO_BRACES,
34+
style,
35+
"check consistent use of braces in macro"
36+
}
37+
38+
const BRACES: &[(&str, &str)] = &[("(", ")"), ("{", "}"), ("[", "]")];
39+
40+
/// The (name, (open brace, close brace), source snippet)
41+
type MacroInfo<'a> = (&'a str, &'a (String, String), String);
42+
43+
#[derive(Clone, Debug, Default)]
44+
pub struct MacroBraces {
45+
macro_braces: FxHashMap<String, (String, String)>,
46+
done: FxHashSet<Span>,
47+
}
48+
49+
impl MacroBraces {
50+
pub fn new(conf: &FxHashSet<MacroMatcher>) -> Self {
51+
let macro_braces = macro_braces(conf.clone());
52+
Self {
53+
macro_braces,
54+
done: FxHashSet::default(),
55+
}
56+
}
57+
}
58+
59+
impl_lint_pass!(MacroBraces => [NONSTANDARD_MACRO_BRACES]);
60+
61+
impl EarlyLintPass for MacroBraces {
62+
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
63+
if let Some((name, braces, snip)) = is_offending_macro(cx, item.span, self) {
64+
let span = item.span.ctxt().outer_expn_data().call_site;
65+
emit_help(cx, snip, braces, name, span);
66+
self.done.insert(span);
67+
}
68+
}
69+
70+
fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) {
71+
if let Some((name, braces, snip)) = is_offending_macro(cx, stmt.span, self) {
72+
let span = stmt.span.ctxt().outer_expn_data().call_site;
73+
emit_help(cx, snip, braces, name, span);
74+
self.done.insert(span);
75+
}
76+
}
77+
78+
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
79+
if let Some((name, braces, snip)) = is_offending_macro(cx, expr.span, self) {
80+
let span = expr.span.ctxt().outer_expn_data().call_site;
81+
emit_help(cx, snip, braces, name, span);
82+
self.done.insert(span);
83+
}
84+
}
85+
86+
fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) {
87+
if let Some((name, braces, snip)) = is_offending_macro(cx, ty.span, self) {
88+
let span = ty.span.ctxt().outer_expn_data().call_site;
89+
emit_help(cx, snip, braces, name, span);
90+
self.done.insert(span);
91+
}
92+
}
93+
}
94+
95+
fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, this: &'a MacroBraces) -> Option<MacroInfo<'a>> {
96+
if_chain! {
97+
if in_macro(span);
98+
if let Some((name, braces)) = find_matching_macro(span, &this.macro_braces);
99+
if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site);
100+
let c = snip.replace(" ", ""); // make formatting consistent
101+
if !c.starts_with(&format!("{}!{}", name, braces.0));
102+
if !this.done.contains(&span.ctxt().outer_expn_data().call_site);
103+
then {
104+
Some((name, braces, snip))
105+
} else {
106+
None
107+
}
108+
}
109+
}
110+
111+
fn emit_help(cx: &EarlyContext<'_>, snip: String, braces: &(String, String), name: &str, span: Span) {
112+
let with_space = &format!("! {}", braces.0);
113+
let without_space = &format!("!{}", braces.0);
114+
let mut help = snip;
115+
for b in BRACES.iter().filter(|b| b.0 != braces.0) {
116+
help = help.replace(b.0, &braces.0).replace(b.1, &braces.1);
117+
// Only `{` traditionally has space before the brace
118+
if braces.0 != "{" && help.contains(with_space) {
119+
help = help.replace(with_space, without_space);
120+
} else if braces.0 == "{" && help.contains(without_space) {
121+
help = help.replace(without_space, with_space);
122+
}
123+
}
124+
span_lint_and_help(
125+
cx,
126+
NONSTANDARD_MACRO_BRACES,
127+
span,
128+
&format!("use of irregular braces for `{}!` macro", name),
129+
Some(span),
130+
&format!("consider writing `{}`", help),
131+
);
132+
}
133+
134+
fn find_matching_macro(
135+
span: Span,
136+
braces: &FxHashMap<String, (String, String)>,
137+
) -> Option<(&String, &(String, String))> {
138+
braces
139+
.iter()
140+
.find(|(macro_name, _)| is_direct_expn_of(span, macro_name).is_some())
141+
}
142+
143+
fn macro_braces(conf: FxHashSet<MacroMatcher>) -> FxHashMap<String, (String, String)> {
144+
let mut braces = vec![
145+
macro_matcher!(
146+
name: "print",
147+
braces: ("(", ")"),
148+
),
149+
macro_matcher!(
150+
name: "println",
151+
braces: ("(", ")"),
152+
),
153+
macro_matcher!(
154+
name: "eprint",
155+
braces: ("(", ")"),
156+
),
157+
macro_matcher!(
158+
name: "eprintln",
159+
braces: ("(", ")"),
160+
),
161+
macro_matcher!(
162+
name: "write",
163+
braces: ("(", ")"),
164+
),
165+
macro_matcher!(
166+
name: "writeln",
167+
braces: ("(", ")"),
168+
),
169+
macro_matcher!(
170+
name: "format",
171+
braces: ("(", ")"),
172+
),
173+
macro_matcher!(
174+
name: "format_args",
175+
braces: ("(", ")"),
176+
),
177+
macro_matcher!(
178+
name: "vec",
179+
braces: ("[", "]"),
180+
),
181+
]
182+
.into_iter()
183+
.collect::<FxHashMap<_, _>>();
184+
// We want users items to override any existing items
185+
for it in conf {
186+
braces.insert(it.name, it.braces);
187+
}
188+
braces
189+
}
190+
191+
macro_rules! macro_matcher {
192+
(name: $name:expr, braces: ($open:expr, $close:expr) $(,)?) => {
193+
($name.to_owned(), ($open.to_owned(), $close.to_owned()))
194+
};
195+
}
196+
pub(crate) use macro_matcher;
197+
198+
#[derive(Clone, Debug)]
199+
pub struct MacroMatcher {
200+
name: String,
201+
braces: (String, String),
202+
}
203+
204+
impl Hash for MacroMatcher {
205+
fn hash<H: Hasher>(&self, state: &mut H) {
206+
self.name.hash(state);
207+
}
208+
}
209+
210+
impl PartialEq for MacroMatcher {
211+
fn eq(&self, other: &Self) -> bool {
212+
self.name == other.name
213+
}
214+
}
215+
impl Eq for MacroMatcher {}
216+
217+
impl<'de> Deserialize<'de> for MacroMatcher {
218+
fn deserialize<D>(deser: D) -> Result<Self, D::Error>
219+
where
220+
D: de::Deserializer<'de>,
221+
{
222+
#[derive(Deserialize)]
223+
#[serde(field_identifier, rename_all = "lowercase")]
224+
enum Field {
225+
Name,
226+
Brace,
227+
}
228+
struct MacVisitor;
229+
impl<'de> de::Visitor<'de> for MacVisitor {
230+
type Value = MacroMatcher;
231+
232+
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
233+
formatter.write_str("struct MacroMatcher")
234+
}
235+
236+
fn visit_map<V>(self, mut map: V) -> Result<Self::Value, V::Error>
237+
where
238+
V: de::MapAccess<'de>,
239+
{
240+
let mut name = None;
241+
let mut brace: Option<&str> = None;
242+
while let Some(key) = map.next_key()? {
243+
match key {
244+
Field::Name => {
245+
if name.is_some() {
246+
return Err(de::Error::duplicate_field("name"));
247+
}
248+
name = Some(map.next_value()?);
249+
},
250+
Field::Brace => {
251+
if brace.is_some() {
252+
return Err(de::Error::duplicate_field("brace"));
253+
}
254+
brace = Some(map.next_value()?);
255+
},
256+
}
257+
}
258+
let name = name.ok_or_else(|| de::Error::missing_field("name"))?;
259+
let brace = brace.ok_or_else(|| de::Error::missing_field("brace"))?;
260+
Ok(MacroMatcher {
261+
name,
262+
braces: BRACES
263+
.iter()
264+
.find(|b| b.0 == brace)
265+
.map(|(o, c)| ((*o).to_owned(), (*c).to_owned()))
266+
.ok_or_else(|| {
267+
de::Error::custom(&format!("expected one of `(`, `{{`, `[` found `{}`", brace))
268+
})?,
269+
})
270+
}
271+
}
272+
273+
const FIELDS: &[&str] = &["name", "brace"];
274+
deser.deserialize_struct("MacroMatcher", FIELDS, MacVisitor)
275+
}
276+
}

clippy_lints/src/utils/conf.rs

+11-5
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ impl TryConf {
2626

2727
macro_rules! define_Conf {
2828
($(
29-
#[doc = $doc:literal]
29+
$(#[doc = $doc:literal])*
3030
$(#[conf_deprecated($dep:literal)])?
3131
($name:ident: $ty:ty = $default:expr),
3232
)*) => {
3333
/// Clippy lint configuration
3434
pub struct Conf {
35-
$(#[doc = $doc] pub $name: $ty,)*
35+
$($(#[doc = $doc])* pub $name: $ty,)*
3636
}
3737

3838
mod defaults {
@@ -109,7 +109,7 @@ macro_rules! define_Conf {
109109
stringify!($name),
110110
stringify!($ty),
111111
format!("{:?}", super::defaults::$name()),
112-
$doc,
112+
concat!($($doc,)*),
113113
deprecation_reason,
114114
)
115115
},
@@ -182,9 +182,9 @@ define_Conf! {
182182
(vec_box_size_threshold: u64 = 4096),
183183
/// Lint: TYPE_REPETITION_IN_BOUNDS. The maximum number of bounds a trait can have to be linted
184184
(max_trait_bounds: u64 = 3),
185-
/// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bools a struct can have
185+
/// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bool fields a struct can have
186186
(max_struct_bools: u64 = 3),
187-
/// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have
187+
/// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bool parameters a function can have
188188
(max_fn_params_bools: u64 = 3),
189189
/// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests).
190190
(warn_on_all_wildcard_imports: bool = false),
@@ -198,6 +198,12 @@ define_Conf! {
198198
(upper_case_acronyms_aggressive: bool = false),
199199
/// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest.
200200
(cargo_ignore_publish: bool = false),
201+
/// Lint: NONSTANDARD_MACRO_BRACES. Enforce the named macros always use the braces specified.
202+
///
203+
/// A `MacroMatcher` can be added like so `{ name = "macro_name", brace = "(" }`.
204+
/// If the macro is could be used with a full path two `MacroMatcher`s have to be added one
205+
/// with the full path `crate_name::macro_name` and one with just the macro name.
206+
(standard_macro_braces: Vec<crate::nonstandard_macro_braces::MacroMatcher> = Vec::new()),
201207
}
202208

203209
/// Search for the configuration file.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
standard-macro-braces = [
2+
{ name = "quote", brace = "{" },
3+
{ name = "quote::quote", brace = "{" },
4+
{ name = "eprint", brace = "[" },
5+
{ name = "type_pos", brace = "[" },
6+
]

0 commit comments

Comments
 (0)