Clippy: forbid println and friends (#305)

We've had recurring issues with `println` calls sneaking in
where `log` crate macros would be more appropriate.
This commit adds a Clippy warning for uses of the global I/O macros,
i.e. `print`, `println`, `eprint`, `eprintln` and `dbg`.

The lint is disabled by a more-specific `clippy.toml` in the `cli` and `sqltest` crates,
as well as using `allow` attributes in `standalone`'s `subscommands` module.

Additionally, this commit converts a handful of prints in `core/utils` to `log::info`.
This commit is contained in:
Phoebe Goldman
2023-09-20 13:24:26 -04:00
committed by GitHub
parent f9c5de8132
commit fca7c27946
15 changed files with 51 additions and 20 deletions
+7
View File
@@ -0,0 +1,7 @@
disallowed-macros = [
{ path = "std::print", reason = "print blocks on a global mutex for synchronization, and its output cannot be filtered. Use a log macro instead, or apply #[allow(disallowed-macros)] if this is test or CLI code." },
{ path = "std::println", reason = "println blocks on a global mutex for synchronization, and its output cannot be filtered. Use a log macro instead, or apply #[allow(disallowed-macros)] if this is test or CLI code." },
{ path = "std::eprint", reason = "eprint blocks on a global mutex for synchronization, and its output cannot be filtered. Use a log macro instead, or apply #[allow(disallowed-macros)] if this is test or CLI code." },
{ path = "std::eprintln", reason = "eprintln blocks on a global mutex for synchronization, and its output cannot be filtered. Use a log macro instead, or apply #[allow(disallowed-macros)] if this is test or CLI code." },
{ path = "std::dbg", reason = "dbg is a debugging tool and should never be committed into the repository." },
]
+16 -4
View File
@@ -848,7 +848,10 @@ fn spacetimedb_tabletype_impl(item: syn::DeriveInput) -> syn::Result<TokenStream
};
if std::env::var("PROC_MACRO_DEBUG").is_ok() {
println!("{}", emission);
{
#![allow(clippy::disallowed_macros)]
println!("{}", emission);
}
}
Ok(emission)
@@ -871,7 +874,10 @@ fn spacetimedb_index(
};
if std::env::var("PROC_MACRO_DEBUG").is_ok() {
println!("{}", output);
{
#![allow(clippy::disallowed_macros)]
println!("{}", output);
}
}
Ok(output)
@@ -908,7 +914,10 @@ fn spacetimedb_connect_disconnect(item: TokenStream, connect: bool) -> syn::Resu
};
if std::env::var("PROC_MACRO_DEBUG").is_ok() {
println!("{}", emission);
{
#![allow(clippy::disallowed_macros)]
println!("{}", emission);
}
}
Ok(emission)
@@ -975,7 +984,10 @@ pub fn schema_type(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
};
if std::env::var("PROC_MACRO_DEBUG").is_ok() {
println!("{}", emission);
{
#![allow(clippy::disallowed_macros)]
println!("{}", emission);
}
}
Ok(emission)
+3
View File
@@ -0,0 +1,3 @@
# Overwrite the root's disallowed-macros,
# as the CLI is allowed to use println and friends.
disallowed-macros = []
+5 -1
View File
@@ -2,7 +2,11 @@ use std::fs;
fn main() {
let proto_dir = "protobuf";
println!("cargo:rerun-if-changed={proto_dir}");
{
#![allow(clippy::disallowed_macros)]
println!("cargo:rerun-if-changed={proto_dir}");
}
let protos = fs::read_dir(proto_dir)
.unwrap()
+2
View File
@@ -269,6 +269,8 @@ impl<'a> Iterator for MessageLogIter<'a> {
#[cfg(test)]
mod tests {
#![allow(clippy::disallowed_macros)]
use super::MessageLog;
use spacetimedb_lib::error::ResultTest;
use tempdir::{self, TempDir};
+1
View File
@@ -588,6 +588,7 @@ pub(crate) mod tests_utils {
#[cfg(test)]
mod tests {
#![allow(clippy::disallowed_macros)]
use std::sync::{Arc, Mutex};
+1 -1
View File
@@ -98,7 +98,7 @@ fn reload_config<S>(conf_file: &Path, reload_handle: &reload::Handle<EnvFilter,
std::thread::sleep(RELOAD_INTERVAL);
if let Ok(modified) = conf_file.metadata().and_then(|m| m.modified()) {
if prev_time.map_or(true, |prev| modified > prev) {
eprintln!("reloading log config...");
log::info!("reloading log config...");
prev_time = Some(modified);
if reload_handle.reload(parse_from_file(conf_file)).is_err() {
break;
+2
View File
@@ -1,3 +1,5 @@
#![allow(clippy::disallowed_macros)]
use std::iter;
use itertools::Itertools as _;
-12
View File
@@ -394,16 +394,4 @@ mod tests {
let algebraic_type = AlgebraicType::meta_type();
AlgebraicType::from_value(&algebraic_type.as_value()).expect("No errors.");
}
fn _legacy_encoding_comparison() {
let algebraic_type = AlgebraicType::meta_type();
let mut buf = Vec::new();
algebraic_type.as_value().encode(&mut buf);
println!("buf: {:?}", buf);
let mut buf = Vec::new();
algebraic_type.encode(&mut buf);
println!("buf: {:?}", buf);
}
}
+3
View File
@@ -0,0 +1,3 @@
# Overwrite the root's disallowed-macros,
# as sqltest is allowed to use println and friends.
disallowed-macros = []
+3
View File
@@ -1,2 +1,5 @@
// CLI commands are allowed to use println and friends.
#![allow(clippy::disallowed_macros)]
pub mod start;
pub mod version;
+2 -2
View File
@@ -7,7 +7,7 @@ use std::path::Path;
/// Otherwise if the directory does exist, do nothing.
pub fn create_dir_or_err(path: &str) -> anyhow::Result<()> {
if !Path::new(path).is_dir() {
println!("Creating directory {}", path);
log::info!("Creating directory {}", path);
std::fs::create_dir_all(path)?;
}
Ok(())
@@ -19,7 +19,7 @@ pub fn create_dir_or_err(path: &str) -> anyhow::Result<()> {
pub fn create_file_with_contents(path: &str, contents: &str) -> anyhow::Result<()> {
create_dir_or_err(Path::new(path).parent().unwrap().to_str().unwrap())?;
if !Path::new(path).is_file() {
println!("Creating file {}", path);
log::info!("Creating file {}", path);
std::fs::write(path, contents)?;
}
Ok(())
+2
View File
@@ -549,6 +549,8 @@ pub fn create_game_data() -> GameData {
#[cfg(test)]
mod tests {
#![allow(clippy::disallowed_macros)]
use super::*;
use crate::dsl::{prefix_op, query, value};
use crate::program::Program;
+2
View File
@@ -20,6 +20,8 @@ fn fib(n: u64) -> u64 {
}
fn main() {
#![allow(clippy::disallowed_macros)]
let mut args = env::args().skip(1);
let first = args.next();
+2
View File
@@ -153,6 +153,8 @@ pub(crate) fn check_types(env: &mut EnvTy, ast: &ExprOpt) -> Result<Ty, ErrorTyp
#[cfg(test)]
mod tests {
#![allow(clippy::disallowed_macros)]
use spacetimedb_lib::identity::AuthCtx;
use spacetimedb_sats::algebraic_type::AlgebraicType;
use spacetimedb_sats::builtin_type::BuiltinType;