about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-07-27 19:52:46 +0000
committerbors <bors@rust-lang.org>2023-07-27 19:52:46 +0000
commit295bdc028f3ed7a314469c4a5a2c98b38a77db30 (patch)
tree5c0699afb5ceb647e055a9910bf200ad8229d858
parentd446378143d23425f5763207359099975ece0da4 (diff)
parent3bfccacca97cb7d4d7751fc90b89b3381b8b8b35 (diff)
downloadrust-295bdc028f3ed7a314469c4a5a2c98b38a77db30.tar.gz
rust-295bdc028f3ed7a314469c4a5a2c98b38a77db30.zip
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.
-rw-r--r--clippy_lints/src/option_env_unwrap.rs39
-rw-r--r--tests/ui/option_env_unwrap.rs1
-rw-r--r--tests/ui/option_env_unwrap.stderr18
3 files changed, 36 insertions, 22 deletions
diff --git a/clippy_lints/src/option_env_unwrap.rs b/clippy_lints/src/option_env_unwrap.rs
index 377bddeaa5f..9c7f7e1cd7f 100644
--- a/clippy_lints/src/option_env_unwrap.rs
+++ b/clippy_lints/src/option_env_unwrap.rs
@@ -1,10 +1,9 @@
 use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::is_direct_expn_of;
-use if_chain::if_chain;
 use rustc_ast::ast::{Expr, ExprKind, MethodCall};
 use rustc_lint::{EarlyContext, EarlyLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::sym;
+use rustc_span::{sym, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -36,21 +35,27 @@ declare_lint_pass!(OptionEnvUnwrap => [OPTION_ENV_UNWRAP]);
 
 impl EarlyLintPass for OptionEnvUnwrap {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
-        if_chain! {
-            if let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &expr.kind;
-            if matches!(seg.ident.name, sym::expect | sym::unwrap);
-            if let ExprKind::Call(caller, _) = &receiver.kind;
-            if is_direct_expn_of(caller.span, "option_env").is_some();
-            then {
-                span_lint_and_help(
-                    cx,
-                    OPTION_ENV_UNWRAP,
-                    expr.span,
-                    "this will panic at run-time if the environment variable doesn't exist at compile-time",
-                    None,
-                    "consider using the `env!` macro instead"
-                );
-            }
+        fn lint(cx: &EarlyContext<'_>, span: Span) {
+            span_lint_and_help(
+                cx,
+                OPTION_ENV_UNWRAP,
+                span,
+                "this will panic at run-time if the environment variable doesn't exist at compile-time",
+                None,
+                "consider using the `env!` macro instead",
+            );
         }
+
+        if let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &expr.kind &&
+		matches!(seg.ident.name, sym::expect | sym::unwrap) {
+			if let ExprKind::Call(caller, _) = &receiver.kind &&
+            // If it exists, it will be ::core::option::Option::Some("<env var>").unwrap() (A method call in the HIR)
+            is_direct_expn_of(caller.span, "option_env").is_some() {
+				lint(cx, expr.span);
+			} 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)
+            is_direct_expn_of(caller.span, "option_env").is_some() {
+				lint(cx, expr.span);
+			}
+		}
     }
 }
diff --git a/tests/ui/option_env_unwrap.rs b/tests/ui/option_env_unwrap.rs
index 65a1b467f81..61dbad939db 100644
--- a/tests/ui/option_env_unwrap.rs
+++ b/tests/ui/option_env_unwrap.rs
@@ -9,6 +9,7 @@ use proc_macros::{external, inline_macros};
 fn main() {
     let _ = option_env!("PATH").unwrap();
     let _ = option_env!("PATH").expect("environment variable PATH isn't set");
+    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.
     let _ = inline!(option_env!($"PATH").unwrap());
     let _ = inline!(option_env!($"PATH").expect($"environment variable PATH isn't set"));
     let _ = external!(option_env!($"PATH").unwrap());
diff --git a/tests/ui/option_env_unwrap.stderr b/tests/ui/option_env_unwrap.stderr
index 7bba62686ee..cfa9dd58a30 100644
--- a/tests/ui/option_env_unwrap.stderr
+++ b/tests/ui/option_env_unwrap.stderr
@@ -16,7 +16,15 @@ LL |     let _ = option_env!("PATH").expect("environment variable PATH isn't set
    = help: consider using the `env!` macro instead
 
 error: this will panic at run-time if the environment variable doesn't exist at compile-time
-  --> $DIR/option_env_unwrap.rs:12:21
+  --> $DIR/option_env_unwrap.rs:12:13
+   |
+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...
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using the `env!` macro instead
+
+error: this will panic at run-time if the environment variable doesn't exist at compile-time
+  --> $DIR/option_env_unwrap.rs:13:21
    |
 LL |     let _ = inline!(option_env!($"PATH").unwrap());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -25,7 +33,7 @@ LL |     let _ = inline!(option_env!($"PATH").unwrap());
    = note: this error originates in the macro `__inline_mac_fn_main` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: this will panic at run-time if the environment variable doesn't exist at compile-time
-  --> $DIR/option_env_unwrap.rs:13:21
+  --> $DIR/option_env_unwrap.rs:14:21
    |
 LL |     let _ = inline!(option_env!($"PATH").expect($"environment variable PATH isn't set"));
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -34,7 +42,7 @@ LL |     let _ = inline!(option_env!($"PATH").expect($"environment variable PATH
    = note: this error originates in the macro `__inline_mac_fn_main` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: this will panic at run-time if the environment variable doesn't exist at compile-time
-  --> $DIR/option_env_unwrap.rs:14:13
+  --> $DIR/option_env_unwrap.rs:15:13
    |
 LL |     let _ = external!(option_env!($"PATH").unwrap());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -43,7 +51,7 @@ LL |     let _ = external!(option_env!($"PATH").unwrap());
    = note: this error originates in the macro `external` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: this will panic at run-time if the environment variable doesn't exist at compile-time
-  --> $DIR/option_env_unwrap.rs:15:13
+  --> $DIR/option_env_unwrap.rs:16:13
    |
 LL |     let _ = external!(option_env!($"PATH").expect($"environment variable PATH isn't set"));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -51,5 +59,5 @@ LL |     let _ = external!(option_env!($"PATH").expect($"environment variable PA
    = help: consider using the `env!` macro instead
    = note: this error originates in the macro `external` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: aborting due to 6 previous errors
+error: aborting due to 7 previous errors