Skip to content

Commit cec4bd1

Browse files
UrgauUrgau
committed
Uplift clippy::precedence as ambiguous_precedence
1 parent c716f18 commit cec4bd1

File tree

7 files changed

+453
-0
lines changed

7 files changed

+453
-0
lines changed

‎compiler/rustc_lint/messages.ftl

+6
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,12 @@ lint_path_statement_drop = path statement drops value
455455
456456
lint_path_statement_no_effect = path statement with no effect
457457
458+
lint_precedence_unary = unary minus has lower precedence than method call
459+
.suggestion = consider adding parentheses to clarify your intent
460+
461+
lint_precedence_unwary = operator precedence can trip the unwary
462+
.suggestion = consider adding parentheses to clarify your intent
463+
458464
lint_ptr_null_checks_fn_ptr = function pointers are not nullable, so checking them for null will always return false
459465
.help = wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value
460466
.label = expression has type `{$orig_ty}`

‎compiler/rustc_lint/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ mod noop_method_call;
8080
mod opaque_hidden_inferred_bound;
8181
mod pass_by_value;
8282
mod passes;
83+
mod precedence;
8384
mod ptr_nulls;
8485
mod redundant_semicolon;
8586
mod reference_casting;
@@ -118,6 +119,7 @@ use nonstandard_style::*;
118119
use noop_method_call::*;
119120
use opaque_hidden_inferred_bound::*;
120121
use pass_by_value::*;
122+
use precedence::*;
121123
use ptr_nulls::*;
122124
use redundant_semicolon::*;
123125
use reference_casting::*;
@@ -180,6 +182,7 @@ early_lint_methods!(
180182
RedundantSemicolons: RedundantSemicolons,
181183
UnusedDocComment: UnusedDocComment,
182184
UnexpectedCfgs: UnexpectedCfgs,
185+
Precedence: Precedence,
183186
]
184187
]
185188
);

‎compiler/rustc_lint/src/lints.rs

+46
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,52 @@ pub struct ExpectationNote {
617617
pub rationale: Symbol,
618618
}
619619

620+
// precedence.rs
621+
#[derive(LintDiagnostic)]
622+
pub enum PrecedenceDiag {
623+
#[diag(lint_precedence_unary)]
624+
Unary {
625+
#[subdiagnostic]
626+
suggestion: PrecedenceUnarySuggestion,
627+
},
628+
#[diag(lint_precedence_unwary)]
629+
Unwary {
630+
#[subdiagnostic]
631+
suggestion: PrecedenceUnwarySuggestion,
632+
},
633+
}
634+
635+
#[derive(Subdiagnostic)]
636+
#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")]
637+
pub struct PrecedenceUnarySuggestion {
638+
#[suggestion_part(code = "(")]
639+
pub start_span: Span,
640+
#[suggestion_part(code = ")")]
641+
pub end_span: Span,
642+
}
643+
644+
#[derive(Subdiagnostic)]
645+
pub enum PrecedenceUnwarySuggestion {
646+
#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")]
647+
OneExpr {
648+
#[suggestion_part(code = "(")]
649+
start_span: Span,
650+
#[suggestion_part(code = ")")]
651+
end_span: Span,
652+
},
653+
#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")]
654+
TwoExpr {
655+
#[suggestion_part(code = "(")]
656+
start_span: Span,
657+
#[suggestion_part(code = ")")]
658+
end_span: Span,
659+
#[suggestion_part(code = "(")]
660+
start2_span: Span,
661+
#[suggestion_part(code = ")")]
662+
end2_span: Span,
663+
},
664+
}
665+
620666
// ptr_nulls.rs
621667
#[derive(LintDiagnostic)]
622668
pub enum PtrNullChecksDiag<'a> {

‎compiler/rustc_lint/src/precedence.rs

+132
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
use rustc_ast::token::LitKind;
2+
use rustc_ast::{BinOpKind, Expr, ExprKind, MethodCall, UnOp};
3+
use rustc_span::source_map::Spanned;
4+
5+
use crate::lints::{PrecedenceDiag, PrecedenceUnarySuggestion, PrecedenceUnwarySuggestion};
6+
use crate::{EarlyContext, EarlyLintPass, LintContext};
7+
8+
// List of functions `f(x)` where `f(-x)=-f(x)` so the
9+
// precedence doens't matter.
10+
const ALLOWED_ODD_FUNCTIONS: [&str; 14] = [
11+
"asin",
12+
"asinh",
13+
"atan",
14+
"atanh",
15+
"cbrt",
16+
"fract",
17+
"round",
18+
"signum",
19+
"sin",
20+
"sinh",
21+
"tan",
22+
"tanh",
23+
"to_degrees",
24+
"to_radians",
25+
];
26+
27+
declare_lint! {
28+
/// The `ambiguous_precedence` lint checks for operations where
29+
/// precedence may be unclear and suggests adding parentheses.
30+
///
31+
/// ### Example
32+
///
33+
/// ```rust
34+
/// # #[allow(unused)]
35+
/// 1 << 2 + 3; // equals 32, while `(1 << 2) + 3` equals 7
36+
/// -1i32.abs(); // equals -1, while `(-1i32).abs()` equals 1
37+
/// ```
38+
///
39+
/// {{produces}}
40+
///
41+
/// ### Explanation
42+
///
43+
/// Unary operations take precedence on binary operations and method
44+
/// calls take precedence over unary precedence. Setting the precedence
45+
/// explicitly makes the code clearer and avoid potential bugs.
46+
pub AMBIGUOUS_PRECEDENCE,
47+
Warn,
48+
"operations where precedence may be unclear"
49+
}
50+
51+
declare_lint_pass!(Precedence => [AMBIGUOUS_PRECEDENCE]);
52+
53+
impl EarlyLintPass for Precedence {
54+
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
55+
if expr.span.from_expansion() {
56+
return;
57+
}
58+
59+
if let ExprKind::Binary(Spanned { node: op, .. }, ref left, ref right) = expr.kind {
60+
if !is_bit_op(op) {
61+
return;
62+
}
63+
64+
let suggestion = match (is_arith_expr(left), is_arith_expr(right)) {
65+
(true, true) => PrecedenceUnwarySuggestion::TwoExpr {
66+
start_span: left.span.shrink_to_lo(),
67+
end_span: left.span.shrink_to_hi(),
68+
start2_span: right.span.shrink_to_lo(),
69+
end2_span: right.span.shrink_to_hi(),
70+
},
71+
(true, false) => PrecedenceUnwarySuggestion::OneExpr {
72+
start_span: left.span.shrink_to_lo(),
73+
end_span: left.span.shrink_to_hi(),
74+
},
75+
(false, true) => PrecedenceUnwarySuggestion::OneExpr {
76+
start_span: right.span.shrink_to_lo(),
77+
end_span: right.span.shrink_to_hi(),
78+
},
79+
(false, false) => return,
80+
};
81+
82+
cx.emit_spanned_lint(
83+
AMBIGUOUS_PRECEDENCE,
84+
expr.span,
85+
PrecedenceDiag::Unwary { suggestion },
86+
);
87+
}
88+
89+
if let ExprKind::Unary(UnOp::Neg, operand) = &expr.kind {
90+
let mut arg = operand;
91+
92+
let mut all_odd = true;
93+
while let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &arg.kind {
94+
let seg_str = seg.ident.name.as_str();
95+
all_odd &=
96+
ALLOWED_ODD_FUNCTIONS.iter().any(|odd_function| **odd_function == *seg_str);
97+
arg = receiver;
98+
}
99+
100+
if !all_odd
101+
&& let ExprKind::Lit(lit) = &arg.kind
102+
&& let LitKind::Integer | LitKind::Float = &lit.kind
103+
{
104+
cx.emit_spanned_lint(AMBIGUOUS_PRECEDENCE, expr.span, PrecedenceDiag::Unary {
105+
suggestion: PrecedenceUnarySuggestion {
106+
start_span: operand.span.shrink_to_lo(),
107+
end_span: operand.span.shrink_to_hi(),
108+
}
109+
});
110+
}
111+
}
112+
}
113+
}
114+
115+
fn is_arith_expr(expr: &Expr) -> bool {
116+
match expr.kind {
117+
ExprKind::Binary(Spanned { node: op, .. }, _, _) => is_arith_op(op),
118+
_ => false,
119+
}
120+
}
121+
122+
#[must_use]
123+
fn is_bit_op(op: BinOpKind) -> bool {
124+
use rustc_ast::ast::BinOpKind::{BitAnd, BitOr, BitXor, Shl, Shr};
125+
matches!(op, BitXor | BitAnd | BitOr | Shl | Shr)
126+
}
127+
128+
#[must_use]
129+
fn is_arith_op(op: BinOpKind) -> bool {
130+
use rustc_ast::ast::BinOpKind::{Add, Div, Mul, Rem, Sub};
131+
matches!(op, Add | Sub | Mul | Div | Rem)
132+
}

‎tests/ui/lint/precedence.fixed

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// run-rustfix
2+
// check-pass
3+
4+
fn main() {
5+
let _ = 1 << (2 + 3);
6+
//~^ WARN operator precedence can trip the unwary
7+
let _ = (1 + 2) << 3;
8+
//~^ WARN operator precedence can trip the unwary
9+
let _ = 4 >> (1 + 1);
10+
//~^ WARN operator precedence can trip the unwary
11+
let _ = (1 + 3) >> 2;
12+
//~^ WARN operator precedence can trip the unwary
13+
let _ = 1 ^ (1 - 1);
14+
//~^ WARN operator precedence can trip the unwary
15+
let _ = 3 | (2 - 1);
16+
//~^ WARN operator precedence can trip the unwary
17+
let _ = 3 & (5 - 2);
18+
//~^ WARN operator precedence can trip the unwary
19+
let _ = (1 + 2) << (3 + 1);
20+
//~^ WARN operator precedence can trip the unwary
21+
let _ = -(1i32.abs());
22+
//~^ WARN unary minus has lower precedence than method call
23+
let _ = -(1f32.abs());
24+
//~^ WARN unary minus has lower precedence than method call
25+
26+
// These should not trigger an error
27+
let _ = (-1i32).abs();
28+
let _ = (-1f32).abs();
29+
let _ = -(1i32).abs();
30+
let _ = -(1f32).abs();
31+
let _ = -(1i32.abs());
32+
let _ = -(1f32.abs());
33+
34+
// Odd functions should not trigger an error
35+
let _ = -1f64.asin();
36+
let _ = -1f64.asinh();
37+
let _ = -1f64.atan();
38+
let _ = -1f64.atanh();
39+
let _ = -1f64.cbrt();
40+
let _ = -1f64.fract();
41+
let _ = -1f64.round();
42+
let _ = -1f64.signum();
43+
let _ = -1f64.sin();
44+
let _ = -1f64.sinh();
45+
let _ = -1f64.tan();
46+
let _ = -1f64.tanh();
47+
let _ = -1f64.to_degrees();
48+
let _ = -1f64.to_radians();
49+
50+
// Chains containing any non-odd function should trigger (issue clippy#5924)
51+
let _ = -(1.0_f64.cos().cos());
52+
//~^ WARN unary minus has lower precedence than method call
53+
let _ = -(1.0_f64.cos().sin());
54+
//~^ WARN unary minus has lower precedence than method call
55+
let _ = -(1.0_f64.sin().cos());
56+
//~^ WARN unary minus has lower precedence than method call
57+
58+
// Chains of odd functions shouldn't trigger
59+
let _ = -1f64.sin().sin();
60+
}

‎tests/ui/lint/precedence.rs

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// run-rustfix
2+
// check-pass
3+
4+
fn main() {
5+
let _ = 1 << 2 + 3;
6+
//~^ WARN operator precedence can trip the unwary
7+
let _ = 1 + 2 << 3;
8+
//~^ WARN operator precedence can trip the unwary
9+
let _ = 4 >> 1 + 1;
10+
//~^ WARN operator precedence can trip the unwary
11+
let _ = 1 + 3 >> 2;
12+
//~^ WARN operator precedence can trip the unwary
13+
let _ = 1 ^ 1 - 1;
14+
//~^ WARN operator precedence can trip the unwary
15+
let _ = 3 | 2 - 1;
16+
//~^ WARN operator precedence can trip the unwary
17+
let _ = 3 & 5 - 2;
18+
//~^ WARN operator precedence can trip the unwary
19+
let _ = 1 + 2 << 3 + 1;
20+
//~^ WARN operator precedence can trip the unwary
21+
let _ = -1i32.abs();
22+
//~^ WARN unary minus has lower precedence than method call
23+
let _ = -1f32.abs();
24+
//~^ WARN unary minus has lower precedence than method call
25+
26+
// These should not trigger an error
27+
let _ = (-1i32).abs();
28+
let _ = (-1f32).abs();
29+
let _ = -(1i32).abs();
30+
let _ = -(1f32).abs();
31+
let _ = -(1i32.abs());
32+
let _ = -(1f32.abs());
33+
34+
// Odd functions should not trigger an error
35+
let _ = -1f64.asin();
36+
let _ = -1f64.asinh();
37+
let _ = -1f64.atan();
38+
let _ = -1f64.atanh();
39+
let _ = -1f64.cbrt();
40+
let _ = -1f64.fract();
41+
let _ = -1f64.round();
42+
let _ = -1f64.signum();
43+
let _ = -1f64.sin();
44+
let _ = -1f64.sinh();
45+
let _ = -1f64.tan();
46+
let _ = -1f64.tanh();
47+
let _ = -1f64.to_degrees();
48+
let _ = -1f64.to_radians();
49+
50+
// Chains containing any non-odd function should trigger (issue clippy#5924)
51+
let _ = -1.0_f64.cos().cos();
52+
//~^ WARN unary minus has lower precedence than method call
53+
let _ = -1.0_f64.cos().sin();
54+
//~^ WARN unary minus has lower precedence than method call
55+
let _ = -1.0_f64.sin().cos();
56+
//~^ WARN unary minus has lower precedence than method call
57+
58+
// Chains of odd functions shouldn't trigger
59+
let _ = -1f64.sin().sin();
60+
}

0 commit comments

Comments
 (0)