1 use clippy_utils
::diagnostics
::{span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then}
;
2 use clippy_utils
::source
::{snippet, snippet_opt}
;
3 use clippy_utils
::ty
::implements_trait
;
4 use if_chain
::if_chain
;
5 use rustc_ast
::ast
::LitKind
;
6 use rustc_errors
::Applicability
;
7 use rustc_hir
::intravisit
::FnKind
;
9 self as hir
, def
, BinOpKind
, BindingAnnotation
, Body
, Expr
, ExprKind
, FnDecl
, HirId
, Mutability
, PatKind
, Stmt
,
10 StmtKind
, TyKind
, UnOp
,
12 use rustc_lint
::{LateContext, LateLintPass}
;
13 use rustc_middle
::lint
::in_external_macro
;
14 use rustc_middle
::ty
::{self, Ty}
;
15 use rustc_session
::{declare_lint_pass, declare_tool_lint}
;
16 use rustc_span
::hygiene
::DesugaringKind
;
17 use rustc_span
::source_map
::{ExpnKind, Span}
;
18 use rustc_span
::symbol
::sym
;
20 use clippy_utils
::consts
::{constant, Constant}
;
21 use clippy_utils
::sugg
::Sugg
;
23 expr_path_res
, get_item_name
, get_parent_expr
, in_constant
, is_diag_trait_item
, is_integer_const
, iter_input_pats
,
24 last_path_segment
, match_any_def_paths
, paths
, unsext
, SpanlessEq
,
27 declare_clippy_lint
! {
29 /// Checks for function arguments and let bindings denoted as
32 /// ### Why is this bad?
33 /// The `ref` declaration makes the function take an owned
34 /// value, but turns the argument into a reference (which means that the value
35 /// is destroyed when exiting the function). This adds not much value: either
36 /// take a reference type, or take an owned value and create references in the
39 /// For let bindings, `let x = &foo;` is preferred over `let ref x = foo`. The
40 /// type of `x` is more obvious with the former.
42 /// ### Known problems
43 /// If the argument is dereferenced within the function,
44 /// removing the `ref` will lead to errors. This can be fixed by removing the
45 /// dereferences, e.g., changing `*x` to `x` within the function.
50 /// fn foo(ref x: u8) -> bool {
55 /// fn foo(x: &u8) -> bool {
61 "an entire binding declared as `ref`, in a function argument or a `let` statement"
64 declare_clippy_lint
! {
66 /// Checks for comparisons to NaN.
68 /// ### Why is this bad?
69 /// NaN does not compare meaningfully to anything – not
70 /// even itself – so those comparisons are simply wrong.
77 /// if x == f32::NAN { }
84 "comparisons to `NAN`, which will always return false, probably not intended"
87 declare_clippy_lint
! {
89 /// Checks for (in-)equality comparisons on floating-point
90 /// values (apart from zero), except in functions called `*eq*` (which probably
91 /// implement equality for a type involving floats).
93 /// ### Why is this bad?
94 /// Floating point calculations are usually imprecise, so
95 /// asking if two values are *exactly* equal is asking for trouble. For a good
96 /// guide on what to do, see [the floating point
97 /// guide](http://www.floating-point-gui.de/errors/comparison).
101 /// let x = 1.2331f64;
102 /// let y = 1.2332f64;
105 /// if y == 1.23f64 { }
106 /// if y != x {} // where both are floats
109 /// let error_margin = f64::EPSILON; // Use an epsilon for comparison
110 /// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
111 /// // let error_margin = std::f64::EPSILON;
112 /// if (y - 1.23f64).abs() < error_margin { }
113 /// if (y - x).abs() > error_margin { }
117 "using `==` or `!=` on float values instead of comparing difference with an epsilon"
120 declare_clippy_lint
! {
122 /// Checks for conversions to owned values just for the sake
125 /// ### Why is this bad?
126 /// The comparison can operate on a reference, so creating
127 /// an owned value effectively throws it away directly afterwards, which is
128 /// needlessly consuming code and heap space.
133 /// # let y = String::from("foo");
134 /// if x.to_owned() == y {}
136 /// Could be written as
139 /// # let y = String::from("foo");
144 "creating owned instances for comparing with others, e.g., `x == \"foo\".to_string()`"
147 declare_clippy_lint
! {
149 /// Checks for getting the remainder of a division by one or minus
152 /// ### Why is this bad?
153 /// The result for a divisor of one can only ever be zero; for
154 /// minus one it can cause panic/overflow (if the left operand is the minimal value of
155 /// the respective integer type) or results in zero. No one will write such code
156 /// deliberately, unless trying to win an Underhanded Rust Contest. Even for that
157 /// contest, it's probably a bad idea. Use something more underhanded.
167 "taking a number modulo +/-1, which can either panic/overflow or always returns 0"
170 declare_clippy_lint
! {
172 /// Checks for the use of bindings with a single leading
175 /// ### Why is this bad?
176 /// A single leading underscore is usually used to indicate
177 /// that a binding will not be used. Using such a binding breaks this
180 /// ### Known problems
181 /// The lint does not work properly with desugaring and
182 /// macro, it has been allowed in the mean time.
187 /// let y = _x + 1; // Here we are using `_x`, even though it has a leading
188 /// // underscore. We should rename `_x` to `x`
190 pub USED_UNDERSCORE_BINDING
,
192 "using a binding which is prefixed with an underscore"
195 declare_clippy_lint
! {
197 /// Checks for the use of short circuit boolean conditions as
201 /// ### Why is this bad?
202 /// Using a short circuit boolean condition as a statement
203 /// may hide the fact that the second part is executed or not depending on the
204 /// outcome of the first part.
208 /// f() && g(); // We should write `if f() { g(); }`.
210 pub SHORT_CIRCUIT_STATEMENT
,
212 "using a short circuit boolean condition as a statement"
215 declare_clippy_lint
! {
217 /// Catch casts from `0` to some pointer type
219 /// ### Why is this bad?
220 /// This generally means `null` and is better expressed as
221 /// {`std`, `core`}`::ptr::`{`null`, `null_mut`}.
226 /// let a = 0 as *const u32;
229 /// let a = std::ptr::null::<u32>();
233 "using `0 as *{const, mut} T`"
236 declare_clippy_lint
! {
238 /// Checks for (in-)equality comparisons on floating-point
239 /// value and constant, except in functions called `*eq*` (which probably
240 /// implement equality for a type involving floats).
242 /// ### Why is this bad?
243 /// Floating point calculations are usually imprecise, so
244 /// asking if two values are *exactly* equal is asking for trouble. For a good
245 /// guide on what to do, see [the floating point
246 /// guide](http://www.floating-point-gui.de/errors/comparison).
250 /// let x: f64 = 1.0;
251 /// const ONE: f64 = 1.00;
254 /// if x == ONE { } // where both are floats
257 /// let error_margin = f64::EPSILON; // Use an epsilon for comparison
258 /// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
259 /// // let error_margin = std::f64::EPSILON;
260 /// if (x - ONE).abs() < error_margin { }
264 "using `==` or `!=` on float constants instead of comparing difference with an epsilon"
267 declare_lint_pass
!(MiscLints
=> [
273 USED_UNDERSCORE_BINDING
,
274 SHORT_CIRCUIT_STATEMENT
,
279 impl<'tcx
> LateLintPass
<'tcx
> for MiscLints
{
282 cx
: &LateContext
<'tcx
>,
284 decl
: &'tcx FnDecl
<'_
>,
285 body
: &'tcx Body
<'_
>,
289 if let FnKind
::Closure
= k
{
290 // Does not apply to closures
293 if in_external_macro(cx
.tcx
.sess
, span
) {
296 for arg
in iter_input_pats(decl
, body
) {
297 if let PatKind
::Binding(BindingAnnotation
::Ref
| BindingAnnotation
::RefMut
, ..) = arg
.pat
.kind
{
302 "`ref` directly on a function argument is ignored. \
303 Consider using a reference type instead",
309 fn check_stmt(&mut self, cx
: &LateContext
<'tcx
>, stmt
: &'tcx Stmt
<'_
>) {
311 if !in_external_macro(cx
.tcx
.sess
, stmt
.span
);
312 if let StmtKind
::Local(local
) = stmt
.kind
;
313 if let PatKind
::Binding(an
, .., name
, None
) = local
.pat
.kind
;
314 if let Some(init
) = local
.init
;
315 if an
== BindingAnnotation
::Ref
|| an
== BindingAnnotation
::RefMut
;
317 // use the macro callsite when the init span (but not the whole local span)
318 // comes from an expansion like `vec![1, 2, 3]` in `let ref _ = vec![1, 2, 3];`
319 let sugg_init
= if init
.span
.from_expansion() && !local
.span
.from_expansion() {
320 Sugg
::hir_with_macro_callsite(cx
, init
, "..")
322 Sugg
::hir(cx
, init
, "..")
324 let (mutopt
, initref
) = if an
== BindingAnnotation
::RefMut
{
325 ("mut ", sugg_init
.mut_addr())
327 ("", sugg_init
.addr())
329 let tyopt
= if let Some(ty
) = local
.ty
{
330 format
!(": &{mutopt}{ty}", mutopt
=mutopt
, ty
=snippet(cx
, ty
.span
, ".."))
334 span_lint_hir_and_then(
339 "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead",
341 diag
.span_suggestion(
345 "let {name}{tyopt} = {initref};",
346 name
=snippet(cx
, name
.span
, ".."),
350 Applicability
::MachineApplicable
,
357 if let StmtKind
::Semi(expr
) = stmt
.kind
;
358 if let ExprKind
::Binary(ref binop
, a
, b
) = expr
.kind
;
359 if binop
.node
== BinOpKind
::And
|| binop
.node
== BinOpKind
::Or
;
360 if let Some(sugg
) = Sugg
::hir_opt(cx
, a
);
362 span_lint_hir_and_then(
364 SHORT_CIRCUIT_STATEMENT
,
367 "boolean short circuit operator in statement may be clearer using an explicit test",
369 let sugg
= if binop
.node
== BinOpKind
::Or { !sugg }
else { sugg }
;
370 diag
.span_suggestion(
376 &snippet(cx
, b
.span
, ".."),
378 Applicability
::MachineApplicable
, // snippet
385 fn check_expr(&mut self, cx
: &LateContext
<'tcx
>, expr
: &'tcx Expr
<'_
>) {
387 ExprKind
::Cast(e
, ty
) => {
388 check_cast(cx
, expr
.span
, e
, ty
);
391 ExprKind
::Binary(ref cmp
, left
, right
) => {
392 check_binary(cx
, expr
, cmp
, left
, right
);
397 if in_attributes_expansion(expr
) || expr
.span
.is_desugaring(DesugaringKind
::Await
) {
398 // Don't lint things expanded by #[derive(...)], etc or `await` desugaring
401 let binding
= match expr
.kind
{
402 ExprKind
::Path(ref qpath
) if !matches
!(qpath
, hir
::QPath
::LangItem(..)) => {
403 let binding
= last_path_segment(qpath
).ident
.as_str();
404 if binding
.starts_with('_'
) &&
405 !binding
.starts_with("__") &&
406 binding
!= "_result" && // FIXME: #944
408 // don't lint if the declaration is in a macro
409 non_macro_local(cx
, cx
.qpath_res(qpath
, expr
.hir_id
))
416 ExprKind
::Field(_
, ident
) => {
417 let name
= ident
.as_str();
418 if name
.starts_with('_'
) && !name
.starts_with("__") {
426 if let Some(binding
) = binding
{
429 USED_UNDERSCORE_BINDING
,
432 "used binding `{}` which is prefixed with an underscore. A leading \
433 underscore signals that a binding will not be used",
441 fn get_lint_and_message(
442 is_comparing_constants
: bool
,
443 is_comparing_arrays
: bool
,
444 ) -> (&'
static rustc_lint
::Lint
, &'
static str) {
445 if is_comparing_constants
{
448 if is_comparing_arrays
{
449 "strict comparison of `f32` or `f64` constant arrays"
451 "strict comparison of `f32` or `f64` constant"
457 if is_comparing_arrays
{
458 "strict comparison of `f32` or `f64` arrays"
460 "strict comparison of `f32` or `f64`"
466 fn check_nan(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>, cmp_expr
: &Expr
<'_
>) {
468 if !in_constant(cx
, cmp_expr
.hir_id
);
469 if let Some((value
, _
)) = constant(cx
, cx
.typeck_results(), expr
);
471 Constant
::F32(num
) => num
.is_nan(),
472 Constant
::F64(num
) => num
.is_nan(),
480 "doomed comparison with `NAN`, use `{f32,f64}::is_nan()` instead",
486 fn is_named_constant
<'tcx
>(cx
: &LateContext
<'tcx
>, expr
: &'tcx Expr
<'_
>) -> bool
{
487 if let Some((_
, res
)) = constant(cx
, cx
.typeck_results(), expr
) {
494 fn is_allowed
<'tcx
>(cx
: &LateContext
<'tcx
>, expr
: &'tcx Expr
<'_
>) -> bool
{
495 match constant(cx
, cx
.typeck_results(), expr
) {
496 Some((Constant
::F32(f
), _
)) => f
== 0.0 || f
.is_infinite(),
497 Some((Constant
::F64(f
), _
)) => f
== 0.0 || f
.is_infinite(),
498 Some((Constant
::Vec(vec
), _
)) => vec
.iter().all(|f
| match f
{
499 Constant
::F32(f
) => *f
== 0.0 || (*f
).is_infinite(),
500 Constant
::F64(f
) => *f
== 0.0 || (*f
).is_infinite(),
507 // Return true if `expr` is the result of `signum()` invoked on a float value.
508 fn is_signum(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>) -> bool
{
509 // The negation of a signum is still a signum
510 if let ExprKind
::Unary(UnOp
::Neg
, child_expr
) = expr
.kind
{
511 return is_signum(cx
, child_expr
);
515 if let ExprKind
::MethodCall(method_name
, _
, [ref self_arg
, ..], _
) = expr
.kind
;
516 if sym
!(signum
) == method_name
.ident
.name
;
517 // Check that the receiver of the signum() is a float (expressions[0] is the receiver of
520 return is_float(cx
, self_arg
);
526 fn is_float(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>) -> bool
{
527 let value
= &cx
.typeck_results().expr_ty(expr
).peel_refs().kind();
529 if let ty
::Array(arr_ty
, _
) = value
{
530 return matches
!(arr_ty
.kind(), ty
::Float(_
));
533 matches
!(value
, ty
::Float(_
))
536 fn is_array(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>) -> bool
{
537 matches
!(&cx
.typeck_results().expr_ty(expr
).peel_refs().kind(), ty
::Array(_
, _
))
540 fn check_to_owned(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>, other
: &Expr
<'_
>, left
: bool
) {
548 fn is_implemented(&self) -> bool
{
549 self.ty_eq_other
|| self.other_eq_ty
553 fn symmetric_partial_eq
<'tcx
>(cx
: &LateContext
<'tcx
>, ty
: Ty
<'tcx
>, other
: Ty
<'tcx
>) -> Option
<EqImpl
> {
554 cx
.tcx
.lang_items().eq_trait().map(|def_id
| EqImpl
{
555 ty_eq_other
: implements_trait(cx
, ty
, def_id
, &[other
.into()]),
556 other_eq_ty
: implements_trait(cx
, other
, def_id
, &[ty
.into()]),
560 let (arg_ty
, snip
) = match expr
.kind
{
561 ExprKind
::MethodCall(.., args
, _
) if args
.len() == 1 => {
563 if let Some(expr_def_id
) = cx
.typeck_results().type_dependent_def_id(expr
.hir_id
);
564 if is_diag_trait_item(cx
, expr_def_id
, sym
::ToString
)
565 || is_diag_trait_item(cx
, expr_def_id
, sym
::ToOwned
);
567 (cx
.typeck_results().expr_ty(&args
[0]), snippet(cx
, args
[0].span
, ".."))
573 ExprKind
::Call(path
, [arg
]) => {
574 if expr_path_res(cx
, path
)
576 .and_then(|id
| match_any_def_paths(cx
, id
, &[&paths
::FROM_STR_METHOD
, &paths
::FROM_FROM
]))
579 (cx
.typeck_results().expr_ty(arg
), snippet(cx
, arg
.span
, ".."))
587 let other_ty
= cx
.typeck_results().expr_ty(other
);
589 let without_deref
= symmetric_partial_eq(cx
, arg_ty
, other_ty
).unwrap_or_default();
590 let with_deref
= arg_ty
592 .and_then(|tam
| symmetric_partial_eq(cx
, tam
.ty
, other_ty
))
593 .unwrap_or_default();
595 if !with_deref
.is_implemented() && !without_deref
.is_implemented() {
599 let other_gets_derefed
= matches
!(other
.kind
, ExprKind
::Unary(UnOp
::Deref
, _
));
601 let lint_span
= if other_gets_derefed
{
602 expr
.span
.to(other
.span
)
611 "this creates an owned instance just for comparison",
613 // This also catches `PartialEq` implementations that call `to_owned`.
614 if other_gets_derefed
{
615 diag
.span_label(lint_span
, "try implementing the comparison without allocating");
621 if with_deref
.is_implemented() {
622 expr_snip
= format
!("*{}", snip
);
623 eq_impl
= with_deref
;
625 expr_snip
= snip
.to_string();
626 eq_impl
= without_deref
;
631 if (eq_impl
.ty_eq_other
&& left
) || (eq_impl
.other_eq_ty
&& !left
) {
635 span
= expr
.span
.to(other
.span
);
636 if eq_impl
.ty_eq_other
{
637 hint
= format
!("{} == {}", expr_snip
, snippet(cx
, other
.span
, ".."));
639 hint
= format
!("{} == {}", snippet(cx
, other
.span
, ".."), expr_snip
);
643 diag
.span_suggestion(
647 Applicability
::MachineApplicable
, // snippet
653 /// Heuristic to see if an expression is used. Should be compatible with
654 /// `unused_variables`'s idea
655 /// of what it means for an expression to be "used".
656 fn is_used(cx
: &LateContext
<'_
>, expr
: &Expr
<'_
>) -> bool
{
657 get_parent_expr(cx
, expr
).map_or(true, |parent
| match parent
.kind
{
658 ExprKind
::Assign(_
, rhs
, _
) | ExprKind
::AssignOp(_
, _
, rhs
) => SpanlessEq
::new(cx
).eq_expr(rhs
, expr
),
659 _
=> is_used(cx
, parent
),
663 /// Tests whether an expression is in a macro expansion (e.g., something
664 /// generated by `#[derive(...)]` or the like).
665 fn in_attributes_expansion(expr
: &Expr
<'_
>) -> bool
{
666 use rustc_span
::hygiene
::MacroKind
;
667 if expr
.span
.from_expansion() {
668 let data
= expr
.span
.ctxt().outer_expn_data();
669 matches
!(data
.kind
, ExpnKind
::Macro(MacroKind
::Attr
, _
))
675 /// Tests whether `res` is a variable defined outside a macro.
676 fn non_macro_local(cx
: &LateContext
<'_
>, res
: def
::Res
) -> bool
{
677 if let def
::Res
::Local(id
) = res
{
678 !cx
.tcx
.hir().span(id
).from_expansion()
684 fn check_cast(cx
: &LateContext
<'_
>, span
: Span
, e
: &Expr
<'_
>, ty
: &hir
::Ty
<'_
>) {
686 if let TyKind
::Ptr(ref mut_ty
) = ty
.kind
;
687 if let ExprKind
::Lit(ref lit
) = e
.kind
;
688 if let LitKind
::Int(0, _
) = lit
.node
;
689 if !in_constant(cx
, e
.hir_id
);
691 let (msg
, sugg_fn
) = match mut_ty
.mutbl
{
692 Mutability
::Mut
=> ("`0 as *mut _` detected", "std::ptr::null_mut"),
693 Mutability
::Not
=> ("`0 as *const _` detected", "std::ptr::null"),
696 let (sugg
, appl
) = if let TyKind
::Infer
= mut_ty
.ty
.kind
{
697 (format
!("{}()", sugg_fn
), Applicability
::MachineApplicable
)
698 } else if let Some(mut_ty_snip
) = snippet_opt(cx
, mut_ty
.ty
.span
) {
699 (format
!("{}::<{}>()", sugg_fn
, mut_ty_snip
), Applicability
::MachineApplicable
)
701 // `MaybeIncorrect` as type inference may not work with the suggested code
702 (format
!("{}()", sugg_fn
), Applicability
::MaybeIncorrect
)
704 span_lint_and_sugg(cx
, ZERO_PTR
, span
, msg
, "try", sugg
, appl
);
710 cx
: &LateContext
<'a
>,
712 cmp
: &rustc_span
::source_map
::Spanned
<rustc_hir
::BinOpKind
>,
717 if op
.is_comparison() {
718 check_nan(cx
, left
, expr
);
719 check_nan(cx
, right
, expr
);
720 check_to_owned(cx
, left
, right
, true);
721 check_to_owned(cx
, right
, left
, false);
723 if (op
== BinOpKind
::Eq
|| op
== BinOpKind
::Ne
) && (is_float(cx
, left
) || is_float(cx
, right
)) {
724 if is_allowed(cx
, left
) || is_allowed(cx
, right
) {
728 // Allow comparing the results of signum()
729 if is_signum(cx
, left
) && is_signum(cx
, right
) {
733 if let Some(name
) = get_item_name(cx
, expr
) {
734 let name
= name
.as_str();
735 if name
== "eq" || name
== "ne" || name
== "is_nan" || name
.starts_with("eq_") || name
.ends_with("_eq") {
739 let is_comparing_arrays
= is_array(cx
, left
) || is_array(cx
, right
);
740 let (lint
, msg
) = get_lint_and_message(
741 is_named_constant(cx
, left
) || is_named_constant(cx
, right
),
744 span_lint_and_then(cx
, lint
, expr
.span
, msg
, |diag
| {
745 let lhs
= Sugg
::hir(cx
, left
, "..");
746 let rhs
= Sugg
::hir(cx
, right
, "..");
748 if !is_comparing_arrays
{
749 diag
.span_suggestion(
751 "consider comparing them within some margin of error",
753 "({}).abs() {} error_margin",
755 if op
== BinOpKind
::Eq { '<' }
else { '>' }
757 Applicability
::HasPlaceholders
, // snippet
760 diag
.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`");
762 } else if op
== BinOpKind
::Rem
{
763 if is_integer_const(cx
, right
, 1) {
764 span_lint(cx
, MODULO_ONE
, expr
.span
, "any number modulo 1 will be 0");
767 if let ty
::Int(ity
) = cx
.typeck_results().expr_ty(right
).kind() {
768 if is_integer_const(cx
, right
, unsext(cx
.tcx
, -1, *ity
)) {
773 "any number modulo -1 will panic/overflow or result in 0",