refactor: Migrate some cases to expect/reason

Thought of this when reviewing new cases for `std::env` in #16131.
This commit is contained in:
Ed Page
2026-01-02 15:41:46 -06:00
parent b8ba7ae88d
commit 91fbe9fbdd
15 changed files with 81 additions and 62 deletions

View File

@@ -8,15 +8,19 @@ fn main() {
commit_info();
compress_man();
windows_manifest();
// ALLOWED: Accessing environment during build time shouldn't be prohibited.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "not `cargo`, not needing to load from config"
)]
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)]
#[expect(
clippy::disallowed_methods,
reason = "not `cargo`, not needing to load from config"
)]
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()
@@ -114,8 +118,10 @@ fn commit_info_from_rustc_source_tarball() -> Option<CommitInfo> {
fn commit_info() {
// Var set by bootstrap whenever omit-git-hash is enabled in rust-lang/rust's config.toml.
println!("cargo:rerun-if-env-changed=CFG_OMIT_GIT_HASH");
// ALLOWED: Accessing environment during build time shouldn't be prohibited.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "not `cargo`, not needing to load from config"
)]
if std::env::var_os("CFG_OMIT_GIT_HASH").is_some() {
return;
}
@@ -129,7 +135,10 @@ fn commit_info() {
println!("cargo:rustc-env=CARGO_COMMIT_DATE={}", git.date);
}
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "not `cargo`, not needing to load from config"
)]
fn windows_manifest() {
use std::env;
let target_os = env::var("CARGO_CFG_TARGET_OS");

View File

@@ -1,5 +1,3 @@
#![allow(clippy::self_named_module_files)] // false positive in `commands/build.rs`
use cargo::core::features;
use cargo::core::shell::Shell;
use cargo::util::network::http::http_handle;
@@ -96,7 +94,7 @@ where
+ Send
+ Sync,
{
#![allow(clippy::disallowed_methods)]
#![expect(clippy::disallowed_methods, reason = "runs before config is loaded")]
if env_to_bool(std::env::var_os("CARGO_LOG_PROFILE").as_deref()) {
let capture_args =

View File

@@ -624,10 +624,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
// 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)]
#[expect(clippy::disallowed_methods, reason = "consistency with rustc")]
if let Ok(show_backtraces) = std::env::var("RUST_BACKTRACE") {
if !built_with_debuginfo && show_backtraces != "0" {
build_error_context.push_str(&format!(
@@ -1077,10 +1074,10 @@ impl BuildOutput {
None => return false,
Some(n) => n,
};
// ALLOWED: the process of rustc bootstrapping reads this through
// `std::env`. We should make the behavior consistent. Also, we
// don't advertise this for bypassing nightly.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "consistency with rustc, not specified behavior"
)]
std::env::var("RUSTC_BOOTSTRAP")
.map_or(false, |var| var.split(',').any(|s| s == name))
};

View File

@@ -1534,15 +1534,17 @@ 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)]
#[expect(
clippy::disallowed_methods,
reason = "testing only, no reason for config support"
)]
if let Ok(override_channel) = env::var("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS") {
return override_channel;
}
// ALLOWED: the process of rustc bootstrapping reads this through
// `std::env`. We should make the behavior consistent. Also, we
// don't advertise this for bypassing nightly.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "consistency with rustc, not specified behavior"
)]
if let Ok(staging) = env::var("RUSTC_BOOTSTRAP") {
if staging == "1" {
return "dev".to_string();
@@ -1556,8 +1558,10 @@ pub fn channel() -> String {
/// 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)]
#[expect(
clippy::disallowed_methods,
reason = "testing only, no reason for config support"
)]
fn cargo_use_gitoxide_instead_of_git2() -> bool {
std::env::var_os("__CARGO_USE_GITOXIDE_INSTEAD_OF_GIT2").map_or(false, |value| value == "1")
}

View File

@@ -255,7 +255,7 @@ pub struct Gc<'a, 'gctx> {
/// This is important to be held, since we don't want multiple cargos to
/// be allowed to write to the cache at the same time, or for others to
/// read while we are modifying the cache.
#[allow(dead_code)] // Held for drop.
#[expect(dead_code, reason = "held for `drop`")]
lock: CacheLock<'gctx>,
}

View File

@@ -1791,7 +1791,10 @@ fn to_timestamp(t: &SystemTime) -> Timestamp {
///
/// If possible, try to avoid calling this too often since accessing clocks
/// can be a little slow on some systems.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "testing only, no reason for config support"
)]
fn now() -> Timestamp {
match std::env::var("__CARGO_TEST_LAST_USE_NOW") {
Ok(now) => now.parse().unwrap(),

View File

@@ -533,8 +533,10 @@ 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<usize> {
// ALLOWED: For testing cargo itself only.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "testing only, no reason for config support"
)]
if let Ok(width) = std::env::var("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS") {
return Some(width.parse().unwrap());
}
@@ -616,7 +618,10 @@ fn supports_unicode(stream: &dyn IsTerminal) -> bool {
}
fn supports_hyperlinks() -> bool {
#[allow(clippy::disallowed_methods)] // We are reading the state of the system, not config
#[expect(
clippy::disallowed_methods,
reason = "reading the state of the system, not config"
)]
if std::env::var_os("TERM_PROGRAM").as_deref() == Some(std::ffi::OsStr::new("iTerm.app")) {
// Override `supports_hyperlinks` as we have an unknown incompatibility with iTerm2
return false;
@@ -626,7 +631,10 @@ fn supports_hyperlinks() -> bool {
}
/// Determines whether the terminal supports ANSI OSC 9;4.
#[allow(clippy::disallowed_methods)] // Read environment variables to detect terminal
#[expect(
clippy::disallowed_methods,
reason = "reading the state of the system, not config"
)]
fn supports_term_integration(stream: &dyn IsTerminal) -> bool {
let windows_terminal = std::env::var("WT_SESSION").is_ok();
let conemu = std::env::var("ConEmuANSI").ok() == Some("ON".into());

View File

@@ -922,8 +922,10 @@ mod tests {
}
/// Check if `url` equals to the overridden crates.io URL.
// ALLOWED: For testing Cargo itself only.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "testing only, no reason for config support"
)]
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)
}

View File

@@ -133,11 +133,11 @@ impl NewOptions {
#[serde(rename_all = "kebab-case")]
struct CargoNewConfig {
#[deprecated = "cargo-new no longer supports adding the authors field"]
#[allow(dead_code)]
#[expect(dead_code, reason = "deprecated")]
name: Option<String>,
#[deprecated = "cargo-new no longer supports adding the authors field"]
#[allow(dead_code)]
#[expect(dead_code, reason = "deprecated")]
email: Option<String>,
#[serde(rename = "vcs")]

View File

@@ -678,9 +678,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<String> {
// ALLOWED: For the internal mechanism of `cargo fix` only.
// Shouldn't be set directly by anyone.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "internal only, no reason for config support"
)]
env::var(FIX_ENV_INTERNAL).ok()
}
@@ -1238,23 +1239,26 @@ impl FixArgs {
}
let file = file.ok_or_else(|| anyhow::anyhow!("could not find .rs file in rustc args"))?;
// ALLOWED: For the internal mechanism of `cargo fix` only.
// Shouldn't be set directly by anyone.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "internal only, no reason for config support"
)]
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)]
#[expect(
clippy::disallowed_methods,
reason = "internal only, no reason for config support"
)]
let prepare_for_edition = env::var(EDITION_ENV_INTERNAL).ok().map(|v| {
let enabled_edition = enabled_edition.unwrap_or(Edition::Edition2015);
let mode = EditionFixMode::from_str(&v);
mode.next_edition(enabled_edition)
});
// ALLOWED: For the internal mechanism of `cargo fix` only.
// Shouldn't be set directly by anyone.
#[allow(clippy::disallowed_methods)]
#[expect(
clippy::disallowed_methods,
reason = "internal only, no reason for config support"
)]
let sysroot = env::var_os(SYSROOT_INTERNAL).map(PathBuf::from);
Ok(FixArgs {

View File

@@ -13,7 +13,7 @@ use std::collections::{HashMap, HashSet};
#[derive(Debug, Copy, Clone)]
pub struct NodeId {
index: usize,
#[allow(dead_code)] // intended for `derive(Debug)`
#[expect(dead_code, reason = "intended for `derive(Debug)`")]
debug: InternedString,
}

View File

@@ -69,11 +69,7 @@ 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 the config system in [`GlobalContext`], please use
// `std::env::var{_os}` and justify the validity of the usage.
#[allow(clippy::disallowed_methods)]
#[expect(clippy::disallowed_methods, reason = "seeds `GlobalContext::get_env`")]
let env: HashMap<_, _> = std::env::vars_os().collect();
let (case_insensitive_env, normalized_env) = make_case_insensitive_and_normalized_env(&env);
Self {

View File

@@ -2269,8 +2269,7 @@ pub fn save_credentials(
}
#[cfg(not(unix))]
#[allow(unused)]
fn set_permissions(file: &File, mode: u32) -> CargoResult<()> {
fn set_permissions(_file: &File, _mode: u32) -> CargoResult<()> {
Ok(())
}
}

View File

@@ -32,9 +32,10 @@ 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)]
#[expect(
clippy::disallowed_methods,
reason = "testing only, no reason for config support"
)]
if env::var("__CARGO_TEST_SETSID_PLEASE_DONT_USE_ELSEWHERE").is_ok() {
// SAFETY: I'm unaware of any safety requirements for this function.
unsafe {

View File

@@ -75,9 +75,7 @@ mod vcs;
mod workspace;
pub fn is_rustup() -> bool {
// ALLOWED: `RUSTUP_HOME` should only be read from process env, otherwise
// other tools may point to executables from incompatible distributions.
#[allow(clippy::disallowed_methods)]
#[expect(clippy::disallowed_methods, reason = "consistency with rustup")]
std::env::var_os("RUSTUP_HOME").is_some()
}