Skip to content

Commit 295bdc0

Browse files
committed
Auto merge of #10759 - blyxyas:unset_opt_env_unwrap, r=flip1995
Now `option_env_unwrap` warns even if a variable isn't set at compiletime Fixes #10742 changelog: Fix false negative where `option_env_unwrap` wouldn't warn if the env variable isn't set at compile-time.
2 parents d446378 + 3bfccac commit 295bdc0

File tree

3 files changed

+36
-22
lines changed

3 files changed

+36
-22
lines changed

clippy_lints/src/option_env_unwrap.rs

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::is_direct_expn_of;
3-
use if_chain::if_chain;
43
use rustc_ast::ast::{Expr, ExprKind, MethodCall};
54
use rustc_lint::{EarlyContext, EarlyLintPass};
65
use rustc_session::{declare_lint_pass, declare_tool_lint};
7-
use rustc_span::sym;
6+
use rustc_span::{sym, Span};
87

98
declare_clippy_lint! {
109
/// ### What it does
@@ -36,21 +35,27 @@ declare_lint_pass!(OptionEnvUnwrap => [OPTION_ENV_UNWRAP]);
3635

3736
impl EarlyLintPass for OptionEnvUnwrap {
3837
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
39-
if_chain! {
40-
if let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &expr.kind;
41-
if matches!(seg.ident.name, sym::expect | sym::unwrap);
42-
if let ExprKind::Call(caller, _) = &receiver.kind;
43-
if is_direct_expn_of(caller.span, "option_env").is_some();
44-
then {
45-
span_lint_and_help(
46-
cx,
47-
OPTION_ENV_UNWRAP,
48-
expr.span,
49-
"this will panic at run-time if the environment variable doesn't exist at compile-time",
50-
None,
51-
"consider using the `env!` macro instead"
52-
);
53-
}
38+
fn lint(cx: &EarlyContext<'_>, span: Span) {
39+
span_lint_and_help(
40+
cx,
41+
OPTION_ENV_UNWRAP,
42+
span,
43+
"this will panic at run-time if the environment variable doesn't exist at compile-time",
44+
None,
45+
"consider using the `env!` macro instead",
46+
);
5447
}
48+
49+
if let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &expr.kind &&
50+
matches!(seg.ident.name, sym::expect | sym::unwrap) {
51+
if let ExprKind::Call(caller, _) = &receiver.kind &&
52+
// If it exists, it will be ::core::option::Option::Some("<env var>").unwrap() (A method call in the HIR)
53+
is_direct_expn_of(caller.span, "option_env").is_some() {
54+
lint(cx, expr.span);
55+
} else if let ExprKind::Path(_, caller) = &receiver.kind && // If it doesn't exist, it will be ::core::option::Option::None::<&'static str>.unwrap() (A path in the HIR)
56+
is_direct_expn_of(caller.span, "option_env").is_some() {
57+
lint(cx, expr.span);
58+
}
59+
}
5560
}
5661
}

tests/ui/option_env_unwrap.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use proc_macros::{external, inline_macros};
99
fn main() {
1010
let _ = option_env!("PATH").unwrap();
1111
let _ = option_env!("PATH").expect("environment variable PATH isn't set");
12+
let _ = option_env!("__Y__do_not_use").unwrap(); // This test only works if you don't have a __Y__do_not_use env variable in your environment.
1213
let _ = inline!(option_env!($"PATH").unwrap());
1314
let _ = inline!(option_env!($"PATH").expect($"environment variable PATH isn't set"));
1415
let _ = external!(option_env!($"PATH").unwrap());

tests/ui/option_env_unwrap.stderr

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,15 @@ LL | let _ = option_env!("PATH").expect("environment variable PATH isn't set
1616
= help: consider using the `env!` macro instead
1717

1818
error: this will panic at run-time if the environment variable doesn't exist at compile-time
19-
--> $DIR/option_env_unwrap.rs:12:21
19+
--> $DIR/option_env_unwrap.rs:12:13
20+
|
21+
LL | let _ = option_env!("__Y__do_not_use").unwrap(); // This test only works if you don't have a __Y__do_not_use env variable in your env...
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
|
24+
= help: consider using the `env!` macro instead
25+
26+
error: this will panic at run-time if the environment variable doesn't exist at compile-time
27+
--> $DIR/option_env_unwrap.rs:13:21
2028
|
2129
LL | let _ = inline!(option_env!($"PATH").unwrap());
2230
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -25,7 +33,7 @@ LL | let _ = inline!(option_env!($"PATH").unwrap());
2533
= note: this error originates in the macro `__inline_mac_fn_main` (in Nightly builds, run with -Z macro-backtrace for more info)
2634

2735
error: this will panic at run-time if the environment variable doesn't exist at compile-time
28-
--> $DIR/option_env_unwrap.rs:13:21
36+
--> $DIR/option_env_unwrap.rs:14:21
2937
|
3038
LL | let _ = inline!(option_env!($"PATH").expect($"environment variable PATH isn't set"));
3139
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -34,7 +42,7 @@ LL | let _ = inline!(option_env!($"PATH").expect($"environment variable PATH
3442
= note: this error originates in the macro `__inline_mac_fn_main` (in Nightly builds, run with -Z macro-backtrace for more info)
3543

3644
error: this will panic at run-time if the environment variable doesn't exist at compile-time
37-
--> $DIR/option_env_unwrap.rs:14:13
45+
--> $DIR/option_env_unwrap.rs:15:13
3846
|
3947
LL | let _ = external!(option_env!($"PATH").unwrap());
4048
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -43,13 +51,13 @@ LL | let _ = external!(option_env!($"PATH").unwrap());
4351
= note: this error originates in the macro `external` (in Nightly builds, run with -Z macro-backtrace for more info)
4452

4553
error: this will panic at run-time if the environment variable doesn't exist at compile-time
46-
--> $DIR/option_env_unwrap.rs:15:13
54+
--> $DIR/option_env_unwrap.rs:16:13
4755
|
4856
LL | let _ = external!(option_env!($"PATH").expect($"environment variable PATH isn't set"));
4957
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5058
|
5159
= help: consider using the `env!` macro instead
5260
= note: this error originates in the macro `external` (in Nightly builds, run with -Z macro-backtrace for more info)
5361

54-
error: aborting due to 6 previous errors
62+
error: aborting due to 7 previous errors
5563

0 commit comments

Comments
 (0)