From d020565ed2d65efbd852aab298c4b8c69de75b12 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 26 Mar 2019 10:55:03 +0100 Subject: [PATCH 1/9] Hacky rustup --- Cargo.toml | 3 +- clippy_lints/src/attrs.rs | 8 ++-- clippy_lints/src/matches.rs | 6 +-- clippy_lints/src/missing_doc.rs | 4 +- clippy_lints/src/missing_inline.rs | 11 +++--- clippy_lints/src/no_effect.rs | 6 +-- clippy_lints/src/question_mark.rs | 2 +- clippy_lints/src/ranges.rs | 14 +++++-- clippy_lints/src/use_self.rs | 2 +- clippy_lints/src/utils/mod.rs | 2 +- clippy_lints/src/vec.rs | 5 +++ tests/compile-test.rs | 46 +++++++++++++++++----- tests/ui/for_loop.stderr | 26 ++++++------ tests/ui/into_iter_on_ref.fixed | 2 +- tests/ui/into_iter_on_ref.stderr | 14 ++----- tests/ui/question_mark.stderr | 63 ------------------------------ 16 files changed, 91 insertions(+), 123 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 024b8ff28ad4..3d36e52d2df8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,8 @@ rustc_tools_util = { version = "0.1.1", path = "rustc_tools_util"} [dev-dependencies] clippy_dev = { version = "0.0.1", path = "clippy_dev" } cargo_metadata = "0.7.1" -compiletest_rs = "0.3.19" +compiletest_rs = { version = "=0.3.19", features = ["tmp", "stable"] } +libtest = "0.0.1" lazy_static = "1.0" serde_derive = "1.0" clippy-mini-macro-test = { version = "0.2", path = "mini-macro" } diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index b4e4d46a33a6..c054a00894e7 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -208,8 +208,8 @@ impl LintPass for AttrPass { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass { fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute) { if let Some(items) = &attr.meta_item_list() { - if let Some(ident) = attr.ident_str() { - match ident { + if let Some(ident) = attr.ident() { + match &*ident.as_str() { "allow" | "warn" | "deny" | "forbid" => { check_clippy_lint_names(cx, items); }, @@ -242,8 +242,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass { for attr in &item.attrs { if let Some(lint_list) = &attr.meta_item_list() { - if let Some(ident) = attr.ident_str() { - match ident { + if let Some(ident) = attr.ident() { + match &*ident.as_str() { "allow" | "warn" | "deny" | "forbid" => { // whitelist `unused_imports` and `deprecated` for `use` items // and `unused_imports` for `extern crate` items with `macro_use` diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 74a85f1f8174..34cb2422e1c5 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -516,11 +516,11 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) { for pat in &arm.pats { if let PatKind::Path(ref path) = pat.deref().node { if let QPath::Resolved(_, p) = path { - missing_variants.retain(|e| e.did != p.def.def_id()); + missing_variants.retain(|e| e.ctor_def_id != Some(p.def.def_id())); } } else if let PatKind::TupleStruct(ref path, ..) = pat.deref().node { if let QPath::Resolved(_, p) = path { - missing_variants.retain(|e| e.did != p.def.def_id()); + missing_variants.retain(|e| e.ctor_def_id != Some(p.def.def_id())); } } } @@ -539,7 +539,7 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) { String::new() }; // This path assumes that the enum type is imported into scope. - format!("{}{}{}", ident_str, cx.tcx.def_path_str(v.did), suffix) + format!("{}{}{}", ident_str, cx.tcx.def_path_str(v.def_id), suffix) }) .collect(); diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index ed984f16db5c..721cfd870201 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -58,9 +58,9 @@ impl MissingDoc { if let Some(meta) = meta; if let MetaItemKind::List(list) = meta.node; if let Some(meta) = list.get(0); - if let Some(name) = meta.ident_str(); + if let Some(name) = meta.ident(); then { - name == "include" + name.as_str() == "include" } else { false } diff --git a/clippy_lints/src/missing_inline.rs b/clippy_lints/src/missing_inline.rs index 090ed6d242d1..c6b1b7eaf517 100644 --- a/clippy_lints/src/missing_inline.rs +++ b/clippy_lints/src/missing_inline.rs @@ -162,12 +162,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingInline { }; if let Some(trait_def_id) = trait_def_id { - if cx.tcx.hir().as_local_node_id(trait_def_id).is_some() { - if !cx.access_levels.is_exported(impl_item.hir_id) { - // If a trait is being implemented for an item, and the - // trait is not exported, we don't need #[inline] - return; - } + if cx.tcx.hir().as_local_node_id(trait_def_id).is_some() && !cx.access_levels.is_exported(impl_item.hir_id) + { + // If a trait is being implemented for an item, and the + // trait is not exported, we don't need #[inline] + return; } } diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index dae59bc84b95..74a9b353ea97 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -72,7 +72,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { if let ExprKind::Path(ref qpath) = callee.node { let def = cx.tables.qpath_def(qpath, callee.hir_id); match def { - Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..) => { + Def::Struct(..) | Def::Variant(..) | Def::Ctor(..) => { !has_drop(cx, cx.tables.expr_ty(expr)) && args.iter().all(|arg| has_no_effect(cx, arg)) }, _ => false, @@ -166,9 +166,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option - { + Def::Struct(..) | Def::Variant(..) | Def::Ctor(..) if !has_drop(cx, cx.tables.expr_ty(expr)) => { Some(args.iter().collect()) }, _ => None, diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 7821ad56ccc8..5c64cd1c15c7 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -128,7 +128,7 @@ impl Pass { }, ExprKind::Ret(Some(ref expr)) => Self::expression_returns_none(cx, expr), ExprKind::Path(ref qp) => { - if let Def::VariantCtor(def_id, _) = cx.tables.qpath_def(qp, expression.hir_id) { + if let Def::Ctor(def_id, def::CtorOf::Variant, _) = cx.tables.qpath_def(qp, expression.hir_id) { return match_def_path(cx.tcx, def_id, &OPTION_NONE); } diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 18dfb963230b..87aeaf521028 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -157,25 +157,31 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { }) = higher::range(cx, expr); if let Some(y) = y_plus_one(end); then { + let span = expr.span + .ctxt() + .outer() + .expn_info() + .map(|info| info.call_site) + .unwrap_or(expr.span); span_lint_and_then( cx, RANGE_PLUS_ONE, - expr.span, + span, "an inclusive range would be more readable", |db| { let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); let end = Sugg::hir(cx, y, "y"); - if let Some(is_wrapped) = &snippet_opt(cx, expr.span) { + if let Some(is_wrapped) = &snippet_opt(cx, span) { if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') { db.span_suggestion( - expr.span, + span, "use", format!("({}..={})", start, end), Applicability::MaybeIncorrect, ); } else { db.span_suggestion( - expr.span, + span, "use", format!("{}..={}", start, end), Applicability::MachineApplicable, // snippet diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index ac901ab062f3..d3b011a3bd98 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -233,7 +233,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> { if path.segments.last().expect(SEGMENTS_MSG).ident.name != SelfUpper.name() { if self.item_path.def == path.def { span_use_self_lint(self.cx, path); - } else if let Def::StructCtor(ctor_did, CtorKind::Fn) = path.def { + } else if let Def::Ctor(ctor_did, def::CtorOf::Struct, CtorKind::Fn) = path.def { if self.item_path.def.opt_def_id() == self.cx.tcx.parent(ctor_did) { span_use_self_lint(self.cx, path); } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 2cf638b939cc..d70d3ff1239d 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -863,7 +863,7 @@ pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat) -> bool { fn is_enum_variant(cx: &LateContext<'_, '_>, qpath: &QPath, id: HirId) -> bool { matches!( cx.tables.qpath_def(qpath, id), - def::Def::Variant(..) | def::Def::VariantCtor(..) + def::Def::Variant(..) | def::Def::Ctor(_, def::CtorOf::Variant, _) ) } diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index 51cedfef0f71..9b8b2372c53b 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -59,6 +59,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { then { // report the error around the `vec!` not inside `:` let span = arg.span + .ctxt() + .outer() + .expn_info() + .map(|info| info.call_site) + .expect("unable to get call_site") .ctxt() .outer() .expn_info() diff --git a/tests/compile-test.rs b/tests/compile-test.rs index b6a4beff0469..37a33911d9e6 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -1,7 +1,7 @@ #![feature(test)] use compiletest_rs as compiletest; -extern crate test; +use libtest::TestDescAndFn; use std::env::{set_var, var}; use std::ffi::OsStr; @@ -74,15 +74,11 @@ fn run_mode(mode: &str, dir: PathBuf) { compiletest::run_tests(&cfg); } -fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec) -> Result { +fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec) -> Result { let mut result = true; let opts = compiletest::test_opts(config); for dir in fs::read_dir(&config.src_base)? { - let dir = dir?; - if !dir.file_type()?.is_dir() { - continue; - } - let dir_path = dir.path(); + let dir_path = dir.unwrap().path(); set_var("CARGO_MANIFEST_DIR", &dir_path); for file in fs::read_dir(&dir_path)? { let file = file?; @@ -101,9 +97,25 @@ fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec {}, diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr index e8964c26796e..c1ad23614840 100644 --- a/tests/ui/for_loop.stderr +++ b/tests/ui/for_loop.stderr @@ -198,7 +198,7 @@ error: it is more concise to loop over references to containers instead of using --> $DIR/for_loop.rs:170:15 | LL | for _v in vec.iter() {} - | ^^^^^^^^^^ help: to write this more concisely, try: `&vec` + | ^^^^^^^^^^ | = note: `-D clippy::explicit-iter-loop` implied by `-D warnings` @@ -206,13 +206,13 @@ error: it is more concise to loop over references to containers instead of using --> $DIR/for_loop.rs:172:15 | LL | for _v in vec.iter_mut() {} - | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec` + | ^^^^^^^^^^^^^^ error: it is more concise to loop over containers instead of using explicit iteration methods` --> $DIR/for_loop.rs:175:15 | LL | for _v in out_vec.into_iter() {} - | ^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `out_vec` + | ^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings` @@ -220,61 +220,61 @@ error: it is more concise to loop over references to containers instead of using --> $DIR/for_loop.rs:178:15 | LL | for _v in array.into_iter() {} - | ^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&array` + | ^^^^^^^^^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:183:15 | LL | for _v in [1, 2, 3].iter() {} - | ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[1, 2, 3]` + | ^^^^^^^^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:187:15 | LL | for _v in [0; 32].iter() {} - | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 32]` + | ^^^^^^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:192:15 | LL | for _v in ll.iter() {} - | ^^^^^^^^^ help: to write this more concisely, try: `&ll` + | ^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:195:15 | LL | for _v in vd.iter() {} - | ^^^^^^^^^ help: to write this more concisely, try: `&vd` + | ^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:198:15 | LL | for _v in bh.iter() {} - | ^^^^^^^^^ help: to write this more concisely, try: `&bh` + | ^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:201:15 | LL | for _v in hm.iter() {} - | ^^^^^^^^^ help: to write this more concisely, try: `&hm` + | ^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:204:15 | LL | for _v in bt.iter() {} - | ^^^^^^^^^ help: to write this more concisely, try: `&bt` + | ^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:207:15 | LL | for _v in hs.iter() {} - | ^^^^^^^^^ help: to write this more concisely, try: `&hs` + | ^^^^^^^^^ error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:210:15 | LL | for _v in bs.iter() {} - | ^^^^^^^^^ help: to write this more concisely, try: `&bs` + | ^^^^^^^^^ error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want --> $DIR/for_loop.rs:212:15 diff --git a/tests/ui/into_iter_on_ref.fixed b/tests/ui/into_iter_on_ref.fixed index f5342be631b0..659fd56f9a9d 100644 --- a/tests/ui/into_iter_on_ref.fixed +++ b/tests/ui/into_iter_on_ref.fixed @@ -10,7 +10,7 @@ fn main() { for _ in &[1, 2, 3] {} for _ in vec![X, X] {} for _ in &vec![X, X] {} - for _ in [1, 2, 3].iter() {} //~ ERROR equivalent to .iter() + for _ in [1, 2, 3].into_iter() {} //~ ERROR equivalent to .iter() let _ = [1, 2, 3].iter(); //~ ERROR equivalent to .iter() let _ = vec![1, 2, 3].into_iter(); diff --git a/tests/ui/into_iter_on_ref.stderr b/tests/ui/into_iter_on_ref.stderr index 931e4880f938..c3e5c85618b8 100644 --- a/tests/ui/into_iter_on_ref.stderr +++ b/tests/ui/into_iter_on_ref.stderr @@ -1,8 +1,8 @@ error: this .into_iter() call is equivalent to .iter() and will not move the array - --> $DIR/into_iter_on_ref.rs:13:24 + --> $DIR/into_iter_on_ref.rs:15:23 | -LL | for _ in [1, 2, 3].into_iter() {} //~ ERROR equivalent to .iter() - | ^^^^^^^^^ help: call directly: `iter` +LL | let _ = [1, 2, 3].into_iter(); //~ ERROR equivalent to .iter() + | ^^^^^^^^^ help: call directly: `iter` | note: lint level defined here --> $DIR/into_iter_on_ref.rs:4:9 @@ -10,12 +10,6 @@ note: lint level defined here LL | #![deny(clippy::into_iter_on_array)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: this .into_iter() call is equivalent to .iter() and will not move the array - --> $DIR/into_iter_on_ref.rs:15:23 - | -LL | let _ = [1, 2, 3].into_iter(); //~ ERROR equivalent to .iter() - | ^^^^^^^^^ help: call directly: `iter` - error: this .into_iter() call is equivalent to .iter() and will not move the Vec --> $DIR/into_iter_on_ref.rs:17:30 | @@ -174,5 +168,5 @@ error: this .into_iter() call is equivalent to .iter() and will not move the Pat LL | let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter() | ^^^^^^^^^ help: call directly: `iter` -error: aborting due to 28 previous errors +error: aborting due to 27 previous errors diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index 522501d58c66..e69de29bb2d1 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -1,63 +0,0 @@ -error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:2:5 - | -LL | / if a.is_none() { -LL | | return None; -LL | | } - | |_____^ help: replace_it_with: `a?;` - | - = note: `-D clippy::question-mark` implied by `-D warnings` - -error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:47:9 - | -LL | / if (self.opt).is_none() { -LL | | return None; -LL | | } - | |_________^ help: replace_it_with: `(self.opt)?;` - -error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:51:9 - | -LL | / if self.opt.is_none() { -LL | | return None -LL | | } - | |_________^ help: replace_it_with: `self.opt?;` - -error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:55:17 - | -LL | let _ = if self.opt.is_none() { - | _________________^ -LL | | return None; -LL | | } else { -LL | | self.opt -LL | | }; - | |_________^ help: replace_it_with: `Some(self.opt?)` - -error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:72:9 - | -LL | / if self.opt.is_none() { -LL | | return None; -LL | | } - | |_________^ help: replace_it_with: `self.opt.as_ref()?;` - -error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:80:9 - | -LL | / if self.opt.is_none() { -LL | | return None; -LL | | } - | |_________^ help: replace_it_with: `self.opt.as_ref()?;` - -error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:88:9 - | -LL | / if self.opt.is_none() { -LL | | return None; -LL | | } - | |_________^ help: replace_it_with: `self.opt.as_ref()?;` - -error: aborting due to 7 previous errors - From 3cff06a0eb27dc5b6d208eb24fb22016222ea9d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Fischer?= Date: Tue, 26 Mar 2019 13:57:03 -0300 Subject: [PATCH 2/9] Fix some test failures --- clippy_lints/src/ranges.rs | 3 +-- tests/compile-test.rs | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 87aeaf521028..84c9509844fc 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -161,8 +161,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { .ctxt() .outer() .expn_info() - .map(|info| info.call_site) - .unwrap_or(expr.span); + .map_or(expr.span, |info| info.call_site); span_lint_and_then( cx, RANGE_PLUS_ONE, diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 37a33911d9e6..33b410dbd99a 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -100,13 +100,13 @@ fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec .position(|test| test.desc.name.to_string() == test_name.to_string()) .expect("The test should be in there"); let opts = libtest::TestOpts { - list: opts.list.clone(), + list: opts.list, filter: opts.filter.clone(), - filter_exact: opts.filter_exact.clone(), + filter_exact: opts.filter_exact, exclude_should_panic: Default::default(), run_ignored: libtest::RunIgnored::No, - run_tests: opts.run_tests.clone(), - bench_benchmarks: opts.bench_benchmarks.clone(), + run_tests: opts.run_tests, + bench_benchmarks: opts.bench_benchmarks, logfile: opts.logfile.clone(), nocapture: opts.nocapture, color: libtest::ColorConfig::AutoColor, From 6f01ecfefde7aee9a2cd62cd4b28e42dfe36410d Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 27 Mar 2019 11:46:33 +0100 Subject: [PATCH 3/9] Fix question_mark lint+test --- clippy_lints/src/utils/mod.rs | 9 ++++- tests/ui/question_mark.stderr | 63 +++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index d70d3ff1239d..1f4b0324700e 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -27,7 +27,7 @@ use rustc::hir::def::Def; use rustc::hir::def_id::CrateNum; use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc::hir::intravisit::{NestedVisitorMap, Visitor}; -use rustc::hir::map::DisambiguatedDefPathData; +use rustc::hir::map::{DefPathData, DisambiguatedDefPathData}; use rustc::hir::Node; use rustc::hir::*; use rustc::lint::{LateContext, Level, Lint, LintContext}; @@ -178,6 +178,13 @@ impl<'tcx> Printer<'tcx, 'tcx> for AbsolutePathPrinter<'_, 'tcx> { disambiguated_data: &DisambiguatedDefPathData, ) -> Result { let mut path = print_prefix(self)?; + + // Skip `::{{constructor}}` on tuple/unit structs. + match disambiguated_data.data { + DefPathData::Ctor => return Ok(path), + _ => {} + } + path.push(disambiguated_data.data.as_interned_str().as_str()); Ok(path) } diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index e69de29bb2d1..522501d58c66 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -0,0 +1,63 @@ +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:2:5 + | +LL | / if a.is_none() { +LL | | return None; +LL | | } + | |_____^ help: replace_it_with: `a?;` + | + = note: `-D clippy::question-mark` implied by `-D warnings` + +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:47:9 + | +LL | / if (self.opt).is_none() { +LL | | return None; +LL | | } + | |_________^ help: replace_it_with: `(self.opt)?;` + +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:51:9 + | +LL | / if self.opt.is_none() { +LL | | return None +LL | | } + | |_________^ help: replace_it_with: `self.opt?;` + +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:55:17 + | +LL | let _ = if self.opt.is_none() { + | _________________^ +LL | | return None; +LL | | } else { +LL | | self.opt +LL | | }; + | |_________^ help: replace_it_with: `Some(self.opt?)` + +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:72:9 + | +LL | / if self.opt.is_none() { +LL | | return None; +LL | | } + | |_________^ help: replace_it_with: `self.opt.as_ref()?;` + +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:80:9 + | +LL | / if self.opt.is_none() { +LL | | return None; +LL | | } + | |_________^ help: replace_it_with: `self.opt.as_ref()?;` + +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:88:9 + | +LL | / if self.opt.is_none() { +LL | | return None; +LL | | } + | |_________^ help: replace_it_with: `self.opt.as_ref()?;` + +error: aborting due to 7 previous errors + From bbb7963735308b5bf2e978c124feed20afcc528c Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 27 Mar 2019 13:22:40 +0100 Subject: [PATCH 4/9] Fix dogfood error of question_mark lint fix --- clippy_lints/src/utils/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 1f4b0324700e..3590b7ae9eea 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -180,9 +180,8 @@ impl<'tcx> Printer<'tcx, 'tcx> for AbsolutePathPrinter<'_, 'tcx> { let mut path = print_prefix(self)?; // Skip `::{{constructor}}` on tuple/unit structs. - match disambiguated_data.data { - DefPathData::Ctor => return Ok(path), - _ => {} + if let DefPathData::Ctor = disambiguated_data.data { + return Ok(path); } path.push(disambiguated_data.data.as_interned_str().as_str()); From 491f72442e891973cf447813794ab472383c810b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Fischer?= Date: Fri, 29 Mar 2019 01:47:09 -0300 Subject: [PATCH 5/9] Updated source to match with recent rustc `master` toolchain changes --- clippy_lints/src/enum_glob_use.rs | 3 ++- clippy_lints/src/lifetimes.rs | 3 ++- src/driver.rs | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/enum_glob_use.rs b/clippy_lints/src/enum_glob_use.rs index 37575f10f195..afdf27376d80 100644 --- a/clippy_lints/src/enum_glob_use.rs +++ b/clippy_lints/src/enum_glob_use.rs @@ -39,9 +39,10 @@ impl LintPass for EnumGlobUse { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EnumGlobUse { fn check_mod(&mut self, cx: &LateContext<'a, 'tcx>, m: &'tcx Mod, _: Span, _: HirId) { + let map = cx.tcx.hir(); // only check top level `use` statements for item in &m.item_ids { - self.lint_item(cx, cx.tcx.hir().expect_item(item.id)); + self.lint_item(cx, map.expect_item(map.hir_to_node_id(item.id))); } } } diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 4b2ba60090f9..cd717e586e71 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -356,7 +356,8 @@ impl<'a, 'tcx> Visitor<'tcx> for RefVisitor<'a, 'tcx> { self.collect_anonymous_lifetimes(path, ty); }, TyKind::Def(item, _) => { - if let ItemKind::Existential(ref exist_ty) = self.cx.tcx.hir().expect_item(item.id).node { + let map = self.cx.tcx.hir(); + if let ItemKind::Existential(ref exist_ty) = map.expect_item(map.hir_to_node_id(item.id)).node { for bound in &exist_ty.bounds { if let GenericBound::Outlives(_) = *bound { self.record(&None); diff --git a/src/driver.rs b/src/driver.rs index 01358f46dd70..0b5c259ec4e9 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -93,7 +93,7 @@ impl rustc_driver::Callbacks for ClippyCallbacks { ls.register_early_pass(Some(sess), true, false, pass); } for pass in late_lint_passes { - ls.register_late_pass(Some(sess), true, pass); + ls.register_late_pass(Some(sess), true, false, pass); } for (name, (to, deprecated_name)) in lint_groups { From 414c34c300f3716c03ae034bd72e4db3170cf0b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sat, 30 Mar 2019 15:58:14 +0100 Subject: [PATCH 6/9] rustup 41316f0449025394fdca6606d3fdb3b8f37a9872 --- src/driver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/driver.rs b/src/driver.rs index 0b5c259ec4e9..834d11861c0d 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -93,7 +93,7 @@ impl rustc_driver::Callbacks for ClippyCallbacks { ls.register_early_pass(Some(sess), true, false, pass); } for pass in late_lint_passes { - ls.register_late_pass(Some(sess), true, false, pass); + ls.register_late_pass(Some(sess), true, false, false, pass); } for (name, (to, deprecated_name)) in lint_groups { From b253c564d5f11888038f1e71a1d8534e0827bd4d Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Mon, 1 Apr 2019 07:19:05 +0200 Subject: [PATCH 7/9] Rustup to https://github.com/rust-lang/rust/pull/58805 --- clippy_lints/src/types.rs | 3 +-- tests/ui/use_self.fixed | 1 + tests/ui/use_self.rs | 1 + tests/ui/use_self.stderr | 14 +++++++------- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 33dea622b358..95609d3f3029 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1533,7 +1533,7 @@ impl LintPass for CharLitAsU8 { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CharLitAsU8 { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - use syntax::ast::{LitKind, UintTy}; + use syntax::ast::LitKind; if let ExprKind::Cast(ref e, _) = expr.node { if let ExprKind::Lit(ref l) = e.node { @@ -1818,7 +1818,6 @@ impl Ord for FullInt { fn numeric_cast_precast_bounds<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<(FullInt, FullInt)> { use std::*; - use syntax::ast::{IntTy, UintTy}; if let ExprKind::Cast(ref cast_exp, _) = expr.node { let pre_cast_ty = cx.tables.expr_ty(cast_exp); diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed index 730d391848ec..68af85030ab2 100644 --- a/tests/ui/use_self.fixed +++ b/tests/ui/use_self.fixed @@ -239,6 +239,7 @@ mod nesting { struct Foo {} impl Foo { fn foo() { + #[allow(unused_imports)] use self::Foo; // Can't use Self here struct Bar { foo: Foo, // Foo != Self diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index 008247315121..7a6d415528ad 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -239,6 +239,7 @@ mod nesting { struct Foo {} impl Foo { fn foo() { + #[allow(unused_imports)] use self::Foo; // Can't use Self here struct Bar { foo: Foo, // Foo != Self diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr index 6e39a28012a8..bf1f41fd64ed 100644 --- a/tests/ui/use_self.stderr +++ b/tests/ui/use_self.stderr @@ -151,43 +151,43 @@ LL | use_self_expand!(); // Should lint in local macros | ------------------- in this macro invocation error: unnecessary structure name repetition - --> $DIR/use_self.rs:260:21 + --> $DIR/use_self.rs:261:21 | LL | fn baz() -> Foo { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:261:13 + --> $DIR/use_self.rs:262:13 | LL | Foo {} | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:248:29 + --> $DIR/use_self.rs:249:29 | LL | fn bar() -> Bar { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:249:21 + --> $DIR/use_self.rs:250:21 | LL | Bar { foo: Foo {} } | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:303:13 + --> $DIR/use_self.rs:304:13 | LL | nested::A::fun_1(); | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:304:13 + --> $DIR/use_self.rs:305:13 | LL | nested::A::A; | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:306:13 + --> $DIR/use_self.rs:307:13 | LL | nested::A {}; | ^^^^^^^^^ help: use the applicable keyword: `Self` From 55f67fc7f433b29f93957574f847a47991634b6b Mon Sep 17 00:00:00 2001 From: flip1995 Date: Mon, 1 Apr 2019 09:49:39 +0200 Subject: [PATCH 8/9] Set level of identity_conversion FP to warn --- tests/compile-test.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 33b410dbd99a..1a53b9659082 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -74,6 +74,7 @@ fn run_mode(mode: &str, dir: PathBuf) { compiletest::run_tests(&cfg); } +#[warn(clippy::identity_conversion)] fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec) -> Result { let mut result = true; let opts = compiletest::test_opts(config); From 41927796bfcc5892e87e26eb50aaa81a5d5d7f77 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Mon, 1 Apr 2019 10:28:07 +0200 Subject: [PATCH 9/9] Run rustfmt --- tests/ui/needless_pass_by_value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui/needless_pass_by_value.rs b/tests/ui/needless_pass_by_value.rs index 0d093adf8cd3..f031dd105c25 100644 --- a/tests/ui/needless_pass_by_value.rs +++ b/tests/ui/needless_pass_by_value.rs @@ -95,7 +95,7 @@ impl S { s.len() + t.capacity() } - fn bar(_t: T // Ok, since `&T: Serialize` too + fn bar(_t: T, // Ok, since `&T: Serialize` too ) { }