Fix nonstandard_macro_braces FP and docs of disallowed_types #7478
Conversation
|
r? @llogiq (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I could make this 2 PRs I just wasn't sure if it would be worth it for how small the change is? |
| @@ -44,7 +44,7 @@ fn main() { | |||
| let _ = format!["ugh {} stop being such a good compiler", "hello"]; | |||
| let _ = quote!(let x = 1;); | |||
| let _ = quote::quote!(match match match); | |||
| let _ = test!(); | |||
| let _ = test!(); // don't trigger for macro calls inside macros | |||
There was a problem hiding this comment.
You know if the macro is defined within the same crate..... I would argue it makes sense to still emit the lint, but maybe it's not worth complicating the logic further.
There was a problem hiding this comment.
I think I got something that works, that was a bit of a rabbit hole, Span is a complicated thing...
|
With #7488 I'm going to remove the commit to fix the docs of |
| @@ -93,13 +93,21 @@ impl EarlyLintPass for MacroBraces { | |||
| } | |||
|
|
|||
| fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a MacroBraces) -> Option<MacroInfo<'a>> { | |||
| let unnested_or_local = || { | |||
| let nested = in_macro(span.ctxt().outer_expn_data().call_site); | |||
| let in_local_macro = nested | |||
There was a problem hiding this comment.
Having nested && here and returning !nested || .. might befuddle at cursory reading. I'd suggest using a guard if instead.
There was a problem hiding this comment.
Is this what you mean?
let unnested_or_local = || {
let nested = in_macro(span.ctxt().outer_expn_data().call_site);
let in_local_macro = matches!(
span.macro_backtrace().last().and_then(|e| e.macro_def_id),
Some(defid) if nested && defid.is_local() // adding `nested` to the if guard?
);
!nested || in_local_macro
};There was a problem hiding this comment.
I was more thinking of
let unnested_or_local = || {
let nested = in_macro(span.ctxt().outer_expn_data().call_site);
!nested || span.macro_backtrace().last().map_or(false, |e|
e.macro_def_id.map_or(false, |defid| defid.is_local()))
)
};There was a problem hiding this comment.
Oh right duh 🤦 I was asking the same question two times in a row (!nested then nested && ..)
|
Should I squash, or is there anything else, thanks again! |
|
No, that should be fine. @bors r+ |
|
📌 Commit bc7fac9 has been approved by |
Fix nonstandard_macro_braces FP and docs of disallowed_types changelog: Fix FP in [`nonstandard_macro_braces`] lint
|
💔 Test failed - checks-action_test |
|
@bors retry |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: Fix FP in [
nonstandard_macro_braces] lint