]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 XL |
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; | |
f20569fa XL |
4 | use if_chain::if_chain; |
5 | use rustc_ast::ast::LitKind; | |
6 | use rustc_errors::Applicability; | |
7 | use rustc_hir::intravisit::FnKind; | |
8 | use rustc_hir::{ | |
9 | self as hir, def, BinOpKind, BindingAnnotation, Body, Expr, ExprKind, FnDecl, HirId, Mutability, PatKind, Stmt, | |
10 | StmtKind, TyKind, UnOp, | |
11 | }; | |
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; | |
19 | ||
17df50a5 | 20 | use clippy_utils::consts::{constant, Constant}; |
cdc7bbd5 XL |
21 | use clippy_utils::sugg::Sugg; |
22 | use clippy_utils::{ | |
5099ac24 FG |
23 | 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, path_def_id, paths, unsext, SpanlessEq, | |
f20569fa XL |
25 | }; |
26 | ||
27 | declare_clippy_lint! { | |
94222f64 XL |
28 | /// ### What it does |
29 | /// Checks for function arguments and let bindings denoted as | |
f20569fa XL |
30 | /// `ref`. |
31 | /// | |
94222f64 XL |
32 | /// ### Why is this bad? |
33 | /// The `ref` declaration makes the function take an owned | |
f20569fa XL |
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 | |
37 | /// body. | |
38 | /// | |
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. | |
41 | /// | |
94222f64 XL |
42 | /// ### Known problems |
43 | /// If the argument is dereferenced within the function, | |
f20569fa XL |
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. | |
46 | /// | |
94222f64 | 47 | /// ### Example |
f20569fa XL |
48 | /// ```rust,ignore |
49 | /// // Bad | |
50 | /// fn foo(ref x: u8) -> bool { | |
51 | /// true | |
52 | /// } | |
53 | /// | |
54 | /// // Good | |
55 | /// fn foo(x: &u8) -> bool { | |
56 | /// true | |
57 | /// } | |
58 | /// ``` | |
a2a8927a | 59 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
60 | pub TOPLEVEL_REF_ARG, |
61 | style, | |
62 | "an entire binding declared as `ref`, in a function argument or a `let` statement" | |
63 | } | |
64 | ||
65 | declare_clippy_lint! { | |
94222f64 XL |
66 | /// ### What it does |
67 | /// Checks for comparisons to NaN. | |
f20569fa | 68 | /// |
94222f64 XL |
69 | /// ### Why is this bad? |
70 | /// NaN does not compare meaningfully to anything – not | |
f20569fa XL |
71 | /// even itself – so those comparisons are simply wrong. |
72 | /// | |
94222f64 | 73 | /// ### Example |
f20569fa XL |
74 | /// ```rust |
75 | /// # let x = 1.0; | |
76 | /// | |
77 | /// // Bad | |
78 | /// if x == f32::NAN { } | |
79 | /// | |
80 | /// // Good | |
81 | /// if x.is_nan() { } | |
82 | /// ``` | |
a2a8927a | 83 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
84 | pub CMP_NAN, |
85 | correctness, | |
86 | "comparisons to `NAN`, which will always return false, probably not intended" | |
87 | } | |
88 | ||
89 | declare_clippy_lint! { | |
94222f64 XL |
90 | /// ### What it does |
91 | /// Checks for (in-)equality comparisons on floating-point | |
f20569fa XL |
92 | /// values (apart from zero), except in functions called `*eq*` (which probably |
93 | /// implement equality for a type involving floats). | |
94 | /// | |
94222f64 XL |
95 | /// ### Why is this bad? |
96 | /// Floating point calculations are usually imprecise, so | |
f20569fa XL |
97 | /// asking if two values are *exactly* equal is asking for trouble. For a good |
98 | /// guide on what to do, see [the floating point | |
99 | /// guide](http://www.floating-point-gui.de/errors/comparison). | |
100 | /// | |
94222f64 | 101 | /// ### Example |
f20569fa XL |
102 | /// ```rust |
103 | /// let x = 1.2331f64; | |
104 | /// let y = 1.2332f64; | |
105 | /// | |
106 | /// // Bad | |
107 | /// if y == 1.23f64 { } | |
108 | /// if y != x {} // where both are floats | |
109 | /// | |
110 | /// // Good | |
111 | /// let error_margin = f64::EPSILON; // Use an epsilon for comparison | |
112 | /// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead. | |
113 | /// // let error_margin = std::f64::EPSILON; | |
114 | /// if (y - 1.23f64).abs() < error_margin { } | |
115 | /// if (y - x).abs() > error_margin { } | |
116 | /// ``` | |
a2a8927a | 117 | #[clippy::version = "pre 1.29.0"] |
f20569fa | 118 | pub FLOAT_CMP, |
c295e0f8 | 119 | pedantic, |
f20569fa XL |
120 | "using `==` or `!=` on float values instead of comparing difference with an epsilon" |
121 | } | |
122 | ||
123 | declare_clippy_lint! { | |
94222f64 XL |
124 | /// ### What it does |
125 | /// Checks for conversions to owned values just for the sake | |
f20569fa XL |
126 | /// of a comparison. |
127 | /// | |
94222f64 XL |
128 | /// ### Why is this bad? |
129 | /// The comparison can operate on a reference, so creating | |
f20569fa XL |
130 | /// an owned value effectively throws it away directly afterwards, which is |
131 | /// needlessly consuming code and heap space. | |
132 | /// | |
94222f64 | 133 | /// ### Example |
f20569fa XL |
134 | /// ```rust |
135 | /// # let x = "foo"; | |
136 | /// # let y = String::from("foo"); | |
137 | /// if x.to_owned() == y {} | |
138 | /// ``` | |
139 | /// Could be written as | |
140 | /// ```rust | |
141 | /// # let x = "foo"; | |
142 | /// # let y = String::from("foo"); | |
143 | /// if x == y {} | |
144 | /// ``` | |
a2a8927a | 145 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
146 | pub CMP_OWNED, |
147 | perf, | |
148 | "creating owned instances for comparing with others, e.g., `x == \"foo\".to_string()`" | |
149 | } | |
150 | ||
151 | declare_clippy_lint! { | |
94222f64 XL |
152 | /// ### What it does |
153 | /// Checks for getting the remainder of a division by one or minus | |
f20569fa XL |
154 | /// one. |
155 | /// | |
94222f64 XL |
156 | /// ### Why is this bad? |
157 | /// The result for a divisor of one can only ever be zero; for | |
f20569fa XL |
158 | /// minus one it can cause panic/overflow (if the left operand is the minimal value of |
159 | /// the respective integer type) or results in zero. No one will write such code | |
160 | /// deliberately, unless trying to win an Underhanded Rust Contest. Even for that | |
161 | /// contest, it's probably a bad idea. Use something more underhanded. | |
162 | /// | |
94222f64 | 163 | /// ### Example |
f20569fa XL |
164 | /// ```rust |
165 | /// # let x = 1; | |
166 | /// let a = x % 1; | |
167 | /// let a = x % -1; | |
168 | /// ``` | |
a2a8927a | 169 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
170 | pub MODULO_ONE, |
171 | correctness, | |
172 | "taking a number modulo +/-1, which can either panic/overflow or always returns 0" | |
173 | } | |
174 | ||
175 | declare_clippy_lint! { | |
94222f64 XL |
176 | /// ### What it does |
177 | /// Checks for the use of bindings with a single leading | |
f20569fa XL |
178 | /// underscore. |
179 | /// | |
94222f64 XL |
180 | /// ### Why is this bad? |
181 | /// A single leading underscore is usually used to indicate | |
f20569fa XL |
182 | /// that a binding will not be used. Using such a binding breaks this |
183 | /// expectation. | |
184 | /// | |
94222f64 XL |
185 | /// ### Known problems |
186 | /// The lint does not work properly with desugaring and | |
f20569fa XL |
187 | /// macro, it has been allowed in the mean time. |
188 | /// | |
94222f64 | 189 | /// ### Example |
f20569fa XL |
190 | /// ```rust |
191 | /// let _x = 0; | |
192 | /// let y = _x + 1; // Here we are using `_x`, even though it has a leading | |
193 | /// // underscore. We should rename `_x` to `x` | |
194 | /// ``` | |
a2a8927a | 195 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
196 | pub USED_UNDERSCORE_BINDING, |
197 | pedantic, | |
198 | "using a binding which is prefixed with an underscore" | |
199 | } | |
200 | ||
201 | declare_clippy_lint! { | |
94222f64 XL |
202 | /// ### What it does |
203 | /// Checks for the use of short circuit boolean conditions as | |
f20569fa XL |
204 | /// a |
205 | /// statement. | |
206 | /// | |
94222f64 XL |
207 | /// ### Why is this bad? |
208 | /// Using a short circuit boolean condition as a statement | |
f20569fa XL |
209 | /// may hide the fact that the second part is executed or not depending on the |
210 | /// outcome of the first part. | |
211 | /// | |
94222f64 | 212 | /// ### Example |
f20569fa XL |
213 | /// ```rust,ignore |
214 | /// f() && g(); // We should write `if f() { g(); }`. | |
215 | /// ``` | |
a2a8927a | 216 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
217 | pub SHORT_CIRCUIT_STATEMENT, |
218 | complexity, | |
219 | "using a short circuit boolean condition as a statement" | |
220 | } | |
221 | ||
222 | declare_clippy_lint! { | |
94222f64 XL |
223 | /// ### What it does |
224 | /// Catch casts from `0` to some pointer type | |
f20569fa | 225 | /// |
94222f64 XL |
226 | /// ### Why is this bad? |
227 | /// This generally means `null` and is better expressed as | |
f20569fa XL |
228 | /// {`std`, `core`}`::ptr::`{`null`, `null_mut`}. |
229 | /// | |
94222f64 | 230 | /// ### Example |
f20569fa XL |
231 | /// ```rust |
232 | /// // Bad | |
233 | /// let a = 0 as *const u32; | |
234 | /// | |
235 | /// // Good | |
236 | /// let a = std::ptr::null::<u32>(); | |
237 | /// ``` | |
a2a8927a | 238 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
239 | pub ZERO_PTR, |
240 | style, | |
241 | "using `0 as *{const, mut} T`" | |
242 | } | |
243 | ||
244 | declare_clippy_lint! { | |
94222f64 XL |
245 | /// ### What it does |
246 | /// Checks for (in-)equality comparisons on floating-point | |
f20569fa XL |
247 | /// value and constant, except in functions called `*eq*` (which probably |
248 | /// implement equality for a type involving floats). | |
249 | /// | |
94222f64 XL |
250 | /// ### Why is this bad? |
251 | /// Floating point calculations are usually imprecise, so | |
f20569fa XL |
252 | /// asking if two values are *exactly* equal is asking for trouble. For a good |
253 | /// guide on what to do, see [the floating point | |
254 | /// guide](http://www.floating-point-gui.de/errors/comparison). | |
255 | /// | |
94222f64 | 256 | /// ### Example |
f20569fa XL |
257 | /// ```rust |
258 | /// let x: f64 = 1.0; | |
259 | /// const ONE: f64 = 1.00; | |
260 | /// | |
261 | /// // Bad | |
262 | /// if x == ONE { } // where both are floats | |
263 | /// | |
264 | /// // Good | |
265 | /// let error_margin = f64::EPSILON; // Use an epsilon for comparison | |
266 | /// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead. | |
267 | /// // let error_margin = std::f64::EPSILON; | |
268 | /// if (x - ONE).abs() < error_margin { } | |
269 | /// ``` | |
a2a8927a | 270 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
271 | pub FLOAT_CMP_CONST, |
272 | restriction, | |
273 | "using `==` or `!=` on float constants instead of comparing difference with an epsilon" | |
274 | } | |
275 | ||
276 | declare_lint_pass!(MiscLints => [ | |
277 | TOPLEVEL_REF_ARG, | |
278 | CMP_NAN, | |
279 | FLOAT_CMP, | |
280 | CMP_OWNED, | |
281 | MODULO_ONE, | |
282 | USED_UNDERSCORE_BINDING, | |
283 | SHORT_CIRCUIT_STATEMENT, | |
284 | ZERO_PTR, | |
285 | FLOAT_CMP_CONST | |
286 | ]); | |
287 | ||
288 | impl<'tcx> LateLintPass<'tcx> for MiscLints { | |
289 | fn check_fn( | |
290 | &mut self, | |
291 | cx: &LateContext<'tcx>, | |
292 | k: FnKind<'tcx>, | |
293 | decl: &'tcx FnDecl<'_>, | |
294 | body: &'tcx Body<'_>, | |
295 | span: Span, | |
296 | _: HirId, | |
297 | ) { | |
298 | if let FnKind::Closure = k { | |
299 | // Does not apply to closures | |
300 | return; | |
301 | } | |
302 | if in_external_macro(cx.tcx.sess, span) { | |
303 | return; | |
304 | } | |
305 | for arg in iter_input_pats(decl, body) { | |
306 | if let PatKind::Binding(BindingAnnotation::Ref | BindingAnnotation::RefMut, ..) = arg.pat.kind { | |
307 | span_lint( | |
308 | cx, | |
309 | TOPLEVEL_REF_ARG, | |
310 | arg.pat.span, | |
311 | "`ref` directly on a function argument is ignored. \ | |
312 | Consider using a reference type instead", | |
313 | ); | |
314 | } | |
315 | } | |
316 | } | |
317 | ||
318 | fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { | |
319 | if_chain! { | |
320 | if !in_external_macro(cx.tcx.sess, stmt.span); | |
cdc7bbd5 | 321 | if let StmtKind::Local(local) = stmt.kind; |
f20569fa | 322 | if let PatKind::Binding(an, .., name, None) = local.pat.kind; |
cdc7bbd5 | 323 | if let Some(init) = local.init; |
cdc7bbd5 | 324 | if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut; |
f20569fa | 325 | then { |
cdc7bbd5 XL |
326 | // use the macro callsite when the init span (but not the whole local span) |
327 | // comes from an expansion like `vec![1, 2, 3]` in `let ref _ = vec![1, 2, 3];` | |
328 | let sugg_init = if init.span.from_expansion() && !local.span.from_expansion() { | |
329 | Sugg::hir_with_macro_callsite(cx, init, "..") | |
330 | } else { | |
331 | Sugg::hir(cx, init, "..") | |
332 | }; | |
333 | let (mutopt, initref) = if an == BindingAnnotation::RefMut { | |
334 | ("mut ", sugg_init.mut_addr()) | |
335 | } else { | |
336 | ("", sugg_init.addr()) | |
337 | }; | |
338 | let tyopt = if let Some(ty) = local.ty { | |
339 | format!(": &{mutopt}{ty}", mutopt=mutopt, ty=snippet(cx, ty.span, "..")) | |
340 | } else { | |
341 | String::new() | |
342 | }; | |
343 | span_lint_hir_and_then( | |
344 | cx, | |
345 | TOPLEVEL_REF_ARG, | |
346 | init.hir_id, | |
347 | local.pat.span, | |
348 | "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead", | |
349 | |diag| { | |
350 | diag.span_suggestion( | |
351 | stmt.span, | |
352 | "try", | |
353 | format!( | |
354 | "let {name}{tyopt} = {initref};", | |
355 | name=snippet(cx, name.span, ".."), | |
356 | tyopt=tyopt, | |
357 | initref=initref, | |
358 | ), | |
359 | Applicability::MachineApplicable, | |
360 | ); | |
361 | } | |
362 | ); | |
f20569fa XL |
363 | } |
364 | }; | |
365 | if_chain! { | |
cdc7bbd5 XL |
366 | if let StmtKind::Semi(expr) = stmt.kind; |
367 | if let ExprKind::Binary(ref binop, a, b) = expr.kind; | |
f20569fa XL |
368 | if binop.node == BinOpKind::And || binop.node == BinOpKind::Or; |
369 | if let Some(sugg) = Sugg::hir_opt(cx, a); | |
370 | then { | |
17df50a5 XL |
371 | span_lint_hir_and_then( |
372 | cx, | |
f20569fa | 373 | SHORT_CIRCUIT_STATEMENT, |
17df50a5 | 374 | expr.hir_id, |
f20569fa XL |
375 | stmt.span, |
376 | "boolean short circuit operator in statement may be clearer using an explicit test", | |
377 | |diag| { | |
378 | let sugg = if binop.node == BinOpKind::Or { !sugg } else { sugg }; | |
379 | diag.span_suggestion( | |
380 | stmt.span, | |
381 | "replace it with", | |
382 | format!( | |
383 | "if {} {{ {}; }}", | |
384 | sugg, | |
385 | &snippet(cx, b.span, ".."), | |
386 | ), | |
387 | Applicability::MachineApplicable, // snippet | |
388 | ); | |
389 | }); | |
390 | } | |
391 | }; | |
392 | } | |
393 | ||
394 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | |
395 | match expr.kind { | |
cdc7bbd5 | 396 | ExprKind::Cast(e, ty) => { |
f20569fa XL |
397 | check_cast(cx, expr.span, e, ty); |
398 | return; | |
399 | }, | |
cdc7bbd5 | 400 | ExprKind::Binary(ref cmp, left, right) => { |
f20569fa XL |
401 | check_binary(cx, expr, cmp, left, right); |
402 | return; | |
403 | }, | |
404 | _ => {}, | |
405 | } | |
406 | if in_attributes_expansion(expr) || expr.span.is_desugaring(DesugaringKind::Await) { | |
407 | // Don't lint things expanded by #[derive(...)], etc or `await` desugaring | |
408 | return; | |
409 | } | |
a2a8927a | 410 | let sym; |
f20569fa XL |
411 | let binding = match expr.kind { |
412 | ExprKind::Path(ref qpath) if !matches!(qpath, hir::QPath::LangItem(..)) => { | |
413 | let binding = last_path_segment(qpath).ident.as_str(); | |
414 | if binding.starts_with('_') && | |
415 | !binding.starts_with("__") && | |
416 | binding != "_result" && // FIXME: #944 | |
417 | is_used(cx, expr) && | |
418 | // don't lint if the declaration is in a macro | |
419 | non_macro_local(cx, cx.qpath_res(qpath, expr.hir_id)) | |
420 | { | |
421 | Some(binding) | |
422 | } else { | |
423 | None | |
424 | } | |
425 | }, | |
426 | ExprKind::Field(_, ident) => { | |
a2a8927a XL |
427 | sym = ident.name; |
428 | let name = sym.as_str(); | |
f20569fa XL |
429 | if name.starts_with('_') && !name.starts_with("__") { |
430 | Some(name) | |
431 | } else { | |
432 | None | |
433 | } | |
434 | }, | |
435 | _ => None, | |
436 | }; | |
437 | if let Some(binding) = binding { | |
438 | span_lint( | |
439 | cx, | |
440 | USED_UNDERSCORE_BINDING, | |
441 | expr.span, | |
442 | &format!( | |
443 | "used binding `{}` which is prefixed with an underscore. A leading \ | |
444 | underscore signals that a binding will not be used", | |
445 | binding | |
446 | ), | |
447 | ); | |
448 | } | |
449 | } | |
450 | } | |
451 | ||
452 | fn get_lint_and_message( | |
453 | is_comparing_constants: bool, | |
454 | is_comparing_arrays: bool, | |
455 | ) -> (&'static rustc_lint::Lint, &'static str) { | |
456 | if is_comparing_constants { | |
457 | ( | |
458 | FLOAT_CMP_CONST, | |
459 | if is_comparing_arrays { | |
460 | "strict comparison of `f32` or `f64` constant arrays" | |
461 | } else { | |
462 | "strict comparison of `f32` or `f64` constant" | |
463 | }, | |
464 | ) | |
465 | } else { | |
466 | ( | |
467 | FLOAT_CMP, | |
468 | if is_comparing_arrays { | |
469 | "strict comparison of `f32` or `f64` arrays" | |
470 | } else { | |
471 | "strict comparison of `f32` or `f64`" | |
472 | }, | |
473 | ) | |
474 | } | |
475 | } | |
476 | ||
477 | fn check_nan(cx: &LateContext<'_>, expr: &Expr<'_>, cmp_expr: &Expr<'_>) { | |
478 | if_chain! { | |
479 | if !in_constant(cx, cmp_expr.hir_id); | |
480 | if let Some((value, _)) = constant(cx, cx.typeck_results(), expr); | |
cdc7bbd5 XL |
481 | if match value { |
482 | Constant::F32(num) => num.is_nan(), | |
483 | Constant::F64(num) => num.is_nan(), | |
484 | _ => false, | |
485 | }; | |
f20569fa | 486 | then { |
cdc7bbd5 XL |
487 | span_lint( |
488 | cx, | |
489 | CMP_NAN, | |
490 | cmp_expr.span, | |
491 | "doomed comparison with `NAN`, use `{f32,f64}::is_nan()` instead", | |
492 | ); | |
f20569fa XL |
493 | } |
494 | } | |
495 | } | |
496 | ||
497 | fn is_named_constant<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { | |
498 | if let Some((_, res)) = constant(cx, cx.typeck_results(), expr) { | |
499 | res | |
500 | } else { | |
501 | false | |
502 | } | |
503 | } | |
504 | ||
505 | fn is_allowed<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { | |
506 | match constant(cx, cx.typeck_results(), expr) { | |
507 | Some((Constant::F32(f), _)) => f == 0.0 || f.is_infinite(), | |
508 | Some((Constant::F64(f), _)) => f == 0.0 || f.is_infinite(), | |
509 | Some((Constant::Vec(vec), _)) => vec.iter().all(|f| match f { | |
510 | Constant::F32(f) => *f == 0.0 || (*f).is_infinite(), | |
511 | Constant::F64(f) => *f == 0.0 || (*f).is_infinite(), | |
512 | _ => false, | |
513 | }), | |
514 | _ => false, | |
515 | } | |
516 | } | |
517 | ||
518 | // Return true if `expr` is the result of `signum()` invoked on a float value. | |
519 | fn is_signum(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { | |
520 | // The negation of a signum is still a signum | |
cdc7bbd5 XL |
521 | if let ExprKind::Unary(UnOp::Neg, child_expr) = expr.kind { |
522 | return is_signum(cx, child_expr); | |
f20569fa XL |
523 | } |
524 | ||
525 | if_chain! { | |
5099ac24 | 526 | if let ExprKind::MethodCall(method_name, [ref self_arg, ..], _) = expr.kind; |
f20569fa XL |
527 | if sym!(signum) == method_name.ident.name; |
528 | // Check that the receiver of the signum() is a float (expressions[0] is the receiver of | |
529 | // the method call) | |
530 | then { | |
c295e0f8 | 531 | return is_float(cx, self_arg); |
f20569fa XL |
532 | } |
533 | } | |
534 | false | |
535 | } | |
536 | ||
537 | fn is_float(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { | |
538 | let value = &cx.typeck_results().expr_ty(expr).peel_refs().kind(); | |
539 | ||
540 | if let ty::Array(arr_ty, _) = value { | |
541 | return matches!(arr_ty.kind(), ty::Float(_)); | |
542 | }; | |
543 | ||
544 | matches!(value, ty::Float(_)) | |
545 | } | |
546 | ||
547 | fn is_array(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { | |
548 | matches!(&cx.typeck_results().expr_ty(expr).peel_refs().kind(), ty::Array(_, _)) | |
549 | } | |
550 | ||
5099ac24 | 551 | #[allow(clippy::too_many_lines)] |
f20569fa XL |
552 | fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left: bool) { |
553 | #[derive(Default)] | |
554 | struct EqImpl { | |
555 | ty_eq_other: bool, | |
556 | other_eq_ty: bool, | |
557 | } | |
558 | ||
559 | impl EqImpl { | |
560 | fn is_implemented(&self) -> bool { | |
561 | self.ty_eq_other || self.other_eq_ty | |
562 | } | |
563 | } | |
564 | ||
565 | fn symmetric_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> Option<EqImpl> { | |
566 | cx.tcx.lang_items().eq_trait().map(|def_id| EqImpl { | |
567 | ty_eq_other: implements_trait(cx, ty, def_id, &[other.into()]), | |
568 | other_eq_ty: implements_trait(cx, other, def_id, &[ty.into()]), | |
569 | }) | |
570 | } | |
571 | ||
572 | let (arg_ty, snip) = match expr.kind { | |
cdc7bbd5 | 573 | ExprKind::MethodCall(.., args, _) if args.len() == 1 => { |
f20569fa XL |
574 | if_chain!( |
575 | if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); | |
cdc7bbd5 XL |
576 | if is_diag_trait_item(cx, expr_def_id, sym::ToString) |
577 | || is_diag_trait_item(cx, expr_def_id, sym::ToOwned); | |
f20569fa XL |
578 | then { |
579 | (cx.typeck_results().expr_ty(&args[0]), snippet(cx, args[0].span, "..")) | |
580 | } else { | |
581 | return; | |
582 | } | |
583 | ) | |
584 | }, | |
cdc7bbd5 | 585 | ExprKind::Call(path, [arg]) => { |
5099ac24 | 586 | if path_def_id(cx, path) |
cdc7bbd5 XL |
587 | .and_then(|id| match_any_def_paths(cx, id, &[&paths::FROM_STR_METHOD, &paths::FROM_FROM])) |
588 | .is_some() | |
589 | { | |
590 | (cx.typeck_results().expr_ty(arg), snippet(cx, arg.span, "..")) | |
f20569fa XL |
591 | } else { |
592 | return; | |
593 | } | |
594 | }, | |
595 | _ => return, | |
596 | }; | |
597 | ||
598 | let other_ty = cx.typeck_results().expr_ty(other); | |
599 | ||
600 | let without_deref = symmetric_partial_eq(cx, arg_ty, other_ty).unwrap_or_default(); | |
601 | let with_deref = arg_ty | |
602 | .builtin_deref(true) | |
603 | .and_then(|tam| symmetric_partial_eq(cx, tam.ty, other_ty)) | |
604 | .unwrap_or_default(); | |
605 | ||
606 | if !with_deref.is_implemented() && !without_deref.is_implemented() { | |
607 | return; | |
608 | } | |
609 | ||
610 | let other_gets_derefed = matches!(other.kind, ExprKind::Unary(UnOp::Deref, _)); | |
611 | ||
612 | let lint_span = if other_gets_derefed { | |
613 | expr.span.to(other.span) | |
614 | } else { | |
615 | expr.span | |
616 | }; | |
617 | ||
618 | span_lint_and_then( | |
619 | cx, | |
620 | CMP_OWNED, | |
621 | lint_span, | |
622 | "this creates an owned instance just for comparison", | |
623 | |diag| { | |
624 | // This also catches `PartialEq` implementations that call `to_owned`. | |
625 | if other_gets_derefed { | |
626 | diag.span_label(lint_span, "try implementing the comparison without allocating"); | |
627 | return; | |
628 | } | |
629 | ||
630 | let expr_snip; | |
631 | let eq_impl; | |
632 | if with_deref.is_implemented() { | |
633 | expr_snip = format!("*{}", snip); | |
634 | eq_impl = with_deref; | |
635 | } else { | |
636 | expr_snip = snip.to_string(); | |
637 | eq_impl = without_deref; | |
638 | }; | |
639 | ||
640 | let span; | |
641 | let hint; | |
642 | if (eq_impl.ty_eq_other && left) || (eq_impl.other_eq_ty && !left) { | |
643 | span = expr.span; | |
644 | hint = expr_snip; | |
645 | } else { | |
646 | span = expr.span.to(other.span); | |
5099ac24 FG |
647 | |
648 | let cmp_span = if other.span < expr.span { | |
649 | other.span.between(expr.span) | |
650 | } else { | |
651 | expr.span.between(other.span) | |
652 | }; | |
f20569fa | 653 | if eq_impl.ty_eq_other { |
5099ac24 FG |
654 | hint = format!( |
655 | "{}{}{}", | |
656 | expr_snip, | |
657 | snippet(cx, cmp_span, ".."), | |
658 | snippet(cx, other.span, "..") | |
659 | ); | |
f20569fa | 660 | } else { |
5099ac24 FG |
661 | hint = format!( |
662 | "{}{}{}", | |
663 | snippet(cx, other.span, ".."), | |
664 | snippet(cx, cmp_span, ".."), | |
665 | expr_snip | |
666 | ); | |
f20569fa XL |
667 | } |
668 | } | |
669 | ||
670 | diag.span_suggestion( | |
671 | span, | |
672 | "try", | |
673 | hint, | |
674 | Applicability::MachineApplicable, // snippet | |
675 | ); | |
676 | }, | |
677 | ); | |
678 | } | |
679 | ||
680 | /// Heuristic to see if an expression is used. Should be compatible with | |
681 | /// `unused_variables`'s idea | |
682 | /// of what it means for an expression to be "used". | |
683 | fn is_used(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { | |
684 | get_parent_expr(cx, expr).map_or(true, |parent| match parent.kind { | |
cdc7bbd5 | 685 | ExprKind::Assign(_, rhs, _) | ExprKind::AssignOp(_, _, rhs) => SpanlessEq::new(cx).eq_expr(rhs, expr), |
f20569fa XL |
686 | _ => is_used(cx, parent), |
687 | }) | |
688 | } | |
689 | ||
690 | /// Tests whether an expression is in a macro expansion (e.g., something | |
691 | /// generated by `#[derive(...)]` or the like). | |
692 | fn in_attributes_expansion(expr: &Expr<'_>) -> bool { | |
693 | use rustc_span::hygiene::MacroKind; | |
694 | if expr.span.from_expansion() { | |
695 | let data = expr.span.ctxt().outer_expn_data(); | |
136023e0 | 696 | matches!(data.kind, ExpnKind::Macro(MacroKind::Attr, _)) |
f20569fa XL |
697 | } else { |
698 | false | |
699 | } | |
700 | } | |
701 | ||
702 | /// Tests whether `res` is a variable defined outside a macro. | |
703 | fn non_macro_local(cx: &LateContext<'_>, res: def::Res) -> bool { | |
704 | if let def::Res::Local(id) = res { | |
705 | !cx.tcx.hir().span(id).from_expansion() | |
706 | } else { | |
707 | false | |
708 | } | |
709 | } | |
710 | ||
711 | fn check_cast(cx: &LateContext<'_>, span: Span, e: &Expr<'_>, ty: &hir::Ty<'_>) { | |
712 | if_chain! { | |
713 | if let TyKind::Ptr(ref mut_ty) = ty.kind; | |
714 | if let ExprKind::Lit(ref lit) = e.kind; | |
715 | if let LitKind::Int(0, _) = lit.node; | |
716 | if !in_constant(cx, e.hir_id); | |
717 | then { | |
718 | let (msg, sugg_fn) = match mut_ty.mutbl { | |
719 | Mutability::Mut => ("`0 as *mut _` detected", "std::ptr::null_mut"), | |
720 | Mutability::Not => ("`0 as *const _` detected", "std::ptr::null"), | |
721 | }; | |
722 | ||
723 | let (sugg, appl) = if let TyKind::Infer = mut_ty.ty.kind { | |
724 | (format!("{}()", sugg_fn), Applicability::MachineApplicable) | |
725 | } else if let Some(mut_ty_snip) = snippet_opt(cx, mut_ty.ty.span) { | |
726 | (format!("{}::<{}>()", sugg_fn, mut_ty_snip), Applicability::MachineApplicable) | |
727 | } else { | |
728 | // `MaybeIncorrect` as type inference may not work with the suggested code | |
729 | (format!("{}()", sugg_fn), Applicability::MaybeIncorrect) | |
730 | }; | |
731 | span_lint_and_sugg(cx, ZERO_PTR, span, msg, "try", sugg, appl); | |
732 | } | |
733 | } | |
734 | } | |
735 | ||
5099ac24 | 736 | fn check_binary<'a>( |
f20569fa XL |
737 | cx: &LateContext<'a>, |
738 | expr: &Expr<'_>, | |
739 | cmp: &rustc_span::source_map::Spanned<rustc_hir::BinOpKind>, | |
740 | left: &'a Expr<'_>, | |
741 | right: &'a Expr<'_>, | |
742 | ) { | |
743 | let op = cmp.node; | |
744 | if op.is_comparison() { | |
745 | check_nan(cx, left, expr); | |
746 | check_nan(cx, right, expr); | |
747 | check_to_owned(cx, left, right, true); | |
748 | check_to_owned(cx, right, left, false); | |
749 | } | |
750 | if (op == BinOpKind::Eq || op == BinOpKind::Ne) && (is_float(cx, left) || is_float(cx, right)) { | |
751 | if is_allowed(cx, left) || is_allowed(cx, right) { | |
752 | return; | |
753 | } | |
754 | ||
755 | // Allow comparing the results of signum() | |
756 | if is_signum(cx, left) && is_signum(cx, right) { | |
757 | return; | |
758 | } | |
759 | ||
760 | if let Some(name) = get_item_name(cx, expr) { | |
761 | let name = name.as_str(); | |
762 | if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_") || name.ends_with("_eq") { | |
763 | return; | |
764 | } | |
765 | } | |
766 | let is_comparing_arrays = is_array(cx, left) || is_array(cx, right); | |
767 | let (lint, msg) = get_lint_and_message( | |
768 | is_named_constant(cx, left) || is_named_constant(cx, right), | |
769 | is_comparing_arrays, | |
770 | ); | |
771 | span_lint_and_then(cx, lint, expr.span, msg, |diag| { | |
772 | let lhs = Sugg::hir(cx, left, ".."); | |
773 | let rhs = Sugg::hir(cx, right, ".."); | |
774 | ||
775 | if !is_comparing_arrays { | |
776 | diag.span_suggestion( | |
777 | expr.span, | |
778 | "consider comparing them within some margin of error", | |
779 | format!( | |
780 | "({}).abs() {} error_margin", | |
781 | lhs - rhs, | |
782 | if op == BinOpKind::Eq { '<' } else { '>' } | |
783 | ), | |
784 | Applicability::HasPlaceholders, // snippet | |
785 | ); | |
786 | } | |
787 | diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`"); | |
788 | }); | |
789 | } else if op == BinOpKind::Rem { | |
790 | if is_integer_const(cx, right, 1) { | |
791 | span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0"); | |
792 | } | |
793 | ||
794 | if let ty::Int(ity) = cx.typeck_results().expr_ty(right).kind() { | |
795 | if is_integer_const(cx, right, unsext(cx.tcx, -1, *ity)) { | |
796 | span_lint( | |
797 | cx, | |
798 | MODULO_ONE, | |
799 | expr.span, | |
800 | "any number modulo -1 will panic/overflow or result in 0", | |
801 | ); | |
802 | } | |
803 | }; | |
804 | } | |
805 | } |