new lint: numbered-fields

Author: llogiq

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/numbered_fields.rs

@@ -0,0 +1,77 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+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.
+    ///
+    /// ### 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()
+                && 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 snippet = format!(
+                    "{}({})",
+                    //TODO: use snippet_with_applicability throughout
+                    snippet(cx, path.span(), ".."),
+                    expr_spans
+                        .into_iter_sorted()
+                        .map(|(_, span)| snippet(cx, span, ".."))
+                        .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,
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
+    }
+}

tests/ui/numbered_fields.fixed

@@ -0,0 +1,20 @@
+//run-rustfix
+#![warn(clippy::numbered_fields)]
+
+#[derive(Default)]
+struct TupleStruct(u32, u32, u8);
+
+fn main() {
+    let tuple_struct = TupleStruct::default();
+
+    // This should lint
+    let _ = TupleStruct(1u32, 42, 23u8);
+
+    // Ok because of default initializer
+    let _ = TupleStruct { 0: 42, ..tuple_struct };
+
+    let _ = TupleStruct {
+        1: 23,
+        ..TupleStruct::default()
+    };
+}

tests/ui/numbered_fields.rs

@@ -0,0 +1,24 @@
+//run-rustfix
+#![warn(clippy::numbered_fields)]
+
+#[derive(Default)]
+struct TupleStruct(u32, u32, u8);
+
+fn main() {
+    let tuple_struct = TupleStruct::default();
+
+    // This should lint
+    let _ = TupleStruct {
+        0: 1u32,
+        1: 42,
+        2: 23u8,
+    };
+
+    // Ok because of default initializer
+    let _ = TupleStruct { 0: 42, ..tuple_struct };
+
+    let _ = TupleStruct {
+        1: 23,
+        ..TupleStruct::default()
+    };
+}

tests/ui/numbered_fields.stderr

@@ -0,0 +1,15 @@
+error: used a field initializer for a tuple struct
+  --> $DIR/numbered_fields.rs:11: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: aborting due to previous error
+