Skip to content

Commit cf04c08

Browse files
new lint partial_ord_enum_struct_variant_fields_reordered (#1165)
last one of: #1132! --------- Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com>
1 parent 908c868 commit cf04c08

File tree

8 files changed

+423
-0
lines changed

8 files changed

+423
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
SemverQuery(
2+
id: "partial_ord_enum_struct_variant_fields_reordered",
3+
human_readable_name: "struct variant fields reordered in #[derive(PartialOrd)] enum",
4+
description: "A public enum that derives PartialOrd had fields reordered within a struct variant, which changes 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 Enum {
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+
variant {
35+
... on StructVariant {
36+
variant_name: name @output @tag
37+
public_api_eligible @filter(op: "=", value: ["$true"])
38+
39+
field {
40+
field_name: name @output @tag
41+
public_api_eligible @filter(op: "=", value: ["$true"])
42+
position @output @tag
43+
}
44+
}
45+
}
46+
}
47+
}
48+
}
49+
current {
50+
item {
51+
... on Enum {
52+
visibility_limit @filter(op: "=", value: ["$public"])
53+
name @output
54+
55+
impl {
56+
negative @filter(op: "=", value: ["$false"])
57+
attrs @filter(op: "contains", value: ["$derived"])
58+
59+
implemented_trait {
60+
trait {
61+
canonical_path {
62+
path @filter(op: "=", value: ["$partial_ord"])
63+
}
64+
}
65+
}
66+
}
67+
68+
importable_path {
69+
path @filter(op: "=", value: ["%path"])
70+
public_api @filter(op: "=", value: ["$true"])
71+
}
72+
73+
variant {
74+
... on StructVariant {
75+
name @filter(op: "=", value: ["%variant_name"])
76+
public_api_eligible @filter(op: "=", value: ["$true"])
77+
78+
field {
79+
name @filter(op: "=", value: ["%field_name"])
80+
public_api_eligible @filter(op: "=", value: ["$true"])
81+
position @filter(op: "!=", value: ["%position"]) @output(name: "new_position")
82+
span_: span @optional {
83+
filename @output
84+
begin_line @output
85+
end_line @output
86+
}
87+
}
88+
}
89+
}
90+
}
91+
}
92+
}
93+
}
94+
}"#,
95+
arguments: {
96+
"public": "public",
97+
"derived": "#[automatically_derived]",
98+
"false": false,
99+
"true": true,
100+
"partial_ord": ["core", "cmp", "PartialOrd"],
101+
},
102+
error_message: "A public enum that derives PartialOrd had fields reordered within a struct variant. #[derive(PartialOrd)] uses field order to determine ordering behavior, so this change may break downstream code that relies on the previous ordering.",
103+
per_result_error_template: Some("{{name}}::{{variant_name}}.{{field_name}} moved from position {{position}} to {{new_position}}, in {{span_filename}}:{{span_begin_line}}"),
104+
)

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_enum_struct_variant_fields_reordered,
12851286
partial_ord_enum_variants_reordered,
12861287
partial_ord_struct_fields_reordered,
12871288
proc_macro_marked_deprecated,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[package]
2+
publish = false
3+
name = "partial_ord_enum_struct_variant_fields_reordered"
4+
version = "0.1.0"
5+
edition = "2021"
6+
7+
[dependencies]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// Public enum with #[derive(PartialOrd)] and struct variant with reordered fields - should trigger warning
2+
#[derive(PartialOrd, PartialEq)]
3+
pub enum PublicEnum {
4+
StructVariant { b: u16, c: u32, a: u8 },
5+
}
6+
7+
// Private enum with #[derive(PartialOrd)] and struct variant with reordered fields - should not trigger
8+
#[derive(PartialOrd, PartialEq)]
9+
enum PrivateEnum {
10+
StructVariant { b: u16, c: u32, a: u8 },
11+
}
12+
13+
// Public enum without #[derive(PartialOrd)] but with struct variant with reordered fields - should not trigger
14+
pub enum RegularEnum {
15+
StructVariant { b: u16, c: u32, a: u8 },
16+
}
17+
18+
// Public enum with #[derive(PartialOrd)] and doc(hidden) - should not trigger
19+
#[doc(hidden)]
20+
#[derive(PartialOrd, PartialEq)]
21+
pub enum DocHiddenEnum {
22+
StructVariant { b: u16, c: u32, a: u8 },
23+
}
24+
25+
// Public enum with #[derive(PartialOrd)] and doc(hidden) variant - should not trigger
26+
#[derive(PartialOrd, PartialEq)]
27+
pub enum EnumWithDocHiddenVariant {
28+
#[doc(hidden)]
29+
StructVariant { b: u16, c: u32, a: u8 },
30+
}
31+
32+
// Public enum with #[derive(PartialOrd)] and doc(hidden) fields - should not trigger
33+
#[derive(PartialOrd, PartialEq)]
34+
pub enum EnumWithDocHiddenFields {
35+
StructVariant {
36+
a: u8,
37+
#[doc(hidden)]
38+
c: u32,
39+
#[doc(hidden)]
40+
b: u16,
41+
},
42+
}
43+
44+
// Public enum with #[derive(PartialOrd)] and same field order - should not trigger
45+
#[derive(PartialOrd, PartialEq)]
46+
pub enum UnchangedEnum {
47+
StructVariant { x: u8, y: u16 },
48+
}
49+
50+
// Test with manual PartialOrd implementation - should not trigger
51+
#[derive(PartialEq)]
52+
pub enum ManuallyImplemented {
53+
StructVariant { b: u16, a: u8 },
54+
}
55+
56+
impl PartialOrd for ManuallyImplemented {
57+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
58+
todo!()
59+
}
60+
}
61+
62+
// Test with multiple variants - should check each struct variant independently
63+
#[derive(PartialOrd, PartialEq)]
64+
pub enum MultipleVariants {
65+
First { b: u16, a: u8 },
66+
Second { y: u64, x: u32 },
67+
Third(u16, u8), // tuple variant reordered but should be ignored
68+
Fourth, // unit variant unchanged
69+
}
70+
71+
// Test case: Used to derive PartialOrd but no longer does - should not trigger this lint
72+
#[derive(PartialEq)]
73+
pub enum NoLongerPartialOrd {
74+
StructVariant { b: u16, c: u32, a: u8 },
75+
}
76+
77+
// Test case: Didn't derive PartialOrd before but now does - should not trigger this lint
78+
#[derive(PartialOrd, PartialEq)]
79+
pub enum NewlyPartialOrd {
80+
StructVariant { b: u16, c: u32, a: u8 },
81+
}
82+
83+
// Test case: Switching from derived to hand-implemented PartialOrd - should not trigger this lint
84+
#[derive(PartialEq)]
85+
pub enum DerivedToHandImpl {
86+
StructVariant { b: u16, c: u32, a: u8 },
87+
}
88+
89+
impl PartialOrd for DerivedToHandImpl {
90+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
91+
todo!()
92+
}
93+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[package]
2+
publish = false
3+
name = "partial_ord_enum_struct_variant_fields_reordered"
4+
version = "0.1.0"
5+
edition = "2021"
6+
7+
[dependencies]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Public enum with #[derive(PartialOrd)] and struct variant with reordered fields - should trigger warning
2+
#[derive(PartialOrd, PartialEq)]
3+
pub enum PublicEnum {
4+
StructVariant { a: u8, b: u16, c: u32 },
5+
}
6+
7+
// Private enum with #[derive(PartialOrd)] and struct variant with reordered fields - should not trigger
8+
#[derive(PartialOrd, PartialEq)]
9+
enum PrivateEnum {
10+
StructVariant { a: u8, b: u16, c: u32 },
11+
}
12+
13+
// Public enum without #[derive(PartialOrd)] but with struct variant with reordered fields - should not trigger
14+
pub enum RegularEnum {
15+
StructVariant { a: u8, b: u16, c: u32 },
16+
}
17+
18+
// Public enum with #[derive(PartialOrd)] and doc(hidden) - should not trigger
19+
#[doc(hidden)]
20+
#[derive(PartialOrd, PartialEq)]
21+
pub enum DocHiddenEnum {
22+
StructVariant { a: u8, b: u16, c: u32 },
23+
}
24+
25+
// Public enum with #[derive(PartialOrd)] and doc(hidden) variant - should not trigger
26+
#[derive(PartialOrd, PartialEq)]
27+
pub enum EnumWithDocHiddenVariant {
28+
#[doc(hidden)]
29+
StructVariant { a: u8, b: u16, c: u32 },
30+
}
31+
32+
// Public enum with #[derive(PartialOrd)] and doc(hidden) fields - should not trigger
33+
#[derive(PartialOrd, PartialEq)]
34+
pub enum EnumWithDocHiddenFields {
35+
StructVariant {
36+
a: u8,
37+
#[doc(hidden)]
38+
b: u16,
39+
#[doc(hidden)]
40+
c: u32,
41+
},
42+
}
43+
44+
// Public enum with #[derive(PartialOrd)] and same field order - should not trigger
45+
#[derive(PartialOrd, PartialEq)]
46+
pub enum UnchangedEnum {
47+
StructVariant { x: u8, y: u16 },
48+
}
49+
50+
// Test with manual PartialOrd implementation - should not trigger
51+
#[derive(PartialEq)]
52+
pub enum ManuallyImplemented {
53+
StructVariant { a: u8, b: u16 },
54+
}
55+
56+
impl PartialOrd for ManuallyImplemented {
57+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
58+
todo!()
59+
}
60+
}
61+
62+
// Test with multiple variants - should check each struct variant independently
63+
#[derive(PartialOrd, PartialEq)]
64+
pub enum MultipleVariants {
65+
First { a: u8, b: u16 },
66+
Second { x: u32, y: u64 },
67+
Third(u8, u16), // tuple variant should be ignored
68+
Fourth, // unit variant should be ignored
69+
}
70+
71+
// Test case: Used to derive PartialOrd but no longer does - should not trigger this lint
72+
#[derive(PartialOrd, PartialEq)]
73+
pub enum NoLongerPartialOrd {
74+
StructVariant { a: u8, b: u16, c: u32 },
75+
}
76+
77+
// Test case: Didn't derive PartialOrd before but now does - should not trigger this lint
78+
pub enum NewlyPartialOrd {
79+
StructVariant { a: u8, b: u16, c: u32 },
80+
}
81+
82+
// Test case: Switching from derived to hand-implemented PartialOrd - should not trigger this lint
83+
#[derive(PartialOrd, PartialEq)]
84+
pub enum DerivedToHandImpl {
85+
StructVariant { a: u8, b: u16, c: u32 },
86+
}

test_outputs/query_execution/derive_trait_impl_removed.snap

+19
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,25 @@ expression: "&query_execution_results"
5656
"visibility_limit": String("public"),
5757
},
5858
],
59+
"./test_crates/partial_ord_enum_struct_variant_fields_reordered/": [
60+
{
61+
"name": String("NoLongerPartialOrd"),
62+
"path": List([
63+
String("partial_ord_enum_struct_variant_fields_reordered"),
64+
String("NoLongerPartialOrd"),
65+
]),
66+
"span_begin_line": Uint64(73),
67+
"span_end_line": Uint64(75),
68+
"span_filename": String("src/lib.rs"),
69+
"trait_name": String("PartialOrd"),
70+
"trait_path": List([
71+
String("core"),
72+
String("cmp"),
73+
String("PartialOrd"),
74+
]),
75+
"visibility_limit": String("public"),
76+
},
77+
],
5978
"./test_crates/partial_ord_enum_variants_reordered/": [
6079
{
6180
"name": String("NoLongerPartialOrd"),

0 commit comments

Comments
 (0)