Skip to content
Merged
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
15 changes: 6 additions & 9 deletions clippy_lints/src/manual_async_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,11 @@ fn desugared_async_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>)
}

fn suggested_ret(cx: &LateContext<'_>, output: &Ty<'_>) -> Option<(&'static str, String)> {
match output.kind {
TyKind::Tup(tys) if tys.is_empty() => {
let sugg = "remove the return type";
Some((sugg, String::new()))
},
_ => {
let sugg = "return the output of the future directly";
snippet_opt(cx, output.span).map(|snip| (sugg, format!(" -> {snip}")))
},
if let TyKind::Tup([]) = output.kind {
let sugg = "remove the return type";
Some((sugg, String::new()))
} else {
let sugg = "return the output of the future directly";
snippet_opt(cx, output.span).map(|snip| (sugg, format!(" -> {snip}")))
}
}
102 changes: 88 additions & 14 deletions clippy_lints/src/matches/redundant_guards.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::path_to_local;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::source::snippet;
use clippy_utils::visitors::{for_each_expr, is_local_used};
use rustc_ast::{BorrowKind, LitKind};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Guard, MatchSource, Node, Pat, PatKind};
use rustc_lint::LateContext;
use rustc_span::symbol::Ident;
use rustc_span::Span;
use rustc_span::{Span, Symbol};
use std::borrow::Cow;
use std::ops::ControlFlow;

use super::REDUNDANT_GUARDS;
Expand Down Expand Up @@ -41,7 +42,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
(PatKind::Ref(..), None) | (_, Some(_)) => continue,
_ => arm.pat.span,
};
emit_redundant_guards(cx, outer_arm, if_expr.span, pat_span, &binding, arm.guard);
emit_redundant_guards(
cx,
outer_arm,
if_expr.span,
snippet(cx, pat_span, "<binding>"),
&binding,
arm.guard,
);
}
// `Some(x) if let Some(2) = x`
else if let Guard::IfLet(let_expr) = guard
Expand All @@ -52,7 +60,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
(PatKind::Ref(..), None) | (_, Some(_)) => continue,
_ => let_expr.pat.span,
};
emit_redundant_guards(cx, outer_arm, let_expr.span, pat_span, &binding, None);
emit_redundant_guards(
cx,
outer_arm,
let_expr.span,
snippet(cx, pat_span, "<binding>"),
&binding,
None,
);
}
// `Some(x) if x == Some(2)`
// `Some(x) if Some(2) == x`
Expand All @@ -78,11 +93,76 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
(ExprKind::AddrOf(..), None) | (_, Some(_)) => continue,
_ => pat.span,
};
emit_redundant_guards(cx, outer_arm, if_expr.span, pat_span, &binding, None);
emit_redundant_guards(
cx,
outer_arm,
if_expr.span,
snippet(cx, pat_span, "<binding>"),
&binding,
None,
);
} else if let Guard::If(if_expr) = guard
&& let ExprKind::MethodCall(path, recv, args, ..) = if_expr.kind
&& let Some(binding) = get_pat_binding(cx, recv, outer_arm)
{
check_method_calls(cx, outer_arm, path.ident.name, recv, args, if_expr, &binding);
}
}
}

fn check_method_calls<'tcx>(
cx: &LateContext<'tcx>,
arm: &Arm<'tcx>,
method: Symbol,
recv: &Expr<'_>,
args: &[Expr<'_>],
if_expr: &Expr<'_>,
binding: &PatBindingInfo,
) {
let ty = cx.typeck_results().expr_ty(recv).peel_refs();
let slice_like = ty.is_slice() || ty.is_array();

let sugg = if method == sym!(is_empty) {
// `s if s.is_empty()` becomes ""
// `arr if arr.is_empty()` becomes []

if ty.is_str() {
r#""""#.into()
} else if slice_like {
"[]".into()
} else {
return;
}
} else if slice_like
&& let Some(needle) = args.first()
&& let ExprKind::AddrOf(.., needle) = needle.kind
&& let ExprKind::Array(needles) = needle.kind
&& needles.iter().all(|needle| expr_can_be_pat(cx, needle))
{
// `arr if arr.starts_with(&[123])` becomes [123, ..]
// `arr if arr.ends_with(&[123])` becomes [.., 123]
// `arr if arr.starts_with(&[])` becomes [..] (why would anyone write this?)

let mut sugg = snippet(cx, needle.span, "<needle>").into_owned();

if needles.is_empty() {
sugg.insert_str(1, "..");
} else if method == sym!(starts_with) {
sugg.insert_str(sugg.len() - 1, ", ..");
} else if method == sym!(ends_with) {
sugg.insert_str(1, ".., ");
} else {
return;
}

sugg.into()
} else {
return;
};

emit_redundant_guards(cx, arm, if_expr.span, sugg, binding, None);
}

struct PatBindingInfo {
span: Span,
byref_ident: Option<Ident>,
Expand Down Expand Up @@ -134,19 +214,16 @@ fn emit_redundant_guards<'tcx>(
cx: &LateContext<'tcx>,
outer_arm: &Arm<'tcx>,
guard_span: Span,
pat_span: Span,
binding_replacement: Cow<'static, str>,
pat_binding: &PatBindingInfo,
inner_guard: Option<Guard<'_>>,
) {
let mut app = Applicability::MaybeIncorrect;

span_lint_and_then(
cx,
REDUNDANT_GUARDS,
guard_span.source_callsite(),
"redundant guard",
|diag| {
let binding_replacement = snippet_with_applicability(cx, pat_span, "<binding_repl>", &mut app);
let suggestion_span = match *pat_binding {
PatBindingInfo {
span,
Expand All @@ -170,14 +247,11 @@ fn emit_redundant_guards<'tcx>(
Guard::IfLet(l) => ("if let", l.span),
};

format!(
" {prefix} {}",
snippet_with_applicability(cx, span, "<guard>", &mut app),
)
format!(" {prefix} {}", snippet(cx, span, "<guard>"))
}),
),
],
app,
Applicability::MaybeIncorrect,
);
},
);
Expand Down
57 changes: 57 additions & 0 deletions tests/ui/redundant_guards.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,60 @@ mod issue11465 {
}
}
}

fn issue11807() {
#![allow(clippy::single_match)]

match Some(Some("")) {
Some(Some("")) => {},
_ => {},
}

match Some(Some(String::new())) {
// Do not lint: String deref-coerces to &str
Some(Some(x)) if x.is_empty() => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some([])) => {},
_ => {},
}

match Some(Some([] as [i32; 0])) {
Some(Some([])) => {},
_ => {},
}

match Some(Some(Vec::<()>::new())) {
// Do not lint: Vec deref-coerces to &[T]
Some(Some(x)) if x.is_empty() => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some([..])) => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some([1, ..])) => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some([1, 2, ..])) => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some([.., 1, 2])) => {},
_ => {},
}

match Some(Some(Vec::<i32>::new())) {
// Do not lint: deref coercion
Some(Some(x)) if x.starts_with(&[1, 2]) => {},
_ => {},
}
}
57 changes: 57 additions & 0 deletions tests/ui/redundant_guards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,60 @@ mod issue11465 {
}
}
}

fn issue11807() {
#![allow(clippy::single_match)]

match Some(Some("")) {
Some(Some(x)) if x.is_empty() => {},
_ => {},
}

match Some(Some(String::new())) {
// Do not lint: String deref-coerces to &str
Some(Some(x)) if x.is_empty() => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some(x)) if x.is_empty() => {},
_ => {},
}

match Some(Some([] as [i32; 0])) {
Some(Some(x)) if x.is_empty() => {},
_ => {},
}

match Some(Some(Vec::<()>::new())) {
// Do not lint: Vec deref-coerces to &[T]
Some(Some(x)) if x.is_empty() => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some(x)) if x.starts_with(&[]) => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some(x)) if x.starts_with(&[1]) => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some(x)) if x.starts_with(&[1, 2]) => {},
_ => {},
}

match Some(Some(&[] as &[i32])) {
Some(Some(x)) if x.ends_with(&[1, 2]) => {},
_ => {},
}

match Some(Some(Vec::<i32>::new())) {
// Do not lint: deref coercion
Some(Some(x)) if x.starts_with(&[1, 2]) => {},
_ => {},
}
}
86 changes: 85 additions & 1 deletion tests/ui/redundant_guards.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -203,5 +203,89 @@ LL - B { ref c, .. } if matches!(c, &1) => {},
LL + B { c: 1, .. } => {},
|

error: aborting due to 17 previous errors
error: redundant guard
--> $DIR/redundant_guards.rs:201:26
|
LL | Some(Some(x)) if x.is_empty() => {},
| ^^^^^^^^^^^^
|
help: try
|
LL - Some(Some(x)) if x.is_empty() => {},
LL + Some(Some("")) => {},
|

error: redundant guard
--> $DIR/redundant_guards.rs:212:26
|
LL | Some(Some(x)) if x.is_empty() => {},
| ^^^^^^^^^^^^
|
help: try
|
LL - Some(Some(x)) if x.is_empty() => {},
LL + Some(Some([])) => {},
|

error: redundant guard
--> $DIR/redundant_guards.rs:217:26
|
LL | Some(Some(x)) if x.is_empty() => {},
| ^^^^^^^^^^^^
|
help: try
|
LL - Some(Some(x)) if x.is_empty() => {},
LL + Some(Some([])) => {},
|

error: redundant guard
--> $DIR/redundant_guards.rs:228:26
|
LL | Some(Some(x)) if x.starts_with(&[]) => {},
| ^^^^^^^^^^^^^^^^^^
|
help: try
|
LL - Some(Some(x)) if x.starts_with(&[]) => {},
LL + Some(Some([..])) => {},
|

error: redundant guard
--> $DIR/redundant_guards.rs:233:26
|
LL | Some(Some(x)) if x.starts_with(&[1]) => {},
| ^^^^^^^^^^^^^^^^^^^
|
help: try
|
LL - Some(Some(x)) if x.starts_with(&[1]) => {},
LL + Some(Some([1, ..])) => {},
|

error: redundant guard
--> $DIR/redundant_guards.rs:238:26
|
LL | Some(Some(x)) if x.starts_with(&[1, 2]) => {},
| ^^^^^^^^^^^^^^^^^^^^^^
|
help: try
|
LL - Some(Some(x)) if x.starts_with(&[1, 2]) => {},
LL + Some(Some([1, 2, ..])) => {},
|

error: redundant guard
--> $DIR/redundant_guards.rs:243:26
|
LL | Some(Some(x)) if x.ends_with(&[1, 2]) => {},
| ^^^^^^^^^^^^^^^^^^^^
|
help: try
|
LL - Some(Some(x)) if x.ends_with(&[1, 2]) => {},
LL + Some(Some([.., 1, 2])) => {},
|

error: aborting due to 24 previous errors