Skip to content

Commit 54db962

Browse files
UrgauUrgau
committed
Uplift clippy::option_env_unwrap to rustc
1 parent 2203df0 commit 54db962

File tree

6 files changed

+254
-0
lines changed

6 files changed

+254
-0
lines changed

‎compiler/rustc_lint/messages.ftl‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ lint_map_unit_fn = `Iterator::map` call that discard the iterator's values
3131
.map_label = after this call to map, the resulting iterator is `impl Iterator<Item = ()>`, which means the only information carried by the iterator is the number of items
3232
.suggestion = you might have meant to use `Iterator::for_each`
3333
34+
lint_option_env_unwrap = incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
35+
.suggestion = consider using the `env!` macro instead
36+
3437
lint_non_binding_let_on_sync_lock =
3538
non-binding let on a synchronization lock
3639

‎compiler/rustc_lint/src/lib.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ mod non_fmt_panic;
7272
mod nonstandard_style;
7373
mod noop_method_call;
7474
mod opaque_hidden_inferred_bound;
75+
mod option_env_unwrap;
7576
mod pass_by_value;
7677
mod passes;
7778
mod redundant_semicolon;
@@ -111,6 +112,7 @@ use non_fmt_panic::NonPanicFmt;
111112
use nonstandard_style::*;
112113
use noop_method_call::*;
113114
use opaque_hidden_inferred_bound::*;
115+
use option_env_unwrap::*;
114116
use pass_by_value::*;
115117
use redundant_semicolon::*;
116118
use traits::*;
@@ -211,6 +213,7 @@ late_lint_methods!(
211213
BoxPointers: BoxPointers,
212214
PathStatements: PathStatements,
213215
LetUnderscore: LetUnderscore,
216+
OptionEnvUnwrap: OptionEnvUnwrap,
214217
// Depends on referenced function signatures in expressions
215218
UnusedResults: UnusedResults,
216219
NonUpperCaseGlobals: NonUpperCaseGlobals,

‎compiler/rustc_lint/src/lints.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,27 @@ pub struct MappingToUnit {
801801
pub replace: String,
802802
}
803803

804+
// option_env_unwrap.rs
805+
#[derive(LintDiagnostic)]
806+
#[diag(lint_option_env_unwrap)]
807+
pub struct OptionEnvUnwrapDiag {
808+
#[subdiagnostic]
809+
pub suggestion: Option<OptionEnvUnwrapSuggestion>,
810+
}
811+
812+
#[derive(Subdiagnostic)]
813+
#[suggestion(
814+
lint_suggestion,
815+
style = "verbose",
816+
code = "env!({replace})",
817+
applicability = "machine-applicable"
818+
)]
819+
pub struct OptionEnvUnwrapSuggestion {
820+
#[primary_span]
821+
pub span: Span,
822+
pub replace: String,
823+
}
824+
804825
// internal.rs
805826
#[derive(LintDiagnostic)]
806827
#[diag(lint_default_hash_types)]

‎compiler/rustc_lint/src/option_env_unwrap.rs‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
use rustc_hir::{Expr, ExprKind};
2+
use rustc_session::{declare_lint, declare_lint_pass};
3+
use rustc_span::{sym, ExpnKind, MacroKind, Span};
4+
5+
use crate::{
6+
lints::{OptionEnvUnwrapDiag, OptionEnvUnwrapSuggestion},
7+
LateContext, LateLintPass, LintContext,
8+
};
9+
10+
declare_lint! {
11+
/// The `incorrect_option_env_unwraps` lint checks for usage of
12+
/// `option_env!(...).unwrap()` and suggests using the `env!` macro instead.
13+
///
14+
/// ### Example
15+
///
16+
/// ```rust,no_run
17+
/// let _ = option_env!("HOME").unwrap();
18+
/// ```
19+
///
20+
/// {{produces}}
21+
///
22+
/// ### Explanation
23+
///
24+
/// Unwrapping the result of `option_env!` will panic
25+
/// at run-time if the environment variable doesn't exist, whereas `env!`
26+
/// catches it at compile-time.
27+
INCORRECT_OPTION_ENV_UNWRAPS,
28+
Warn,
29+
"using `option_env!(...).unwrap()` to get environment variable"
30+
}
31+
32+
declare_lint_pass!(OptionEnvUnwrap => [INCORRECT_OPTION_ENV_UNWRAPS]);
33+
34+
impl<'tcx> LateLintPass<'tcx> for OptionEnvUnwrap {
35+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
36+
let ExprKind::MethodCall(_, receiver, _, _) = expr.kind else { return; };
37+
38+
let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) else { return; };
39+
40+
if !matches!(
41+
cx.tcx.get_diagnostic_name(method_def_id),
42+
Some(sym::option_unwrap | sym::option_expect)
43+
) {
44+
return;
45+
}
46+
47+
// Handle found environment variable `Option::Some(...)`
48+
let caller_span = if let ExprKind::Call(caller, _) = &receiver.kind {
49+
caller.span
50+
// Handle not found environment variable `Option::None`
51+
} else if let ExprKind::Path(qpath) = &receiver.kind {
52+
qpath.span()
53+
} else {
54+
return;
55+
};
56+
57+
if is_direct_expn_of_option_env(caller_span) {
58+
cx.emit_spanned_lint(
59+
INCORRECT_OPTION_ENV_UNWRAPS,
60+
expr.span,
61+
OptionEnvUnwrapDiag {
62+
suggestion: extract_inner_macro_call(cx, caller_span)
63+
.map(|replace| OptionEnvUnwrapSuggestion { span: expr.span, replace }),
64+
},
65+
)
66+
}
67+
}
68+
}
69+
70+
fn is_direct_expn_of_option_env(span: Span) -> bool {
71+
if span.from_expansion() {
72+
let data = span.ctxt().outer_expn_data();
73+
74+
if let ExpnKind::Macro(MacroKind::Bang, mac_name) = data.kind {
75+
return mac_name == sym::option_env;
76+
}
77+
}
78+
79+
false
80+
}
81+
82+
/// Given a Span representing a macro call: `option_env! ( \"j)j\")` get the inner
83+
/// content, ie. ` \"j)j\"`
84+
fn extract_inner_macro_call(cx: &LateContext<'_>, span: Span) -> Option<String> {
85+
let snippet = cx.sess().parse_sess.source_map().span_to_snippet(span).ok()?;
86+
87+
let mut inner = snippet.chars().skip_while(|c| !"([{".contains(*c)).collect::<String>();
88+
89+
// remove last character, ie one of `)]}`
90+
inner.pop()?;
91+
// remove first character, ie one of `([{`
92+
inner.remove(0);
93+
94+
Some(inner)
95+
}

‎tests/ui/lint/option_env_unwrap.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// check-pass
2+
3+
fn main() {
4+
let _ = option_env!("PATH").unwrap();
5+
//~^ WARN incorrect usage of `option_env!`
6+
let _ = option_env!("PATH").expect("environment variable PATH isn't set");
7+
//~^ WARN incorrect usage of `option_env!`
8+
let _ = option_env!("NOT_IN_ENV").unwrap();
9+
//~^ WARN incorrect usage of `option_env!`
10+
let _ = option_env!("NOT_IN_ENV").expect("environment variable NOT_IN_ENV isn't set");
11+
//~^ WARN incorrect usage of `option_env!`
12+
let _ = assert_ne!(option_env!("PATH").unwrap(), "a");
13+
//~^ WARN incorrect usage of `option_env!`
14+
15+
// just verify suggestion
16+
let _ = option_env!("PATH")
17+
//~^ WARN incorrect usage of `option_env!`
18+
.unwrap();
19+
let _ = option_env!(
20+
//~^ WARN incorrect usage of `option_env!`
21+
"PATH"
22+
)
23+
. unwrap();
24+
let _ = (option_env!("NOT_IN_ENV")).expect("aaa");
25+
//~^ WARN incorrect usage of `option_env!`
26+
27+
// should not lint
28+
let _ = option_env!("PATH").unwrap_or("...");
29+
let _ = option_env!("PATH").unwrap_or_else(|| "...");
30+
}

‎tests/ui/lint/option_env_unwrap.stderr‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
warning: incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
2+
--> $DIR/option_env_unwrap.rs:4:13
3+
|
4+
LL | let _ = option_env!("PATH").unwrap();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `#[warn(incorrect_option_env_unwraps)]` on by default
8+
help: consider using the `env!` macro instead
9+
|
10+
LL | let _ = env!("PATH");
11+
| ~~~~~~~~~~~~
12+
13+
warning: incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
14+
--> $DIR/option_env_unwrap.rs:6:13
15+
|
16+
LL | let _ = option_env!("PATH").expect("environment variable PATH isn't set");
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
|
19+
help: consider using the `env!` macro instead
20+
|
21+
LL | let _ = env!("PATH");
22+
| ~~~~~~~~~~~~
23+
24+
warning: incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
25+
--> $DIR/option_env_unwrap.rs:8:13
26+
|
27+
LL | let _ = option_env!("NOT_IN_ENV").unwrap();
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
|
30+
help: consider using the `env!` macro instead
31+
|
32+
LL | let _ = env!("NOT_IN_ENV");
33+
| ~~~~~~~~~~~~~~~~~~
34+
35+
warning: incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
36+
--> $DIR/option_env_unwrap.rs:10:13
37+
|
38+
LL | let _ = option_env!("NOT_IN_ENV").expect("environment variable NOT_IN_ENV isn't set");
39+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
40+
|
41+
help: consider using the `env!` macro instead
42+
|
43+
LL | let _ = env!("NOT_IN_ENV");
44+
| ~~~~~~~~~~~~~~~~~~
45+
46+
warning: incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
47+
--> $DIR/option_env_unwrap.rs:12:24
48+
|
49+
LL | let _ = assert_ne!(option_env!("PATH").unwrap(), "a");
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
51+
|
52+
help: consider using the `env!` macro instead
53+
|
54+
LL | let _ = assert_ne!(env!("PATH"), "a");
55+
| ~~~~~~~~~~~~
56+
57+
warning: incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
58+
--> $DIR/option_env_unwrap.rs:16:13
59+
|
60+
LL | let _ = option_env!("PATH")
61+
| _____________^
62+
LL | |
63+
LL | | .unwrap();
64+
| |_________________^
65+
|
66+
help: consider using the `env!` macro instead
67+
|
68+
LL | let _ = env!("PATH");
69+
| ~~~~~~~~~~~~
70+
71+
warning: incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
72+
--> $DIR/option_env_unwrap.rs:19:13
73+
|
74+
LL | let _ = option_env!(
75+
| _____________^
76+
LL | |
77+
LL | | "PATH"
78+
LL | | )
79+
LL | | . unwrap();
80+
| |__________________^
81+
|
82+
help: consider using the `env!` macro instead
83+
|
84+
LL ~ let _ = env!(
85+
LL +
86+
LL + "PATH"
87+
LL ~ );
88+
|
89+
90+
warning: incorrect usage of `option_env!`, it will panic at run-time if the environment variable doesn't exist at compile-time
91+
--> $DIR/option_env_unwrap.rs:24:13
92+
|
93+
LL | let _ = (option_env!("NOT_IN_ENV")).expect("aaa");
94+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
95+
|
96+
help: consider using the `env!` macro instead
97+
|
98+
LL | let _ = env!("NOT_IN_ENV");
99+
| ~~~~~~~~~~~~~~~~~~
100+
101+
warning: 8 warnings emitted
102+

0 commit comments

Comments
 (0)