Add inconsistent_struct_pattern lint

examples/README.md

@@ -27,6 +27,7 @@ The example libraries are separated into the following three categories:
 | -------------------------------------------------------------------------------------- | -------------------------------------------------------------- |
 | [`commented_code`](./supplementary/commented_code)                                     | Code that has been commented out                               |
 | [`escaping_doc_link`](./supplementary/escaping_doc_link)                               | Doc comment links that escape their packages                   |
+| [`inconsistent_struct_pattern`](./supplementary/inconsistent_struct_pattern)           | Struct patterns whose fields do not match their declared order |
 | [`local_ref_cell`](./supplementary/local_ref_cell)                                     | `RefCell` local variables                                      |
 | [`redundant_reference`](./supplementary/redundant_reference)                           | Reference fields used only to read one copyable subfield       |
 | [`unnamed_constant`](./supplementary/unnamed_constant)                                 | Unnamed constants, aka magic numbers                           |

examples/general/await_holding_span_guard/src/lib.rs

@@ -88,12 +88,12 @@ const TRACING_SPAN_ENTERED_GUARD: [&str; 3] = ["tracing", "span", "EnteredSpan"]
 impl LateLintPass<'_> for AwaitHoldingSpanGuard {
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ hir::Expr<'_>) {
         if let hir::ExprKind::Closure(hir::Closure {
+            def_id,
             kind:
                 hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
                     hir::CoroutineDesugaring::Async,
                     _,
                 )),
-            def_id,
             ..
         }) = expr.kind
         {

examples/restriction/overscoped_allow/src/lib.rs

@@ -403,8 +403,8 @@ fn local_path_from_span(cx: &LateContext<'_>, span: Span) -> Option<PathBuf> {
 fn is_extern_crate_test(cx: &LateContext<'_>, hir_id: HirId) -> bool {
     let node = cx.tcx.hir_node(hir_id);
     if let Node::Item(Item {
-        kind: ItemKind::ExternCrate(None),
         ident,
+        kind: ItemKind::ExternCrate(None),
         ..
     }) = node
     {

examples/supplementary/Cargo.toml

@@ -12,6 +12,9 @@ crate-type = ["cdylib"]
 [dependencies]
 commented_code = { path = "commented_code", features = ["rlib"] }
 escaping_doc_link = { path = "escaping_doc_link", features = ["rlib"] }
+inconsistent_struct_pattern = { path = "inconsistent_struct_pattern", features = [
+    "rlib",
+] }
 local_ref_cell = { path = "local_ref_cell", features = ["rlib"] }
 redundant_reference = { path = "redundant_reference", features = ["rlib"] }
 unnamed_constant = { path = "unnamed_constant", features = ["rlib"] }
@@ -31,6 +34,7 @@ rustc_private = true
 members = [
     "commented_code",
     "escaping_doc_link",
+    "inconsistent_struct_pattern",
     "local_ref_cell",
     "redundant_reference",
     "unnamed_constant",

examples/supplementary/inconsistent_struct_pattern/Cargo.toml

@@ -0,0 +1,27 @@
+[package]
+name = "inconsistent_struct_pattern"
+version = "3.2.1"
+authors = ["Samuel E. Moelius III <[email protected]>"]
+description = "A lint to check for struct patterns whose fields do not match their declared order"
+edition = "2021"
+publish = false
+
+[lib]
+crate-type = ["cdylib", "rlib"]
+
+[dependencies]
+clippy_utils = { workspace = true }
+
+dylint_linting = { path = "../../../utils/linting" }
+
+[dev-dependencies]
+dylint_testing = { path = "../../../utils/testing" }
+
+[features]
+rlib = ["dylint_linting/constituent"]
+
+[lints]
+workspace = true
+
+[package.metadata.rust-analyzer]
+rustc_private = true

examples/supplementary/inconsistent_struct_pattern/README.md

@@ -0,0 +1,26 @@
+# inconsistent_struct_pattern
+
+### What it does
+Checks for struct patterns whose fields whose fields do not match their declared order.
+
+### Why is this bad?
+It can be harder to spot mistakes in inconsistent code.
+
+### Example
+```rust
+struct Struct {
+    a: bool,
+    b: bool,
+};
+let strukt = Struct { a: false, b: true };
+let Struct { b, a } = strukt;
+```
+Use instead:
+```rust
+struct Struct {
+    a: bool,
+    b: bool,
+};
+let strukt = Struct { a: false, b: true };
+let Struct { a, b } = strukt;
+```

examples/supplementary/inconsistent_struct_pattern/src/lib.rs

@@ -0,0 +1,154 @@
+#![feature(rustc_private)]
+#![feature(let_chains)]
+#![warn(unused_extern_crates)]
+
+extern crate rustc_data_structures;
+extern crate rustc_errors;
+extern crate rustc_hir;
+extern crate rustc_span;
+
+use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_opt};
+use rustc_data_structures::fx::FxHashMap;
+use rustc_errors::Applicability;
+use rustc_hir::{
+    def::{DefKind, Res},
+    Pat, PatField, PatKind, Path, QPath,
+};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_span::Symbol;
+
+dylint_linting::declare_late_lint! {
+    /// ### What it does
+    /// Checks for struct patterns whose fields whose fields do not match their declared order.
+    ///
+    /// ### Why is this bad?
+    /// It can be harder to spot mistakes in inconsistent code.
+    ///
+    /// ### Example
+    /// ```rust
+    /// struct Struct {
+    ///     a: bool,
+    ///     b: bool,
+    /// };
+    /// let strukt = Struct { a: false, b: true };
+    /// let Struct { b, a } = strukt;
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// struct Struct {
+    ///     a: bool,
+    ///     b: bool,
+    /// };
+    /// let strukt = Struct { a: false, b: true };
+    /// let Struct { a, b } = strukt;
+    /// ```
+    pub INCONSISTENT_STRUCT_PATTERN,
+    Warn,
+    "struct patterns whose fields do not match their declared order"
+}
+
+impl<'tcx> LateLintPass<'tcx> for InconsistentStructPattern {
+    fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'tcx>) {
+        let PatKind::Struct(
+            QPath::Resolved(
+                _,
+                Path {
+                    res: Res::Def(DefKind::Struct, def_id),
+                    ..
+                },
+            ),
+            fields,
+            _,
+        ) = pat.kind
+        else {
+            return;
+        };
+
+        let adt_def = cx.tcx.adt_def(def_id);
+        let variant_def = adt_def.variants().iter().next().unwrap();
+
+        let mut def_order_map = FxHashMap::default();
+        for (idx, field) in variant_def.fields.iter().enumerate() {
+            def_order_map.insert(field.name, idx);
+        }
+
+        if is_consistent_order(fields, &def_order_map) {
+            return;
+        }
+
+        let span = fields
+            .first()
+            .unwrap()
+            .span
+            .with_hi(fields.last().unwrap().span.hi());
+        let sugg = suggestion(cx, fields, &def_order_map);
+
+        span_lint_and_sugg(
+            cx,
+            INCONSISTENT_STRUCT_PATTERN,
+            span,
+            "struct pattern field order is inconsistent with struct definition field order",
+            "use",
+            sugg,
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
+fn suggestion(
+    cx: &LateContext<'_>,
+    fields: &[PatField],
+    def_order_map: &FxHashMap<Symbol, usize>,
+) -> String {
+    let ws = fields
+        .windows(2)
+        .map(|w| {
+            let span = w[0].span.with_hi(w[1].span.lo()).with_lo(w[0].span.hi());
+            snippet_opt(cx, span).unwrap()
+        })
+        .collect::<Vec<_>>();
+
+    let mut fields = fields.to_vec();
+    fields.sort_unstable_by_key(|field| def_order_map[&field.ident.name]);
+    let pat_snippets = fields
+        .iter()
+        .map(|field| snippet_opt(cx, field.span).unwrap())
+        .collect::<Vec<_>>();
+
+    assert_eq!(pat_snippets.len(), ws.len() + 1);
+
+    let mut sugg = String::new();
+    for i in 0..pat_snippets.len() {
+        sugg += &pat_snippets[i];
+        if i < ws.len() {
+            sugg += &ws[i];
+        }
+    }
+    sugg
+}
+
+// smoelius: `is_consistent_order` is based on:
+// https://github.com/rust-lang/rust-clippy/blob/35e8be7407198565c434b69c5b9f85c71f156539/clippy_lints/src/inconsistent_struct_constructor.rs#L120-L133
+
+// Check whether the order of the fields in the constructor is consistent with the order in the
+// definition.
+fn is_consistent_order<'tcx>(
+    fields: &'tcx [PatField<'tcx>],
+    def_order_map: &FxHashMap<Symbol, usize>,
+) -> bool {
+    let mut cur_idx = usize::MIN;
+    for f in fields {
+        let next_idx = def_order_map[&f.ident.name];
+        if cur_idx > next_idx {
+            return false;
+        }
+        cur_idx = next_idx;
+    }
+
+    true
+}
+
+#[test]
+fn ui() {
+    dylint_testing::ui_test(env!("CARGO_PKG_NAME"), "ui");
+}

examples/supplementary/inconsistent_struct_pattern/ui/main.fixed

@@ -0,0 +1,24 @@
+// run-rustfix
+#![allow(unused)]
+
+#[derive(Default)]
+struct Struct {
+    a: bool,
+    b: bool,
+    c: bool,
+}
+
+fn main() {
+    let strukt = Struct::default();
+
+    // should not lint
+    let Struct { a, b, c } = strukt;
+    let Struct { a, b, .. } = strukt;
+    let Struct { a, c, .. } = strukt;
+    let Struct { b, c, .. } = strukt;
+
+    // should lint
+    let Struct { a, b, c } = strukt;
+    let Struct { a, b, c } = strukt;
+    let Struct { a, b, c } = strukt;
+}

examples/supplementary/inconsistent_struct_pattern/ui/main.rs

@@ -0,0 +1,24 @@
+// run-rustfix
+#![allow(unused)]
+
+#[derive(Default)]
+struct Struct {
+    a: bool,
+    b: bool,
+    c: bool,
+}
+
+fn main() {
+    let strukt = Struct::default();
+
+    // should not lint
+    let Struct { a, b, c } = strukt;
+    let Struct { a, b, .. } = strukt;
+    let Struct { a, c, .. } = strukt;
+    let Struct { b, c, .. } = strukt;
+
+    // should lint
+    let Struct { a, c, b } = strukt;
+    let Struct { b, a, c } = strukt;
+    let Struct { c, b, a } = strukt;
+}

examples/supplementary/inconsistent_struct_pattern/ui/main.stderr

@@ -0,0 +1,22 @@
+warning: struct pattern field order is inconsistent with struct definition field order
+  --> $DIR/main.rs:21:18
+   |
+LL |     let Struct { a, c, b } = strukt;
+   |                  ^^^^^^^ help: use: `a, b, c`
+   |
+   = note: `#[warn(inconsistent_struct_pattern)]` on by default
+
+warning: struct pattern field order is inconsistent with struct definition field order
+  --> $DIR/main.rs:22:18
+   |
+LL |     let Struct { b, a, c } = strukt;
+   |                  ^^^^^^^ help: use: `a, b, c`
+
+warning: struct pattern field order is inconsistent with struct definition field order
+  --> $DIR/main.rs:23:18
+   |
+LL |     let Struct { c, b, a } = strukt;
+   |                  ^^^^^^^ help: use: `a, b, c`
+
+warning: 3 warnings emitted
+

examples/supplementary/src/lib.rs

@@ -12,6 +12,7 @@ pub fn register_lints(sess: &rustc_session::Session, lint_store: &mut rustc_lint
     // smoelius: Please keep the following `register_lints` calls sorted by crate name.
     commented_code::register_lints(sess, lint_store);
     escaping_doc_link::register_lints(sess, lint_store);
+    inconsistent_struct_pattern::register_lints(sess, lint_store);
     redundant_reference::register_lints(sess, lint_store);
     unnamed_constant::register_lints(sess, lint_store);
     unnecessary_borrow_mut::register_lints(sess, lint_store);