squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Fix suggestions for ref matches

Browse source
This commit is contained in:
Karim Snj 2018-02-04 13:41:54 +01:00
parent 8e8cf2feb1
commit 9575dac491
2 changed files with 47 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

69
clippy_lints/src/matches.rs
View file

@ -11,8 +11,8 @@ use syntax::ast::LitKind;
use syntax::ast::NodeId;
use syntax::codemap::Span;
use utils::paths;
use utils::{expr_block, in_external_macro, is_allowed, is_expn_of, match_qpath, match_type, remove_blocks,
snippet, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty};
use utils::{expr_block, in_external_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 utils::sugg::Sugg;
/// **What it does:** Checks for matches with a single arm where an `if let`
@ -195,8 +195,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchPass {
check_wild_err_arm(cx, ex, arms);
check_match_as_ref(cx, ex, arms, expr);
}
if let ExprMatch(ref ex, ref arms, source) = expr.node {
check_match_ref_pats(cx, ex, arms, source, expr);
if let ExprMatch(ref ex, ref arms, _) = expr.node {
check_match_ref_pats(cx, ex, arms, expr);
}
}
}
@ -400,37 +400,34 @@ fn is_panic_block(block: &Block) -> bool {
}
}
fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: MatchSource, expr: &Expr) {
fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
if has_only_ref_pats(arms) {
if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
span_lint_and_then(
cx,
MATCH_REF_PATS,
expr.span,
let mut suggs = Vec::new();
let (title, msg) = if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
suggs.push((ex.span, Sugg::hir(cx, inner, "..").to_string()));
(
"you don't need to add `&` to both the expression and the patterns",
|db| {
let inner = Sugg::hir(cx, inner, "..");
let template = match_template(expr.span, source, &inner);
db.span_suggestion(expr.span, "try", template);
},
);
"try",
)
} else {
span_lint_and_then(
cx,
MATCH_REF_PATS,
expr.span,
suggs.push((ex.span, Sugg::hir(cx, ex, "..").deref().to_string()));
(
"you don't need to add `&` to all patterns",
|db| {
let ex = Sugg::hir(cx, ex, "..");
let template = match_template(expr.span, source, &ex.deref());
db.span_suggestion(
expr.span,
"instead of prefixing all patterns with `&`, you can dereference the expression",
template,
);
},
);
}
"instead of prefixing all patterns with `&`, you can dereference the expression",
)
};
suggs.extend(arms.iter().flat_map(|a| &a.pats).filter_map(|p| {
if let PatKind::Ref(ref refp, _) = p.node {
Some((p.span, snippet(cx, refp.span, "..").to_string()))
} else {
None
}
}));
span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |db| {
multispan_sugg(db, msg.to_owned(), suggs);
});
}
}
@ -615,16 +612,6 @@ fn has_only_ref_pats(arms: &[Arm]) -> bool {
mapped.map_or(false, |v| v.iter().any(|el| *el))
}
fn match_template(span: Span, source: MatchSource, expr: &Sugg) -> String {
match source {
MatchSource::Normal => format!("match {} {{ .. }}", expr),
MatchSource::IfLetDesugar { .. } => format!("if let .. = {} {{ .. }}", expr),
MatchSource::WhileLetDesugar => format!("while let .. = {} {{ .. }}", expr),
MatchSource::ForLoopDesugar => span_bug!(span, "for loop desugared to match with &-patterns!"),
MatchSource::TryDesugar => span_bug!(span, "`?` operator desugared to match with &-patterns!"),
}
}
pub fn overlapping<T>(ranges: &[SpannedRange<T>]) -> Option<(&SpannedRange<T>, &SpannedRange<T>)>
where
T: Copy + Ord,

24
tests/ui/matches.stderr
View file

@ -132,7 +132,9 @@ error: you don't need to add `&` to all patterns
= note: `-D match-ref-pats` implied by `-D warnings`
help: instead of prefixing all patterns with `&`, you can dereference the expression
|
138 | match *v { .. }
138 | match *v {
139 | Some(v) => println!("{:?}", v),
140 | None => println!("none"),
|
error: you don't need to add `&` to all patterns
@ -145,7 +147,8 @@ error: you don't need to add `&` to all patterns
| |_____^
help: instead of prefixing all patterns with `&`, you can dereference the expression
|
148 | match *tup { .. }
148 | match *tup {
149 | (v, 1) => println!("{}", v),
|
error: you don't need to add `&` to both the expression and the patterns
@ -155,7 +158,13 @@ error: you don't need to add `&` to both the expression and the patterns
155 | | &Some(v) => println!("{:?}", v),
156 | | &None => println!("none"),
157 | | }
| |_____^ help: try: `match w { .. }`
| |_____^
help: try
|
154 | match w {
155 | Some(v) => println!("{:?}", v),
156 | None => println!("none"),
|
error: you don't need to add `&` to all patterns
--> $DIR/matches.rs:165:5
@ -166,7 +175,7 @@ error: you don't need to add `&` to all patterns
| |_____^
help: instead of prefixing all patterns with `&`, you can dereference the expression
|
165 | if let .. = *a { .. }
165 | if let None = *a {
|
error: you don't need to add `&` to both the expression and the patterns
@ -175,7 +184,11 @@ error: you don't need to add `&` to both the expression and the patterns
170 | / if let &None = &b {
171 | | println!("none");
172 | | }
| |_____^ help: try: `if let .. = b { .. }`
| |_____^
help: try
|
170 | if let None = b {
|
error: some ranges overlap
--> $DIR/matches.rs:179:9
@ -450,3 +463,4 @@ error: use as_mut() instead
error: aborting due to 37 previous errors