Skip to content

Commit c2d9d46

Browse files
authored
Merge pull request #1592 from Manishearth/node_id_to_type
Bugfixes
2 parents 268b15f + ac48e09 commit c2d9d46

File tree

5 files changed

+61
-76
lines changed

5 files changed

+61
-76
lines changed

clippy_lints/src/consts.rs

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,6 @@ pub enum Constant {
5353
Tuple(Vec<Constant>),
5454
}
5555

56-
impl Constant {
57-
/// Convert to `u64` if possible.
58-
///
59-
/// # panics
60-
///
61-
/// If the constant could not be converted to `u64` losslessly.
62-
fn as_u64(&self) -> u64 {
63-
if let Constant::Int(val) = *self {
64-
val.to_u64().expect("negative constant can't be casted to `u64`")
65-
} else {
66-
panic!("Could not convert a `{:?}` to `u64`", self);
67-
}
68-
}
69-
}
70-
7156
impl PartialEq for Constant {
7257
fn eq(&self, other: &Constant) -> bool {
7358
match (self, other) {
@@ -266,11 +251,12 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
266251
ExprLit(ref lit) => Some(lit_to_constant(&lit.node, self.tcx, self.tables.expr_ty(e))),
267252
ExprArray(ref vec) => self.multi(vec).map(Constant::Vec),
268253
ExprTup(ref tup) => self.multi(tup).map(Constant::Tuple),
269-
ExprRepeat(ref value, number_id) => {
270-
let val = &self.tcx.hir.body(number_id).value;
271-
self.binop_apply(value,
272-
val,
273-
|v, n| Some(Constant::Repeat(Box::new(v), n.as_u64() as usize)))
254+
ExprRepeat(ref value, _) => {
255+
let n = match self.tables.expr_ty(e).sty {
256+
ty::TyArray(_, n) => n,
257+
_ => span_bug!(e.span, "typeck error"),
258+
};
259+
self.expr(value).map(|v| Constant::Repeat(Box::new(v), n))
274260
},
275261
ExprUnary(op, ref operand) => {
276262
self.expr(operand).and_then(|o| match op {
@@ -375,15 +361,4 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
375361
_ => None,
376362
}
377363
}
378-
379-
380-
fn binop_apply<F>(&mut self, left: &Expr, right: &Expr, op: F) -> Option<Constant>
381-
where F: Fn(Constant, Constant) -> Option<Constant>
382-
{
383-
if let (Some(lc), Some(rc)) = (self.expr(left), self.expr(right)) {
384-
op(lc, rc)
385-
} else {
386-
None
387-
}
388-
}
389364
}

clippy_lints/src/matches.rs

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use syntax::ast::LitKind;
1010
use syntax::codemap::Span;
1111
use utils::paths;
1212
use utils::{match_type, snippet, span_note_and_lint, span_lint_and_then, in_external_macro, expr_block, walk_ptrs_ty,
13-
is_expn_of};
13+
is_expn_of, remove_blocks};
1414
use utils::sugg::Sugg;
1515

1616
/// **What it does:** Checks for matches with a single arm where an `if let`
@@ -179,11 +179,12 @@ fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
179179
if arms.len() == 2 &&
180180
arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
181181
arms[1].pats.len() == 1 && arms[1].guard.is_none() {
182-
let els = if is_unit_expr(&arms[1].body) {
182+
let els = remove_blocks(&arms[1].body);
183+
let els = if is_unit_expr(els) {
183184
None
184-
} else if let ExprBlock(_) = arms[1].body.node {
185+
} else if let ExprBlock(_) = els.node {
185186
// matches with blocks that contain statements are prettier as `if let + else`
186-
Some(&*arms[1].body)
187+
Some(els)
187188
} else {
188189
// allow match arms with just expressions
189190
return;
@@ -198,29 +199,33 @@ fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
198199

199200
fn check_single_match_single_pattern(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr, els: Option<&Expr>) {
200201
if arms[1].pats[0].node == PatKind::Wild {
201-
let lint = if els.is_some() {
202-
SINGLE_MATCH_ELSE
203-
} else {
204-
SINGLE_MATCH
205-
};
206-
let els_str = els.map_or(String::new(), |els| format!(" else {}", expr_block(cx, els, None, "..")));
207-
span_lint_and_then(cx,
208-
lint,
209-
expr.span,
210-
"you seem to be trying to use match for destructuring a single pattern. \
211-
Consider using `if let`",
212-
|db| {
213-
db.span_suggestion(expr.span,
214-
"try this",
215-
format!("if let {} = {} {}{}",
216-
snippet(cx, arms[0].pats[0].span, ".."),
217-
snippet(cx, ex.span, ".."),
218-
expr_block(cx, &arms[0].body, None, ".."),
219-
els_str));
220-
});
202+
report_single_match_single_pattern(cx, ex, arms, expr, els);
221203
}
222204
}
223205

206+
fn report_single_match_single_pattern(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr, els: Option<&Expr>) {
207+
let lint = if els.is_some() {
208+
SINGLE_MATCH_ELSE
209+
} else {
210+
SINGLE_MATCH
211+
};
212+
let els_str = els.map_or(String::new(), |els| format!(" else {}", expr_block(cx, els, None, "..")));
213+
span_lint_and_then(cx,
214+
lint,
215+
expr.span,
216+
"you seem to be trying to use match for destructuring a single pattern. \
217+
Consider using `if let`",
218+
|db| {
219+
db.span_suggestion(expr.span,
220+
"try this",
221+
format!("if let {} = {} {}{}",
222+
snippet(cx, arms[0].pats[0].span, ".."),
223+
snippet(cx, ex.span, ".."),
224+
expr_block(cx, &arms[0].body, None, ".."),
225+
els_str));
226+
});
227+
}
228+
224229
fn check_single_match_opt_like(
225230
cx: &LateContext,
226231
ex: &Expr,
@@ -253,26 +258,7 @@ fn check_single_match_opt_like(
253258

254259
for &(ty_path, pat_path) in candidates {
255260
if &path == pat_path && match_type(cx, ty, ty_path) {
256-
let lint = if els.is_some() {
257-
SINGLE_MATCH_ELSE
258-
} else {
259-
SINGLE_MATCH
260-
};
261-
let els_str = els.map_or(String::new(), |els| format!(" else {}", expr_block(cx, els, None, "..")));
262-
span_lint_and_then(cx,
263-
lint,
264-
expr.span,
265-
"you seem to be trying to use match for destructuring a single pattern. Consider \
266-
using `if let`",
267-
|db| {
268-
db.span_suggestion(expr.span,
269-
"try this",
270-
format!("if let {} = {} {}{}",
271-
snippet(cx, arms[0].pats[0].span, ".."),
272-
snippet(cx, ex.span, ".."),
273-
expr_block(cx, &arms[0].body, None, ".."),
274-
els_str));
275-
});
261+
report_single_match_single_pattern(cx, ex, arms, expr, els);
276262
}
277263
}
278264
}

clippy_lints/src/utils/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option<Name> {
385385
/// snippet(cx, expr.span, "..")
386386
/// ```
387387
pub fn snippet<'a, 'b, T: LintContext<'b>>(cx: &T, span: Span, default: &'a str) -> Cow<'a, str> {
388-
cx.sess().codemap().span_to_snippet(span).map(From::from).unwrap_or_else(|_| Cow::Borrowed(default))
388+
snippet_opt(cx, span).map_or_else(|| Cow::Borrowed(default), From::from)
389389
}
390390

391391
/// Convert a span to a code snippet. Returns `None` if not available.

tests/run-pass/ice-1588.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#![feature(plugin)]
2+
#![plugin(clippy)]
3+
#![allow(clippy)]
4+
5+
fn main() {
6+
match 1 {
7+
1 => {}
8+
2 => {
9+
[0; 1];
10+
}
11+
_ => {}
12+
}
13+
}

tests/run-pass/single-match-else.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#![feature(plugin)]
2+
#![plugin(clippy)]
3+
#![warn(single_match_else)]
4+
5+
fn main() {
6+
let n = match (42, 43) {
7+
(42, n) => n,
8+
_ => panic!("typeck error"),
9+
};
10+
assert_eq!(n, 43);
11+
}

0 commit comments

Comments
 (0)