Skip to content

Commit 31dc4ab

Browse files
committed
Fix formatting for ref_mut_iter_method_chain
1 parent 453c07b commit 31dc4ab

File tree

2 files changed

+94
-35
lines changed

2 files changed

+94
-35
lines changed

clippy_lints/src/methods/ref_mut_iter_method_chain.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::source::{snippet_expr, TargetPrecedence};
2+
use clippy_utils::source::{snippet_expr, ExprPosition};
33
use clippy_utils::ty::implements_trait;
44
use if_chain::if_chain;
55
use rustc_errors::Applicability;
@@ -17,7 +17,8 @@ pub(crate) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>) {
1717
if implements_trait(cx, cx.typeck_results().expr_ty(base_expr).peel_refs(), iter_trait, &[]);
1818
then {
1919
let snip_expr = match base_expr.kind {
20-
ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty(e).is_ref() && !base_expr.span.from_expansion()
20+
ExprKind::Unary(UnOp::Deref, e)
21+
if cx.typeck_results().expr_ty(e).is_ref() && !base_expr.span.from_expansion()
2122
=> e,
2223
_ => base_expr,
2324
};
@@ -30,7 +31,7 @@ pub(crate) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>) {
3031
"try",
3132
format!(
3233
"{}.by_ref()",
33-
snippet_expr(cx, snip_expr, TargetPrecedence::Postfix, self_arg.span.ctxt(), &mut app),
34+
snippet_expr(cx, snip_expr, ExprPosition::Postfix, self_arg.span.ctxt(), &mut app),
3435
),
3536
app,
3637
);

clippy_utils/src/source.rs

+90-32
Original file line numberDiff line numberDiff line change
@@ -307,20 +307,34 @@ pub fn snippet_with_context(
307307
}
308308

309309
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
310-
pub enum TargetPrecedence {
311-
Closure,
312-
Assignment,
313-
Range,
314-
Or,
315-
And,
316-
Eq,
317-
BitOr,
318-
BitXor,
319-
BitAnd,
320-
Shift,
321-
Add,
322-
Mul,
323-
As,
310+
pub enum ExprPosition {
311+
// Also includes `return`, `yield`, `break` and closures
312+
Paren,
313+
AssignmentRhs,
314+
AssignmentLhs,
315+
RangeLhs,
316+
RangeRhs,
317+
OrLhs,
318+
OrRhs,
319+
AndLhs,
320+
AndRhs,
321+
Let,
322+
EqLhs,
323+
EqRhs,
324+
BitOrLhs,
325+
BitOrRhs,
326+
BitXorLhs,
327+
BitXorRhs,
328+
BitAndLhs,
329+
BitAndRhs,
330+
ShiftLhs,
331+
ShiftRhs,
332+
AddLhs,
333+
AddRhs,
334+
MulLhs,
335+
MulRhs,
336+
// Also includes type ascription
337+
Cast,
324338
Prefix,
325339
Postfix,
326340
}
@@ -345,7 +359,7 @@ pub enum TargetPrecedence {
345359
pub fn snippet_expr(
346360
cx: &LateContext<'_>,
347361
expr: &Expr<'_>,
348-
target_precedence: TargetPrecedence,
362+
target_position: ExprPosition,
349363
ctxt: SyntaxContext,
350364
app: &mut Applicability,
351365
) -> String {
@@ -355,29 +369,73 @@ pub fn snippet_expr(
355369
Cow::Borrowed(snip) => snip.to_owned(),
356370
Cow::Owned(snip) if is_mac_call => snip,
357371
Cow::Owned(mut snip) => {
358-
let needs_paren = match expr.kind {
372+
let ctxt = expr.span.ctxt();
373+
374+
// Attempt to determine if parenthesis are needed base on the target position. The snippet may have
375+
// parenthesis already, so attempt to find those.
376+
// TODO: Remove parenthesis if they aren't needed at the target position.
377+
let needs_paren = match expr.peel_drop_temps().kind {
378+
ExprKind::Binary(_, lhs, rhs)
379+
if (ctxt == lhs.span.ctxt() && expr.span.lo() != lhs.span.lo())
380+
|| (ctxt == rhs.span.ctxt() && expr.span.hi() != rhs.span.hi()) =>
381+
{
382+
false
383+
},
359384
ExprKind::Binary(op, ..) => match op.node {
360-
BinOpKind::Add | BinOpKind::Sub => target_precedence > TargetPrecedence::Add,
361-
BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem => target_precedence > TargetPrecedence::Mul,
362-
BinOpKind::And => target_precedence > TargetPrecedence::And,
363-
BinOpKind::Or => target_precedence > TargetPrecedence::Or,
364-
BinOpKind::BitXor => target_precedence > TargetPrecedence::BitXor,
365-
BinOpKind::BitAnd => target_precedence > TargetPrecedence::BitAnd,
366-
BinOpKind::BitOr => target_precedence > TargetPrecedence::BitOr,
367-
BinOpKind::Shl | BinOpKind::Shr => target_precedence > TargetPrecedence::Shift,
385+
BinOpKind::Add | BinOpKind::Sub => target_position > ExprPosition::AddLhs,
386+
BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem => target_position > ExprPosition::MulLhs,
387+
BinOpKind::And => target_position > ExprPosition::AndLhs,
388+
BinOpKind::Or => target_position > ExprPosition::OrLhs,
389+
BinOpKind::BitXor => target_position > ExprPosition::BitXorLhs,
390+
BinOpKind::BitAnd => target_position > ExprPosition::BitAndLhs,
391+
BinOpKind::BitOr => target_position > ExprPosition::BitOrLhs,
392+
BinOpKind::Shl | BinOpKind::Shr => target_position > ExprPosition::ShiftLhs,
368393
BinOpKind::Eq | BinOpKind::Lt | BinOpKind::Le | BinOpKind::Ne | BinOpKind::Gt | BinOpKind::Ge => {
369-
target_precedence > TargetPrecedence::Eq
394+
target_position > ExprPosition::EqLhs
370395
},
371396
},
372-
ExprKind::Unary(..) | ExprKind::AddrOf(..) => target_precedence > TargetPrecedence::Prefix,
373-
ExprKind::Cast(..) => target_precedence > TargetPrecedence::As,
374-
ExprKind::Box(..)
375-
| ExprKind::Closure(..)
397+
ExprKind::Box(..) | ExprKind::Unary(..) | ExprKind::AddrOf(..) if snip.starts_with('(') => false,
398+
ExprKind::Box(..) | ExprKind::Unary(..) | ExprKind::AddrOf(..) => {
399+
target_position > ExprPosition::Prefix
400+
},
401+
ExprKind::Let(..) if snip.starts_with('(') => false,
402+
ExprKind::Let(..) => target_position > ExprPosition::Let,
403+
ExprKind::Cast(lhs, rhs)
404+
if (ctxt == lhs.span.ctxt() && expr.span.lo() != lhs.span.lo())
405+
|| (ctxt == rhs.span.ctxt() && expr.span.hi() != rhs.span.hi()) =>
406+
{
407+
false
408+
},
409+
ExprKind::Cast(..) | ExprKind::Type(..) => target_position > ExprPosition::Cast,
410+
411+
ExprKind::Closure(..)
376412
| ExprKind::Break(..)
377413
| ExprKind::Ret(..)
378-
| ExprKind::Yield(..) => target_precedence > TargetPrecedence::Closure,
379-
ExprKind::Assign(..) | ExprKind::AssignOp(..) => target_precedence > TargetPrecedence::Assignment,
380-
_ => false,
414+
| ExprKind::Yield(..)
415+
| ExprKind::Assign(..)
416+
| ExprKind::AssignOp(..) => target_position > ExprPosition::AssignmentRhs,
417+
418+
// Postfix operators, or expression with braces of some form
419+
ExprKind::Array(_)
420+
| ExprKind::Call(..)
421+
| ExprKind::ConstBlock(_)
422+
| ExprKind::MethodCall(..)
423+
| ExprKind::Tup(..)
424+
| ExprKind::Lit(..)
425+
| ExprKind::DropTemps(_)
426+
| ExprKind::If(..)
427+
| ExprKind::Loop(..)
428+
| ExprKind::Match(..)
429+
| ExprKind::Block(..)
430+
| ExprKind::Field(..)
431+
| ExprKind::Index(..)
432+
| ExprKind::Path(_)
433+
| ExprKind::Continue(_)
434+
| ExprKind::InlineAsm(_)
435+
| ExprKind::LlvmInlineAsm(_)
436+
| ExprKind::Struct(..)
437+
| ExprKind::Repeat(..)
438+
| ExprKind::Err => false,
381439
};
382440

383441
if needs_paren {

0 commit comments

Comments
 (0)