Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ builtin_macros_duplicate_macro_attribute = duplicated attribute
builtin_macros_env_not_defined = environment variable `{$var}` not defined at compile time
.cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead
.cargo_typo = there is a similar Cargo environment variable: `{$suggested_var}`
.custom = use `std::env::var({$var_expr})` to read the variable at run time
builtin_macros_env_not_unicode = environment variable `{$var}` is not a valid Unicode string
Expand Down
53 changes: 53 additions & 0 deletions compiler/rustc_builtin_macros/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_ast::token::{self, LitKind};
use rustc_ast::tokenstream::TokenStream;
use rustc_ast::{ExprKind, GenericArg, Mutability};
use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacEager, MacroExpanderResult};
use rustc_span::edit_distance::edit_distance;
use rustc_span::{Ident, Span, Symbol, kw, sym};
use thin_vec::thin_vec;

Expand Down Expand Up @@ -144,6 +145,12 @@ pub(crate) fn expand_env<'cx>(
if let Some(msg_from_user) = custom_msg {
cx.dcx()
.emit_err(errors::EnvNotDefinedWithUserMessage { span, msg_from_user })
} else if let Some(suggested_var) = find_similar_cargo_var(var.as_str()) {
cx.dcx().emit_err(errors::EnvNotDefined::CargoEnvVarTypo {
span,
var: *symbol,
suggested_var: Symbol::intern(suggested_var),
})
} else if is_cargo_env_var(var.as_str()) {
cx.dcx().emit_err(errors::EnvNotDefined::CargoEnvVar {
span,
Expand Down Expand Up @@ -176,3 +183,49 @@ fn is_cargo_env_var(var: &str) -> bool {
|| var.starts_with("DEP_")
|| matches!(var, "OUT_DIR" | "OPT_LEVEL" | "PROFILE" | "HOST" | "TARGET")
}

const KNOWN_CARGO_VARS: &[&str] = &[
// List of known Cargo environment variables that are set for crates (not build scripts, OUT_DIR etc).
// See: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates
Comment on lines +187 to +189
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion (non-blocking): I have two reservations about making this suggestion:

  1. Wouldn't this depend on the specific cargo version?
  2. Basically, what I was trying to get at, is that in principle I feel like rustc should not make assumptions about cargo (that is, rustc should not "know" about cargo's existence), because cargo isn't the only way to invoke rustc.

I guess I could be convinced by "well, most users do build with cargo", so don't consider this a hard-blocking concern.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I'm not sure, there is a similar list for this in rust-analyzer: https://github.com/chenyukang/rust/blob/702bf000a0b5062d8b5ced717a26d70919df6ba0/src/tools/rust-analyzer/crates/project-model/src/env.rs#L9-L49 , I guess there is a better and general way to keep it synced.
  2. I agree rustc should not knowing details about cargo, I think it's ok since we need to check the prefix with CARGO_ of env:
    https://github.com/rust-lang/rust/pull/148559/files#diff-0eaa1095d59f2f3d9c015005d15fd8f7cda13121a8404d77b2955fc5fcd40449R213, and it's a note but not code suggestions, it's maybe incorrect for rare scenarios.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, seems alright, was just curious to hear what you think.

"CARGO_PKG_VERSION",
"CARGO_PKG_VERSION_MAJOR",
"CARGO_PKG_VERSION_MINOR",
"CARGO_PKG_VERSION_PATCH",
"CARGO_PKG_VERSION_PRE",
"CARGO_PKG_AUTHORS",
"CARGO_PKG_NAME",
"CARGO_PKG_DESCRIPTION",
"CARGO_PKG_HOMEPAGE",
"CARGO_PKG_REPOSITORY",
"CARGO_PKG_LICENSE",
"CARGO_PKG_LICENSE_FILE",
"CARGO_PKG_RUST_VERSION",
"CARGO_PKG_README",
"CARGO_MANIFEST_DIR",
"CARGO_MANIFEST_PATH",
"CARGO_CRATE_NAME",
"CARGO_BIN_NAME",
"CARGO_PRIMARY_PACKAGE",
];

fn find_similar_cargo_var(var: &str) -> Option<&'static str> {
if !var.starts_with("CARGO_") {
return None;
}

let lookup_len = var.chars().count();
let max_dist = std::cmp::max(lookup_len, 3) / 3;
let mut best_match = None;
let mut best_distance = usize::MAX;

for &known_var in KNOWN_CARGO_VARS {
if let Some(distance) = edit_distance(var, known_var, max_dist) {
if distance < best_distance {
best_distance = distance;
best_match = Some(known_var);
}
}
}

best_match
}
8 changes: 8 additions & 0 deletions compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,14 @@ pub(crate) enum EnvNotDefined<'a> {
var_expr: &'a rustc_ast::Expr,
},
#[diag(builtin_macros_env_not_defined)]
#[help(builtin_macros_cargo_typo)]
CargoEnvVarTypo {
#[primary_span]
span: Span,
var: Symbol,
suggested_var: Symbol,
},
#[diag(builtin_macros_env_not_defined)]
#[help(builtin_macros_custom)]
CustomEnvVar {
#[primary_span]
Expand Down
50 changes: 50 additions & 0 deletions tests/ui/env-macro/env-cargo-var-typo-issue-148439.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//@ edition: 2021

// Regression test for issue #148439
// Ensure that when using misspelled Cargo environment variables in env!(),

fn test_cargo_package_version() {
let _ = env!("CARGO_PACKAGE_VERSION");
//~^ ERROR environment variable `CARGO_PACKAGE_VERSION` not defined at compile time
//~| HELP there is a similar Cargo environment variable: `CARGO_PKG_VERSION`
}

fn test_cargo_package_name() {
let _ = env!("CARGO_PACKAGE_NAME");
//~^ ERROR environment variable `CARGO_PACKAGE_NAME` not defined at compile time
//~| HELP there is a similar Cargo environment variable: `CARGO_PKG_NAME`
}

fn test_cargo_package_authors() {
let _ = env!("CARGO_PACKAGE_AUTHORS");
//~^ ERROR environment variable `CARGO_PACKAGE_AUTHORS` not defined at compile time
//~| HELP there is a similar Cargo environment variable: `CARGO_PKG_AUTHORS`
}

fn test_cargo_manifest_directory() {
let _ = env!("CARGO_MANIFEST_DIRECTORY");
//~^ ERROR environment variable `CARGO_MANIFEST_DIRECTORY` not defined at compile time
//~| HELP there is a similar Cargo environment variable: `CARGO_MANIFEST_DIR`
}

fn test_cargo_pkg_version_typo() {
let _ = env!("CARGO_PKG_VERSIO");
//~^ ERROR environment variable `CARGO_PKG_VERSIO` not defined at compile time
//~| HELP there is a similar Cargo environment variable: `CARGO_PKG_VERSION`
}

fn test_non_cargo_var() {
// Non-Cargo variable should get different help message
let _ = env!("MY_CUSTOM_VAR");
//~^ ERROR environment variable `MY_CUSTOM_VAR` not defined at compile time
//~| HELP use `std::env::var("MY_CUSTOM_VAR")` to read the variable at run time
}

fn test_cargo_unknown_var() {
// Cargo-prefixed but not similar to any known variable
let _ = env!("CARGO_SOMETHING_TOTALLY_UNKNOWN");
//~^ ERROR environment variable `CARGO_SOMETHING_TOTALLY_UNKNOWN` not defined at compile time
//~| HELP Cargo sets build script variables at run time. Use `std::env::var("CARGO_SOMETHING_TOTALLY_UNKNOWN")` instead
}

fn main() {}
58 changes: 58 additions & 0 deletions tests/ui/env-macro/env-cargo-var-typo-issue-148439.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
error: environment variable `CARGO_PACKAGE_VERSION` not defined at compile time
--> $DIR/env-cargo-var-typo-issue-148439.rs:7:13
|
LL | let _ = env!("CARGO_PACKAGE_VERSION");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: there is a similar Cargo environment variable: `CARGO_PKG_VERSION`

error: environment variable `CARGO_PACKAGE_NAME` not defined at compile time
--> $DIR/env-cargo-var-typo-issue-148439.rs:13:13
|
LL | let _ = env!("CARGO_PACKAGE_NAME");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: there is a similar Cargo environment variable: `CARGO_PKG_NAME`

error: environment variable `CARGO_PACKAGE_AUTHORS` not defined at compile time
--> $DIR/env-cargo-var-typo-issue-148439.rs:19:13
|
LL | let _ = env!("CARGO_PACKAGE_AUTHORS");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: there is a similar Cargo environment variable: `CARGO_PKG_AUTHORS`

error: environment variable `CARGO_MANIFEST_DIRECTORY` not defined at compile time
--> $DIR/env-cargo-var-typo-issue-148439.rs:25:13
|
LL | let _ = env!("CARGO_MANIFEST_DIRECTORY");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: there is a similar Cargo environment variable: `CARGO_MANIFEST_DIR`

error: environment variable `CARGO_PKG_VERSIO` not defined at compile time
--> $DIR/env-cargo-var-typo-issue-148439.rs:31:13
|
LL | let _ = env!("CARGO_PKG_VERSIO");
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: there is a similar Cargo environment variable: `CARGO_PKG_VERSION`

error: environment variable `MY_CUSTOM_VAR` not defined at compile time
--> $DIR/env-cargo-var-typo-issue-148439.rs:38:13
|
LL | let _ = env!("MY_CUSTOM_VAR");
| ^^^^^^^^^^^^^^^^^^^^^
|
= help: use `std::env::var("MY_CUSTOM_VAR")` to read the variable at run time

error: environment variable `CARGO_SOMETHING_TOTALLY_UNKNOWN` not defined at compile time
--> $DIR/env-cargo-var-typo-issue-148439.rs:45:13
|
LL | let _ = env!("CARGO_SOMETHING_TOTALLY_UNKNOWN");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Cargo sets build script variables at run time. Use `std::env::var("CARGO_SOMETHING_TOTALLY_UNKNOWN")` instead

error: aborting due to 7 previous errors

Loading