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.

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

Fix single_match suggestions with expanded macros #3272

Open
wants to merge 1 commit into
base: master
from
Jump to file or symbol
Failed to load files and symbols.
+57 −24
Diff settings

Always

Just for now

@@ -13,12 +13,13 @@ use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass, in_exte
use crate::rustc::{declare_tool_lint, lint_array};
use if_chain::if_chain;
use crate::rustc::ty::{self, Ty};
use std::borrow::Cow;
use std::cmp::Ordering;
use std::collections::Bound;
use crate::syntax::ast::LitKind;
use crate::syntax::source_map::Span;
use crate::utils::paths;
use crate::utils::{expr_block, is_allowed, is_expn_of, match_qpath, match_type, multispan_sugg,
use crate::utils::{expr_block, in_macro, is_allowed, is_expn_of, match_qpath, match_type, multispan_sugg,
remove_blocks, snippet, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty};
use crate::utils::sugg::Sugg;
use crate::consts::{constant, Constant};
@@ -254,7 +255,18 @@ fn report_single_match_single_pattern(cx: &LateContext<'_, '_>, ex: &Expr, arms:
} else {
SINGLE_MATCH
};
let els_str = els.map_or(String::new(), |els| format!(" else {}", expr_block(cx, els, None, "..")));
let els_str = els.map_or(String::new(), |els| {
if in_macro(els.span) {
" else { .. }".to_string()

This comment has been minimized.

@flip1995

flip1995 Oct 15, 2018

Collaborator

Could we maybe get a better suggestion with source_callsite()

Everything else LGTM.

@flip1995

flip1995 Oct 15, 2018

Collaborator

Could we maybe get a better suggestion with source_callsite()

Everything else LGTM.

This comment has been minimized.

@phansch

phansch Oct 16, 2018

Collaborator

I will give it a try, thanks!

@phansch

phansch Oct 16, 2018

Collaborator

I will give it a try, thanks!

} else {
format!(" else {}", expr_block(cx, els, None, ".."))
}
});
let expr_block = if in_macro(arms[0].body.span) {
Cow::Owned("{ .. }".to_string())
} else {
expr_block(cx, &arms[0].body, None, "..")
};
span_lint_and_sugg(
cx,
lint,
@@ -266,7 +278,7 @@ fn report_single_match_single_pattern(cx: &LateContext<'_, '_>, ex: &Expr, arms:
"if let {} = {} {}{}",
snippet(cx, arms[0].pats[0].span, ".."),
snippet(cx, ex.span, ".."),
expr_block(cx, &arms[0].body, None, ".."),
expr_block,
els_str
),
);
View
@@ -33,7 +33,7 @@ error: you seem to be trying to use match for destructuring a single pattern. Co
51 | | &(v, 1) => println!("{}", v),
52 | | _ => println!("none"),
53 | | }
| |_____^ help: try this: `if let &(v, 1) = tup { $ crate :: io :: _print ( format_args_nl ! ( $ ( $ arg ) * ) ) ; } else { $ crate :: io :: _print ( format_args_nl ! ( $ ( $ arg ) * ) ) ; }`
| |_____^ help: try this: `if let &(v, 1) = tup { .. } else { .. }`
error: you don't need to add `&` to all patterns
--> $DIR/matches.rs:50:5
View
@@ -23,6 +23,15 @@ fn single_match(){
_ => ()
};
let x = Some(1u8);
match x {
// Note the missing block braces.
// We suggest `if let Some(y) = x { .. }` because the macro
// is expanded before we can do anything.
Some(y) => println!("{:?}", y),
_ => ()
}
let z = (1u8,1u8);
match z {
(2...3, 7...9) => dummy(),
@@ -12,38 +12,50 @@ error: you seem to be trying to use match for destructuring a single pattern. Co
error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:27:5
|
27 | / match z {
28 | | (2...3, 7...9) => dummy(),
29 | | _ => {}
30 | | };
27 | / match x {
28 | | // Note the missing block braces.
29 | | // We suggest `if let Some(y) = x { .. }` because the macro
30 | | // is expanded before we can do anything.
31 | | Some(y) => println!("{:?}", y),
32 | | _ => ()
33 | | }
| |_____^ help: try this: `if let Some(y) = x { .. }`
error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:36:5
|
36 | / match z {
37 | | (2...3, 7...9) => dummy(),
38 | | _ => {}
39 | | };
| |_____^ help: try this: `if let (2...3, 7...9) = z { dummy() }`
error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:53:5
--> $DIR/single_match.rs:62:5
|
53 | / match x {
54 | | Some(y) => dummy(),
55 | | None => ()
56 | | };
62 | / match x {
63 | | Some(y) => dummy(),
64 | | None => ()
65 | | };
| |_____^ help: try this: `if let Some(y) = x { dummy() }`
error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:58:5
--> $DIR/single_match.rs:67:5
|
58 | / match y {
59 | | Ok(y) => dummy(),
60 | | Err(..) => ()
61 | | };
67 | / match y {
68 | | Ok(y) => dummy(),
69 | | Err(..) => ()
70 | | };
| |_____^ help: try this: `if let Ok(y) = y { dummy() }`
error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:65:5
--> $DIR/single_match.rs:74:5
|
65 | / match c {
66 | | Cow::Borrowed(..) => dummy(),
67 | | Cow::Owned(..) => (),
68 | | };
74 | / match c {
75 | | Cow::Borrowed(..) => dummy(),
76 | | Cow::Owned(..) => (),
77 | | };
| |_____^ help: try this: `if let Cow::Borrowed(..) = c { dummy() }`
error: aborting due to 5 previous errors
error: aborting due to 6 previous errors
ProTip! Use n and p to navigate between commits in a pull request.