Skip to content

Commit 475656a

Browse files
committed
1 parent b6dc257 commit 475656a

File tree

13 files changed

+173
-34
lines changed

13 files changed

+173
-34
lines changed

clippy_lints/src/lifetimes.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ fn check_fn_inner<'a, 'tcx>(
113113
let mut bounds_lts = Vec::new();
114114
let types = generics.params.iter().filter(|param| match param.kind {
115115
GenericParamKind::Type { .. } => true,
116-
GenericParamKind::Lifetime { .. } => false,
116+
_ => false,
117117
});
118118
for typ in types {
119119
for bound in &typ.bounds {
@@ -133,7 +133,7 @@ fn check_fn_inner<'a, 'tcx>(
133133
if let Some(ref params) = *params {
134134
let lifetimes = params.args.iter().filter_map(|arg| match arg {
135135
GenericArg::Lifetime(lt) => Some(lt),
136-
GenericArg::Type(_) => None,
136+
_ => None,
137137
});
138138
for bound in lifetimes {
139139
if bound.name != LifetimeName::Static && !bound.is_elided() {
@@ -316,7 +316,7 @@ impl<'v, 't> RefVisitor<'v, 't> {
316316
if !last_path_segment.parenthesized
317317
&& !last_path_segment.args.iter().any(|arg| match arg {
318318
GenericArg::Lifetime(_) => true,
319-
GenericArg::Type(_) => false,
319+
_ => false,
320320
})
321321
{
322322
let hir_id = self.cx.tcx.hir().node_to_hir_id(ty.id);

clippy_lints/src/matches.rs

Lines changed: 85 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ use crate::utils::{
66
snippet_with_applicability, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty,
77
};
88
use if_chain::if_chain;
9+
use rustc::hir::def::CtorKind;
910
use rustc::hir::*;
1011
use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
11-
use rustc::ty::{self, Ty};
12+
use rustc::ty::{self, Ty, TyKind};
1213
use rustc::{declare_tool_lint, lint_array};
1314
use rustc_errors::Applicability;
1415
use std::cmp::Ordering;
1516
use std::collections::Bound;
17+
use std::ops::Deref;
1618
use syntax::ast::LitKind;
1719
use syntax::source_map::Span;
1820

@@ -191,7 +193,8 @@ declare_clippy_lint! {
191193
///
192194
/// **Why is this bad?** New enum variants added by library updates can be missed.
193195
///
194-
/// **Known problems:** Nested wildcards a la `Foo(_)` are currently not detected.
196+
/// **Known problems:** Suggested replacements may be incorrect if guards exhaustively cover some
197+
/// variants, and also may not use correct path to enum if it's not present in the current scope.
195198
///
196199
/// **Example:**
197200
/// ```rust
@@ -464,19 +467,89 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
464467
}
465468

466469
fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
467-
if cx.tables.expr_ty(ex).is_enum() {
470+
let ty = cx.tables.expr_ty(ex);
471+
if !ty.is_enum() {
472+
// If there isn't a nice closed set of possible values that can be conveniently enumerated,
473+
// don't complain about not enumerating the mall.
474+
return;
475+
}
476+
477+
// First pass - check for violation, but don't do much book-keeping because this is hopefully
478+
// the uncommon case, and the book-keeping is slightly expensive.
479+
let mut wildcard_span = None;
480+
let mut wildcard_ident = None;
481+
for arm in arms {
482+
for pat in &arm.pats {
483+
if let PatKind::Wild = pat.node {
484+
wildcard_span = Some(pat.span);
485+
} else if let PatKind::Binding(_, _, _, ident, None) = pat.node {
486+
wildcard_span = Some(pat.span);
487+
wildcard_ident = Some(ident);
488+
}
489+
}
490+
}
491+
492+
if let Some(wildcard_span) = wildcard_span {
493+
// Accumulate the variants which should be put in place of the wildcard because they're not
494+
// already covered.
495+
496+
let mut missing_variants = vec![];
497+
if let TyKind::Adt(def, _) = ty.sty {
498+
for variant in &def.variants {
499+
missing_variants.push(variant);
500+
}
501+
}
502+
468503
for arm in arms {
469-
if is_wild(&arm.pats[0]) {
470-
span_note_and_lint(
471-
cx,
472-
WILDCARD_ENUM_MATCH_ARM,
473-
arm.pats[0].span,
474-
"wildcard match will miss any future added variants.",
475-
arm.pats[0].span,
476-
"to resolve, match each variant explicitly",
477-
);
504+
if arm.guard.is_some() {
505+
// Guards mean that this case probably isn't exhaustively covered. Technically
506+
// this is incorrect, as we should really check whether each variant is exhaustively
507+
// covered by the set of guards that cover it, but that's really hard to do.
508+
continue;
478509
}
510+
for pat in &arm.pats {
511+
if let PatKind::Path(ref path) = pat.deref().node {
512+
if let QPath::Resolved(_, p) = path {
513+
missing_variants.retain(|e| e.did != p.def.def_id());
514+
}
515+
} else if let PatKind::TupleStruct(ref path, ..) = pat.deref().node {
516+
if let QPath::Resolved(_, p) = path {
517+
missing_variants.retain(|e| e.did != p.def.def_id());
518+
}
519+
}
520+
}
521+
}
522+
523+
let suggestion: Vec<String> = missing_variants
524+
.iter()
525+
.map(|v| {
526+
let suffix = match v.ctor_kind {
527+
CtorKind::Fn => "(..)",
528+
CtorKind::Const | CtorKind::Fictive => "",
529+
};
530+
let ident_str = if let Some(ident) = wildcard_ident {
531+
format!("{} @ ", ident.name)
532+
} else {
533+
String::new()
534+
};
535+
// This path assumes that the enum type is imported into scope.
536+
format!("{}{}{}", ident_str, cx.tcx.item_path_str(v.did), suffix)
537+
})
538+
.collect();
539+
540+
if suggestion.is_empty() {
541+
return;
479542
}
543+
544+
span_lint_and_sugg(
545+
cx,
546+
WILDCARD_ENUM_MATCH_ARM,
547+
wildcard_span,
548+
"wildcard match will miss any future added variants.",
549+
"try this",
550+
suggestion.join(" | "),
551+
Applicability::MachineApplicable,
552+
)
480553
}
481554
}
482555

clippy_lints/src/misc.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ declare_clippy_lint! {
215215
///
216216
/// **Example:**
217217
/// ```rust
218-
/// const ONE == 1.00f64
218+
/// const ONE = 1.00f64;
219219
/// x == ONE // where both are floats
220220
/// ```
221221
declare_clippy_lint! {

clippy_lints/src/needless_pass_by_value.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
234234
.and_then(|ps| ps.args.as_ref())
235235
.map(|params| params.args.iter().find_map(|arg| match arg {
236236
GenericArg::Type(ty) => Some(ty),
237-
GenericArg::Lifetime(_) => None,
237+
_ => None,
238238
}).unwrap());
239239
then {
240240
let slice_ty = format!("&[{}]", snippet(cx, elem_ty.span, "_"));

clippy_lints/src/ptr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ fn check_fn(cx: &LateContext<'_, '_>, decl: &FnDecl, fn_id: NodeId, opt_body_id:
235235
if !params.parenthesized;
236236
if let Some(inner) = params.args.iter().find_map(|arg| match arg {
237237
GenericArg::Type(ty) => Some(ty),
238-
GenericArg::Lifetime(_) => None,
238+
_ => None,
239239
});
240240
then {
241241
let replacement = snippet_opt(cx, inner.span);

clippy_lints/src/transmute.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,8 @@ declare_clippy_lint! {
159159

160160
/// **What it does:** Checks for transmutes from an integer to a float.
161161
///
162-
/// **Why is this bad?** This might result in an invalid in-memory representation of a float.
162+
/// **Why is this bad?** Transmutes are dangerous and error-prone, whereas `from_bits` is intuitive
163+
/// and safe.
163164
///
164165
/// **Known problems:** None.
165166
///
@@ -497,7 +498,7 @@ fn get_type_snippet(cx: &LateContext<'_, '_>, path: &QPath, to_ref_ty: Ty<'_>) -
497498
if !params.parenthesized;
498499
if let Some(to_ty) = params.args.iter().filter_map(|arg| match arg {
499500
GenericArg::Type(ty) => Some(ty),
500-
GenericArg::Lifetime(_) => None,
501+
_ => None,
501502
}).nth(1);
502503
if let TyKind::Rptr(_, ref to_ty) = to_ty.node;
503504
then {

clippy_lints/src/types.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath, path: &[&str])
223223
if !params.parenthesized;
224224
if let Some(ty) = params.args.iter().find_map(|arg| match arg {
225225
GenericArg::Type(ty) => Some(ty),
226-
GenericArg::Lifetime(_) => None,
226+
_ => None,
227227
});
228228
if let TyKind::Path(ref qpath) = ty.node;
229229
if let Some(did) = opt_def_id(cx.tables.qpath_def(qpath, cx.tcx.hir().node_to_hir_id(ty.id)));
@@ -267,7 +267,7 @@ fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) {
267267
if let Some(ref last) = last_path_segment(qpath).args;
268268
if let Some(ty) = last.args.iter().find_map(|arg| match arg {
269269
GenericArg::Type(ty) => Some(ty),
270-
GenericArg::Lifetime(_) => None,
270+
_ => None,
271271
});
272272
// ty is now _ at this point
273273
if let TyKind::Path(ref ty_qpath) = ty.node;
@@ -278,7 +278,7 @@ fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) {
278278
if let Some(ref last) = last_path_segment(ty_qpath).args;
279279
if let Some(boxed_ty) = last.args.iter().find_map(|arg| match arg {
280280
GenericArg::Type(ty) => Some(ty),
281-
GenericArg::Lifetime(_) => None,
281+
_ => None,
282282
});
283283
then {
284284
let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty);
@@ -327,7 +327,7 @@ fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) {
327327
.map_or_else(|| [].iter(), |params| params.args.iter())
328328
.filter_map(|arg| match arg {
329329
GenericArg::Type(ty) => Some(ty),
330-
GenericArg::Lifetime(_) => None,
330+
_ => None,
331331
})
332332
}) {
333333
check_ty(cx, ty, is_local);
@@ -340,7 +340,7 @@ fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) {
340340
.map_or_else(|| [].iter(), |params| params.args.iter())
341341
.filter_map(|arg| match arg {
342342
GenericArg::Type(ty) => Some(ty),
343-
GenericArg::Lifetime(_) => None,
343+
_ => None,
344344
})
345345
}) {
346346
check_ty(cx, ty, is_local);
@@ -351,7 +351,7 @@ fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) {
351351
if let Some(ref params) = seg.args {
352352
for ty in params.args.iter().filter_map(|arg| match arg {
353353
GenericArg::Type(ty) => Some(ty),
354-
GenericArg::Lifetime(_) => None,
354+
_ => None,
355355
}) {
356356
check_ty(cx, ty, is_local);
357357
}
@@ -387,7 +387,7 @@ fn check_ty_rptr(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool, lt:
387387
if !params.parenthesized;
388388
if let Some(inner) = params.args.iter().find_map(|arg| match arg {
389389
GenericArg::Type(ty) => Some(ty),
390-
GenericArg::Lifetime(_) => None,
390+
_ => None,
391391
});
392392
then {
393393
if is_any_trait(inner) {
@@ -2138,7 +2138,7 @@ impl<'tcx> ImplicitHasherType<'tcx> {
21382138
.iter()
21392139
.filter_map(|arg| match arg {
21402140
GenericArg::Type(ty) => Some(ty),
2141-
GenericArg::Lifetime(_) => None,
2141+
_ => None,
21422142
})
21432143
.collect();
21442144
let params_len = params.len();
@@ -2239,8 +2239,10 @@ impl<'a, 'b, 'tcx: 'a + 'b> ImplicitHasherConstructorVisitor<'a, 'b, 'tcx> {
22392239

22402240
impl<'a, 'b, 'tcx: 'a + 'b> Visitor<'tcx> for ImplicitHasherConstructorVisitor<'a, 'b, 'tcx> {
22412241
fn visit_body(&mut self, body: &'tcx Body) {
2242+
let prev_body = self.body;
22422243
self.body = self.cx.tcx.body_tables(body.id());
22432244
walk_body(self, body);
2245+
self.body = prev_body;
22442246
}
22452247

22462248
fn visit_expr(&mut self, e: &'tcx Expr) {

clippy_lints/src/use_self.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf {
181181
let should_check = if let Some(ref params) = *parameters {
182182
!params.parenthesized && !params.args.iter().any(|arg| match arg {
183183
GenericArg::Lifetime(_) => true,
184-
GenericArg::Type(_) => false,
184+
_ => false,
185185
})
186186
} else {
187187
true

tests/ui/ice-3717.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
use std::collections::HashSet;
2+
3+
fn main() {}
4+
5+
pub fn ice_3717(_: &HashSet<usize>) {
6+
let _ = [0u8; 0];
7+
let _: HashSet<usize> = HashSet::new();
8+
}

tests/ui/ice-3717.stderr

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: parameter of type `HashSet` should be generalized over different hashers
2+
--> $DIR/ice-3717.rs:5:21
3+
|
4+
LL | pub fn ice_3717(_: &HashSet<usize>) {
5+
| ^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::implicit-hasher` implied by `-D warnings`
8+
help: consider adding a type parameter
9+
|
10+
LL | pub fn ice_3717<S: ::std::hash::BuildHasher + Default>(_: &HashSet<usize, S>) {
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^
12+
help: ...and use generic constructor
13+
|
14+
LL | let _: HashSet<usize> = HashSet::default();
15+
| ^^^^^^^^^^^^^^^^^^
16+
17+
error: aborting due to previous error
18+

0 commit comments

Comments
 (0)