Auto merge of #4837 - flip1995:integration, r=<try>

[WIP] RIIR: Integration tests

In #4825 the `rust-lang/chalk` test failed because the output was too large. I didn't want to completely disabling the output, since showing the backtrace of an ICE directly in travis is pretty useful. Since finding strings in command outputs is easier in Rust, than in bash, I just RIIRed it.

This and also rewriting our tests in Rust may help with trying out new CI platforms (cc #4577)

Based on #4825, so I could test it.

changelog: none

.travis.yml

@@ -65,9 +65,8 @@ matrix:
       if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try)
     - env: INTEGRATION=Geal/nom
       if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try)
-    # FIXME blocked on https://github.com/rust-lang/rust-clippy/issues/4727
-    #- env: INTEGRATION=rust-lang/rustfmt
-    #  if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try)
+    - env: INTEGRATION=rust-lang/rustfmt
+      if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try)
     - env: INTEGRATION=hyperium/hyper
       if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try)
     - env: INTEGRATION=bluss/rust-itertools
@@ -125,7 +124,7 @@ before_script:
 script:
   - |
     if [[ -n ${INTEGRATION} ]]; then
-      ./ci/integration-tests.sh && sleep 5
+      cargo test --test integration --features integration && sleep 5
     else
       ./ci/base-tests.sh && sleep 5
     fi

Cargo.toml

@@ -38,10 +38,13 @@ clippy_lints = { version = "0.0.212", path = "clippy_lints" }
 regex = "1"
 semver = "0.9"
 rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}
+git2 = { version = "0.10", optional = true }
+tempfile = { version = "3.1.0", optional = true }
 
 [dev-dependencies]
 cargo_metadata = "0.9.0"
-compiletest_rs = { version = "0.3.24", features = ["tmp"] }
+compiletest_rs = { version = "0.4.0", features = ["tmp"] }
+tester = "0.7"
 lazy_static = "1.0"
 clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }
 serde = { version = "1.0", features = ["derive"] }
@@ -58,3 +61,4 @@ rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}
 [features]
 deny-warnings = []
 debugging = []
+integration = ["git2", "tempfile"]

clippy_lints/src/lib.rs

@@ -2,7 +2,6 @@
 
 #![feature(box_syntax)]
 #![feature(box_patterns)]
-#![feature(never_type)]
 #![feature(rustc_private)]
 #![feature(slice_patterns)]
 #![feature(stmt_expr_attributes)]

clippy_lints/src/loops.rs

@@ -720,7 +720,7 @@ fn never_loop_expr(expr: &Expr, main_loop_id: HirId) -> NeverLoopResult {
         ExprKind::Struct(_, _, None)
         | ExprKind::Yield(_, _)
         | ExprKind::Closure(_, _, _, _, _)
-        | ExprKind::InlineAsm(_, _, _)
+        | ExprKind::InlineAsm(_)
         | ExprKind::Path(_)
         | ExprKind::Lit(_)
         | ExprKind::Err => NeverLoopResult::Otherwise,

clippy_lints/src/utils/author.rs

@@ -458,8 +458,8 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
                     println!("Ret(None) = {};", current);
                 }
             },
-            ExprKind::InlineAsm(_, ref _input, ref _output) => {
-                println!("InlineAsm(_, ref input, ref output) = {};", current);
+            ExprKind::InlineAsm(_) => {
+                println!("InlineAsm(_) = {};", current);
                 println!("    // unimplemented: `ExprKind::InlineAsm` is not further destructured at the moment");
             },
             ExprKind::Struct(ref path, ref fields, ref opt_base) => {

clippy_lints/src/utils/inspector.rs

@@ -282,14 +282,16 @@ fn print_expr(cx: &LateContext<'_, '_>, expr: &hir::Expr, indent: usize) {
                 print_expr(cx, e, indent + 1);
             }
         },
-        hir::ExprKind::InlineAsm(_, ref input, ref output) => {
+        hir::ExprKind::InlineAsm(ref asm) => {
+            let inputs = &asm.inputs_exprs;
+            let outputs = &asm.outputs_exprs;
             println!("{}InlineAsm", ind);
             println!("{}inputs:", ind);
-            for e in input {
+            for e in inputs.iter() {
                 print_expr(cx, e, indent + 1);
             }
             println!("{}outputs:", ind);
-            for e in output {
+            for e in outputs.iter() {
                 print_expr(cx, e, indent + 1);
             }
         },

clippy_lints/src/write.rs

@@ -8,6 +8,7 @@ use rustc_errors::Applicability;
 use rustc_lexer::unescape::{self, EscapeError};
 use rustc_parse::parser;
 use syntax::ast::*;
+use syntax::symbol::Symbol;
 use syntax::token;
 use syntax::tokenstream::TokenStream;
 use syntax_pos::{BytePos, Span};
@@ -190,7 +191,7 @@ impl EarlyLintPass for Write {
         if mac.path == sym!(println) {
             span_lint(cx, PRINT_STDOUT, mac.span, "use of `println!`");
             if let (Some(fmt_str), _) = check_tts(cx, &mac.tts, false) {
-                if fmt_str.contents.is_empty() {
+                if fmt_str.symbol == Symbol::intern("") {
                     span_lint_and_sugg(
                         cx,
                         PRINTLN_EMPTY_STRING,
@@ -205,7 +206,7 @@ impl EarlyLintPass for Write {
         } else if mac.path == sym!(print) {
             span_lint(cx, PRINT_STDOUT, mac.span, "use of `print!`");
             if let (Some(fmt_str), _) = check_tts(cx, &mac.tts, false) {
-                if check_newlines(&fmt_str.contents, fmt_str.style) {
+                if check_newlines(&fmt_str) {
                     span_lint_and_then(
                         cx,
                         PRINT_WITH_NEWLINE,
@@ -216,7 +217,7 @@ impl EarlyLintPass for Write {
                                 "use `println!` instead",
                                 vec![
                                     (mac.path.span, String::from("println")),
-                                    (fmt_str.newline_span(), String::new()),
+                                    (newline_span(&fmt_str), String::new()),
                                 ],
                                 Applicability::MachineApplicable,
                             );
@@ -226,7 +227,7 @@ impl EarlyLintPass for Write {
             }
         } else if mac.path == sym!(write) {
             if let (Some(fmt_str), _) = check_tts(cx, &mac.tts, true) {
-                if check_newlines(&fmt_str.contents, fmt_str.style) {
+                if check_newlines(&fmt_str) {
                     span_lint_and_then(
                         cx,
                         WRITE_WITH_NEWLINE,
@@ -237,7 +238,7 @@ impl EarlyLintPass for Write {
                                 "use `writeln!()` instead",
                                 vec![
                                     (mac.path.span, String::from("writeln")),
-                                    (fmt_str.newline_span(), String::new()),
+                                    (newline_span(&fmt_str), String::new()),
                                 ],
                                 Applicability::MachineApplicable,
                             );
@@ -247,7 +248,7 @@ impl EarlyLintPass for Write {
             }
         } else if mac.path == sym!(writeln) {
             if let (Some(fmt_str), expr) = check_tts(cx, &mac.tts, true) {
-                if fmt_str.contents.is_empty() {
+                if fmt_str.symbol == Symbol::intern("") {
                     let mut applicability = Applicability::MachineApplicable;
                     let suggestion = expr.map_or_else(
                         move || {
@@ -272,37 +273,27 @@ impl EarlyLintPass for Write {
     }
 }
 
-/// The arguments of a `print[ln]!` or `write[ln]!` invocation.
-struct FmtStr {
-    /// The contents of the format string (inside the quotes).
-    contents: String,
-    style: StrStyle,
-    /// The span of the format string, including quotes, the raw marker, and any raw hashes.
-    span: Span,
-}
-
-impl FmtStr {
-    /// Given a format string that ends in a newline and its span, calculates the span of the
-    /// newline.
-    fn newline_span(&self) -> Span {
-        let sp = self.span;
+/// Given a format string that ends in a newline and its span, calculates the span of the
+/// newline.
+fn newline_span(fmtstr: &StrLit) -> Span {
+    let sp = fmtstr.span;
+    let contents = &fmtstr.symbol.as_str();
 
-        let newline_sp_hi = sp.hi()
-            - match self.style {
-                StrStyle::Cooked => BytePos(1),
-                StrStyle::Raw(hashes) => BytePos((1 + hashes).into()),
-            };
-
-        let newline_sp_len = if self.contents.ends_with('\n') {
-            BytePos(1)
-        } else if self.contents.ends_with(r"\n") {
-            BytePos(2)
-        } else {
-            panic!("expected format string to contain a newline");
+    let newline_sp_hi = sp.hi()
+        - match fmtstr.style {
+            StrStyle::Cooked => BytePos(1),
+            StrStyle::Raw(hashes) => BytePos((1 + hashes).into()),
         };
 
-        sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
-    }
+    let newline_sp_len = if contents.ends_with('\n') {
+        BytePos(1)
+    } else if contents.ends_with(r"\n") {
+        BytePos(2)
+    } else {
+        panic!("expected format string to contain a newline");
+    };
+
+    sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
 }
 
 /// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
@@ -325,7 +316,7 @@ impl FmtStr {
 /// (Some("string to write: {}"), Some(buf))
 /// ```
 #[allow(clippy::too_many_lines)]
-fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<FmtStr>, Option<Expr>) {
+fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<StrLit>, Option<Expr>) {
     use fmt_macros::*;
     let tts = tts.clone();
 
@@ -342,12 +333,11 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
         }
     }
 
-    let (fmtstr, fmtstyle) = match parser.parse_str().map_err(|mut err| err.cancel()) {
-        Ok((fmtstr, fmtstyle)) => (fmtstr.to_string(), fmtstyle),
+    let fmtstr = match parser.parse_str_lit() {
+        Ok(fmtstr) => fmtstr,
         Err(_) => return (None, expr),
     };
-    let fmtspan = parser.prev_span;
-    let tmp = fmtstr.clone();
+    let tmp = fmtstr.symbol.as_str();
     let mut args = vec![];
     let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
     while let Some(piece) = fmt_parser.next() {
@@ -377,26 +367,12 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
             ty_span: None,
         };
         if !parser.eat(&token::Comma) {
-            return (
-                Some(FmtStr {
-                    contents: fmtstr,
-                    style: fmtstyle,
-                    span: fmtspan,
-                }),
-                expr,
-            );
+            return (Some(fmtstr), expr);
         }
         let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
             expr
         } else {
-            return (
-                Some(FmtStr {
-                    contents: fmtstr,
-                    style: fmtstyle,
-                    span: fmtspan,
-                }),
-                None,
-            );
+            return (Some(fmtstr), None);
         };
         match &token_expr.kind {
             ExprKind::Lit(_) => {
@@ -448,11 +424,13 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
 /// Checks if the format string contains a single newline that terminates it.
 ///
 /// Literal and escaped newlines are both checked (only literal for raw strings).
-fn check_newlines(contents: &str, style: StrStyle) -> bool {
+fn check_newlines(fmtstr: &StrLit) -> bool {
     let mut has_internal_newline = false;
     let mut last_was_cr = false;
     let mut should_lint = false;
 
+    let contents = &fmtstr.symbol.as_str();
+
     let mut cb = |r: Range<usize>, c: Result<char, EscapeError>| {
         let c = c.unwrap();
 
@@ -466,7 +444,7 @@ fn check_newlines(contents: &str, style: StrStyle) -> bool {
         }
     };
 
-    match style {
+    match fmtstr.style {
         StrStyle::Cooked => unescape::unescape_str(contents, &mut cb),
         StrStyle::Raw(_) => unescape::unescape_raw_str(contents, &mut cb),
     }

src/driver.rs

@@ -75,6 +75,12 @@ impl rustc_driver::Callbacks for ClippyCallbacks {
             clippy_lints::register_pre_expansion_lints(&mut lint_store, &conf);
             clippy_lints::register_renamed(&mut lint_store);
         }));
+
+        // FIXME: #4825; This is required, because Clippy lints that are based on MIR have to be
+        // run on the unoptimized MIR. On the other hand this results in some false negatives. If
+        // MIR passes can be enabled / disabled separately, we should figure out, what passes to
+        // use for Clippy.
+        config.opts.debugging_opts.mir_opt_level = 0;
     }
 }
 

tests/compile-test.rs

@@ -1,7 +1,7 @@
 #![feature(test)]
 
 use compiletest_rs as compiletest;
-extern crate test;
+extern crate tester as test;
 
 use std::env::{set_var, var};
 use std::ffi::OsStr;