Stop inserting redundant parenthesis around desugared match expressions (#16102)
Currently the `sugg` utility treats all `ExprKind::Match` expressions as potentially needing brackets, and therefore many lints will add parenthesis around them. However this includes desugared match expressions like the `?` and `.await` operators. In this PR I have updated the utility to only treat match expressions which include a code block as needing parenthesis, as the other types have similar precedence rules and expectations to things like member access and I think can be treated like as such. While this change is small on paper it touches a large amount of code due to changing a cross cutting concern, I am happy to add additional tests if we think it is needed, but I wanted to get a feel for if this is even a sensible change to be doing and what the expectations were around the level of testing needed before investing more time into it. Regarding not putting a specific lint in the changelog, determining all the lints this could possibly effect would be possible but take some time, and I wonder if it would be a bit too noisy in the changelog. Open to suggestions about how best to address that. fixes rust-lang/rust-clippy#16045 changelog: stop inserting unnecessary brackets around `x?` and `x.await` expressions in suggestions
This commit is contained in:
commit
a6dfbb3524
6 changed files with 96 additions and 5 deletions
|
|
@ -8,7 +8,7 @@ use rustc_ast::util::parser::AssocOp;
|
|||
use rustc_ast::{UnOp, ast};
|
||||
use rustc_data_structures::fx::FxHashSet;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{self as hir, Closure, ExprKind, HirId, MutTy, Node, TyKind};
|
||||
use rustc_hir::{self as hir, Closure, ExprKind, HirId, MatchSource, MutTy, Node, TyKind};
|
||||
use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
|
||||
use rustc_lint::{EarlyContext, LateContext, LintContext};
|
||||
use rustc_middle::hir::place::ProjectionKind;
|
||||
|
|
@ -146,7 +146,9 @@ impl<'a> Sugg<'a> {
|
|||
| ExprKind::Let(..)
|
||||
| ExprKind::Closure { .. }
|
||||
| ExprKind::Unary(..)
|
||||
| ExprKind::Match(..) => Sugg::MaybeParen(get_snippet(expr.span)),
|
||||
| ExprKind::Match(_, _,
|
||||
MatchSource::Normal | MatchSource::Postfix | MatchSource::ForLoopDesugar
|
||||
) => Sugg::MaybeParen(get_snippet(expr.span)),
|
||||
ExprKind::Continue(..)
|
||||
| ExprKind::Yield(..)
|
||||
| ExprKind::Array(..)
|
||||
|
|
@ -169,7 +171,10 @@ impl<'a> Sugg<'a> {
|
|||
| ExprKind::Tup(..)
|
||||
| ExprKind::Use(..)
|
||||
| ExprKind::Err(_)
|
||||
| ExprKind::UnsafeBinderCast(..) => Sugg::NonParen(get_snippet(expr.span)),
|
||||
| ExprKind::UnsafeBinderCast(..)
|
||||
| ExprKind::Match(_, _,
|
||||
MatchSource::AwaitDesugar | MatchSource::TryDesugar(_) | MatchSource::FormatArgs
|
||||
) => Sugg::NonParen(get_snippet(expr.span)),
|
||||
ExprKind::DropTemps(inner) => Self::hir_from_snippet(cx, inner, get_snippet),
|
||||
ExprKind::Assign(lhs, rhs, _) => {
|
||||
Sugg::BinOp(AssocOp::Assign, get_snippet(lhs.span), get_snippet(rhs.span))
|
||||
|
|
|
|||
|
|
@ -582,3 +582,13 @@ mod issue14150 {
|
|||
//~^ cast_possible_wrap
|
||||
}
|
||||
}
|
||||
|
||||
fn issue16045() {
|
||||
fn f() -> Result<(), ()> {
|
||||
let val = Ok::<_, ()>(0u8);
|
||||
_ = val? as i8;
|
||||
//~^ cast_possible_wrap
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -764,5 +764,11 @@ error: casting `u8` to `i8` may wrap around the value
|
|||
LL | _ = 1u8 as i8;
|
||||
| ^^^^^^^^^
|
||||
|
||||
error: aborting due to 94 previous errors
|
||||
error: casting `u8` to `i8` may wrap around the value
|
||||
--> tests/ui/cast.rs:589:13
|
||||
|
|
||||
LL | _ = val? as i8;
|
||||
| ^^^^^^^^^^ help: if this is intentional, use `cast_signed()` instead: `val?.cast_signed()`
|
||||
|
||||
error: aborting due to 95 previous errors
|
||||
|
||||
|
|
|
|||
|
|
@ -166,3 +166,32 @@ fn issue13902() {
|
|||
//~^ redundant_pattern_matching
|
||||
}
|
||||
}
|
||||
|
||||
fn issue16045() {
|
||||
fn f() -> Result<(), ()> {
|
||||
let x = Ok::<_, ()>(Some(123));
|
||||
if x?.is_some() {
|
||||
//~^ redundant_pattern_matching
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn g() {
|
||||
struct F {
|
||||
x: Option<u32>,
|
||||
}
|
||||
|
||||
impl Future for F {
|
||||
type Output = Option<u32>;
|
||||
|
||||
fn poll(self: std::pin::Pin<&mut Self>, _: &mut std::task::Context<'_>) -> std::task::Poll<Self::Output> {
|
||||
std::task::Poll::Ready(self.x)
|
||||
}
|
||||
}
|
||||
let x = F { x: Some(123) };
|
||||
if x.await.is_some() {
|
||||
//~^ redundant_pattern_matching
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -202,3 +202,32 @@ fn issue13902() {
|
|||
//~^ redundant_pattern_matching
|
||||
}
|
||||
}
|
||||
|
||||
fn issue16045() {
|
||||
fn f() -> Result<(), ()> {
|
||||
let x = Ok::<_, ()>(Some(123));
|
||||
if let Some(_) = x? {
|
||||
//~^ redundant_pattern_matching
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn g() {
|
||||
struct F {
|
||||
x: Option<u32>,
|
||||
}
|
||||
|
||||
impl Future for F {
|
||||
type Output = Option<u32>;
|
||||
|
||||
fn poll(self: std::pin::Pin<&mut Self>, _: &mut std::task::Context<'_>) -> std::task::Poll<Self::Output> {
|
||||
std::task::Poll::Ready(self.x)
|
||||
}
|
||||
}
|
||||
let x = F { x: Some(123) };
|
||||
if let Some(_) = x.await {
|
||||
//~^ redundant_pattern_matching
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -224,5 +224,17 @@ error: redundant pattern matching, consider using `is_none()`
|
|||
LL | let _ = matches!(*p, None);
|
||||
| ^^^^^^^^^^^^^^^^^^ help: try: `(*p).is_none()`
|
||||
|
||||
error: aborting due to 31 previous errors
|
||||
error: redundant pattern matching, consider using `is_some()`
|
||||
--> tests/ui/redundant_pattern_matching_option.rs:209:16
|
||||
|
|
||||
LL | if let Some(_) = x? {
|
||||
| -------^^^^^^^----- help: try: `if x?.is_some()`
|
||||
|
||||
error: redundant pattern matching, consider using `is_some()`
|
||||
--> tests/ui/redundant_pattern_matching_option.rs:229:16
|
||||
|
|
||||
LL | if let Some(_) = x.await {
|
||||
| -------^^^^^^^---------- help: try: `if x.await.is_some()`
|
||||
|
||||
error: aborting due to 33 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue