Skip to content

Commit

Permalink
Add inconsistent_struct_pattern lint
Browse files Browse the repository at this point in the history
  • Loading branch information
smoelius committed Nov 19, 2024
1 parent f4f281e commit d4a3acd
Show file tree
Hide file tree
Showing 9 changed files with 210 additions and 0 deletions.
1 change: 1 addition & 0 deletions examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
10 changes: 10 additions & 0 deletions examples/supplementary/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions examples/supplementary/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand All @@ -31,6 +34,7 @@ rustc_private = true
members = [
"commented_code",
"escaping_doc_link",
"inconsistent_struct_pattern",
"local_ref_cell",
"redundant_reference",
"unnamed_constant",
Expand Down
27 changes: 27 additions & 0 deletions examples/supplementary/inconsistent_struct_pattern/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
[package]
name = "inconsistent_struct_pattern"
version = "3.2.1"
authors = ["Samuel E. Moelius III <sam@moeli.us>"]
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
20 changes: 20 additions & 0 deletions examples/supplementary/inconsistent_struct_pattern/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# 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;
```
104 changes: 104 additions & 0 deletions examples/supplementary/inconsistent_struct_pattern/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
#![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<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");
}
21 changes: 21 additions & 0 deletions examples/supplementary/inconsistent_struct_pattern/ui/main.rs
Original file line number Diff line number Diff line change
@@ -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;
}
22 changes: 22 additions & 0 deletions examples/supplementary/inconsistent_struct_pattern/ui/main.stderr
Original file line number Diff line number Diff line change
@@ -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

1 change: 1 addition & 0 deletions examples/supplementary/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit d4a3acd

Please sign in to comment.