diff --git a/build.rs b/build.rs index 13e88935e09..752221f8cdc 100644 --- a/build.rs +++ b/build.rs @@ -7,13 +7,15 @@ use std::process::Command; fn main() { commit_info(); compress_man(); - println!( - "cargo:rustc-env=RUST_HOST_TARGET={}", - std::env::var("TARGET").unwrap() - ); + // ALLOWED: Accessing environment during build time shouldn't be prohibited. + #[allow(clippy::disallowed_methods)] + let target = std::env::var("TARGET").unwrap(); + println!("cargo:rustc-env=RUST_HOST_TARGET={target}"); } fn compress_man() { + // ALLOWED: Accessing environment during build time shouldn't be prohibited. + #[allow(clippy::disallowed_methods)] let out_path = Path::new(&std::env::var("OUT_DIR").unwrap()).join("man.tgz"); let dst = fs::File::create(out_path).unwrap(); let encoder = GzBuilder::new() diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000000..4f9be8f9b08 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,6 @@ +disallowed-methods = [ + { path = "std::env::var", reason = "Use `Config::get_env` instead. See rust-lang/cargo#11588" }, + { path = "std::env::var_os", reason = "Use `Config::get_env_os` instead. See rust-lang/cargo#11588" }, + { path = "std::env::vars", reason = "Not recommended to use in Cargo. See rust-lang/cargo#11588" }, + { path = "std::env::vars_os", reason = "Not recommended to use in Cargo. See rust-lang/cargo#11588" }, +] diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 99b0ccef21e..f84ec53ad68 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -413,6 +413,9 @@ impl GlobalArgs { } pub fn cli() -> Command { + // ALLOWED: `RUSTUP_HOME` should only be read from process env, otherwise + // other tools may point to executables from incompatible distributions. + #[allow(clippy::disallowed_methods)] let is_rustup = std::env::var_os("RUSTUP_HOME").is_some(); let usage = if is_rustup { "cargo [+toolchain] [OPTIONS] [COMMAND]" diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index 28cd18d248b..55da2997fdb 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -1,5 +1,6 @@ #![warn(rust_2018_idioms)] // while we're getting used to 2018 #![allow(clippy::all)] +#![warn(clippy::disallowed_methods)] use cargo::util::toml::StringOrVec; use cargo::util::CliError; diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 40df2261abb..5728d0c8573 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -410,6 +410,10 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { // If we're opting into backtraces, mention that build dependencies' backtraces can // be improved by requesting debuginfo to be built, if we're not building with // debuginfo already. + // + // ALLOWED: Other tools like `rustc` might read it directly + // through `std::env`. We should make their behavior consistent. + #[allow(clippy::disallowed_methods)] if let Ok(show_backtraces) = std::env::var("RUST_BACKTRACE") { if !built_with_debuginfo && show_backtraces != "0" { build_error_context.push_str(&format!( @@ -727,6 +731,10 @@ impl BuildOutput { None => return false, Some(n) => n, }; + // ALLOWED: the process of rustc boostrapping reads this through + // `std::env`. We should make the behavior consistent. Also, we + // don't advertise this for bypassing nightly. + #[allow(clippy::disallowed_methods)] std::env::var("RUSTC_BOOTSTRAP") .map_or(false, |var| var.split(',').any(|s| s == name)) }; diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index e47ae07f844..7bd61d42967 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -764,6 +764,19 @@ pub enum StaleItem { } impl LocalFingerprint { + /// Read the environment variable of the given env `key`, and creates a new + /// [`LocalFingerprint::RerunIfEnvChanged`] for it. + /// + // TODO: This is allowed at this moment. Should figure out if it makes + // sense if permitting to read env from the config system. + #[allow(clippy::disallowed_methods)] + fn from_env>(key: K) -> LocalFingerprint { + let key = key.as_ref(); + let var = key.to_owned(); + let val = env::var(key).ok(); + LocalFingerprint::RerunIfEnvChanged { var, val } + } + /// Checks dynamically at runtime if this `LocalFingerprint` has a stale /// item inside of it. /// @@ -1661,10 +1674,7 @@ fn local_fingerprints_deps( local.extend( deps.rerun_if_env_changed .iter() - .map(|var| LocalFingerprint::RerunIfEnvChanged { - var: var.clone(), - val: env::var(var).ok(), - }), + .map(LocalFingerprint::from_env), ); local diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 55ffa31424a..7f16e79cfa6 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -946,10 +946,7 @@ impl CliUnstable { self.add(flag, &mut warnings)?; } - if self.gitoxide.is_none() - && std::env::var_os("__CARGO_USE_GITOXIDE_INSTEAD_OF_GIT2") - .map_or(false, |value| value == "1") - { + if self.gitoxide.is_none() && cargo_use_gitoxide_instead_of_git2() { self.gitoxide = GitoxideFeatures::safe().into(); } Ok(warnings) @@ -1171,9 +1168,15 @@ impl CliUnstable { /// Returns the current release channel ("stable", "beta", "nightly", "dev"). pub fn channel() -> String { + // ALLOWED: For testing cargo itself only. + #[allow(clippy::disallowed_methods)] if let Ok(override_channel) = env::var("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS") { return override_channel; } + // ALLOWED: the process of rustc boostrapping reads this through + // `std::env`. We should make the behavior consistent. Also, we + // don't advertise this for bypassing nightly. + #[allow(clippy::disallowed_methods)] if let Ok(staging) = env::var("RUSTC_BOOTSTRAP") { if staging == "1" { return "dev".to_string(); @@ -1183,3 +1186,12 @@ pub fn channel() -> String { .release_channel .unwrap_or_else(|| String::from("dev")) } + +/// Only for testing and developing. See ["Running with gitoxide as default git backend in tests"][1]. +/// +/// [1]: https://doc.crates.io/contrib/tests/running.html#running-with-gitoxide-as-default-git-backend-in-tests +// ALLOWED: For testing cargo itself only. +#[allow(clippy::disallowed_methods)] +fn cargo_use_gitoxide_instead_of_git2() -> bool { + std::env::var_os("__CARGO_USE_GITOXIDE_INSTEAD_OF_GIT2").map_or(false, |value| value == "1") +} diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 6ea5b796a31..40bdb6c211d 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -15,6 +15,9 @@ pub struct ResolverProgress { time_to_print: Duration, printed: bool, deps_time: Duration, + /// Provides an escape hatch for machine with slow CPU for debugging and + /// testing Cargo itself. + /// See [rust-lang/cargo#6596](https://github.com/rust-lang/cargo/pull/6596) for more. #[cfg(debug_assertions)] slow_cpu_multiplier: u64, } @@ -31,6 +34,9 @@ impl ResolverProgress { // Architectures that do not have a modern processor, hardware emulation, etc. // In the test code we have `slow_cpu_multiplier`, but that is not accessible here. #[cfg(debug_assertions)] + // ALLOWED: For testing cargo itself only. However, it was communicated as an public + // interface to other developers, so keep it as-is, shouldn't add `__CARGO` prefix. + #[allow(clippy::disallowed_methods)] slow_cpu_multiplier: std::env::var("CARGO_TEST_SLOW_CPU_MULTIPLIER") .ok() .and_then(|m| m.parse().ok()) diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs index 79f09e6f9ce..fdae617c4f8 100644 --- a/src/cargo/core/shell.rs +++ b/src/cargo/core/shell.rs @@ -17,6 +17,8 @@ impl TtyWidth { /// Returns the width of the terminal to use for diagnostics (which is /// relayed to rustc via `--diagnostic-width`). pub fn diagnostic_terminal_width(&self) -> Option { + // ALLOWED: For testing cargo itself only. + #[allow(clippy::disallowed_methods)] if let Ok(width) = std::env::var("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS") { return Some(width.parse().unwrap()); } diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 67dbab7b958..034d7ed590c 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -428,9 +428,7 @@ impl SourceId { _ => return false, } let url = self.inner.url.as_str(); - url == CRATES_IO_INDEX - || url == CRATES_IO_HTTP_INDEX - || std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS").as_deref() == Ok(url) + url == CRATES_IO_INDEX || url == CRATES_IO_HTTP_INDEX || is_overridden_crates_io_url(url) } /// Hashes `self`. @@ -884,3 +882,10 @@ mod tests { assert_eq!(source_id, deserialized); } } + +/// Check if `url` equals to the overridden crates.io URL. +// ALLOWED: For testing Cargo itself only. +#[allow(clippy::disallowed_methods)] +fn is_overridden_crates_io_url(url: &str) -> bool { + std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS").map_or(false, |v| v == url) +} diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 8ebc63ba1d4..3a5a9d20a95 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -4,6 +4,7 @@ // Due to some of the default clippy lints being somewhat subjective and not // necessarily an improvement, we prefer to not use them at this time. #![allow(clippy::all)] +#![warn(clippy::disallowed_methods)] #![warn(clippy::self_named_module_files)] #![allow(rustdoc::private_intra_doc_links)] diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 9cc2fddad28..be24967f8b9 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -17,10 +17,10 @@ //! Cargo begins a normal `cargo check` operation with itself set as a proxy //! for rustc by setting `primary_unit_rustc` in the build config. When //! cargo launches rustc to check a crate, it is actually launching itself. -//! The `FIX_ENV` environment variable is set so that cargo knows it is in +//! The `FIX_ENV_INTERNAL` environment variable is set so that cargo knows it is in //! fix-proxy-mode. //! -//! Each proxied cargo-as-rustc detects it is in fix-proxy-mode (via `FIX_ENV` +//! Each proxied cargo-as-rustc detects it is in fix-proxy-mode (via `FIX_ENV_INTERNAL` //! environment variable in `main`) and does the following: //! //! - Acquire a lock from the `LockServer` from the master cargo process. @@ -63,10 +63,20 @@ use crate::util::Config; use crate::util::{existing_vcs_repo, LockServer, LockServerClient}; use crate::{drop_eprint, drop_eprintln}; -const FIX_ENV: &str = "__CARGO_FIX_PLZ"; -const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE"; -const EDITION_ENV: &str = "__CARGO_FIX_EDITION"; -const IDIOMS_ENV: &str = "__CARGO_FIX_IDIOMS"; +/// **Internal only.** +/// Indicates Cargo is in fix-proxy-mode if presents. +/// The value of it is the socket address of the [`LockServer`] being used. +/// See the [module-level documentation](mod@super::fix) for more. +const FIX_ENV_INTERNAL: &str = "__CARGO_FIX_PLZ"; +/// **Internal only.** +/// For passing [`FixOptions::broken_code`] through to cargo running in proxy mode. +const BROKEN_CODE_ENV_INTERNAL: &str = "__CARGO_FIX_BROKEN_CODE"; +/// **Internal only.** +/// For passing [`FixOptions::edition`] through to cargo running in proxy mode. +const EDITION_ENV_INTERNAL: &str = "__CARGO_FIX_EDITION"; +/// **Internal only.** +/// For passing [`FixOptions::idioms`] through to cargo running in proxy mode. +const IDIOMS_ENV_INTERNAL: &str = "__CARGO_FIX_IDIOMS"; pub struct FixOptions { pub edition: bool, @@ -87,20 +97,20 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions) -> CargoResult<()> { // Spin up our lock server, which our subprocesses will use to synchronize fixes. let lock_server = LockServer::new()?; let mut wrapper = ProcessBuilder::new(env::current_exe()?); - wrapper.env(FIX_ENV, lock_server.addr().to_string()); + wrapper.env(FIX_ENV_INTERNAL, lock_server.addr().to_string()); let _started = lock_server.start()?; opts.compile_opts.build_config.force_rebuild = true; if opts.broken_code { - wrapper.env(BROKEN_CODE_ENV, "1"); + wrapper.env(BROKEN_CODE_ENV_INTERNAL, "1"); } if opts.edition { - wrapper.env(EDITION_ENV, "1"); + wrapper.env(EDITION_ENV_INTERNAL, "1"); } if opts.idioms { - wrapper.env(IDIOMS_ENV, "1"); + wrapper.env(IDIOMS_ENV_INTERNAL, "1"); } *opts @@ -339,7 +349,10 @@ to prevent this issue from happening. /// Returns `None` if `fix` is not being run (not in proxy mode). Returns /// `Some(...)` if in `fix` proxy mode pub fn fix_get_proxy_lock_addr() -> Option { - env::var(FIX_ENV).ok() + // ALLOWED: For the internal mechanism of `cargo fix` only. + // Shouldn't be set directly by anyone. + #[allow(clippy::disallowed_methods)] + env::var(FIX_ENV_INTERNAL).ok() } /// Entry point for `cargo` running as a proxy for `rustc`. @@ -360,7 +373,7 @@ pub fn fix_exec_rustc(config: &Config, lock_addr: &str) -> CargoResult<()> { .ok(); let mut rustc = ProcessBuilder::new(&args.rustc).wrapped(workspace_rustc.as_ref()); rustc.retry_with_argfile(true); - rustc.env_remove(FIX_ENV); + rustc.env_remove(FIX_ENV_INTERNAL); args.apply(&mut rustc); trace!("start rustfixing {:?}", args.file); @@ -404,7 +417,7 @@ pub fn fix_exec_rustc(config: &Config, lock_addr: &str) -> CargoResult<()> { // user's code with our changes. Back out everything and fall through // below to recompile again. if !output.status.success() { - if config.get_env_os(BROKEN_CODE_ENV).is_none() { + if config.get_env_os(BROKEN_CODE_ENV_INTERNAL).is_none() { for (path, file) in fixes.files.iter() { debug!("reverting {:?} due to errors", path); paths::write(path, &file.original_code)?; @@ -578,7 +591,7 @@ fn rustfix_and_fix( // worse by applying fixes where a bug could cause *more* broken code. // Instead, punt upwards which will reexec rustc over the original code, // displaying pretty versions of the diagnostics we just read out. - if !output.status.success() && config.get_env_os(BROKEN_CODE_ENV).is_none() { + if !output.status.success() && config.get_env_os(BROKEN_CODE_ENV_INTERNAL).is_none() { debug!( "rustfixing `{:?}` failed, rustc exited with {:?}", filename, @@ -847,9 +860,15 @@ impl FixArgs { } let file = file.ok_or_else(|| anyhow::anyhow!("could not find .rs file in rustc args"))?; - let idioms = env::var(IDIOMS_ENV).is_ok(); - - let prepare_for_edition = env::var(EDITION_ENV).ok().map(|_| { + // ALLOWED: For the internal mechanism of `cargo fix` only. + // Shouldn't be set directly by anyone. + #[allow(clippy::disallowed_methods)] + let idioms = env::var(IDIOMS_ENV_INTERNAL).is_ok(); + + // ALLOWED: For the internal mechanism of `cargo fix` only. + // Shouldn't be set directly by anyone. + #[allow(clippy::disallowed_methods)] + let prepare_for_edition = env::var(EDITION_ENV_INTERNAL).ok().map(|_| { enabled_edition .unwrap_or(Edition::Edition2015) .saturating_next() diff --git a/src/cargo/util/config/environment.rs b/src/cargo/util/config/environment.rs index bb049546a61..0172c88c07f 100644 --- a/src/cargo/util/config/environment.rs +++ b/src/cargo/util/config/environment.rs @@ -66,6 +66,11 @@ pub struct Env { impl Env { /// Create a new `Env` from process's environment variables. pub fn new() -> Self { + // ALLOWED: This is the only permissible usage of `std::env::vars{_os}` + // within cargo. If you do need access to individual variables without + // interacting with `Config` system, please use `std::env::var{_os}` + // and justify the validity of the usage. + #[allow(clippy::disallowed_methods)] let env: HashMap<_, _> = std::env::vars_os().collect(); let (case_insensitive_env, normalized_env) = make_case_insensitive_and_normalized_env(&env); Self { diff --git a/src/cargo/util/job.rs b/src/cargo/util/job.rs index 9ff9b53042d..f2bcf94a26b 100644 --- a/src/cargo/util/job.rs +++ b/src/cargo/util/job.rs @@ -32,6 +32,9 @@ mod imp { // when-cargo-is-killed-subprocesses-are-also-killed, but that requires // one cargo spawned to become its own session leader, so we do that // here. + // + // ALLOWED: For testing cargo itself only. + #[allow(clippy::disallowed_methods)] if env::var("__CARGO_TEST_SETSID_PLEASE_DONT_USE_ELSEWHERE").is_ok() { libc::setsid(); } diff --git a/src/cargo/util/profile.rs b/src/cargo/util/profile.rs index c655b5488bc..79b544d98c8 100644 --- a/src/cargo/util/profile.rs +++ b/src/cargo/util/profile.rs @@ -22,6 +22,8 @@ pub struct Profiler { } fn enabled_level() -> Option { + // ALLOWED: for profiling Cargo itself, not intended to be used beyond Cargo contributors. + #[allow(clippy::disallowed_methods)] env::var("CARGO_PROFILE").ok().and_then(|s| s.parse().ok()) }