nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-02-21 03:14:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

Make an attempt of creating suggestions

Browse Source 
They aren't perfect but better than nothing
This commit is contained in:
est31 2022-10-10 07:17:04 +02:00
parent 9bd70dbb88
commit a1db9311dc
4 changed files with 157 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
clippy_lints/src/manual_let_else.rs
View File

@ -1,16 +1,19 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::IfLetOrMatch;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::{for_each_expr, Descend};
use clippy_utils::{meets_msrv, msrvs, peel_blocks};
use if_chain::if_chain;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, MatchSource, Pat, PatKind, QPath, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::sym;
use rustc_span::Span;
use serde::Deserialize;
use std::ops::ControlFlow;
@ -80,20 +83,15 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
};
match if_let_or_match {
IfLetOrMatch::IfLet(_let_expr, let_pat, if_then, if_else) => if_chain! {
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! {
if expr_is_simple_identity(let_pat, if_then);
if let Some(if_else) = if_else;
if expr_diverges(cx, if_else);
then {
span_lint(
cx,
MANUAL_LET_ELSE,
stmt.span,
"this could be rewritten as `let else`",
);
emit_manual_let_else(cx, stmt.span, if_let_expr, let_pat, if_else);
}
},
IfLetOrMatch::Match(_match_expr, arms, source) => {
IfLetOrMatch::Match(match_expr, arms, source) => {
if self.matches_behaviour == MatchLintBehaviour::Never {
return;
}
@ -105,27 +103,22 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
if arms.len() != 2 {
return;
}
let check_types = self.matches_behaviour == MatchLintBehaviour::WellKnownTypes;
// We iterate over both arms, trying to find one that is an identity,
// one that diverges. Our check needs to work regardless of the order
// of both arms.
let mut found_identity_arm = false;
let mut found_diverging_arm = false;
for arm in arms {
// Guards don't give us an easy mapping to let else
if arm.guard.is_some() {
return;
}
if expr_is_simple_identity(arm.pat, arm.body) {
found_identity_arm = true;
} else if expr_diverges(cx, arm.body) && pat_allowed_for_else(cx, arm.pat, check_types) {
found_diverging_arm = true;
}
}
if !(found_identity_arm && found_diverging_arm) {
// Guards don't give us an easy mapping either
if arms.iter().any(|arm| arm.guard.is_some()) {
return;
}
span_lint(cx, MANUAL_LET_ELSE, stmt.span, "this could be rewritten as `let else`");
let check_types = self.matches_behaviour == MatchLintBehaviour::WellKnownTypes;
let diverging_arm_opt = arms
.iter()
.enumerate()
.find(|(_, arm)| expr_diverges(cx, arm.body) && pat_allowed_for_else(cx, arm.pat, check_types));
let Some((idx, diverging_arm)) = diverging_arm_opt else { return; };
let pat_arm = &arms[1 - idx];
if !expr_is_simple_identity(pat_arm.pat, pat_arm.body) {
return;
}
emit_manual_let_else(cx, stmt.span, match_expr, pat_arm.pat, diverging_arm.body);
},
}
}
@ -133,6 +126,37 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
extract_msrv_attr!(LateContext);
}
fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: &Pat<'_>, else_body: &Expr<'_>) {
span_lint_and_then(
cx,
MANUAL_LET_ELSE,
span,
"this could be rewritten as `let...else`",
|diag| {
// This is far from perfect, for example there needs to be:
// * mut additions for the bindings
// * renamings of the bindings
// * unused binding collision detection with existing ones
// * putting patterns with at the top level | inside ()
// for this to be machine applicable.
let app = Applicability::HasPlaceholders;
if let Some(sn_pat) = snippet_opt(cx, pat.span) &&
let Some(sn_expr) = snippet_opt(cx, expr.span) &&
let Some(sn_else) = snippet_opt(cx, else_body.span)
{
let else_bl = if matches!(else_body.kind, ExprKind::Block(..)) {
sn_else
} else {
format!("{{ {sn_else} }}")
};
let sugg = format!("let {sn_pat} = {sn_expr} else {else_bl};");
diag.span_suggestion(span, "consider writing", sugg, app);
}
},
);
}
fn expr_diverges(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
fn is_never(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
if let Some(ty) = cx.typeck_results().expr_ty_opt(expr) {

11
tests/ui/manual_let_else.rs
View File

@ -158,14 +158,13 @@ fn not_fire() {
}
};
let v = if let Some(v_some) = g() {
enum Uninhabited {}
fn un() -> Uninhabited {
panic!()
}
let v = if let Some(v_some) = None {
v_some
} else {
enum Uninhabited {}
fn un() -> Uninhabited {
panic!()
}
// Don't lint if the type is uninhabited but not !
un()
};

106
tests/ui/manual_let_else.stderr
View File

@ -1,12 +1,12 @@
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:17:5
|
LL | let v = if let Some(v_some) = g() { v_some } else { return };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { return };`
|
= note: `-D clippy::manual-let-else` implied by `-D warnings`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:18:5
|
LL | / let v = if let Some(v_some) = g() {
@ -15,8 +15,15 @@ LL | | } else {
LL | | return;
LL | | };
| |______^
|
help: consider writing
|
LL ~ let Some(v_some) = g() else {
LL + return;
LL + };
|
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:24:5
|
LL | / let v = if let Some(v) = g() {
@ -27,26 +34,35 @@ LL | | { v }
LL | | return;
LL | | };
| |______^
|
help: consider writing
|
LL ~ let Some(v) = g() else {
LL + // Some computation should still make it fire
LL + g();
LL + return;
LL + };
|
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:37:9
|
LL | let v = if let Some(v_some) = g() { v_some } else { continue };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { continue };`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:38:9
|
LL | let v = if let Some(v_some) = g() { v_some } else { break };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { break };`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:42:5
|
LL | let v = if let Some(v_some) = g() { v_some } else { panic!() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { panic!() };`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:45:5
|
LL | / let v = if let Some(v_some) = g() {
@ -55,8 +71,15 @@ LL | | } else {
LL | | std::process::abort()
LL | | };
| |______^
|
help: consider writing
|
LL ~ let Some(v_some) = g() else {
LL + std::process::abort()
LL + };
|
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:52:5
|
LL | / let v = if let Some(v_some) = g() {
@ -65,8 +88,15 @@ LL | | } else {
LL | | if true { return } else { panic!() }
LL | | };
| |______^
|
help: consider writing
|
LL ~ let Some(v_some) = g() else {
LL + if true { return } else { panic!() }
LL + };
|
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:59:5
|
LL | / let v = if let Some(v_some) = g() {
@ -77,8 +107,17 @@ LL | | } else {
LL | | panic!("diverge");
LL | | };
| |______^
|
help: consider writing
|
LL ~ let Some(v_some) = g() else { if true {
LL + return;
LL + } else {
LL + panic!("diverge");
LL + } };
|
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:68:5
|
LL | / let v = if let Some(v_some) = g() {
@ -89,8 +128,25 @@ LL | | match (g(), g()) {
LL | | }
LL | | };
| |______^
|
help: consider writing
|
LL ~ let Some(v_some) = g() else {
LL + match (g(), g()) {
LL + (Some(_), None) => return,
LL + (None, Some(_)) => {
LL + if true {
LL + return;
LL + } else {
LL + panic!();
LL + }
LL + },
LL + _ => return,
LL + }
LL + };
|
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:85:5
|
LL | / let (v, w) = if let Some(v_some) = g().map(|v| (v, 42)) {
@ -99,8 +155,15 @@ LL | | } else {
LL | | return;
LL | | };
| |______^
|
help: consider writing
|
LL ~ let Some(v_some) = g().map(|v| (v, 42)) else {
LL + return;
LL + };
|
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:92:5
|
LL | / let v = if let (Some(v_some), w_some) = (g(), 0) {
@ -109,12 +172,19 @@ LL | | } else {
LL | | return;
LL | | };
| |______^
|
help: consider writing
|
LL ~ let (Some(v_some), w_some) = (g(), 0) else {
LL + return;
LL + };
|
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else.rs:101:13
|
LL | let $n = if let Some(v) = $e { v } else { return };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return };`
...
LL | create_binding_if_some!(w, g());
| ------------------------------- in this macro invocation

24
tests/ui/manual_let_else_match.stderr
View File

@ -1,58 +1,58 @@
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else_match.rs:32:5
|
LL | / let v = match g() {
LL | | Some(v_some) => v_some,
LL | | None => return,
LL | | };
| |______^
| |______^ help: consider writing: `let Some(v_some) = g() else { return };`
|
= note: `-D clippy::manual-let-else` implied by `-D warnings`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else_match.rs:37:5
|
LL | / let v = match g() {
LL | | Some(v_some) => v_some,
LL | | _ => return,
LL | | };
| |______^
| |______^ help: consider writing: `let Some(v_some) = g() else { return };`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else_match.rs:44:9
|
LL | / let v = match h() {
LL | | (Some(_), Some(_)) | (None, None) => continue,
LL | | (Some(v), None) | (None, Some(v)) => v,
LL | | };
| |__________^
| |__________^ help: consider writing: `let (Some(v), None) | (None, Some(v)) = h() else { continue };`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else_match.rs:49:9
|
LL | / let v = match build_enum() {
LL | | _ => continue,
LL | | Variant::Bar(v) | Variant::Baz(v) => v,
LL | | };
| |__________^
| |__________^ help: consider writing: `let Variant::Bar(v) | Variant::Baz(v) = build_enum() else { continue };`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else_match.rs:57:5
|
LL | / let v = match f() {
LL | | Ok(v) => v,
LL | | Err(_) => return,
LL | | };
| |______^
| |______^ help: consider writing: `let Ok(v) = f() else { return };`
error: this could be rewritten as `let else`
error: this could be rewritten as `let...else`
--> $DIR/manual_let_else_match.rs:63:5
|
LL | / let v = match f().map_err(|_| ()) {
LL | | Ok(v) => v,
LL | | Err(()) => return,
LL | | };
| |______^
| |______^ help: consider writing: `let Ok(v) = f().map_err(|_| ()) else { return };`
error: aborting due to 6 previous errors