1 use if_chain
::if_chain
;
2 use rustc_ast
::ast
::LitKind
;
3 use rustc_errors
::Applicability
;
4 use rustc_hir
::intravisit
::FnKind
;
6 self as hir
, def
, BinOpKind
, BindingAnnotation
, Body
, Expr
, ExprKind
, FnDecl
, HirId
, Mutability
, PatKind
, Stmt
,
7 StmtKind
, TyKind
, UnOp
,
9 use rustc_lint
::{LateContext, LateLintPass}
;
10 use rustc_middle
::lint
::in_external_macro
;
11 use rustc_middle
::ty
::{self, Ty}
;
12 use rustc_session
::{declare_lint_pass, declare_tool_lint}
;
13 use rustc_span
::hygiene
::DesugaringKind
;
14 use rustc_span
::source_map
::{ExpnKind, Span}
;
15 use rustc_span
::symbol
::sym
;
17 use crate::consts
::{constant, Constant}
;
18 use crate::utils
::sugg
::Sugg
;
20 get_item_name
, get_parent_expr
, higher
, implements_trait
, in_constant
, is_diagnostic_assoc_item
, is_integer_const
,
21 iter_input_pats
, last_path_segment
, match_qpath
, snippet
, snippet_opt
, span_lint
, span_lint_and_sugg
,
22 span_lint_and_then
, span_lint_hir_and_then
, unsext
, SpanlessEq
,
25 declare_clippy_lint
! {
26 /// **What it does:** Checks for function arguments and let bindings denoted as
29 /// **Why is this bad?** The `ref` declaration makes the function take an owned
30 /// value, but turns the argument into a reference (which means that the value
31 /// is destroyed when exiting the function). This adds not much value: either
32 /// take a reference type, or take an owned value and create references in the
35 /// For let bindings, `let x = &foo;` is preferred over `let ref x = foo`. The
36 /// type of `x` is more obvious with the former.
38 /// **Known problems:** If the argument is dereferenced within the function,
39 /// removing the `ref` will lead to errors. This can be fixed by removing the
40 /// dereferences, e.g., changing `*x` to `x` within the function.
45 /// fn foo(ref x: u8) -> bool {
50 /// fn foo(x: &u8) -> bool {
56 "an entire binding declared as `ref`, in a function argument or a `let` statement"
59 declare_clippy_lint
! {
60 /// **What it does:** Checks for comparisons to NaN.
62 /// **Why is this bad?** NaN does not compare meaningfully to anything – not
63 /// even itself – so those comparisons are simply wrong.
65 /// **Known problems:** None.
72 /// if x == f32::NAN { }
79 "comparisons to `NAN`, which will always return false, probably not intended"
82 declare_clippy_lint
! {
83 /// **What it does:** Checks for (in-)equality comparisons on floating-point
84 /// values (apart from zero), except in functions called `*eq*` (which probably
85 /// implement equality for a type involving floats).
87 /// **Why is this bad?** Floating point calculations are usually imprecise, so
88 /// asking if two values are *exactly* equal is asking for trouble. For a good
89 /// guide on what to do, see [the floating point
90 /// guide](http://www.floating-point-gui.de/errors/comparison).
92 /// **Known problems:** None.
96 /// let x = 1.2331f64;
97 /// let y = 1.2332f64;
100 /// if y == 1.23f64 { }
101 /// if y != x {} // where both are floats
104 /// let error_margin = f64::EPSILON; // Use an epsilon for comparison
105 /// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
106 /// // let error_margin = std::f64::EPSILON;
107 /// if (y - 1.23f64).abs() < error_margin { }
108 /// if (y - x).abs() > error_margin { }
112 "using `==` or `!=` on float values instead of comparing difference with an epsilon"
115 declare_clippy_lint
! {
116 /// **What it does:** Checks for conversions to owned values just for the sake
119 /// **Why is this bad?** The comparison can operate on a reference, so creating
120 /// an owned value effectively throws it away directly afterwards, which is
121 /// needlessly consuming code and heap space.
123 /// **Known problems:** None.
128 /// # let y = String::from("foo");
129 /// if x.to_owned() == y {}
131 /// Could be written as
134 /// # let y = String::from("foo");
139 "creating owned instances for comparing with others, e.g., `x == \"foo\".to_string()`"
142 declare_clippy_lint
! {
143 /// **What it does:** Checks for getting the remainder of a division by one or minus
146 /// **Why is this bad?** The result for a divisor of one can only ever be zero; for
147 /// minus one it can cause panic/overflow (if the left operand is the minimal value of
148 /// the respective integer type) or results in zero. No one will write such code
149 /// deliberately, unless trying to win an Underhanded Rust Contest. Even for that
150 /// contest, it's probably a bad idea. Use something more underhanded.
152 /// **Known problems:** None.
162 "taking a number modulo +/-1, which can either panic/overflow or always returns 0"
165 declare_clippy_lint
! {
166 /// **What it does:** Checks for the use of bindings with a single leading
169 /// **Why is this bad?** A single leading underscore is usually used to indicate
170 /// that a binding will not be used. Using such a binding breaks this
173 /// **Known problems:** The lint does not work properly with desugaring and
174 /// macro, it has been allowed in the mean time.
179 /// let y = _x + 1; // Here we are using `_x`, even though it has a leading
180 /// // underscore. We should rename `_x` to `x`
182 pub USED_UNDERSCORE_BINDING
,
184 "using a binding which is prefixed with an underscore"
187 declare_clippy_lint
! {
188 /// **What it does:** Checks for the use of short circuit boolean conditions as
192 /// **Why is this bad?** Using a short circuit boolean condition as a statement
193 /// may hide the fact that the second part is executed or not depending on the
194 /// outcome of the first part.
196 /// **Known problems:** None.
200 /// f() && g(); // We should write `if f() { g(); }`.
202 pub SHORT_CIRCUIT_STATEMENT
,
204 "using a short circuit boolean condition as a statement"
207 declare_clippy_lint
! {
208 /// **What it does:** Catch casts from `0` to some pointer type
210 /// **Why is this bad?** This generally means `null` and is better expressed as
211 /// {`std`, `core`}`::ptr::`{`null`, `null_mut`}.
213 /// **Known problems:** None.
219 /// let a = 0 as *const u32;
222 /// let a = std::ptr::null::<u32>();
226 "using `0 as *{const, mut} T`"
229 declare_clippy_lint
! {
230 /// **What it does:** Checks for (in-)equality comparisons on floating-point
231 /// value and constant, except in functions called `*eq*` (which probably
232 /// implement equality for a type involving floats).
234 /// **Why is this bad?** Floating point calculations are usually imprecise, so
235 /// asking if two values are *exactly* equal is asking for trouble. For a good
236 /// guide on what to do, see [the floating point
237 /// guide](http://www.floating-point-gui.de/errors/comparison).
239 /// **Known problems:** None.
243 /// let x: f64 = 1.0;
244 /// const ONE: f64 = 1.00;
247 /// if x == ONE { } // where both are floats
250 /// let error_margin = f64::EPSILON; // Use an epsilon for comparison
251 /// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
252 /// // let error_margin = std::f64::EPSILON;
253 /// if (x - ONE).abs() < error_margin { }
257 "using `==` or `!=` on float constants instead of comparing difference with an epsilon"
260 declare_lint_pass
!(MiscLints
=> [
266 USED_UNDERSCORE_BINDING
,
267 SHORT_CIRCUIT_STATEMENT
,
272 impl<'tcx
> LateLintPass
<'tcx
> for MiscLints
{
275 cx
: &LateContext
<'tcx
>,
277 decl
: &'tcx FnDecl
<'_
>,
278 body
: &'tcx Body
<'_
>,
282 if let FnKind
::Closure
= k
{
283 // Does not apply to closures
286 if in_external_macro(cx
.tcx
.sess
, span
) {
289 for arg
in iter_input_pats(decl
, body
) {
290 if let PatKind
::Binding(BindingAnnotation
::Ref
| BindingAnnotation
::RefMut
, ..) = arg
.pat
.kind
{
295 "`ref` directly on a function argument is ignored. \
296 Consider using a reference type instead",
302 fn check_stmt(&mut self, cx
: &LateContext
<'tcx
>, stmt
: &'tcx Stmt
<'_
>) {
304 if !in_external_macro(cx
.tcx
.sess
, stmt
.span
);
305 if let StmtKind
::Local(ref local
) = stmt
.kind
;
306 if let PatKind
::Binding(an
, .., name
, None
) = local
.pat
.kind
;
307 if let Some(ref init
) = local
.init
;
308 if !higher
::is_from_for_desugar(local
);
310 if an
== BindingAnnotation
::Ref
|| an
== BindingAnnotation
::RefMut
{
311 // use the macro callsite when the init span (but not the whole local span)
312 // comes from an expansion like `vec![1, 2, 3]` in `let ref _ = vec![1, 2, 3];`
313 let sugg_init
= if init
.span
.from_expansion() && !local
.span
.from_expansion() {
314 Sugg
::hir_with_macro_callsite(cx
, init
, "..")
316 Sugg
::hir(cx
, init
, "..")
318 let (mutopt
, initref
) = if an
== BindingAnnotation
::RefMut
{
319 ("mut ", sugg_init
.mut_addr())
321 ("", sugg_init
.addr())
323 let tyopt
= if let Some(ref ty
) = local
.ty
{
324 format
!(": &{mutopt}{ty}", mutopt
=mutopt
, ty
=snippet(cx
, ty
.span
, ".."))
328 span_lint_hir_and_then(
333 "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead",
335 diag
.span_suggestion(
339 "let {name}{tyopt} = {initref};",
340 name
=snippet(cx
, name
.span
, ".."),
344 Applicability
::MachineApplicable
,
352 if let StmtKind
::Semi(ref expr
) = stmt
.kind
;
353 if let ExprKind
::Binary(ref binop
, ref a
, ref b
) = expr
.kind
;
354 if binop
.node
== BinOpKind
::And
|| binop
.node
== BinOpKind
::Or
;
355 if let Some(sugg
) = Sugg
::hir_opt(cx
, a
);
357 span_lint_and_then(cx
,
358 SHORT_CIRCUIT_STATEMENT
,
360 "boolean short circuit operator in statement may be clearer using an explicit test",
362 let sugg
= if binop
.node
== BinOpKind
::Or { !sugg }
else { sugg }
;
363 diag
.span_suggestion(
369 &snippet(cx
, b
.span
, ".."),
371 Applicability
::MachineApplicable
, // snippet
378 fn check_expr(&mut self, cx
: &LateContext
<'tcx
>, expr
: &'tcx Expr
<'_
>) {
380 ExprKind
::Cast(ref e
, ref ty
) => {
381 check_cast(cx
, expr
.span
, e
, ty
);
384 ExprKind
::Binary(ref cmp
, ref left
, ref right
) => {
385 check_binary(cx
, expr
, cmp
, left
, right
);
390 if in_attributes_expansion(expr
) || expr
.span
.is_desugaring(DesugaringKind
::Await
) {
391 // Don't lint things expanded by #[derive(...)], etc or `await` desugaring
394 let binding
= match expr
.kind
{
395 ExprKind
::Path(ref qpath
) if !matches
!(qpath
, hir
::QPath
::LangItem(..)) => {
396 let binding
= last_path_segment(qpath
).ident
.as_str();
397 if binding
.starts_with('_'
) &&
398 !binding
.starts_with("__") &&
399 binding
!= "_result" && // FIXME: #944
401 // don't lint if the declaration is in a macro
402 non_macro_local(cx
, cx
.qpath_res(qpath
, expr
.hir_id
))
409 ExprKind
::Field(_
, ident
) => {
410 let name
= ident
.as_str();
411 if name
.starts_with('_'
) && !name
.starts_with("__") {
419 if let Some(binding
) = binding
{
422 USED_UNDERSCORE_BINDING
,
425 "used binding `{}` which is prefixed with an underscore. A leading \
426 underscore signals that a binding will not be used",
434 fn get_lint_and_message(
435 is_comparing_constants
: bool
,
436 is_comparing_arrays
: bool
,
437 ) -> (&'
static rustc_lint
::Lint
, &'
static str) {
438 if is_comparing_constants
{
441 if is_comparing_arrays
{
442 "strict comparison of `f32` or `f64` constant arrays"
444 "strict comparison of `f32` or `f64` constant"
450 if is_comparing_arrays
{
451 "strict comparison of `f32` or `f64` arrays"
453 "strict comparison of `f32` or `f64`"
459 fn check_nan(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>, cmp_expr
: &Expr
<'_
>) {
461 if !in_constant(cx
, cmp_expr
.hir_id
);
462 if let Some((value
, _
)) = constant(cx
, cx
.typeck_results(), expr
);
464 let needs_lint
= match value
{
465 Constant
::F32(num
) => num
.is_nan(),
466 Constant
::F64(num
) => num
.is_nan(),
475 "doomed comparison with `NAN`, use `{f32,f64}::is_nan()` instead",
482 fn is_named_constant
<'tcx
>(cx
: &LateContext
<'tcx
>, expr
: &'tcx Expr
<'_
>) -> bool
{
483 if let Some((_
, res
)) = constant(cx
, cx
.typeck_results(), expr
) {
490 fn is_allowed
<'tcx
>(cx
: &LateContext
<'tcx
>, expr
: &'tcx Expr
<'_
>) -> bool
{
491 match constant(cx
, cx
.typeck_results(), expr
) {
492 Some((Constant
::F32(f
), _
)) => f
== 0.0 || f
.is_infinite(),
493 Some((Constant
::F64(f
), _
)) => f
== 0.0 || f
.is_infinite(),
494 Some((Constant
::Vec(vec
), _
)) => vec
.iter().all(|f
| match f
{
495 Constant
::F32(f
) => *f
== 0.0 || (*f
).is_infinite(),
496 Constant
::F64(f
) => *f
== 0.0 || (*f
).is_infinite(),
503 // Return true if `expr` is the result of `signum()` invoked on a float value.
504 fn is_signum(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>) -> bool
{
505 // The negation of a signum is still a signum
506 if let ExprKind
::Unary(UnOp
::Neg
, ref child_expr
) = expr
.kind
{
507 return is_signum(cx
, &child_expr
);
511 if let ExprKind
::MethodCall(ref method_name
, _
, ref expressions
, _
) = expr
.kind
;
512 if sym
!(signum
) == method_name
.ident
.name
;
513 // Check that the receiver of the signum() is a float (expressions[0] is the receiver of
516 return is_float(cx
, &expressions
[0]);
522 fn is_float(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>) -> bool
{
523 let value
= &cx
.typeck_results().expr_ty(expr
).peel_refs().kind();
525 if let ty
::Array(arr_ty
, _
) = value
{
526 return matches
!(arr_ty
.kind(), ty
::Float(_
));
529 matches
!(value
, ty
::Float(_
))
532 fn is_array(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>) -> bool
{
533 matches
!(&cx
.typeck_results().expr_ty(expr
).peel_refs().kind(), ty
::Array(_
, _
))
536 fn check_to_owned(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>, other
: &Expr
<'_
>, left
: bool
) {
544 fn is_implemented(&self) -> bool
{
545 self.ty_eq_other
|| self.other_eq_ty
549 fn symmetric_partial_eq
<'tcx
>(cx
: &LateContext
<'tcx
>, ty
: Ty
<'tcx
>, other
: Ty
<'tcx
>) -> Option
<EqImpl
> {
550 cx
.tcx
.lang_items().eq_trait().map(|def_id
| EqImpl
{
551 ty_eq_other
: implements_trait(cx
, ty
, def_id
, &[other
.into()]),
552 other_eq_ty
: implements_trait(cx
, other
, def_id
, &[ty
.into()]),
556 let (arg_ty
, snip
) = match expr
.kind
{
557 ExprKind
::MethodCall(.., ref args
, _
) if args
.len() == 1 => {
559 if let Some(expr_def_id
) = cx
.typeck_results().type_dependent_def_id(expr
.hir_id
);
560 if is_diagnostic_assoc_item(cx
, expr_def_id
, sym
::ToString
)
561 || is_diagnostic_assoc_item(cx
, expr_def_id
, sym
::ToOwned
);
563 (cx
.typeck_results().expr_ty(&args
[0]), snippet(cx
, args
[0].span
, ".."))
569 ExprKind
::Call(ref path
, ref v
) if v
.len() == 1 => {
570 if let ExprKind
::Path(ref path
) = path
.kind
{
571 if match_qpath(path
, &["String", "from_str"]) || match_qpath(path
, &["String", "from"]) {
572 (cx
.typeck_results().expr_ty(&v
[0]), snippet(cx
, v
[0].span
, ".."))
583 let other_ty
= cx
.typeck_results().expr_ty(other
);
585 let without_deref
= symmetric_partial_eq(cx
, arg_ty
, other_ty
).unwrap_or_default();
586 let with_deref
= arg_ty
588 .and_then(|tam
| symmetric_partial_eq(cx
, tam
.ty
, other_ty
))
589 .unwrap_or_default();
591 if !with_deref
.is_implemented() && !without_deref
.is_implemented() {
595 let other_gets_derefed
= matches
!(other
.kind
, ExprKind
::Unary(UnOp
::Deref
, _
));
597 let lint_span
= if other_gets_derefed
{
598 expr
.span
.to(other
.span
)
607 "this creates an owned instance just for comparison",
609 // This also catches `PartialEq` implementations that call `to_owned`.
610 if other_gets_derefed
{
611 diag
.span_label(lint_span
, "try implementing the comparison without allocating");
617 if with_deref
.is_implemented() {
618 expr_snip
= format
!("*{}", snip
);
619 eq_impl
= with_deref
;
621 expr_snip
= snip
.to_string();
622 eq_impl
= without_deref
;
627 if (eq_impl
.ty_eq_other
&& left
) || (eq_impl
.other_eq_ty
&& !left
) {
631 span
= expr
.span
.to(other
.span
);
632 if eq_impl
.ty_eq_other
{
633 hint
= format
!("{} == {}", expr_snip
, snippet(cx
, other
.span
, ".."));
635 hint
= format
!("{} == {}", snippet(cx
, other
.span
, ".."), expr_snip
);
639 diag
.span_suggestion(
643 Applicability
::MachineApplicable
, // snippet
649 /// Heuristic to see if an expression is used. Should be compatible with
650 /// `unused_variables`'s idea
651 /// of what it means for an expression to be "used".
652 fn is_used(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>) -> bool
{
653 get_parent_expr(cx
, expr
).map_or(true, |parent
| match parent
.kind
{
654 ExprKind
::Assign(_
, ref rhs
, _
) | ExprKind
::AssignOp(_
, _
, ref rhs
) => SpanlessEq
::new(cx
).eq_expr(rhs
, expr
),
655 _
=> is_used(cx
, parent
),
659 /// Tests whether an expression is in a macro expansion (e.g., something
660 /// generated by `#[derive(...)]` or the like).
661 fn in_attributes_expansion(expr
: &Expr
<'_
>) -> bool
{
662 use rustc_span
::hygiene
::MacroKind
;
663 if expr
.span
.from_expansion() {
664 let data
= expr
.span
.ctxt().outer_expn_data();
665 matches
!(data
.kind
, ExpnKind
::Macro(MacroKind
::Attr
, _
))
671 /// Tests whether `res` is a variable defined outside a macro.
672 fn non_macro_local(cx
: &LateContext
<'_
>, res
: def
::Res
) -> bool
{
673 if let def
::Res
::Local(id
) = res
{
674 !cx
.tcx
.hir().span(id
).from_expansion()
680 fn check_cast(cx
: &LateContext
<'_
>, span
: Span
, e
: &Expr
<'_
>, ty
: &hir
::Ty
<'_
>) {
682 if let TyKind
::Ptr(ref mut_ty
) = ty
.kind
;
683 if let ExprKind
::Lit(ref lit
) = e
.kind
;
684 if let LitKind
::Int(0, _
) = lit
.node
;
685 if !in_constant(cx
, e
.hir_id
);
687 let (msg
, sugg_fn
) = match mut_ty
.mutbl
{
688 Mutability
::Mut
=> ("`0 as *mut _` detected", "std::ptr::null_mut"),
689 Mutability
::Not
=> ("`0 as *const _` detected", "std::ptr::null"),
692 let (sugg
, appl
) = if let TyKind
::Infer
= mut_ty
.ty
.kind
{
693 (format
!("{}()", sugg_fn
), Applicability
::MachineApplicable
)
694 } else if let Some(mut_ty_snip
) = snippet_opt(cx
, mut_ty
.ty
.span
) {
695 (format
!("{}::<{}>()", sugg_fn
, mut_ty_snip
), Applicability
::MachineApplicable
)
697 // `MaybeIncorrect` as type inference may not work with the suggested code
698 (format
!("{}()", sugg_fn
), Applicability
::MaybeIncorrect
)
700 span_lint_and_sugg(cx
, ZERO_PTR
, span
, msg
, "try", sugg
, appl
);
706 cx
: &LateContext
<'a
>,
708 cmp
: &rustc_span
::source_map
::Spanned
<rustc_hir
::BinOpKind
>,
713 if op
.is_comparison() {
714 check_nan(cx
, left
, expr
);
715 check_nan(cx
, right
, expr
);
716 check_to_owned(cx
, left
, right
, true);
717 check_to_owned(cx
, right
, left
, false);
719 if (op
== BinOpKind
::Eq
|| op
== BinOpKind
::Ne
) && (is_float(cx
, left
) || is_float(cx
, right
)) {
720 if is_allowed(cx
, left
) || is_allowed(cx
, right
) {
724 // Allow comparing the results of signum()
725 if is_signum(cx
, left
) && is_signum(cx
, right
) {
729 if let Some(name
) = get_item_name(cx
, expr
) {
730 let name
= name
.as_str();
731 if name
== "eq" || name
== "ne" || name
== "is_nan" || name
.starts_with("eq_") || name
.ends_with("_eq") {
735 let is_comparing_arrays
= is_array(cx
, left
) || is_array(cx
, right
);
736 let (lint
, msg
) = get_lint_and_message(
737 is_named_constant(cx
, left
) || is_named_constant(cx
, right
),
740 span_lint_and_then(cx
, lint
, expr
.span
, msg
, |diag
| {
741 let lhs
= Sugg
::hir(cx
, left
, "..");
742 let rhs
= Sugg
::hir(cx
, right
, "..");
744 if !is_comparing_arrays
{
745 diag
.span_suggestion(
747 "consider comparing them within some margin of error",
749 "({}).abs() {} error_margin",
751 if op
== BinOpKind
::Eq { '<' }
else { '>' }
753 Applicability
::HasPlaceholders
, // snippet
756 diag
.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`");
758 } else if op
== BinOpKind
::Rem
{
759 if is_integer_const(cx
, right
, 1) {
760 span_lint(cx
, MODULO_ONE
, expr
.span
, "any number modulo 1 will be 0");
763 if let ty
::Int(ity
) = cx
.typeck_results().expr_ty(right
).kind() {
764 if is_integer_const(cx
, right
, unsext(cx
.tcx
, -1, *ity
)) {
769 "any number modulo -1 will panic/overflow or result in 0",