Auto merge of #3410 - fmdkdd:fix-3390-message-format-parse-stderr, r=…

…alexcrichton

Fix `--message-format JSON` when rustc emits non-JSON warnings

The `--message-format JSON` flag parses all the stderr output of rustc to JSON,
but rustc can emit non-JSON lines to stderr (e.g., for warning about the
unstable `-Z` flag on the stable channel), causing cargo to fail reporting
compilation errors when using `--message-format JSON`.

This commit adds a check to look for lines beginning with `{` to only parse
these lines as JSON.  Other lines from rustc are forwarded to the stderr of
cargo.

Fixes #3390.

src/cargo/ops/cargo_rustc/mod.rs

@@ -2,6 +2,7 @@ use std::collections::{HashMap, HashSet};
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fs;
+use std::io::{self, Write};
 use std::path::{self, PathBuf};
 use std::sync::Arc;
 
@@ -297,15 +298,22 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
                     Ok(())
                 },
                 &mut |line| {
-                    let compiler_message = json::Json::from_str(line).map_err(|_| {
-                        internal(&format!("compiler produced invalid json: `{}`", line))
-                    })?;
-
-                    machine_message::emit(machine_message::FromCompiler {
-                        package_id: &package_id,
-                        target: &target,
-                        message: compiler_message,
-                    });
+                    // stderr from rustc can have a mix of JSON and non-JSON output
+                    if line.starts_with("{") {
+                        // Handle JSON lines
+                        let compiler_message = json::Json::from_str(line).map_err(|_| {
+                            internal(&format!("compiler produced invalid json: `{}`", line))
+                        })?;
+
+                        machine_message::emit(machine_message::FromCompiler {
+                            package_id: &package_id,
+                            target: &target,
+                            message: compiler_message,
+                        });
+                    } else {
+                        // Forward non-JSON to stderr
+                        writeln!(io::stderr(), "{}", line)?;
+                    }
                     Ok(())
                 },
             ).map(|_| ())

tests/build.rs

@@ -2470,6 +2470,55 @@ fn wrong_message_format_option() {
 r#"[ERROR] Could not match 'xml' with any of the allowed variants: ["Human", "Json"]"#));
 }
 
+#[test]
+fn message_format_json_forward_stderr() {
+    if is_nightly() { return }
+
+    let p = project("foo")
+        .file("Cargo.toml", &basic_bin_manifest("foo"))
+        .file("src/main.rs", "fn main() { let unused = 0; }");
+
+    assert_that(p.cargo_process("rustc").arg("--bin").arg("foo")
+                .arg("--message-format").arg("JSON").arg("--").arg("-Zno-trans"),
+                execs()
+                .with_stderr_contains("[WARNING] the option `Z` is unstable [..]")
+                .with_json(r#"
+    {
+        "reason":"compiler-message",
+        "package_id":"foo 0.5.0 ([..])",
+        "target":{"kind":["bin"],"name":"foo","src_path":"[..]"},
+        "message":{
+            "children":[],"code":null,"level":"warning","rendered":null,
+            "message":"unused variable: `unused`, #[warn(unused_variables)] on by default",
+            "spans":[{
+                "byte_end":22,"byte_start":16,"column_end":23,"column_start":17,"expansion":null,
+                "file_name":"[..]","is_primary":true,"label":null,"line_end":1,"line_start":1,
+                "suggested_replacement":null,
+                "text":[{
+                    "highlight_end":23,
+                    "highlight_start":17,
+                    "text":"fn main() { let unused = 0; }"
+                }]
+            }]
+        }
+    }
+
+    {
+        "reason":"compiler-artifact",
+        "package_id":"foo 0.5.0 ([..])",
+        "target":{"kind":["bin"],"name":"foo","src_path":"[..]"},
+        "profile":{
+            "debug_assertions":true,
+            "debuginfo":true,
+            "opt_level":"0",
+            "test":false
+        },
+        "features":[],
+        "filenames":["[..]"]
+    }
+"#));
+}
+
 #[test]
 fn no_warn_about_package_metadata() {
     let p = project("foo")