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.

Sign up for GitHub

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

Jump to bottom

Don't lint suspicious_else_formatting inside proc-macros #7707

Merged
merged 2 commits into from Sep 23, 2021
Merged

Don't lint suspicious_else_formatting inside proc-macros #7707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5efd6bc
Don't lint `suspicious_else_formatting` inside proc-macros
Jarcho Sep 23, 2021
e69154f
Add test for #7650
Jarcho Sep 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 31 additions & 26 deletions clippy_lints/src/formatting.rs
View file
Open in desktop
Expand Up @@ -286,34 +286,39 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {
}

fn check_missing_else(cx: &EarlyContext<'_>, first: &Expr, second: &Expr) {
if !differing_macro_contexts(first.span, second.span)
&& !first.span.from_expansion()
&& is_if(first)
&& (is_block(second) || is_if(second))
{
// where the else would be
let else_span = first.span.between(second.span);
if_chain! {
if !differing_macro_contexts(first.span, second.span);
if !first.span.from_expansion();
if let ExprKind::If(cond_expr, ..) = &first.kind;
if is_block(second) || is_if(second);

if let Some(else_snippet) = snippet_opt(cx, else_span) {
if !else_snippet.contains('\n') {
let (looks_like, next_thing) = if is_if(second) {
("an `else if`", "the second `if`")
} else {
("an `else {..}`", "the next block")
};
// Proc-macros can give weird spans. Make sure this is actually an `if`.
if let Some(if_snip) = snippet_opt(cx, first.span.until(cond_expr.span));
if if_snip.starts_with("if");

span_lint_and_note(
cx,
SUSPICIOUS_ELSE_FORMATTING,
else_span,
&format!("this looks like {} but the `else` is missing", looks_like),
None,
&format!(
"to remove this lint, add the missing `else` or add a new line before {}",
next_thing,
),
);
}
// If there is a line break between the two expressions, don't lint.
// If there is a non-whitespace character, this span came from a proc-macro.
let else_span = first.span.between(second.span);
if let Some(else_snippet) = snippet_opt(cx, else_span);
if !else_snippet.chars().any(|c| c == '\n' || !c.is_whitespace());
then {
let (looks_like, next_thing) = if is_if(second) {
("an `else if`", "the second `if`")
} else {
("an `else {..}`", "the next block")
};

span_lint_and_note(
cx,
SUSPICIOUS_ELSE_FORMATTING,
else_span,
&format!("this looks like {} but the `else` is missing", looks_like),
None,
&format!(
"to remove this lint, add the missing `else` or add a new line before {}",
next_thing,
),
);
}
}
}
Expand Down
75 changes: 75 additions & 0 deletions tests/ui/auxiliary/proc_macro_suspicious_else_formatting.rs
View file
Open in desktop
@@ -0,0 +1,75 @@
// compile-flags: --emit=link
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;
use proc_macro::{token_stream, Delimiter, Group, Ident, Span, TokenStream, TokenTree};
use std::iter::FromIterator;

fn read_ident(iter: &mut token_stream::IntoIter) -> Ident {
match iter.next() {
Some(TokenTree::Ident(i)) => i,
_ => panic!("expected ident"),
}
}

#[proc_macro_derive(DeriveBadSpan)]
pub fn derive_bad_span(input: TokenStream) -> TokenStream {
let mut input = input.into_iter();
assert_eq!(read_ident(&mut input).to_string(), "struct");
let ident = read_ident(&mut input);
let mut tys = match input.next() {
Some(TokenTree::Group(g)) if g.delimiter() == Delimiter::Parenthesis => g.stream().into_iter(),
_ => panic!(),
};
let field1 = read_ident(&mut tys);
tys.next();
let field2 = read_ident(&mut tys);

<TokenStream as FromIterator<TokenTree>>::from_iter(
[
Ident::new("impl", Span::call_site()).into(),
ident.into(),
Group::new(
Delimiter::Brace,
<TokenStream as FromIterator<TokenTree>>::from_iter(
[
Ident::new("fn", Span::call_site()).into(),
Ident::new("_foo", Span::call_site()).into(),
Group::new(Delimiter::Parenthesis, TokenStream::new()).into(),
Group::new(
Delimiter::Brace,
<TokenStream as FromIterator<TokenTree>>::from_iter(
[
Ident::new("if", field1.span()).into(),
Ident::new("true", field1.span()).into(),
{
let mut group = Group::new(Delimiter::Brace, TokenStream::new());
group.set_span(field1.span());
group.into()
},
Ident::new("if", field2.span()).into(),
Ident::new("true", field2.span()).into(),
{
let mut group = Group::new(Delimiter::Brace, TokenStream::new());
group.set_span(field2.span());
group.into()
},
]
.iter()
.cloned(),
),
)
.into(),
]
.iter()
.cloned(),
),
)
.into(),
]
.iter()
.cloned(),
)
}
9 changes: 9 additions & 0 deletions tests/ui/suspicious_else_formatting.rs
View file
Open in desktop
@@ -1,5 +1,10 @@
// aux-build:proc_macro_suspicious_else_formatting.rs

#![warn(clippy::suspicious_else_formatting)]

extern crate proc_macro_suspicious_else_formatting;
use proc_macro_suspicious_else_formatting::DeriveBadSpan;

fn foo() -> bool {
true
}
Expand Down Expand Up @@ -103,3 +108,7 @@ fn main() {
{
}
}

// #7650 - Don't lint. Proc-macro using bad spans for `if` expressions.
#[derive(DeriveBadSpan)]
struct _Foo(u32, u32);
18 changes: 9 additions & 9 deletions tests/ui/suspicious_else_formatting.stderr
View file
Open in desktop
@@ -1,5 +1,5 @@
error: this looks like an `else {..}` but the `else` is missing
--> $DIR/suspicious_else_formatting.rs:11:6
--> $DIR/suspicious_else_formatting.rs:16:6
|
LL | } {
| ^
Expand All @@ -8,31 +8,31 @@ LL | } {
= note: to remove this lint, add the missing `else` or add a new line before the next block

error: this looks like an `else if` but the `else` is missing
--> $DIR/suspicious_else_formatting.rs:15:6
--> $DIR/suspicious_else_formatting.rs:20:6
|
LL | } if foo() {
| ^
|
= note: to remove this lint, add the missing `else` or add a new line before the second `if`

error: this looks like an `else if` but the `else` is missing
--> $DIR/suspicious_else_formatting.rs:22:10
--> $DIR/suspicious_else_formatting.rs:27:10
|
LL | } if foo() {
| ^
|
= note: to remove this lint, add the missing `else` or add a new line before the second `if`

error: this looks like an `else if` but the `else` is missing
--> $DIR/suspicious_else_formatting.rs:30:10
--> $DIR/suspicious_else_formatting.rs:35:10
|
LL | } if foo() {
| ^
|
= note: to remove this lint, add the missing `else` or add a new line before the second `if`

error: this is an `else {..}` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:39:6
--> $DIR/suspicious_else_formatting.rs:44:6
|
LL | } else
| ______^
Expand All @@ -42,7 +42,7 @@ LL | | {
= note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}`

error: this is an `else if` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:51:6
--> $DIR/suspicious_else_formatting.rs:56:6
|
LL | } else
| ______^
Expand All @@ -52,7 +52,7 @@ LL | | if foo() { // the span of the above error should continue here
= note: to remove this lint, remove the `else` or remove the new line between `else` and `if`

error: this is an `else if` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:56:6
--> $DIR/suspicious_else_formatting.rs:61:6
|
LL | }
| ______^
Expand All @@ -63,7 +63,7 @@ LL | | if foo() { // the span of the above error should continue here
= note: to remove this lint, remove the `else` or remove the new line between `else` and `if`

error: this is an `else {..}` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:83:6
--> $DIR/suspicious_else_formatting.rs:88:6
|
LL | }
| ______^
Expand All @@ -75,7 +75,7 @@ LL | | {
= note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}`

error: this is an `else {..}` but the formatting might hide it
--> $DIR/suspicious_else_formatting.rs:91:6
--> $DIR/suspicious_else_formatting.rs:96:6
|
LL | }
| ______^
Expand Down