Auto merge of #8170 - rust-lang:numbered-fields, r=xFrednet

new lint: `init-numbered-fields`

This fixes #7985.

r? `@xFrednet`

---

changelog: new lint: [`init_numbered_fields`]

Author: bors

CHANGELOG.md

@@ -3026,6 +3026,7 @@ Released 2018-09-13
 [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter
 [`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string
 [`inherent_to_string_shadow_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display
+[`init_numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#init_numbered_fields
 [`inline_always`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_always
 [`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax
 [`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax

clippy_lints/src/init_numbered_fields.rs

@@ -0,0 +1,80 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::in_macro;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use std::borrow::Cow;
+use std::cmp::Reverse;
+use std::collections::BinaryHeap;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for tuple structs initialized with field syntax.
+    /// It will however not lint if a base initializer is present.
+    /// The lint will also ignore code in macros.
+    ///
+    /// ### Why is this bad?
+    /// This may be confusing to the uninitiated and adds no
+    /// benefit as opposed to tuple initializers
+    ///
+    /// ### Example
+    /// ```rust
+    /// struct TupleStruct(u8, u16);
+    ///
+    /// let _ = TupleStruct {
+    ///     0: 1,
+    ///     1: 23,
+    /// };
+    ///
+    /// // should be written as
+    /// let base = TupleStruct(1, 23);
+    ///
+    /// // This is OK however
+    /// let _ = TupleStruct { 0: 42, ..base };
+    /// ```
+    #[clippy::version = "1.59.0"]
+    pub INIT_NUMBERED_FIELDS,
+    style,
+    "numbered fields in tuple struct initializer"
+}
+
+declare_lint_pass!(NumberedFields => [INIT_NUMBERED_FIELDS]);
+
+impl<'tcx> LateLintPass<'tcx> for NumberedFields {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
+        if let ExprKind::Struct(path, fields, None) = e.kind {
+            if !fields.is_empty()
+                && !in_macro(e.span)
+                && fields
+                    .iter()
+                    .all(|f| f.ident.as_str().as_bytes().iter().all(u8::is_ascii_digit))
+            {
+                let expr_spans = fields
+                    .iter()
+                    .map(|f| (Reverse(f.ident.as_str().parse::<usize>().unwrap()), f.expr.span))
+                    .collect::<BinaryHeap<_>>();
+                let mut appl = Applicability::MachineApplicable;
+                let snippet = format!(
+                    "{}({})",
+                    snippet_with_applicability(cx, path.span(), "..", &mut appl),
+                    expr_spans
+                        .into_iter_sorted()
+                        .map(|(_, span)| snippet_with_applicability(cx, span, "..", &mut appl))
+                        .intersperse(Cow::Borrowed(", "))
+                        .collect::<String>()
+                );
+                span_lint_and_sugg(
+                    cx,
+                    INIT_NUMBERED_FIELDS,
+                    e.span,
+                    "used a field initializer for a tuple struct",
+                    "try this instead",
+                    snippet,
+                    appl,
+                );
+            }
+        }
+    }
+}

clippy_lints/src/lib.register_all.rs

@@ -81,6 +81,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(infinite_iter::INFINITE_ITER),
     LintId::of(inherent_to_string::INHERENT_TO_STRING),
     LintId::of(inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
+    LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS),
     LintId::of(inline_fn_without_body::INLINE_FN_WITHOUT_BODY),
     LintId::of(int_plus_one::INT_PLUS_ONE),
     LintId::of(large_const_arrays::LARGE_CONST_ARRAYS),

clippy_lints/src/lib.register_lints.rs

@@ -178,6 +178,7 @@ store.register_lints(&[
     inherent_impl::MULTIPLE_INHERENT_IMPL,
     inherent_to_string::INHERENT_TO_STRING,
     inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY,
+    init_numbered_fields::INIT_NUMBERED_FIELDS,
     inline_fn_without_body::INLINE_FN_WITHOUT_BODY,
     int_plus_one::INT_PLUS_ONE,
     integer_division::INTEGER_DIVISION,

clippy_lints/src/lib.register_style.rs

@@ -29,6 +29,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(functions::MUST_USE_UNIT),
     LintId::of(functions::RESULT_UNIT_ERR),
     LintId::of(inherent_to_string::INHERENT_TO_STRING),
+    LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS),
     LintId::of(len_zero::COMPARISON_TO_EMPTY),
     LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY),
     LintId::of(len_zero::LEN_ZERO),

clippy_lints/src/lib.rs

@@ -1,13 +1,15 @@
 // error-pattern:cargo-clippy
 
+#![feature(binary_heap_into_iter_sorted)]
 #![feature(box_patterns)]
+#![feature(control_flow_enum)]
 #![feature(drain_filter)]
 #![feature(in_band_lifetimes)]
+#![feature(iter_intersperse)]
+#![feature(let_else)]
 #![feature(once_cell)]
 #![feature(rustc_private)]
 #![feature(stmt_expr_attributes)]
-#![feature(control_flow_enum)]
-#![feature(let_else)]
 #![recursion_limit = "512"]
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 #![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)]
@@ -242,6 +244,7 @@ mod indexing_slicing;
 mod infinite_iter;
 mod inherent_impl;
 mod inherent_to_string;
+mod init_numbered_fields;
 mod inline_fn_without_body;
 mod int_plus_one;
 mod integer_division;
@@ -854,6 +857,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes));
     store.register_late_pass(|| Box::new(needless_late_init::NeedlessLateInit));
     store.register_late_pass(|| Box::new(return_self_not_must_use::ReturnSelfNotMustUse));
+    store.register_late_pass(|| Box::new(init_numbered_fields::NumberedFields));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/macro_use.rs

@@ -1,4 +1,5 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::in_macro;
 use clippy_utils::source::snippet;
 use hir::def::{DefKind, Res};
 use if_chain::if_chain;
@@ -8,7 +9,6 @@ use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::hygiene::ExpnKind;
 use rustc_span::{edition::Edition, sym, Span};
 
 declare_clippy_lint! {
@@ -214,7 +214,3 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
         }
     }
 }
-
-fn in_macro(span: Span) -> bool {
-    span.from_expansion() && !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..))
-}

clippy_utils/src/lib.rs

@@ -142,6 +142,13 @@ macro_rules! extract_msrv_attr {
     };
 }
 
+/// Returns `true` if the span comes from a macro expansion, no matter if from a
+/// macro by example or from a procedural macro
+#[must_use]
+pub fn in_macro(span: Span) -> bool {
+    span.from_expansion() && !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..))
+}
+
 /// Returns `true` if the two spans come from differing expansions (i.e., one is
 /// from a macro and one isn't).
 #[must_use]

tests/ui/numbered_fields.fixed

@@ -0,0 +1,33 @@
+//run-rustfix
+#![warn(clippy::init_numbered_fields)]
+
+#[derive(Default)]
+struct TupleStruct(u32, u32, u8);
+
+// This shouldn't lint because it's in a macro
+macro_rules! tuple_struct_init {
+    () => {
+        TupleStruct { 0: 0, 1: 1, 2: 2 }
+    };
+}
+
+fn main() {
+    let tuple_struct = TupleStruct::default();
+
+    // This should lint
+    let _ = TupleStruct(1u32, 42, 23u8);
+
+    // This should also lint and order the fields correctly
+    let _ = TupleStruct(1u32, 3u32, 2u8);
+
+    // Ok because of default initializer
+    let _ = TupleStruct { 0: 42, ..tuple_struct };
+
+    let _ = TupleStruct {
+        1: 23,
+        ..TupleStruct::default()
+    };
+
+    // Ok because it's in macro
+    let _ = tuple_struct_init!();
+}

tests/ui/numbered_fields.rs

@@ -0,0 +1,41 @@
+//run-rustfix
+#![warn(clippy::init_numbered_fields)]
+
+#[derive(Default)]
+struct TupleStruct(u32, u32, u8);
+
+// This shouldn't lint because it's in a macro
+macro_rules! tuple_struct_init {
+    () => {
+        TupleStruct { 0: 0, 1: 1, 2: 2 }
+    };
+}
+
+fn main() {
+    let tuple_struct = TupleStruct::default();
+
+    // This should lint
+    let _ = TupleStruct {
+        0: 1u32,
+        1: 42,
+        2: 23u8,
+    };
+
+    // This should also lint and order the fields correctly
+    let _ = TupleStruct {
+        0: 1u32,
+        2: 2u8,
+        1: 3u32,
+    };
+
+    // Ok because of default initializer
+    let _ = TupleStruct { 0: 42, ..tuple_struct };
+
+    let _ = TupleStruct {
+        1: 23,
+        ..TupleStruct::default()
+    };
+
+    // Ok because it's in macro
+    let _ = tuple_struct_init!();
+}

tests/ui/numbered_fields.stderr

@@ -0,0 +1,26 @@
+error: used a field initializer for a tuple struct
+  --> $DIR/numbered_fields.rs:18:13
+   |
+LL |       let _ = TupleStruct {
+   |  _____________^
+LL | |         0: 1u32,
+LL | |         1: 42,
+LL | |         2: 23u8,
+LL | |     };
+   | |_____^ help: try this instead: `TupleStruct(1u32, 42, 23u8)`
+   |
+   = note: `-D clippy::init-numbered-fields` implied by `-D warnings`
+
+error: used a field initializer for a tuple struct
+  --> $DIR/numbered_fields.rs:25:13
+   |
+LL |       let _ = TupleStruct {
+   |  _____________^
+LL | |         0: 1u32,
+LL | |         2: 2u8,
+LL | |         1: 3u32,
+LL | |     };
+   | |_____^ help: try this instead: `TupleStruct(1u32, 3u32, 2u8)`
+
+error: aborting due to 2 previous errors
+