Forbid certain macros in the codebase.

Author: ehuss

src/cargo/core/compiler/build_plan.rs

@@ -14,7 +14,7 @@ use serde::Serialize;
 use super::context::OutputFile;
 use super::{CompileKind, CompileMode, Context, Unit};
 use crate::core::TargetKind;
-use crate::util::{internal, CargoResult, ProcessBuilder};
+use crate::util::{internal, CargoResult, Config, ProcessBuilder};
 
 #[derive(Debug, Serialize)]
 struct Invocation {
@@ -146,9 +146,9 @@ impl BuildPlan {
         self.plan.inputs = inputs;
     }
 
-    pub fn output_plan(self) {
+    pub fn output_plan(self, config: &Config) {
         let encoded = serde_json::to_string(&self.plan).unwrap();
-        println!("{}", encoded);
+        crate::drop_println!(config, "{}", encoded);
     }
 }
 

src/cargo/core/compiler/context/mod.rs

@@ -157,7 +157,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
         if build_plan {
             plan.set_inputs(self.build_plan_inputs()?);
-            plan.output_plan();
+            plan.output_plan(self.bcx.config);
         }
 
         // Collect the result of the build into `self.compilation`.

src/cargo/ops/fix.rs

@@ -41,6 +41,7 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::env;
 use std::ffi::OsString;
+use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process::{self, Command, ExitStatus};
 use std::str;
@@ -526,7 +527,11 @@ fn exit_with(status: ExitStatus) -> ! {
     {
         use std::os::unix::prelude::*;
         if let Some(signal) = status.signal() {
-            eprintln!("child failed with signal `{}`", signal);
+            drop(writeln!(
+                std::io::stderr().lock(),
+                "child failed with signal `{}`",
+                signal
+            ));
             process::exit(2);
         }
     }

tests/internal.rs

@@ -0,0 +1,93 @@
+//! Tests for internal code checks.
+use std::fs;
+
+#[test]
+fn check_forbidden_code() {
+    // Do not use certain macros, functions, etc.
+    if !cargo::util::is_ci() {
+        // Only check these on CI, otherwise it could be annoying.
+        return;
+    }
+    let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src");
+    for entry in walkdir::WalkDir::new(path)
+        .into_iter()
+        .filter_map(|e| e.ok())
+    {
+        let path = entry.path();
+        if !entry
+            .file_name()
+            .to_str()
+            .map(|s| s.ends_with(".rs"))
+            .unwrap_or(false)
+        {
+            continue;
+        }
+        let c = fs::read_to_string(path).unwrap();
+        for (line_index, line) in c.lines().enumerate() {
+            if line_has_print(line) {
+                if entry.file_name().to_str().unwrap() == "cargo_new.rs" && line.contains("Hello") {
+                    // An exception.
+                    continue;
+                }
+                panic!(
+                    "found print macro in {}:{}\n\n{}\n\n\
+                    print! macros should not be used in Cargo because they can panic.\n\
+                    Use one of the drop_print macros instead.\n\
+                    ",
+                    path.display(),
+                    line_index,
+                    line
+                );
+            }
+            if line_has_macro(line, "dbg") {
+                panic!(
+                    "found dbg! macro in {}:{}\n\n{}\n\n\
+                    dbg! should not be used outside of debugging.",
+                    path.display(),
+                    line_index,
+                    line
+                );
+            }
+        }
+    }
+}
+
+fn line_has_print(line: &str) -> bool {
+    line_has_macro(line, "print")
+        || line_has_macro(line, "eprint")
+        || line_has_macro(line, "println")
+        || line_has_macro(line, "eprintln")
+}
+
+#[test]
+fn line_has_print_works() {
+    assert!(line_has_print("print!"));
+    assert!(line_has_print("println!"));
+    assert!(line_has_print("eprint!"));
+    assert!(line_has_print("eprintln!"));
+    assert!(line_has_print("(print!(\"hi!\"))"));
+    assert!(!line_has_print("print"));
+    assert!(!line_has_print("i like to print things"));
+    assert!(!line_has_print("drop_print!"));
+    assert!(!line_has_print("drop_println!"));
+    assert!(!line_has_print("drop_eprint!"));
+    assert!(!line_has_print("drop_eprintln!"));
+}
+
+fn line_has_macro(line: &str, mac: &str) -> bool {
+    for (i, _) in line.match_indices(mac) {
+        if line.get(i + mac.len()..i + mac.len() + 1) != Some("!") {
+            continue;
+        }
+        if i == 0 {
+            return true;
+        }
+        // Check for identifier boundary start.
+        let prev1 = line.get(i - 1..i).unwrap().chars().next().unwrap();
+        if prev1.is_alphanumeric() || prev1 == '_' {
+            continue;
+        }
+        return true;
+    }
+    false
+}