]> git.lizzy.rs Git - rust.git/blob - clippy_lints/src/misc_early.rs
Auto merge of #6924 - mgacek8:issue6727_copy_types, r=llogiq
[rust.git] / clippy_lints / src / misc_early.rs
1 use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
2 use clippy_utils::source::snippet_opt;
3 use rustc_ast::ast::{
4     BindingMode, Expr, ExprKind, GenericParamKind, Generics, Lit, LitFloatType, LitIntType, LitKind, Mutability,
5     NodeId, Pat, PatKind, UnOp,
6 };
7 use rustc_ast::visit::FnKind;
8 use rustc_data_structures::fx::FxHashMap;
9 use rustc_errors::Applicability;
10 use rustc_hir::PrimTy;
11 use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
12 use rustc_middle::lint::in_external_macro;
13 use rustc_session::{declare_lint_pass, declare_tool_lint};
14 use rustc_span::source_map::Span;
15
16 declare_clippy_lint! {
17     /// **What it does:** Checks for structure field patterns bound to wildcards.
18     ///
19     /// **Why is this bad?** Using `..` instead is shorter and leaves the focus on
20     /// the fields that are actually bound.
21     ///
22     /// **Known problems:** None.
23     ///
24     /// **Example:**
25     /// ```rust
26     /// # struct Foo {
27     /// #     a: i32,
28     /// #     b: i32,
29     /// #     c: i32,
30     /// # }
31     /// let f = Foo { a: 0, b: 0, c: 0 };
32     ///
33     /// // Bad
34     /// match f {
35     ///     Foo { a: _, b: 0, .. } => {},
36     ///     Foo { a: _, b: _, c: _ } => {},
37     /// }
38     ///
39     /// // Good
40     /// match f {
41     ///     Foo { b: 0, .. } => {},
42     ///     Foo { .. } => {},
43     /// }
44     /// ```
45     pub UNNEEDED_FIELD_PATTERN,
46     restriction,
47     "struct fields bound to a wildcard instead of using `..`"
48 }
49
50 declare_clippy_lint! {
51     /// **What it does:** Checks for function arguments having the similar names
52     /// differing by an underscore.
53     ///
54     /// **Why is this bad?** It affects code readability.
55     ///
56     /// **Known problems:** None.
57     ///
58     /// **Example:**
59     /// ```rust
60     /// // Bad
61     /// fn foo(a: i32, _a: i32) {}
62     ///
63     /// // Good
64     /// fn bar(a: i32, _b: i32) {}
65     /// ```
66     pub DUPLICATE_UNDERSCORE_ARGUMENT,
67     style,
68     "function arguments having names which only differ by an underscore"
69 }
70
71 declare_clippy_lint! {
72     /// **What it does:** Detects expressions of the form `--x`.
73     ///
74     /// **Why is this bad?** It can mislead C/C++ programmers to think `x` was
75     /// decremented.
76     ///
77     /// **Known problems:** None.
78     ///
79     /// **Example:**
80     /// ```rust
81     /// let mut x = 3;
82     /// --x;
83     /// ```
84     pub DOUBLE_NEG,
85     style,
86     "`--x`, which is a double negation of `x` and not a pre-decrement as in C/C++"
87 }
88
89 declare_clippy_lint! {
90     /// **What it does:** Warns on hexadecimal literals with mixed-case letter
91     /// digits.
92     ///
93     /// **Why is this bad?** It looks confusing.
94     ///
95     /// **Known problems:** None.
96     ///
97     /// **Example:**
98     /// ```rust
99     /// // Bad
100     /// let y = 0x1a9BAcD;
101     ///
102     /// // Good
103     /// let y = 0x1A9BACD;
104     /// ```
105     pub MIXED_CASE_HEX_LITERALS,
106     style,
107     "hex literals whose letter digits are not consistently upper- or lowercased"
108 }
109
110 declare_clippy_lint! {
111     /// **What it does:** Warns if literal suffixes are not separated by an
112     /// underscore.
113     ///
114     /// **Why is this bad?** It is much less readable.
115     ///
116     /// **Known problems:** None.
117     ///
118     /// **Example:**
119     /// ```rust
120     /// // Bad
121     /// let y = 123832i32;
122     ///
123     /// // Good
124     /// let y = 123832_i32;
125     /// ```
126     pub UNSEPARATED_LITERAL_SUFFIX,
127     pedantic,
128     "literals whose suffix is not separated by an underscore"
129 }
130
131 declare_clippy_lint! {
132     /// **What it does:** Warns if an integral constant literal starts with `0`.
133     ///
134     /// **Why is this bad?** In some languages (including the infamous C language
135     /// and most of its
136     /// family), this marks an octal constant. In Rust however, this is a decimal
137     /// constant. This could
138     /// be confusing for both the writer and a reader of the constant.
139     ///
140     /// **Known problems:** None.
141     ///
142     /// **Example:**
143     ///
144     /// In Rust:
145     /// ```rust
146     /// fn main() {
147     ///     let a = 0123;
148     ///     println!("{}", a);
149     /// }
150     /// ```
151     ///
152     /// prints `123`, while in C:
153     ///
154     /// ```c
155     /// #include <stdio.h>
156     ///
157     /// int main() {
158     ///     int a = 0123;
159     ///     printf("%d\n", a);
160     /// }
161     /// ```
162     ///
163     /// prints `83` (as `83 == 0o123` while `123 == 0o173`).
164     pub ZERO_PREFIXED_LITERAL,
165     complexity,
166     "integer literals starting with `0`"
167 }
168
169 declare_clippy_lint! {
170     /// **What it does:** Warns if a generic shadows a built-in type.
171     ///
172     /// **Why is this bad?** This gives surprising type errors.
173     ///
174     /// **Known problems:** None.
175     ///
176     /// **Example:**
177     ///
178     /// ```ignore
179     /// impl<u32> Foo<u32> {
180     ///     fn impl_func(&self) -> u32 {
181     ///         42
182     ///     }
183     /// }
184     /// ```
185     pub BUILTIN_TYPE_SHADOW,
186     style,
187     "shadowing a builtin type"
188 }
189
190 declare_clippy_lint! {
191     /// **What it does:** Checks for patterns in the form `name @ _`.
192     ///
193     /// **Why is this bad?** It's almost always more readable to just use direct
194     /// bindings.
195     ///
196     /// **Known problems:** None.
197     ///
198     /// **Example:**
199     /// ```rust
200     /// # let v = Some("abc");
201     ///
202     /// // Bad
203     /// match v {
204     ///     Some(x) => (),
205     ///     y @ _ => (),
206     /// }
207     ///
208     /// // Good
209     /// match v {
210     ///     Some(x) => (),
211     ///     y => (),
212     /// }
213     /// ```
214     pub REDUNDANT_PATTERN,
215     style,
216     "using `name @ _` in a pattern"
217 }
218
219 declare_clippy_lint! {
220     /// **What it does:** Checks for tuple patterns with a wildcard
221     /// pattern (`_`) is next to a rest pattern (`..`).
222     ///
223     /// _NOTE_: While `_, ..` means there is at least one element left, `..`
224     /// means there are 0 or more elements left. This can make a difference
225     /// when refactoring, but shouldn't result in errors in the refactored code,
226     /// since the wildcard pattern isn't used anyway.
227     /// **Why is this bad?** The wildcard pattern is unneeded as the rest pattern
228     /// can match that element as well.
229     ///
230     /// **Known problems:** None.
231     ///
232     /// **Example:**
233     /// ```rust
234     /// # struct TupleStruct(u32, u32, u32);
235     /// # let t = TupleStruct(1, 2, 3);
236     /// // Bad
237     /// match t {
238     ///     TupleStruct(0, .., _) => (),
239     ///     _ => (),
240     /// }
241     ///
242     /// // Good
243     /// match t {
244     ///     TupleStruct(0, ..) => (),
245     ///     _ => (),
246     /// }
247     /// ```
248     pub UNNEEDED_WILDCARD_PATTERN,
249     complexity,
250     "tuple patterns with a wildcard pattern (`_`) is next to a rest pattern (`..`)"
251 }
252
253 declare_lint_pass!(MiscEarlyLints => [
254     UNNEEDED_FIELD_PATTERN,
255     DUPLICATE_UNDERSCORE_ARGUMENT,
256     DOUBLE_NEG,
257     MIXED_CASE_HEX_LITERALS,
258     UNSEPARATED_LITERAL_SUFFIX,
259     ZERO_PREFIXED_LITERAL,
260     BUILTIN_TYPE_SHADOW,
261     REDUNDANT_PATTERN,
262     UNNEEDED_WILDCARD_PATTERN,
263 ]);
264
265 impl EarlyLintPass for MiscEarlyLints {
266     fn check_generics(&mut self, cx: &EarlyContext<'_>, gen: &Generics) {
267         for param in &gen.params {
268             if let GenericParamKind::Type { .. } = param.kind {
269                 if let Some(prim_ty) = PrimTy::from_name(param.ident.name) {
270                     span_lint(
271                         cx,
272                         BUILTIN_TYPE_SHADOW,
273                         param.ident.span,
274                         &format!("this generic shadows the built-in type `{}`", prim_ty.name()),
275                     );
276                 }
277             }
278         }
279     }
280
281     fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) {
282         if let PatKind::Struct(ref npat, ref pfields, _) = pat.kind {
283             let mut wilds = 0;
284             let type_name = npat
285                 .segments
286                 .last()
287                 .expect("A path must have at least one segment")
288                 .ident
289                 .name;
290
291             for field in pfields {
292                 if let PatKind::Wild = field.pat.kind {
293                     wilds += 1;
294                 }
295             }
296             if !pfields.is_empty() && wilds == pfields.len() {
297                 span_lint_and_help(
298                     cx,
299                     UNNEEDED_FIELD_PATTERN,
300                     pat.span,
301                     "all the struct fields are matched to a wildcard pattern, consider using `..`",
302                     None,
303                     &format!("try with `{} {{ .. }}` instead", type_name),
304                 );
305                 return;
306             }
307             if wilds > 0 {
308                 for field in pfields {
309                     if let PatKind::Wild = field.pat.kind {
310                         wilds -= 1;
311                         if wilds > 0 {
312                             span_lint(
313                                 cx,
314                                 UNNEEDED_FIELD_PATTERN,
315                                 field.span,
316                                 "you matched a field with a wildcard pattern, consider using `..` instead",
317                             );
318                         } else {
319                             let mut normal = vec![];
320
321                             for field in pfields {
322                                 match field.pat.kind {
323                                     PatKind::Wild => {},
324                                     _ => {
325                                         if let Ok(n) = cx.sess().source_map().span_to_snippet(field.span) {
326                                             normal.push(n);
327                                         }
328                                     },
329                                 }
330                             }
331
332                             span_lint_and_help(
333                                 cx,
334                                 UNNEEDED_FIELD_PATTERN,
335                                 field.span,
336                                 "you matched a field with a wildcard pattern, consider using `..` \
337                                  instead",
338                                 None,
339                                 &format!("try with `{} {{ {}, .. }}`", type_name, normal[..].join(", ")),
340                             );
341                         }
342                     }
343                 }
344             }
345         }
346
347         if let PatKind::Ident(left, ident, Some(ref right)) = pat.kind {
348             let left_binding = match left {
349                 BindingMode::ByRef(Mutability::Mut) => "ref mut ",
350                 BindingMode::ByRef(Mutability::Not) => "ref ",
351                 BindingMode::ByValue(..) => "",
352             };
353
354             if let PatKind::Wild = right.kind {
355                 span_lint_and_sugg(
356                     cx,
357                     REDUNDANT_PATTERN,
358                     pat.span,
359                     &format!(
360                         "the `{} @ _` pattern can be written as just `{}`",
361                         ident.name, ident.name,
362                     ),
363                     "try",
364                     format!("{}{}", left_binding, ident.name),
365                     Applicability::MachineApplicable,
366                 );
367             }
368         }
369
370         check_unneeded_wildcard_pattern(cx, pat);
371     }
372
373     fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: Span, _: NodeId) {
374         let mut registered_names: FxHashMap<String, Span> = FxHashMap::default();
375
376         for arg in &fn_kind.decl().inputs {
377             if let PatKind::Ident(_, ident, None) = arg.pat.kind {
378                 let arg_name = ident.to_string();
379
380                 if let Some(arg_name) = arg_name.strip_prefix('_') {
381                     if let Some(correspondence) = registered_names.get(arg_name) {
382                         span_lint(
383                             cx,
384                             DUPLICATE_UNDERSCORE_ARGUMENT,
385                             *correspondence,
386                             &format!(
387                                 "`{}` already exists, having another argument having almost the same \
388                                  name makes code comprehension and documentation more difficult",
389                                 arg_name
390                             ),
391                         );
392                     }
393                 } else {
394                     registered_names.insert(arg_name, arg.pat.span);
395                 }
396             }
397         }
398     }
399
400     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
401         if in_external_macro(cx.sess(), expr.span) {
402             return;
403         }
404         match expr.kind {
405             ExprKind::Unary(UnOp::Neg, ref inner) => {
406                 if let ExprKind::Unary(UnOp::Neg, _) = inner.kind {
407                     span_lint(
408                         cx,
409                         DOUBLE_NEG,
410                         expr.span,
411                         "`--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op",
412                     );
413                 }
414             },
415             ExprKind::Lit(ref lit) => Self::check_lit(cx, lit),
416             _ => (),
417         }
418     }
419 }
420
421 impl MiscEarlyLints {
422     fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) {
423         // We test if first character in snippet is a number, because the snippet could be an expansion
424         // from a built-in macro like `line!()` or a proc-macro like `#[wasm_bindgen]`.
425         // Note that this check also covers special case that `line!()` is eagerly expanded by compiler.
426         // See <https://github.com/rust-lang/rust-clippy/issues/4507> for a regression.
427         // FIXME: Find a better way to detect those cases.
428         let lit_snip = match snippet_opt(cx, lit.span) {
429             Some(snip) if snip.chars().next().map_or(false, |c| c.is_digit(10)) => snip,
430             _ => return,
431         };
432
433         if let LitKind::Int(value, lit_int_type) = lit.kind {
434             let suffix = match lit_int_type {
435                 LitIntType::Signed(ty) => ty.name_str(),
436                 LitIntType::Unsigned(ty) => ty.name_str(),
437                 LitIntType::Unsuffixed => "",
438             };
439
440             let maybe_last_sep_idx = if let Some(val) = lit_snip.len().checked_sub(suffix.len() + 1) {
441                 val
442             } else {
443                 return; // It's useless so shouldn't lint.
444             };
445             // Do not lint when literal is unsuffixed.
446             if !suffix.is_empty() && lit_snip.as_bytes()[maybe_last_sep_idx] != b'_' {
447                 span_lint_and_sugg(
448                     cx,
449                     UNSEPARATED_LITERAL_SUFFIX,
450                     lit.span,
451                     "integer type suffix should be separated by an underscore",
452                     "add an underscore",
453                     format!("{}_{}", &lit_snip[..=maybe_last_sep_idx], suffix),
454                     Applicability::MachineApplicable,
455                 );
456             }
457
458             if lit_snip.starts_with("0x") {
459                 if maybe_last_sep_idx <= 2 {
460                     // It's meaningless or causes range error.
461                     return;
462                 }
463                 let mut seen = (false, false);
464                 for ch in lit_snip.as_bytes()[2..=maybe_last_sep_idx].iter() {
465                     match ch {
466                         b'a'..=b'f' => seen.0 = true,
467                         b'A'..=b'F' => seen.1 = true,
468                         _ => {},
469                     }
470                     if seen.0 && seen.1 {
471                         span_lint(
472                             cx,
473                             MIXED_CASE_HEX_LITERALS,
474                             lit.span,
475                             "inconsistent casing in hexadecimal literal",
476                         );
477                         break;
478                     }
479                 }
480             } else if lit_snip.starts_with("0b") || lit_snip.starts_with("0o") {
481                 /* nothing to do */
482             } else if value != 0 && lit_snip.starts_with('0') {
483                 span_lint_and_then(
484                     cx,
485                     ZERO_PREFIXED_LITERAL,
486                     lit.span,
487                     "this is a decimal constant",
488                     |diag| {
489                         diag.span_suggestion(
490                             lit.span,
491                             "if you mean to use a decimal constant, remove the `0` to avoid confusion",
492                             lit_snip.trim_start_matches(|c| c == '_' || c == '0').to_string(),
493                             Applicability::MaybeIncorrect,
494                         );
495                         diag.span_suggestion(
496                             lit.span,
497                             "if you mean to use an octal constant, use `0o`",
498                             format!("0o{}", lit_snip.trim_start_matches(|c| c == '_' || c == '0')),
499                             Applicability::MaybeIncorrect,
500                         );
501                     },
502                 );
503             }
504         } else if let LitKind::Float(_, LitFloatType::Suffixed(float_ty)) = lit.kind {
505             let suffix = float_ty.name_str();
506             let maybe_last_sep_idx = if let Some(val) = lit_snip.len().checked_sub(suffix.len() + 1) {
507                 val
508             } else {
509                 return; // It's useless so shouldn't lint.
510             };
511             if lit_snip.as_bytes()[maybe_last_sep_idx] != b'_' {
512                 span_lint_and_sugg(
513                     cx,
514                     UNSEPARATED_LITERAL_SUFFIX,
515                     lit.span,
516                     "float type suffix should be separated by an underscore",
517                     "add an underscore",
518                     format!("{}_{}", &lit_snip[..=maybe_last_sep_idx], suffix),
519                     Applicability::MachineApplicable,
520                 );
521             }
522         }
523     }
524 }
525
526 fn check_unneeded_wildcard_pattern(cx: &EarlyContext<'_>, pat: &Pat) {
527     if let PatKind::TupleStruct(_, ref patterns) | PatKind::Tuple(ref patterns) = pat.kind {
528         fn span_lint(cx: &EarlyContext<'_>, span: Span, only_one: bool) {
529             span_lint_and_sugg(
530                 cx,
531                 UNNEEDED_WILDCARD_PATTERN,
532                 span,
533                 if only_one {
534                     "this pattern is unneeded as the `..` pattern can match that element"
535                 } else {
536                     "these patterns are unneeded as the `..` pattern can match those elements"
537                 },
538                 if only_one { "remove it" } else { "remove them" },
539                 "".to_string(),
540                 Applicability::MachineApplicable,
541             );
542         }
543
544         if let Some(rest_index) = patterns.iter().position(|pat| pat.is_rest()) {
545             if let Some((left_index, left_pat)) = patterns[..rest_index]
546                 .iter()
547                 .rev()
548                 .take_while(|pat| matches!(pat.kind, PatKind::Wild))
549                 .enumerate()
550                 .last()
551             {
552                 span_lint(cx, left_pat.span.until(patterns[rest_index].span), left_index == 0);
553             }
554
555             if let Some((right_index, right_pat)) = patterns[rest_index + 1..]
556                 .iter()
557                 .take_while(|pat| matches!(pat.kind, PatKind::Wild))
558                 .enumerate()
559                 .last()
560             {
561                 span_lint(
562                     cx,
563                     patterns[rest_index].span.shrink_to_hi().to(right_pat.span),
564                     right_index == 0,
565                 );
566             }
567         }
568     }
569 }