Skip to content

Commit 908c868

Browse files
Frank-IIIFrank-III
authored
new lint: partial_ord_enum_variants_reordered (#1163)
part of: #1132
1 parent 7d72104 commit 908c868

File tree

9 files changed

+1058
-0
lines changed

9 files changed

+1058
-0
lines changed

‎src/lints/partial_ord_enum_variants_reordered.ron

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
SemverQuery(
2+
id: "partial_ord_enum_variants_reordered",
3+
human_readable_name: "enum variants reordered in #[derive(PartialOrd)] enum",
4+
description: "A public enum that derives PartialOrd had its variants reordered, 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+
variant_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 Enum {
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+
variant {
67+
name @filter(op: "=", value: ["%variant_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 enum that derives PartialOrd had its variants reordered. #[derive(PartialOrd)] uses the enum variant order to set the enum's ordering behavior, so this change may break downstream code that relies on the previous order.",
90+
per_result_error_template: Some("{{name}}::{{variant_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_enum_variants_reordered,
12851286
partial_ord_struct_fields_reordered,
12861287
proc_macro_marked_deprecated,
12871288
proc_macro_now_doc_hidden,

‎test_crates/partial_ord_enum_variants_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_enum_variants_reordered"
4+
version = "0.1.0"
5+
edition = "2021"
6+
7+
[dependencies]

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

+145
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
// Public enum with #[derive(PartialOrd)] and reordered variants - should trigger warning
2+
#[derive(PartialOrd, PartialEq)]
3+
pub enum PublicEnum {
4+
B,
5+
C,
6+
A,
7+
}
8+
9+
// Private enum with #[derive(PartialOrd)] and reordered variants - should not trigger
10+
#[derive(PartialOrd, PartialEq)]
11+
enum PrivateEnum {
12+
B,
13+
C,
14+
A,
15+
}
16+
17+
// Public enum without #[derive(PartialOrd)] but with reordered variants - should not trigger
18+
pub enum RegularEnum {
19+
B,
20+
C,
21+
A,
22+
}
23+
24+
// Public enum with #[derive(PartialOrd)] and doc(hidden) - should not trigger
25+
#[doc(hidden)]
26+
#[derive(PartialOrd, PartialEq)]
27+
pub enum DocHiddenEnum {
28+
B,
29+
C,
30+
A,
31+
}
32+
33+
// Public enum with #[derive(PartialOrd)] and doc(hidden) variants - should not trigger
34+
#[derive(PartialOrd, PartialEq)]
35+
pub enum EnumWithDocHiddenVariants {
36+
A,
37+
#[doc(hidden)]
38+
C,
39+
#[doc(hidden)]
40+
B,
41+
D,
42+
}
43+
44+
// Public enum with #[derive(PartialOrd)] and same order - should not trigger
45+
#[derive(PartialOrd, PartialEq)]
46+
pub enum UnchangedEnum {
47+
X,
48+
Y,
49+
}
50+
51+
// Test with manual PartialOrd implementation - should not trigger
52+
#[derive(PartialEq)]
53+
pub enum ManuallyImplemented {
54+
B,
55+
A,
56+
}
57+
58+
impl PartialOrd for ManuallyImplemented {
59+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
60+
match (self, other) {
61+
(Self::A, Self::A) => Some(std::cmp::Ordering::Equal),
62+
(Self::A, Self::B) => Some(std::cmp::Ordering::Less),
63+
(Self::B, Self::A) => Some(std::cmp::Ordering::Greater),
64+
(Self::B, Self::B) => Some(std::cmp::Ordering::Equal),
65+
}
66+
}
67+
}
68+
69+
// Test with multiple derives including PartialOrd - should trigger
70+
#[derive(Debug, PartialOrd, Clone, PartialEq)]
71+
pub enum MultipleDerivesWithPartialOrd {
72+
B,
73+
C,
74+
A,
75+
}
76+
77+
// Test with mixed variant types - should trigger
78+
#[derive(PartialOrd, PartialEq)]
79+
pub enum MixedVariantTypes {
80+
Tuple(i32, String),
81+
Struct { x: i32, y: String },
82+
Unit,
83+
}
84+
85+
// Test with struct variants - should trigger
86+
#[derive(PartialOrd, PartialEq)]
87+
pub enum StructVariants {
88+
B { y: String },
89+
C { z: bool },
90+
A { x: i32 },
91+
}
92+
93+
// Test with tuple variants - should trigger
94+
#[derive(PartialOrd, PartialEq)]
95+
pub enum TupleVariants {
96+
B(String),
97+
C(bool),
98+
A(i32),
99+
}
100+
101+
// Test with discriminants - should trigger
102+
#[derive(PartialOrd, PartialEq)]
103+
pub enum WithDiscriminants {
104+
B = 20,
105+
C = 30,
106+
A = 10,
107+
}
108+
109+
// Test case: Used to derive PartialOrd but no longer does - should not trigger this lint
110+
#[derive(PartialEq)]
111+
pub enum NoLongerPartialOrd {
112+
B,
113+
C,
114+
A,
115+
}
116+
117+
// Test case: Didn't derive PartialOrd before but now does - should not trigger this lint
118+
#[derive(PartialOrd, PartialEq)]
119+
pub enum NewlyPartialOrd {
120+
B,
121+
C,
122+
A,
123+
}
124+
125+
// Test case: Switching from derived to hand-implemented PartialOrd - should not trigger this lint
126+
#[derive(PartialEq)]
127+
pub enum DerivedToHandImpl {
128+
B,
129+
C,
130+
A,
131+
}
132+
133+
impl PartialOrd for DerivedToHandImpl {
134+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
135+
todo!()
136+
}
137+
}
138+
139+
// Test case: Switching from hand-implemented to derived PartialOrd - should not trigger this lint
140+
#[derive(PartialOrd, PartialEq)]
141+
pub enum HandImplToDerived {
142+
B,
143+
C,
144+
A,
145+
}

‎test_crates/partial_ord_enum_variants_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_enum_variants_reordered"
4+
version = "0.1.0"
5+
edition = "2021"
6+
7+
[dependencies]

0 commit comments

Comments
 (0)