Skip to content

Commit

Permalink
Forbid certain macros in the codebase.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed May 13, 2020
1 parent 7274307 commit 62711bd
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 5 deletions.
6 changes: 3 additions & 3 deletions src/cargo/core/compiler/build_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
7 changes: 6 additions & 1 deletion src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down
93 changes: 93 additions & 0 deletions tests/internal.rs
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 62711bd

Please sign in to comment.