Skip to content

Commit 7d72104

Browse files
Frank-IIIFrank-III
authored
new lint partial_ord_struct_field_reordered (#1161)
part of: #1132
1 parent f559342 commit 7d72104

File tree

8 files changed

+483
-0
lines changed

8 files changed

+483
-0
lines changed

‎src/lints/partial_ord_struct_fields_reordered.ron

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
SemverQuery(
2+
id: "partial_ord_struct_fields_reordered",
3+
human_readable_name: "struct fields reordered in #[derive(PartialOrd)] struct",
4+
description: "A public struct that derives PartialOrd had its fields reordered, which can change its ordering behavior.",
5+
required_update: Major,
6+
lint_level: Warn,
7+
reference_link: Some("https://doc.rust-lang.org/std/cmp/trait.PartialOrd.html#derivable"),
8+
query: r#"
9+
{
10+
CrateDiff {
11+
baseline {
12+
item {
13+
... on Struct {
14+
visibility_limit @filter(op: "=", value: ["$public"])
15+
16+
impl {
17+
negative @filter(op: "=", value: ["$false"])
18+
attrs @filter(op: "contains", value: ["$derived"])
19+
20+
implemented_trait {
21+
trait {
22+
canonical_path {
23+
path @filter(op: "=", value: ["$partial_ord"])
24+
}
25+
}
26+
}
27+
}
28+
29+
importable_path {
30+
path @tag @output
31+
public_api @filter(op: "=", value: ["$true"])
32+
}
33+
34+
field {
35+
field_name: name @output @tag
36+
public_api_eligible @filter(op: "=", value: ["$true"])
37+
position @output @tag
38+
}
39+
}
40+
}
41+
}
42+
current {
43+
item {
44+
... on Struct {
45+
visibility_limit @filter(op: "=", value: ["$public"])
46+
name @output
47+
48+
impl {
49+
negative @filter(op: "=", value: ["$false"])
50+
attrs @filter(op: "contains", value: ["$derived"])
51+
52+
implemented_trait {
53+
trait {
54+
canonical_path {
55+
path @filter(op: "=", value: ["$partial_ord"])
56+
}
57+
}
58+
}
59+
}
60+
61+
importable_path {
62+
path @filter(op: "=", value: ["%path"])
63+
public_api @filter(op: "=", value: ["$true"])
64+
}
65+
66+
field {
67+
name @filter(op: "=", value: ["%field_name"])
68+
public_api_eligible @filter(op: "=", value: ["$true"])
69+
position @filter(op: "!=", value: ["%position"]) @output(name: "new_position")
70+
71+
span_: span @optional {
72+
filename @output
73+
begin_line @output
74+
end_line @output
75+
}
76+
}
77+
}
78+
}
79+
}
80+
}
81+
}"#,
82+
arguments: {
83+
"public": "public",
84+
"derived": "#[automatically_derived]",
85+
"false": false,
86+
"true": true,
87+
"partial_ord": ["core", "cmp", "PartialOrd"],
88+
},
89+
error_message: "A public struct that derives PartialOrd had its fields reordered. #[derive(PartialOrd)] uses the field order to set the struct's ordering behavior, so this change may break downstream code that relies on the previous order.",
90+
per_result_error_template: Some("{{name}}.{{field_name}} moved from position {{position}} to {{new_position}}, in {{span_filename}}:{{span_begin_line}}"),
91+
)

‎src/query.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,7 @@ add_lints!(
12821282
method_requires_different_generic_type_params,
12831283
module_missing,
12841284
non_exhaustive_struct_changed_type,
1285+
partial_ord_struct_fields_reordered,
12851286
proc_macro_marked_deprecated,
12861287
proc_macro_now_doc_hidden,
12871288
pub_module_level_const_missing,

‎test_crates/partial_ord_struct_fields_reordered/new/Cargo.toml

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[package]
2+
publish = false
3+
name = "partial_ord_struct_fields_reordered"
4+
version = "0.1.0"
5+
edition = "2021"
6+
7+
[dependencies]

‎test_crates/partial_ord_struct_fields_reordered/new/src/lib.rs

+128
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// Public struct with #[derive(PartialOrd)] and reordered fields - should trigger warning
2+
#[derive(PartialOrd, PartialEq)]
3+
pub struct PublicStruct {
4+
pub b: u16, // moved
5+
pub c: u32, // moved
6+
pub a: u8, // moved
7+
}
8+
9+
// Private struct with #[derive(PartialOrd)] and reordered fields - should not trigger
10+
#[derive(PartialOrd, PartialEq)]
11+
struct PrivateStruct {
12+
pub b: u16,
13+
pub c: u32,
14+
pub a: u8,
15+
}
16+
17+
// Public struct without #[derive(PartialOrd)] but with reordered fields - should not trigger
18+
pub struct RegularStruct {
19+
pub b: u16,
20+
pub c: u32,
21+
pub a: u8,
22+
}
23+
24+
// Public struct with #[derive(PartialOrd)] and doc(hidden) - should not trigger
25+
#[doc(hidden)]
26+
#[derive(PartialOrd, PartialEq)]
27+
pub struct DocHiddenStruct {
28+
pub b: u16,
29+
pub c: u32,
30+
pub a: u8,
31+
}
32+
33+
// Public struct with #[derive(PartialOrd)] and doc(hidden) fields swapped - should not trigger
34+
#[derive(PartialOrd, PartialEq)]
35+
pub struct PublicStructWithDocHiddenFieldsSwapped {
36+
pub a: u8,
37+
#[doc(hidden)]
38+
pub c: u32,
39+
#[doc(hidden)]
40+
pub b: u16,
41+
pub d: u64,
42+
}
43+
44+
// Public struct with #[derive(PartialOrd)] and same order - should not trigger
45+
#[derive(PartialOrd, PartialEq)]
46+
pub struct UnchangedStruct {
47+
pub x: u8, // same position
48+
pub y: u16, // same position
49+
}
50+
51+
// Test with manual PartialOrd implementation - should not trigger
52+
#[derive(PartialEq)]
53+
pub struct ManuallyImplemented {
54+
pub b: u16, // reordered but has manual impl
55+
pub a: u8,
56+
}
57+
58+
impl PartialOrd for ManuallyImplemented {
59+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
60+
Some(self.a.cmp(&other.a))
61+
}
62+
}
63+
64+
// Test with other derives - should not trigger
65+
#[derive(Debug, Clone, PartialEq)]
66+
pub struct OtherDerives {
67+
pub b: u16,
68+
pub a: u8,
69+
}
70+
71+
// Test with multiple derives including PartialOrd - should trigger
72+
#[derive(Debug, PartialOrd, Clone, PartialEq)]
73+
pub struct MultipleDerivesWithPartialOrd {
74+
pub b: u16, // moved
75+
pub c: u32, // moved
76+
pub a: u8, // moved
77+
}
78+
79+
// Struct that no longer derives PartialOrd but has reordered fields - should not trigger
80+
pub struct NoLongerPartialOrd {
81+
pub b: u16,
82+
pub c: u32,
83+
pub a: u8,
84+
}
85+
86+
// Struct that newly derives PartialOrd and has reordered fields - should not trigger
87+
#[derive(PartialOrd, PartialEq)]
88+
pub struct NewlyPartialOrd {
89+
pub b: u16,
90+
pub c: u32,
91+
pub a: u8,
92+
}
93+
94+
// Struct that switches from derived to hand-implemented PartialOrd - should not trigger
95+
pub struct SwitchToHandImpl {
96+
pub b: u16,
97+
pub c: u32,
98+
pub a: u8,
99+
}
100+
101+
impl PartialEq for SwitchToHandImpl {
102+
fn eq(&self, other: &Self) -> bool {
103+
self.a == other.a && self.b == other.b && self.c == other.c
104+
}
105+
}
106+
107+
impl PartialOrd for SwitchToHandImpl {
108+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
109+
// Custom implementation that maintains original ordering logic
110+
match self.a.partial_cmp(&other.a) {
111+
Some(core::cmp::Ordering::Equal) => {}
112+
ord => return ord,
113+
}
114+
match self.b.partial_cmp(&other.b) {
115+
Some(core::cmp::Ordering::Equal) => {}
116+
ord => return ord,
117+
}
118+
self.c.partial_cmp(&other.c)
119+
}
120+
}
121+
122+
// Struct that switches from hand-implemented to derived PartialOrd - should not trigger
123+
#[derive(PartialOrd, PartialEq)]
124+
pub struct SwitchToDerived {
125+
pub b: u16,
126+
pub c: u32,
127+
pub a: u8,
128+
}

‎test_crates/partial_ord_struct_fields_reordered/old/Cargo.toml

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[package]
2+
publish = false
3+
name = "partial_ord_struct_fields_reordered"
4+
version = "0.1.0"
5+
edition = "2021"
6+
7+
[dependencies]

‎test_crates/partial_ord_struct_fields_reordered/old/src/lib.rs

+127
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
// Public struct with #[derive(PartialOrd)] and reordered fields - should trigger warning
2+
#[derive(PartialOrd, PartialEq)]
3+
pub struct PublicStruct {
4+
pub a: u8,
5+
pub b: u16,
6+
pub c: u32,
7+
}
8+
9+
// Private struct with #[derive(PartialOrd)] and reordered fields - should not trigger
10+
#[derive(PartialOrd, PartialEq)]
11+
struct PrivateStruct {
12+
pub a: u8,
13+
pub b: u16,
14+
pub c: u32,
15+
}
16+
17+
// Public struct without #[derive(PartialOrd)] but with reordered fields - should not trigger
18+
pub struct RegularStruct {
19+
pub a: u8,
20+
pub b: u16,
21+
pub c: u32,
22+
}
23+
24+
// Public struct with #[derive(PartialOrd)] and doc(hidden) - should not trigger
25+
#[doc(hidden)]
26+
#[derive(PartialOrd, PartialEq)]
27+
pub struct DocHiddenStruct {
28+
pub a: u8,
29+
pub b: u16,
30+
pub c: u32,
31+
}
32+
33+
// Public struct with #[derive(PartialOrd)] and doc(hidden) fields swapped - should not trigger
34+
#[derive(PartialOrd, PartialEq)]
35+
pub struct PublicStructWithDocHiddenFieldsSwapped {
36+
pub a: u8,
37+
#[doc(hidden)]
38+
pub b: u16,
39+
#[doc(hidden)]
40+
pub c: u32,
41+
pub d: u64,
42+
}
43+
44+
// Public struct with #[derive(PartialOrd)] and same order - should not trigger
45+
#[derive(PartialOrd, PartialEq)]
46+
pub struct UnchangedStruct {
47+
pub x: u8,
48+
pub y: u16,
49+
}
50+
51+
// Test with manual PartialOrd implementation - should not trigger
52+
#[derive(PartialEq)]
53+
pub struct ManuallyImplemented {
54+
pub a: u8,
55+
pub b: u16,
56+
}
57+
58+
impl PartialOrd for ManuallyImplemented {
59+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
60+
Some(self.a.cmp(&other.a))
61+
}
62+
}
63+
64+
// Test with other derives - should not trigger
65+
#[derive(Debug, Clone, PartialEq)]
66+
pub struct OtherDerives {
67+
pub a: u8,
68+
pub b: u16,
69+
}
70+
71+
// Test with multiple derives including PartialOrd - should trigger
72+
#[derive(Debug, PartialOrd, Clone, PartialEq)]
73+
pub struct MultipleDerivesWithPartialOrd {
74+
pub a: u8,
75+
pub b: u16,
76+
pub c: u32,
77+
}
78+
79+
// Struct that no longer derives PartialOrd but has reordered fields - should not trigger
80+
#[derive(PartialOrd, PartialEq)]
81+
pub struct NoLongerPartialOrd {
82+
pub a: u8,
83+
pub b: u16,
84+
pub c: u32,
85+
}
86+
87+
// Struct that newly derives PartialOrd and has reordered fields - should not trigger
88+
pub struct NewlyPartialOrd {
89+
pub a: u8,
90+
pub b: u16,
91+
pub c: u32,
92+
}
93+
94+
// Struct that switches from derived to hand-implemented PartialOrd - should not trigger
95+
#[derive(PartialOrd, PartialEq)]
96+
pub struct SwitchToHandImpl {
97+
pub a: u8,
98+
pub b: u16,
99+
pub c: u32,
100+
}
101+
102+
// Struct that switches from hand-implemented to derived PartialOrd - should not trigger
103+
pub struct SwitchToDerived {
104+
pub a: u8,
105+
pub b: u16,
106+
pub c: u32,
107+
}
108+
109+
impl PartialEq for SwitchToDerived {
110+
fn eq(&self, other: &Self) -> bool {
111+
self.a == other.a && self.b == other.b && self.c == other.c
112+
}
113+
}
114+
115+
impl PartialOrd for SwitchToDerived {
116+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
117+
match self.a.partial_cmp(&other.a) {
118+
Some(core::cmp::Ordering::Equal) => {}
119+
ord => return ord,
120+
}
121+
match self.b.partial_cmp(&other.b) {
122+
Some(core::cmp::Ordering::Equal) => {}
123+
ord => return ord,
124+
}
125+
self.c.partial_cmp(&other.c)
126+
}
127+
}

0 commit comments

Comments
 (0)