new lint: numbered-fields

CHANGELOG.md

@@ -3171,6 +3171,7 @@ Released 2018-09-13
 [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
 [`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces
 [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
+[`numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#numbered_fields
 [`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes
 [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
 [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref

clippy_lints/src/lib.register_all.rs

@@ -221,6 +221,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
     LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
     LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
+    LintId::of(numbered_fields::NUMBERED_FIELDS),
     LintId::of(octal_escapes::OCTAL_ESCAPES),
     LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
     LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),

clippy_lints/src/lib.register_lints.rs

@@ -382,6 +382,7 @@ store.register_lints(&[
     non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS,
     non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY,
     nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES,
+    numbered_fields::NUMBERED_FIELDS,
     octal_escapes::OCTAL_ESCAPES,
     open_options::NONSENSICAL_OPEN_OPTIONS,
     option_env_unwrap::OPTION_ENV_UNWRAP,

clippy_lints/src/lib.register_style.rs

@@ -88,6 +88,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),
     LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
     LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
+    LintId::of(numbered_fields::NUMBERED_FIELDS),
     LintId::of(ptr::CMP_NULL),
     LintId::of(ptr::PTR_ARG),
     LintId::of(ptr_eq::PTR_EQ),

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)]
@@ -312,6 +314,7 @@ mod non_expressive_names;
 mod non_octal_unix_permissions;
 mod non_send_fields_in_send_ty;
 mod nonstandard_macro_braces;
+mod numbered_fields;
 mod octal_escapes;
 mod open_options;
 mod option_env_unwrap;
@@ -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(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_lints/src/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 NUMBERED_FIELDS,
+    style,
+    "numbered fields in tuple struct initializer"
+}
+
+declare_lint_pass!(NumberedFields => [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,
+                    NUMBERED_FIELDS,
+                    e.span,
+                    "used a field initializer for a tuple struct",
+                    "try this instead",
+                    snippet,
+                    appl,
+                );
+            }
+        }
+    }
+}

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::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::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::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
+