Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uplift clippy::option_env_unwrap lint #111738

Closed
wants to merge 4 commits into from
Closed
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
3 changes: 3 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ lint_map_unit_fn = `Iterator::map` call that discard the iterator's values
.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
.suggestion = you might have meant to use `Iterator::for_each`

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
.suggestion = consider using the `env!` macro instead

lint_non_binding_let_on_sync_lock =
non-binding let on a synchronization lock

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ mod non_fmt_panic;
mod nonstandard_style;
mod noop_method_call;
mod opaque_hidden_inferred_bound;
mod option_env_unwrap;
mod pass_by_value;
mod passes;
mod redundant_semicolon;
Expand Down Expand Up @@ -111,6 +112,7 @@ use non_fmt_panic::NonPanicFmt;
use nonstandard_style::*;
use noop_method_call::*;
use opaque_hidden_inferred_bound::*;
use option_env_unwrap::*;
use pass_by_value::*;
use redundant_semicolon::*;
use traits::*;
Expand Down Expand Up @@ -211,6 +213,7 @@ late_lint_methods!(
BoxPointers: BoxPointers,
PathStatements: PathStatements,
LetUnderscore: LetUnderscore,
OptionEnvUnwrap: OptionEnvUnwrap,
// Depends on referenced function signatures in expressions
UnusedResults: UnusedResults,
NonUpperCaseGlobals: NonUpperCaseGlobals,
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,27 @@ pub struct MappingToUnit {
pub replace: String,
}

// option_env_unwrap.rs
#[derive(LintDiagnostic)]
#[diag(lint_option_env_unwrap)]
pub struct OptionEnvUnwrapDiag {
#[subdiagnostic]
pub suggestion: Option<OptionEnvUnwrapSuggestion>,
}

#[derive(Subdiagnostic)]
#[suggestion(
lint_suggestion,
style = "verbose",
code = "env!({replace})",
applicability = "machine-applicable"
)]
pub struct OptionEnvUnwrapSuggestion {
#[primary_span]
pub span: Span,
pub replace: String,
}

// internal.rs
#[derive(LintDiagnostic)]
#[diag(lint_default_hash_types)]
Expand Down
95 changes: 95 additions & 0 deletions compiler/rustc_lint/src/option_env_unwrap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
use rustc_hir::{Expr, ExprKind};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::{sym, ExpnKind, MacroKind, Span};

use crate::{
lints::{OptionEnvUnwrapDiag, OptionEnvUnwrapSuggestion},
LateContext, LateLintPass, LintContext,
};

declare_lint! {
/// The `incorrect_option_env_unwraps` lint checks for usage of
/// `option_env!(...).unwrap()` and suggests using the `env!` macro instead.
///
/// ### Example
///
/// ```rust,no_run
/// let _ = option_env!("HOME").unwrap();
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Unwrapping the result of `option_env!` will panic
/// at run-time if the environment variable doesn't exist, whereas `env!`
/// catches it at compile-time.
INCORRECT_OPTION_ENV_UNWRAPS,
Warn,
"using `option_env!(...).unwrap()` to get environment variable"
}

declare_lint_pass!(OptionEnvUnwrap => [INCORRECT_OPTION_ENV_UNWRAPS]);

impl<'tcx> LateLintPass<'tcx> for OptionEnvUnwrap {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
let ExprKind::MethodCall(_, receiver, _, _) = expr.kind else { return; };

let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) else { return; };

if !matches!(
cx.tcx.get_diagnostic_name(method_def_id),
Some(sym::option_unwrap | sym::option_expect)
) {
return;
}

// Handle found environment variable `Option::Some(...)`
let caller_span = if let ExprKind::Call(caller, _) = &receiver.kind {
caller.span
// Handle not found environment variable `Option::None`
} else if let ExprKind::Path(qpath) = &receiver.kind {
qpath.span()
} else {
return;
};

if is_direct_expn_of_option_env(caller_span) {
cx.emit_spanned_lint(
INCORRECT_OPTION_ENV_UNWRAPS,
expr.span,
OptionEnvUnwrapDiag {
suggestion: extract_inner_macro_call(cx, caller_span)
.map(|replace| OptionEnvUnwrapSuggestion { span: expr.span, replace }),
},
)
}
}
}

fn is_direct_expn_of_option_env(span: Span) -> bool {
if span.from_expansion() {
let data = span.ctxt().outer_expn_data();

if let ExpnKind::Macro(MacroKind::Bang, mac_name) = data.kind {
return mac_name == sym::option_env;
}
}

false
}

/// Given a Span representing a macro call: `option_env! ( \"j)j\")` get the inner
/// content, ie. ` \"j)j\"`
fn extract_inner_macro_call(cx: &LateContext<'_>, span: Span) -> Option<String> {
let snippet = cx.sess().parse_sess.source_map().span_to_snippet(span).ok()?;

let mut inner = snippet.chars().skip_while(|c| !"([{".contains(*c)).collect::<String>();

// remove last character, ie one of `)]}`
inner.pop()?;
// remove first character, ie one of `([{`
inner.remove(0);

Some(inner)
}
2 changes: 1 addition & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ pub fn host_triple() -> &'static str {
// Instead of grabbing the host triple (for the current host), we grab (at
// compile time) the target triple that this rustc is built with and
// calling that (at runtime) the host triple.
(option_env!("CFG_COMPILER_HOST_TRIPLE")).expect("CFG_COMPILER_HOST_TRIPLE")
env!("CFG_COMPILER_HOST_TRIPLE")
}

impl Default for Options {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,9 @@ symbols! {
optin_builtin_traits,
option,
option_env,
option_expect,
option_payload_ptr,
option_unwrap,
options,
or,
or_patterns,
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,7 @@ impl<T> Option<T> {
#[track_caller]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_option", issue = "67441")]
#[rustc_diagnostic_item = "option_expect"]
pub const fn expect(self, msg: &str) -> T {
match self {
Some(val) => val,
Expand Down Expand Up @@ -929,6 +930,7 @@ impl<T> Option<T> {
#[track_caller]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_option", issue = "67441")]
#[rustc_diagnostic_item = "option_unwrap"]
pub const fn unwrap(self) -> T {
match self {
Some(val) => val,
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::operators::PTR_EQ_INFO,
crate::operators::SELF_ASSIGNMENT_INFO,
crate::operators::VERBOSE_BIT_MASK_INFO,
crate::option_env_unwrap::OPTION_ENV_UNWRAP_INFO,
crate::option_if_let_else::OPTION_IF_LET_ELSE_INFO,
crate::overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL_INFO,
crate::panic_in_result_fn::PANIC_IN_RESULT_FN_INFO,
Expand Down
2 changes: 0 additions & 2 deletions src/tools/clippy/clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ mod nonstandard_macro_braces;
mod octal_escapes;
mod only_used_in_recursion;
mod operators;
mod option_env_unwrap;
mod option_if_let_else;
mod overflow_check_conditional;
mod panic_in_result_fn;
Expand Down Expand Up @@ -811,7 +810,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
max_fn_params_bools,
))
});
store.register_early_pass(|| Box::new(option_env_unwrap::OptionEnvUnwrap));
let warn_on_all_wildcard_imports = conf.warn_on_all_wildcard_imports;
store.register_late_pass(move |_| Box::new(wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports)));
store.register_late_pass(|_| Box::<redundant_pub_crate::RedundantPubCrate>::default());
Expand Down
56 changes: 0 additions & 56 deletions src/tools/clippy/clippy_lints/src/option_env_unwrap.rs

This file was deleted.

1 change: 1 addition & 0 deletions src/tools/clippy/clippy_lints/src/renamed_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,5 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
("clippy::temporary_cstring_as_ptr", "temporary_cstring_as_ptr"),
("clippy::unknown_clippy_lints", "unknown_lints"),
("clippy::unused_label", "unused_labels"),
("clippy::option_env_unwrap", "incorrect_option_env_unwraps"),
];
16 changes: 0 additions & 16 deletions src/tools/clippy/tests/ui/option_env_unwrap.rs

This file was deleted.

55 changes: 0 additions & 55 deletions src/tools/clippy/tests/ui/option_env_unwrap.stderr

This file was deleted.

2 changes: 2 additions & 0 deletions src/tools/clippy/tests/ui/rename.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#![allow(temporary_cstring_as_ptr)]
#![allow(unknown_lints)]
#![allow(unused_labels)]
#![allow(option_env_unwrap)]
#![warn(clippy::almost_complete_range)]
#![warn(clippy::disallowed_names)]
#![warn(clippy::blocks_in_if_conditions)]
Expand Down Expand Up @@ -92,5 +93,6 @@
#![warn(temporary_cstring_as_ptr)]
#![warn(unknown_lints)]
#![warn(unused_labels)]
#![warn(incorrect_option_env_unwraps)]

fn main() {}
2 changes: 2 additions & 0 deletions src/tools/clippy/tests/ui/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#![allow(temporary_cstring_as_ptr)]
#![allow(unknown_lints)]
#![allow(unused_labels)]
#![allow(option_env_unwrap)]
#![warn(clippy::almost_complete_letter_range)]
#![warn(clippy::blacklisted_name)]
#![warn(clippy::block_in_if_condition_expr)]
Expand Down Expand Up @@ -92,5 +93,6 @@
#![warn(clippy::temporary_cstring_as_ptr)]
#![warn(clippy::unknown_clippy_lints)]
#![warn(clippy::unused_label)]
#![warn(clippy::incorrect_option_env_unwraps)]

fn main() {}
Loading