Auto merge of #3905 - rust-lang:compiletest, r=<try>

bors · bors · commit 027a659e0c8f · 2019-04-01T19:29:41.000Z
Hacky rustup

This gets our tests back working, but we now are calling `transmute` in the compiletest code.

Also I couldn't quickly figure out some test failures and won't have time to get to them until thursday, if someone could take them over that would be great (maybe just merge this PR and open issues for all failures and start addressing them?)
diff --git 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
@@ -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/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
@@ -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/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
@@ -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
@@ -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
@@ -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<Vec
             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(..)
-                        if !has_drop(cx, cx.tables.expr_ty(expr)) =>
-                    {
+                    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
@@ -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
@@ -157,25 +157,30 @@ 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_or(expr.span, |info| info.call_site);
                 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/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/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
@@ -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,12 @@ impl<'tcx> Printer<'tcx, 'tcx> for AbsolutePathPrinter<'_, 'tcx> {
         disambiguated_data: &DisambiguatedDefPathData,
     ) -> Result<Self::Path, Self::Error> {
         let mut path = print_prefix(self)?;
+
+        // Skip `::{{constructor}}` on tuple/unit structs.
+        if let DefPathData::Ctor = disambiguated_data.data {
+            return Ok(path);
+        }
+
         path.push(disambiguated_data.data.as_interned_str().as_str());
         Ok(path)
     }
@@ -863,7 +869,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
@@ -59,6 +59,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
             then {
                 // report the error around the `vec!` not inside `<std macros>:`
                 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/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, false, pass);
         }
 
         for (name, (to, deprecated_name)) in lint_groups {
diff --git 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,12 @@ fn run_mode(mode: &str, dir: PathBuf) {
     compiletest::run_tests(&cfg);
 }
 
-fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec<test::TestDescAndFn>) -> Result<bool, io::Error> {
+#[warn(clippy::identity_conversion)]
+fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec<TestDescAndFn>) -> Result<bool, io::Error> {
     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 +98,25 @@ fn run_ui_toml_tests(config: &compiletest::Config, mut tests: Vec<test::TestDesc
             let test_name = compiletest::make_test_name(&config, &paths);
             let index = tests
                 .iter()
-                .position(|test| test.desc.name == test_name)
+                .position(|test| test.desc.name.to_string() == test_name.to_string())
                 .expect("The test should be in there");
-            result &= test::run_tests_console(&opts, vec![tests.swap_remove(index)])?;
+            let opts = libtest::TestOpts {
+                list: opts.list,
+                filter: opts.filter.clone(),
+                filter_exact: opts.filter_exact,
+                exclude_should_panic: Default::default(),
+                run_ignored: libtest::RunIgnored::No,
+                run_tests: opts.run_tests,
+                bench_benchmarks: opts.bench_benchmarks,
+                logfile: opts.logfile.clone(),
+                nocapture: opts.nocapture,
+                color: libtest::ColorConfig::AutoColor,
+                format: libtest::OutputFormat::Pretty,
+                test_threads: opts.test_threads,
+                skip: opts.skip.clone(),
+                options: libtest::Options::new(),
+            };
+            result &= libtest::run_tests_console(&opts, vec![tests.swap_remove(index)])?;
         }
     }
     Ok(result)
@@ -114,6 +127,22 @@ fn run_ui_toml() {
     let config = config("ui", path);
     let tests = compiletest::make_tests(&config);
 
+    let tests = tests
+        .into_iter()
+        .map(|test| {
+            libtest::TestDescAndFn {
+                desc: libtest::TestDesc {
+                    name: libtest::TestName::DynTestName(test.desc.name.to_string()),
+                    ignore: test.desc.ignore,
+                    allow_fail: test.desc.allow_fail,
+                    should_panic: libtest::ShouldPanic::No,
+                },
+                // oli obk giving up
+                testfn: unsafe { std::mem::transmute(test.testfn) },
+            }
+        })
+        .collect();
+
     let res = run_ui_toml_tests(&config, tests);
     match res {
         Ok(true) => {},
diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr
diff --git a/tests/ui/into_iter_on_ref.fixed b/tests/ui/into_iter_on_ref.fixed
diff --git a/tests/ui/into_iter_on_ref.stderr b/tests/ui/into_iter_on_ref.stderr
diff --git a/tests/ui/needless_pass_by_value.rs b/tests/ui/needless_pass_by_value.rs
diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed
diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs
diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr

Original file line number	Diff line number	Diff line change
`@@ -39,9 +39,10 @@ impl LintPass for EnumGlobUse {`
`39`	`39`
`40`	`40`	`impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EnumGlobUse {`
`41`	`41`	`fn check_mod(&mut self, cx: &LateContext<'a, 'tcx>, m: &'tcx Mod, _: Span, _: HirId) {`
	`42`	`+ let map = cx.tcx.hir();`
`42`	`43`	// only check top level `use` statements
`43`	`44`	`for item in &m.item_ids {`
`44`		`- self.lint_item(cx, cx.tcx.hir().expect_item(item.id));`
	`45`	`+ self.lint_item(cx, map.expect_item(map.hir_to_node_id(item.id)));`
`45`	`46`	`}`
`46`	`47`	`}`
`47`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -516,11 +516,11 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {`
`516`	`516`	`for pat in &arm.pats {`
`517`	`517`	`if let PatKind::Path(ref path) = pat.deref().node {`
`518`	`518`	`if let QPath::Resolved(_, p) = path {`
`519`		`- missing_variants.retain(\|e\| e.did != p.def.def_id());`
	`519`	`+ missing_variants.retain(\|e\| e.ctor_def_id != Some(p.def.def_id()));`
`520`	`520`	`}`
`521`	`521`	`} else if let PatKind::TupleStruct(ref path, ..) = pat.deref().node {`
`522`	`522`	`if let QPath::Resolved(_, p) = path {`
`523`		`- missing_variants.retain(\|e\| e.did != p.def.def_id());`
	`523`	`+ missing_variants.retain(\|e\| e.ctor_def_id != Some(p.def.def_id()));`
`524`	`524`	`}`
`525`	`525`	`}`
`526`	`526`	`}`
`@@ -539,7 +539,7 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {`
`539`	`539`	`String::new()`
`540`	`540`	`};`
`541`	`541`	`// This path assumes that the enum type is imported into scope.`
`542`		`- format!("{}{}{}", ident_str, cx.tcx.def_path_str(v.did), suffix)`
	`542`	`+ format!("{}{}{}", ident_str, cx.tcx.def_path_str(v.def_id), suffix)`
`543`	`543`	`})`
`544`	`544`	`.collect();`
`545`	`545`
Original file line number	Diff line number	Diff line change
`@@ -128,7 +128,7 @@ impl Pass {`
`128`	`128`	`},`
`129`	`129`	`ExprKind::Ret(Some(ref expr)) => Self::expression_returns_none(cx, expr),`
`130`	`130`	`ExprKind::Path(ref qp) => {`
`131`		`- if let Def::VariantCtor(def_id, _) = cx.tables.qpath_def(qp, expression.hir_id) {`
	`131`	`+ if let Def::Ctor(def_id, def::CtorOf::Variant, _) = cx.tables.qpath_def(qp, expression.hir_id) {`
`132`	`132`	`return match_def_path(cx.tcx, def_id, &OPTION_NONE);`
`133`	`133`	`}`
`134`	`134`