From aa62f12206196b59d4f135cd920b117d554a1fe1 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Tue, 19 Nov 2024 12:26:03 -0500 Subject: [PATCH 1/5] Add `inconsistent_struct_pattern` lint --- examples/README.md | 1 + examples/supplementary/Cargo.lock | 10 ++ examples/supplementary/Cargo.toml | 4 + .../inconsistent_struct_pattern/Cargo.toml | 27 +++++ .../inconsistent_struct_pattern/README.md | 26 +++++ .../inconsistent_struct_pattern/src/lib.rs | 110 ++++++++++++++++++ .../inconsistent_struct_pattern/ui/main.rs | 21 ++++ .../ui/main.stderr | 22 ++++ examples/supplementary/src/lib.rs | 1 + 9 files changed, 222 insertions(+) create mode 100644 examples/supplementary/inconsistent_struct_pattern/Cargo.toml create mode 100644 examples/supplementary/inconsistent_struct_pattern/README.md create mode 100644 examples/supplementary/inconsistent_struct_pattern/src/lib.rs create mode 100644 examples/supplementary/inconsistent_struct_pattern/ui/main.rs create mode 100644 examples/supplementary/inconsistent_struct_pattern/ui/main.stderr diff --git a/examples/README.md b/examples/README.md index 4cb142319..7e22f00dc 100644 --- a/examples/README.md +++ b/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 | diff --git a/examples/supplementary/Cargo.lock b/examples/supplementary/Cargo.lock index eb4e470fc..270e5f1d5 100644 --- a/examples/supplementary/Cargo.lock +++ b/examples/supplementary/Cargo.lock @@ -646,6 +646,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "inconsistent_struct_pattern" +version = "3.2.1" +dependencies = [ + "clippy_utils", + "dylint_linting", + "dylint_testing", +] + [[package]] name = "indexmap" version = "2.6.0" @@ -1134,6 +1143,7 @@ dependencies = [ "commented_code", "dylint_linting", "escaping_doc_link", + "inconsistent_struct_pattern", "local_ref_cell", "redundant_reference", "unnamed_constant", diff --git a/examples/supplementary/Cargo.toml b/examples/supplementary/Cargo.toml index 812142168..32d6cd05e 100644 --- a/examples/supplementary/Cargo.toml +++ b/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", diff --git a/examples/supplementary/inconsistent_struct_pattern/Cargo.toml b/examples/supplementary/inconsistent_struct_pattern/Cargo.toml new file mode 100644 index 000000000..e5529e18f --- /dev/null +++ b/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 "] +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 diff --git a/examples/supplementary/inconsistent_struct_pattern/README.md b/examples/supplementary/inconsistent_struct_pattern/README.md new file mode 100644 index 000000000..ff3bd6810 --- /dev/null +++ b/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; +``` diff --git a/examples/supplementary/inconsistent_struct_pattern/src/lib.rs b/examples/supplementary/inconsistent_struct_pattern/src/lib.rs new file mode 100644 index 000000000..133db29f8 --- /dev/null +++ b/examples/supplementary/inconsistent_struct_pattern/src/lib.rs @@ -0,0 +1,110 @@ +#![feature(rustc_private)] +#![feature(let_chains)] +#![warn(unused_extern_crates)] + +extern crate rustc_data_structures; +extern crate rustc_hir; +extern crate rustc_span; + +use clippy_utils::diagnostics::span_lint; +use rustc_data_structures::fx::FxHashMap; +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; + } + + span_lint( + cx, + INCONSISTENT_STRUCT_PATTERN, + pat.span, + "struct pattern field order is inconsistent with struct definition field order", + ); + } +} + +// 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, +) -> 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"); +} diff --git a/examples/supplementary/inconsistent_struct_pattern/ui/main.rs b/examples/supplementary/inconsistent_struct_pattern/ui/main.rs new file mode 100644 index 000000000..78fb37231 --- /dev/null +++ b/examples/supplementary/inconsistent_struct_pattern/ui/main.rs @@ -0,0 +1,21 @@ +#[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; +} diff --git a/examples/supplementary/inconsistent_struct_pattern/ui/main.stderr b/examples/supplementary/inconsistent_struct_pattern/ui/main.stderr new file mode 100644 index 000000000..44d64a0e8 --- /dev/null +++ b/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:18:9 + | +LL | let Struct { a, c, b } = strukt; + | ^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(inconsistent_struct_pattern)]` on by default + +warning: struct pattern field order is inconsistent with struct definition field order + --> $DIR/main.rs:19:9 + | +LL | let Struct { b, a, c } = strukt; + | ^^^^^^^^^^^^^^^^^^ + +warning: struct pattern field order is inconsistent with struct definition field order + --> $DIR/main.rs:20:9 + | +LL | let Struct { c, b, a } = strukt; + | ^^^^^^^^^^^^^^^^^^ + +warning: 3 warnings emitted + diff --git a/examples/supplementary/src/lib.rs b/examples/supplementary/src/lib.rs index 0c5b02a67..d87b2ee6b 100644 --- a/examples/supplementary/src/lib.rs +++ b/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); From aa10afa071874934703860d8d69275a39731e859 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Tue, 19 Nov 2024 17:51:44 -0500 Subject: [PATCH 2/5] Add suggestions to `inconsistent_struct_pattern` --- .../inconsistent_struct_pattern/src/lib.rs | 50 +++++++++++++++++-- .../inconsistent_struct_pattern/ui/main.fixed | 24 +++++++++ .../inconsistent_struct_pattern/ui/main.rs | 3 ++ .../ui/main.stderr | 12 ++--- 4 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 examples/supplementary/inconsistent_struct_pattern/ui/main.fixed diff --git a/examples/supplementary/inconsistent_struct_pattern/src/lib.rs b/examples/supplementary/inconsistent_struct_pattern/src/lib.rs index 133db29f8..f398c05e6 100644 --- a/examples/supplementary/inconsistent_struct_pattern/src/lib.rs +++ b/examples/supplementary/inconsistent_struct_pattern/src/lib.rs @@ -3,11 +3,13 @@ #![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; +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, @@ -74,15 +76,57 @@ impl<'tcx> LateLintPass<'tcx> for InconsistentStructPattern { return; } - span_lint( + 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, - pat.span, + 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, +) -> 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::>(); + + 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::>(); + + 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 diff --git a/examples/supplementary/inconsistent_struct_pattern/ui/main.fixed b/examples/supplementary/inconsistent_struct_pattern/ui/main.fixed new file mode 100644 index 000000000..c05f35b32 --- /dev/null +++ b/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; +} diff --git a/examples/supplementary/inconsistent_struct_pattern/ui/main.rs b/examples/supplementary/inconsistent_struct_pattern/ui/main.rs index 78fb37231..83bbc4b05 100644 --- a/examples/supplementary/inconsistent_struct_pattern/ui/main.rs +++ b/examples/supplementary/inconsistent_struct_pattern/ui/main.rs @@ -1,3 +1,6 @@ +// run-rustfix +#![allow(unused)] + #[derive(Default)] struct Struct { a: bool, diff --git a/examples/supplementary/inconsistent_struct_pattern/ui/main.stderr b/examples/supplementary/inconsistent_struct_pattern/ui/main.stderr index 44d64a0e8..463349424 100644 --- a/examples/supplementary/inconsistent_struct_pattern/ui/main.stderr +++ b/examples/supplementary/inconsistent_struct_pattern/ui/main.stderr @@ -1,22 +1,22 @@ warning: struct pattern field order is inconsistent with struct definition field order - --> $DIR/main.rs:18:9 + --> $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:19:9 + --> $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:20:9 + --> $DIR/main.rs:23:18 | LL | let Struct { c, b, a } = strukt; - | ^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ help: use: `a, b, c` warning: 3 warnings emitted From 82a5e909200db6ab2aeb8d5576948d39c2a3347a Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Tue, 19 Nov 2024 17:52:22 -0500 Subject: [PATCH 3/5] Fix `await_holding_span_guard` --- examples/general/await_holding_span_guard/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/general/await_holding_span_guard/src/lib.rs b/examples/general/await_holding_span_guard/src/lib.rs index 4935783ba..e0a8b846a 100644 --- a/examples/general/await_holding_span_guard/src/lib.rs +++ b/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 { From 3ef4afcb92266682a2ee665154cda647c34d7f3c Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Wed, 20 Nov 2024 09:44:48 -0500 Subject: [PATCH 4/5] Fix `overscoped_allow` --- examples/restriction/overscoped_allow/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/restriction/overscoped_allow/src/lib.rs b/examples/restriction/overscoped_allow/src/lib.rs index a2f00efd3..d1fba8a08 100644 --- a/examples/restriction/overscoped_allow/src/lib.rs +++ b/examples/restriction/overscoped_allow/src/lib.rs @@ -403,8 +403,8 @@ fn local_path_from_span(cx: &LateContext<'_>, span: Span) -> Option { 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 { From 7b96a2edd5327811f6926cfdf82e37098ec529d4 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Wed, 20 Nov 2024 12:27:06 -0500 Subject: [PATCH 5/5] Update `ctor` to version 0.2.9 --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 33a7838ee..88ed36e27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -719,9 +719,9 @@ checksum = "026ac6ceace6298d2c557ef5ed798894962296469ec7842288ea64674201a2d1" [[package]] name = "ctor" -version = "0.2.8" +version = "0.2.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "edb49164822f3ee45b17acd4a208cfc1251410cf0cad9a833234c9890774dd9f" +checksum = "32a2785755761f3ddc1492979ce1e48d2c00d09311c39e4466429188f3dd6501" dependencies = [ "quote", "syn",