]>
git.proxmox.com Git - rustc.git/blob - src/tools/clippy/clippy_lints/src/misc.rs
3 use rustc
::hir
::intravisit
::FnKind
;
6 use syntax
::codemap
::{ExpnFormat, Span}
;
7 use utils
::{get_item_name
, get_parent_expr
, implements_trait
, in_constant
, in_macro
, is_integer_literal
,
8 iter_input_pats
, last_path_segment
, match_qpath
, match_trait_method
, paths
, snippet
, span_lint
,
9 span_lint_and_then
, walk_ptrs_ty
};
10 use utils
::sugg
::Sugg
;
11 use syntax
::ast
::{LitKind, CRATE_NODE_ID}
;
12 use consts
::{constant, Constant}
;
14 /// **What it does:** Checks for function arguments and let bindings denoted as
17 /// **Why is this bad?** The `ref` declaration makes the function take an owned
18 /// value, but turns the argument into a reference (which means that the value
19 /// is destroyed when exiting the function). This adds not much value: either
20 /// take a reference type, or take an owned value and create references in the
23 /// For let bindings, `let x = &foo;` is preferred over `let ref x = foo`. The
24 /// type of `x` is more obvious with the former.
26 /// **Known problems:** If the argument is dereferenced within the function,
27 /// removing the `ref` will lead to errors. This can be fixed by removing the
28 /// dereferences, e.g. changing `*x` to `x` within the function.
32 /// fn foo(ref x: u8) -> bool { .. }
34 declare_clippy_lint
! {
37 "an entire binding declared as `ref`, in a function argument or a `let` statement"
40 /// **What it does:** Checks for comparisons to NaN.
42 /// **Why is this bad?** NaN does not compare meaningfully to anything – not
43 /// even itself – so those comparisons are simply wrong.
45 /// **Known problems:** None.
51 declare_clippy_lint
! {
54 "comparisons to NAN, which will always return false, probably not intended"
57 /// **What it does:** Checks for (in-)equality comparisons on floating-point
58 /// values (apart from zero), except in functions called `*eq*` (which probably
59 /// implement equality for a type involving floats).
61 /// **Why is this bad?** Floating point calculations are usually imprecise, so
62 /// asking if two values are *exactly* equal is asking for trouble. For a good
63 /// guide on what to do, see [the floating point
64 /// guide](http://www.floating-point-gui.de/errors/comparison).
66 /// **Known problems:** None.
71 /// y != x // where both are floats
73 declare_clippy_lint
! {
76 "using `==` or `!=` on float values instead of comparing difference with an epsilon"
79 /// **What it does:** Checks for conversions to owned values just for the sake
82 /// **Why is this bad?** The comparison can operate on a reference, so creating
83 /// an owned value effectively throws it away directly afterwards, which is
84 /// needlessly consuming code and heap space.
86 /// **Known problems:** None.
92 declare_clippy_lint
! {
95 "creating owned instances for comparing with others, e.g. `x == \"foo\".to_string()`"
98 /// **What it does:** Checks for getting the remainder of a division by one.
100 /// **Why is this bad?** The result can only ever be zero. No one will write
101 /// such code deliberately, unless trying to win an Underhanded Rust
102 /// Contest. Even for that contest, it's probably a bad idea. Use something more
105 /// **Known problems:** None.
111 declare_clippy_lint
! {
114 "taking a number modulo 1, which always returns 0"
117 /// **What it does:** Checks for patterns in the form `name @ _`.
119 /// **Why is this bad?** It's almost always more readable to just use direct
122 /// **Known problems:** None.
128 /// y @ _ => (), // easier written as `y`,
131 declare_clippy_lint
! {
132 pub REDUNDANT_PATTERN
,
134 "using `name @ _` in a pattern"
137 /// **What it does:** Checks for the use of bindings with a single leading
140 /// **Why is this bad?** A single leading underscore is usually used to indicate
141 /// that a binding will not be used. Using such a binding breaks this
144 /// **Known problems:** The lint does not work properly with desugaring and
145 /// macro, it has been allowed in the mean time.
150 /// let y = _x + 1; // Here we are using `_x`, even though it has a leading
151 /// // underscore. We should rename `_x` to `x`
153 declare_clippy_lint
! {
154 pub USED_UNDERSCORE_BINDING
,
156 "using a binding which is prefixed with an underscore"
159 /// **What it does:** Checks for the use of short circuit boolean conditions as
163 /// **Why is this bad?** Using a short circuit boolean condition as a statement
164 /// may hide the fact that the second part is executed or not depending on the
165 /// outcome of the first part.
167 /// **Known problems:** None.
171 /// f() && g(); // We should write `if f() { g(); }`.
173 declare_clippy_lint
! {
174 pub SHORT_CIRCUIT_STATEMENT
,
176 "using a short circuit boolean condition as a statement"
179 /// **What it does:** Catch casts from `0` to some pointer type
181 /// **Why is this bad?** This generally means `null` and is better expressed as
182 /// {`std`, `core`}`::ptr::`{`null`, `null_mut`}.
184 /// **Known problems:** None.
191 declare_clippy_lint
! {
194 "using 0 as *{const, mut} T"
197 /// **What it does:** Checks for (in-)equality comparisons on floating-point
198 /// value and constant, except in functions called `*eq*` (which probably
199 /// implement equality for a type involving floats).
201 /// **Why is this bad?** Floating point calculations are usually imprecise, so
202 /// asking if two values are *exactly* equal is asking for trouble. For a good
203 /// guide on what to do, see [the floating point
204 /// guide](http://www.floating-point-gui.de/errors/comparison).
206 /// **Known problems:** None.
210 /// const ONE == 1.00f64
211 /// x == ONE // where both are floats
213 declare_clippy_lint
! {
216 "using `==` or `!=` on float constants instead of comparing difference with an epsilon"
219 #[derive(Copy, Clone)]
222 impl LintPass
for Pass
{
223 fn get_lints(&self) -> LintArray
{
231 USED_UNDERSCORE_BINDING
,
232 SHORT_CIRCUIT_STATEMENT
,
239 impl<'a
, 'tcx
> LateLintPass
<'a
, 'tcx
> for Pass
{
242 cx
: &LateContext
<'a
, 'tcx
>,
249 if let FnKind
::Closure(_
) = k
{
250 // Does not apply to closures
253 for arg
in iter_input_pats(decl
, body
) {
255 PatKind
::Binding(BindingAnnotation
::Ref
, _
, _
, _
) |
256 PatKind
::Binding(BindingAnnotation
::RefMut
, _
, _
, _
) => {
261 "`ref` directly on a function argument is ignored. Consider using a reference type \
270 fn check_stmt(&mut self, cx
: &LateContext
<'a
, 'tcx
>, s
: &'tcx Stmt
) {
272 if let StmtDecl(ref d
, _
) = s
.node
;
273 if let DeclLocal(ref l
) = d
.node
;
274 if let PatKind
::Binding(an
, _
, i
, None
) = l
.pat
.node
;
275 if let Some(ref init
) = l
.init
;
277 if an
== BindingAnnotation
::Ref
|| an
== BindingAnnotation
::RefMut
{
278 let init
= Sugg
::hir(cx
, init
, "..");
279 let (mutopt
,initref
) = if an
== BindingAnnotation
::RefMut
{
280 ("mut ", init
.mut_addr())
284 let tyopt
= if let Some(ref ty
) = l
.ty
{
285 format
!(": &{mutopt}{ty}", mutopt
=mutopt
, ty
=snippet(cx
, ty
.span
, "_"))
289 span_lint_and_then(cx
,
292 "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead",
294 db
.span_suggestion(s
.span
,
296 format
!("let {name}{tyopt} = {initref};",
297 name
=snippet(cx
, i
.span
, "_"),
306 if let StmtSemi(ref expr
, _
) = s
.node
;
307 if let Expr_
::ExprBinary(ref binop
, ref a
, ref b
) = expr
.node
;
308 if binop
.node
== BiAnd
|| binop
.node
== BiOr
;
309 if let Some(sugg
) = Sugg
::hir_opt(cx
, a
);
311 span_lint_and_then(cx
,
312 SHORT_CIRCUIT_STATEMENT
,
314 "boolean short circuit operator in statement may be clearer using an explicit test",
316 let sugg
= if binop
.node
== BiOr { !sugg }
else { sugg }
;
317 db
.span_suggestion(s
.span
, "replace it with",
318 format
!("if {} {{ {}; }}", sugg
, &snippet(cx
, b
.span
, "..")));
324 fn check_expr(&mut self, cx
: &LateContext
<'a
, 'tcx
>, expr
: &'tcx Expr
) {
326 ExprCast(ref e
, ref ty
) => {
327 check_cast(cx
, expr
.span
, e
, ty
);
330 ExprBinary(ref cmp
, ref left
, ref right
) => {
332 if op
.is_comparison() {
333 if let ExprPath(QPath
::Resolved(_
, ref path
)) = left
.node
{
334 check_nan(cx
, path
, expr
);
336 if let ExprPath(QPath
::Resolved(_
, ref path
)) = right
.node
{
337 check_nan(cx
, path
, expr
);
339 check_to_owned(cx
, left
, right
);
340 check_to_owned(cx
, right
, left
);
342 if (op
== BiEq
|| op
== BiNe
) && (is_float(cx
, left
) || is_float(cx
, right
)) {
343 if is_allowed(cx
, left
) || is_allowed(cx
, right
) {
346 if let Some(name
) = get_item_name(cx
, expr
) {
347 let name
= name
.as_str();
348 if name
== "eq" || name
== "ne" || name
== "is_nan" || name
.starts_with("eq_")
349 || name
.ends_with("_eq")
354 let (lint
, msg
) = if is_named_constant(cx
, left
) || is_named_constant(cx
, right
) {
355 (FLOAT_CMP_CONST
, "strict comparison of f32 or f64 constant")
357 (FLOAT_CMP
, "strict comparison of f32 or f64")
359 span_lint_and_then(cx
, lint
, expr
.span
, msg
, |db
| {
360 let lhs
= Sugg
::hir(cx
, left
, "..");
361 let rhs
= Sugg
::hir(cx
, right
, "..");
365 "consider comparing them within some error",
366 format
!("({}).abs() < error", lhs
- rhs
),
368 db
.span_note(expr
.span
, "std::f32::EPSILON and std::f64::EPSILON are available.");
370 } else if op
== BiRem
&& is_integer_literal(right
, 1) {
371 span_lint(cx
, MODULO_ONE
, expr
.span
, "any number modulo 1 will be 0");
376 if in_attributes_expansion(expr
) {
377 // Don't lint things expanded by #[derive(...)], etc
380 let binding
= match expr
.node
{
381 ExprPath(ref qpath
) => {
382 let binding
= last_path_segment(qpath
).name
.as_str();
383 if binding
.starts_with('_'
) &&
384 !binding
.starts_with("__") &&
385 binding
!= "_result" && // FIXME: #944
387 // don't lint if the declaration is in a macro
388 non_macro_local(cx
, &cx
.tables
.qpath_def(qpath
, expr
.hir_id
))
395 ExprField(_
, spanned
) => {
396 let name
= spanned
.node
.as_str();
397 if name
.starts_with('_'
) && !name
.starts_with("__") {
405 if let Some(binding
) = binding
{
408 USED_UNDERSCORE_BINDING
,
411 "used binding `{}` which is prefixed with an underscore. A leading \
412 underscore signals that a binding will not be used.",
419 fn check_pat(&mut self, cx
: &LateContext
<'a
, 'tcx
>, pat
: &'tcx Pat
) {
420 if let PatKind
::Binding(_
, _
, ref ident
, Some(ref right
)) = pat
.node
{
421 if right
.node
== PatKind
::Wild
{
426 &format
!("the `{} @ _` pattern can be written as just `{}`", ident
.node
, ident
.node
),
433 fn check_nan(cx
: &LateContext
, path
: &Path
, expr
: &Expr
) {
434 if !in_constant(cx
, expr
.id
) {
435 if let Some(seg
) = path
.segments
.last() {
436 if seg
.name
== "NAN" {
441 "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead",
448 fn is_named_constant
<'a
, 'tcx
>(cx
: &LateContext
<'a
, 'tcx
>, expr
: &'tcx Expr
) -> bool
{
449 if let Some((_
, res
)) = constant(cx
, cx
.tables
, expr
) {
456 fn is_allowed
<'a
, 'tcx
>(cx
: &LateContext
<'a
, 'tcx
>, expr
: &'tcx Expr
) -> bool
{
457 match constant(cx
, cx
.tables
, expr
) {
458 Some((Constant
::F32(f
), _
)) => f
== 0.0 || f
.is_infinite(),
459 Some((Constant
::F64(f
), _
)) => f
== 0.0 || f
.is_infinite(),
464 fn is_float(cx
: &LateContext
, expr
: &Expr
) -> bool
{
465 matches
!(walk_ptrs_ty(cx
.tables
.expr_ty(expr
)).sty
, ty
::TyFloat(_
))
468 fn check_to_owned(cx
: &LateContext
, expr
: &Expr
, other
: &Expr
) {
469 let (arg_ty
, snip
) = match expr
.node
{
470 ExprMethodCall(.., ref args
) if args
.len() == 1 => {
471 if match_trait_method(cx
, expr
, &paths
::TO_STRING
) || match_trait_method(cx
, expr
, &paths
::TO_OWNED
) {
472 (cx
.tables
.expr_ty_adjusted(&args
[0]), snippet(cx
, args
[0].span
, ".."))
477 ExprCall(ref path
, ref v
) if v
.len() == 1 => if let ExprPath(ref path
) = path
.node
{
478 if match_qpath(path
, &["String", "from_str"]) || match_qpath(path
, &["String", "from"]) {
479 (cx
.tables
.expr_ty_adjusted(&v
[0]), snippet(cx
, v
[0].span
, ".."))
489 let other_ty
= cx
.tables
.expr_ty_adjusted(other
);
490 let partial_eq_trait_id
= match cx
.tcx
.lang_items().eq_trait() {
495 // *arg impls PartialEq<other>
498 .map_or(false, |tam
| implements_trait(cx
, tam
.ty
, partial_eq_trait_id
, &[other_ty
]))
499 // arg impls PartialEq<*other>
502 .map_or(false, |tam
| implements_trait(cx
, arg_ty
, partial_eq_trait_id
, &[tam
.ty
]))
503 // arg impls PartialEq<other>
504 && !implements_trait(cx
, arg_ty
, partial_eq_trait_id
, &[other_ty
])
513 "this creates an owned instance just for comparison",
515 // this is as good as our recursion check can get, we can't prove that the
516 // current function is
518 // PartialEq::eq, but we can at least ensure that this code is not part of it
519 let parent_fn
= cx
.tcx
.hir
.get_parent(expr
.id
);
520 let parent_impl
= cx
.tcx
.hir
.get_parent(parent_fn
);
521 if parent_impl
!= CRATE_NODE_ID
{
522 if let map
::NodeItem(item
) = cx
.tcx
.hir
.get(parent_impl
) {
523 if let ItemImpl(.., Some(ref trait_ref
), _
, _
) = item
.node
{
524 if trait_ref
.path
.def
.def_id() == partial_eq_trait_id
{
525 // we are implementing PartialEq, don't suggest not doing `to_owned`, otherwise
528 db
.span_label(expr
.span
, "try calling implementing the comparison without allocating");
534 db
.span_suggestion(expr
.span
, "try", snip
.to_string());
539 /// Heuristic to see if an expression is used. Should be compatible with
540 /// `unused_variables`'s idea
541 /// of what it means for an expression to be "used".
542 fn is_used(cx
: &LateContext
, expr
: &Expr
) -> bool
{
543 if let Some(parent
) = get_parent_expr(cx
, expr
) {
545 ExprAssign(_
, ref rhs
) | ExprAssignOp(_
, _
, ref rhs
) => **rhs
== *expr
,
546 _
=> is_used(cx
, parent
),
553 /// Test whether an expression is in a macro expansion (e.g. something
555 /// `#[derive(...)`] or the like).
556 fn in_attributes_expansion(expr
: &Expr
) -> bool
{
561 .map_or(false, |info
| matches
!(info
.callee
.format
, ExpnFormat
::MacroAttribute(_
)))
564 /// Test whether `def` is a variable defined outside a macro.
565 fn non_macro_local(cx
: &LateContext
, def
: &def
::Def
) -> bool
{
567 def
::Def
::Local(id
) | def
::Def
::Upvar(id
, _
, _
) => !in_macro(cx
.tcx
.hir
.span(id
)),
572 fn check_cast(cx
: &LateContext
, span
: Span
, e
: &Expr
, ty
: &Ty
) {
574 if let TyPtr(MutTy { mutbl, .. }
) = ty
.node
;
575 if let ExprLit(ref lit
) = e
.node
;
576 if let LitKind
::Int(value
, ..) = lit
.node
;
578 if !in_constant(cx
, e
.id
);
580 let msg
= match mutbl
{
581 Mutability
::MutMutable
=> "`0 as *mut _` detected. Consider using `ptr::null_mut()`",
582 Mutability
::MutImmutable
=> "`0 as *const _` detected. Consider using `ptr::null()`",
584 span_lint(cx
, ZERO_PTR
, span
, msg
);