Skip to content

Commit 0e47ff5

Browse files
samueltardieusamueltardieu
committed
New lint: detect unnecessary struct building
1 parent 5afa93b commit 0e47ff5

13 files changed

+355
-54
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4986,6 +4986,7 @@ Released 2018-09-13
49864986
[`unnecessary_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc
49874987
[`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
49884988
[`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
4989+
[`unnecessary_struct`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_struct
49894990
[`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
49904991
[`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
49914992
[`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
617617
crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
618618
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
619619
crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
620+
crate::unnecessary_struct::UNNECESSARY_STRUCT_INFO,
620621
crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
621622
crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO,
622623
crate::unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME_INFO,

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ mod unit_types;
301301
mod unnamed_address;
302302
mod unnecessary_owned_empty_strings;
303303
mod unnecessary_self_imports;
304+
mod unnecessary_struct;
304305
mod unnecessary_wraps;
305306
mod unnested_or_patterns;
306307
mod unsafe_removed_from_name;
@@ -936,6 +937,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
936937
store.register_early_pass(|| Box::new(redundant_async_block::RedundantAsyncBlock));
937938
store.register_late_pass(|_| Box::new(let_with_type_underscore::UnderscoreTyped));
938939
store.register_late_pass(|_| Box::new(allow_attributes::AllowAttribute));
940+
store.register_late_pass(|_| Box::new(unnecessary_struct::UnnecessaryStruct));
939941
// add lints here, do not remove this comment, it's used in `new_lint`
940942
}
941943

‎clippy_lints/src/unnecessary_struct.rs

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
use clippy_utils::{diagnostics::span_lint_and_sugg, get_parent_expr, path_to_local, source::snippet, ty::is_copy};
2+
use rustc_hir::*;
3+
use rustc_lint::{LateContext, LateLintPass};
4+
use rustc_session::{declare_lint_pass, declare_tool_lint};
5+
6+
declare_clippy_lint! {
7+
/// ### What it does
8+
/// Checks for initialization of a `struct` by copying a base without setting
9+
/// any field.
10+
///
11+
/// ### Why is this bad?
12+
/// Readibility suffers from unnecessary struct building.
13+
///
14+
/// ### Example
15+
/// ```rust
16+
/// struct S { s: String }
17+
///
18+
/// let a = S { s: String::from("Hello, world!") };
19+
/// let b = S { ..a };
20+
/// ```
21+
/// Use instead:
22+
/// ```rust
23+
/// struct S { s: String }
24+
///
25+
/// let a = S { s: String::from("Hello, world!") };
26+
/// let b = a;
27+
/// ```
28+
#[clippy::version = "1.70.0"]
29+
pub UNNECESSARY_STRUCT,
30+
complexity,
31+
"struct built from a base that can be written mode concisely"
32+
}
33+
declare_lint_pass!(UnnecessaryStruct => [UNNECESSARY_STRUCT]);
34+
35+
impl LateLintPass<'_> for UnnecessaryStruct {
36+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
37+
if let ExprKind::Struct(_, &[], Some(base)) = expr.kind {
38+
if let Some(parent) = get_parent_expr(cx, expr) &&
39+
let parent_ty = cx.typeck_results().expr_ty_adjusted(parent) &&
40+
parent_ty.is_any_ptr()
41+
{
42+
if is_copy(cx, cx.typeck_results().expr_ty(expr)) && path_to_local(base).is_some() {
43+
// When the type implements `Copy`, a reference to the new struct works on the
44+
// copy. Using the original would borrow it.
45+
return;
46+
}
47+
48+
if parent_ty.is_mutable_ptr() && !is_mutable(cx, base) {
49+
// The original can be used in a mutable reference context only if it is mutable.
50+
return;
51+
}
52+
}
53+
54+
// TODO: do not propose to replace *XX if XX is not Copy
55+
if let ExprKind::Unary(UnOp::Deref, target) = base.kind &&
56+
matches!(target.kind, ExprKind::Path(..)) &&
57+
!is_copy(cx, cx.typeck_results().expr_ty(expr))
58+
{
59+
// `*base` cannot be used instead of the struct in the general case if it is not Copy.
60+
return;
61+
}
62+
63+
span_lint_and_sugg(
64+
cx,
65+
UNNECESSARY_STRUCT,
66+
expr.span,
67+
"unnecessary struct building",
68+
"replace with",
69+
snippet(cx, base.span, "..").into_owned(),
70+
rustc_errors::Applicability::MachineApplicable,
71+
);
72+
}
73+
}
74+
}
75+
76+
fn is_mutable(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
77+
if let Some(hir_id) = path_to_local(expr) &&
78+
let Node::Pat(pat) = cx.tcx.hir().get(hir_id)
79+
{
80+
matches!(pat.kind, PatKind::Binding(BindingAnnotation::MUT, ..))
81+
} else {
82+
true
83+
}
84+
}

‎tests/ui/needless_update.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::needless_update)]
2-
#![allow(clippy::no_effect)]
2+
#![allow(clippy::no_effect, clippy::unnecessary_struct)]
33

44
struct S {
55
pub a: i32,

‎tests/ui/no_effect.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
#![feature(box_syntax, fn_traits, unboxed_closures)]
22
#![warn(clippy::no_effect_underscore_binding)]
33
#![allow(dead_code, path_statements)]
4-
#![allow(clippy::deref_addrof, clippy::redundant_field_names, clippy::uninlined_format_args)]
4+
#![allow(
5+
clippy::deref_addrof,
6+
clippy::redundant_field_names,
7+
clippy::uninlined_format_args,
8+
clippy::unnecessary_struct
9+
)]
510

611
struct Unit;
712
struct Tuple(i32);

‎tests/ui/no_effect.stderr

+30-30
Original file line numberDiff line numberDiff line change
@@ -1,183 +1,183 @@
11
error: statement with no effect
2-
--> $DIR/no_effect.rs:92:5
2+
--> $DIR/no_effect.rs:97:5
33
|
44
LL | 0;
55
| ^^
66
|
77
= note: `-D clippy::no-effect` implied by `-D warnings`
88

99
error: statement with no effect
10-
--> $DIR/no_effect.rs:93:5
10+
--> $DIR/no_effect.rs:98:5
1111
|
1212
LL | s2;
1313
| ^^^
1414

1515
error: statement with no effect
16-
--> $DIR/no_effect.rs:94:5
16+
--> $DIR/no_effect.rs:99:5
1717
|
1818
LL | Unit;
1919
| ^^^^^
2020

2121
error: statement with no effect
22-
--> $DIR/no_effect.rs:95:5
22+
--> $DIR/no_effect.rs:100:5
2323
|
2424
LL | Tuple(0);
2525
| ^^^^^^^^^
2626

2727
error: statement with no effect
28-
--> $DIR/no_effect.rs:96:5
28+
--> $DIR/no_effect.rs:101:5
2929
|
3030
LL | Struct { field: 0 };
3131
| ^^^^^^^^^^^^^^^^^^^^
3232

3333
error: statement with no effect
34-
--> $DIR/no_effect.rs:97:5
34+
--> $DIR/no_effect.rs:102:5
3535
|
3636
LL | Struct { ..s };
3737
| ^^^^^^^^^^^^^^^
3838

3939
error: statement with no effect
40-
--> $DIR/no_effect.rs:98:5
40+
--> $DIR/no_effect.rs:103:5
4141
|
4242
LL | Union { a: 0 };
4343
| ^^^^^^^^^^^^^^^
4444

4545
error: statement with no effect
46-
--> $DIR/no_effect.rs:99:5
46+
--> $DIR/no_effect.rs:104:5
4747
|
4848
LL | Enum::Tuple(0);
4949
| ^^^^^^^^^^^^^^^
5050

5151
error: statement with no effect
52-
--> $DIR/no_effect.rs:100:5
52+
--> $DIR/no_effect.rs:105:5
5353
|
5454
LL | Enum::Struct { field: 0 };
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
5656

5757
error: statement with no effect
58-
--> $DIR/no_effect.rs:101:5
58+
--> $DIR/no_effect.rs:106:5
5959
|
6060
LL | 5 + 6;
6161
| ^^^^^^
6262

6363
error: statement with no effect
64-
--> $DIR/no_effect.rs:102:5
64+
--> $DIR/no_effect.rs:107:5
6565
|
6666
LL | *&42;
6767
| ^^^^^
6868

6969
error: statement with no effect
70-
--> $DIR/no_effect.rs:103:5
70+
--> $DIR/no_effect.rs:108:5
7171
|
7272
LL | &6;
7373
| ^^^
7474

7575
error: statement with no effect
76-
--> $DIR/no_effect.rs:104:5
76+
--> $DIR/no_effect.rs:109:5
7777
|
7878
LL | (5, 6, 7);
7979
| ^^^^^^^^^^
8080

8181
error: statement with no effect
82-
--> $DIR/no_effect.rs:105:5
82+
--> $DIR/no_effect.rs:110:5
8383
|
8484
LL | box 42;
8585
| ^^^^^^^
8686

8787
error: statement with no effect
88-
--> $DIR/no_effect.rs:106:5
88+
--> $DIR/no_effect.rs:111:5
8989
|
9090
LL | ..;
9191
| ^^^
9292

9393
error: statement with no effect
94-
--> $DIR/no_effect.rs:107:5
94+
--> $DIR/no_effect.rs:112:5
9595
|
9696
LL | 5..;
9797
| ^^^^
9898

9999
error: statement with no effect
100-
--> $DIR/no_effect.rs:108:5
100+
--> $DIR/no_effect.rs:113:5
101101
|
102102
LL | ..5;
103103
| ^^^^
104104

105105
error: statement with no effect
106-
--> $DIR/no_effect.rs:109:5
106+
--> $DIR/no_effect.rs:114:5
107107
|
108108
LL | 5..6;
109109
| ^^^^^
110110

111111
error: statement with no effect
112-
--> $DIR/no_effect.rs:110:5
112+
--> $DIR/no_effect.rs:115:5
113113
|
114114
LL | 5..=6;
115115
| ^^^^^^
116116

117117
error: statement with no effect
118-
--> $DIR/no_effect.rs:111:5
118+
--> $DIR/no_effect.rs:116:5
119119
|
120120
LL | [42, 55];
121121
| ^^^^^^^^^
122122

123123
error: statement with no effect
124-
--> $DIR/no_effect.rs:112:5
124+
--> $DIR/no_effect.rs:117:5
125125
|
126126
LL | [42, 55][1];
127127
| ^^^^^^^^^^^^
128128

129129
error: statement with no effect
130-
--> $DIR/no_effect.rs:113:5
130+
--> $DIR/no_effect.rs:118:5
131131
|
132132
LL | (42, 55).1;
133133
| ^^^^^^^^^^^
134134

135135
error: statement with no effect
136-
--> $DIR/no_effect.rs:114:5
136+
--> $DIR/no_effect.rs:119:5
137137
|
138138
LL | [42; 55];
139139
| ^^^^^^^^^
140140

141141
error: statement with no effect
142-
--> $DIR/no_effect.rs:115:5
142+
--> $DIR/no_effect.rs:120:5
143143
|
144144
LL | [42; 55][13];
145145
| ^^^^^^^^^^^^^
146146

147147
error: statement with no effect
148-
--> $DIR/no_effect.rs:117:5
148+
--> $DIR/no_effect.rs:122:5
149149
|
150150
LL | || x += 5;
151151
| ^^^^^^^^^^
152152

153153
error: statement with no effect
154-
--> $DIR/no_effect.rs:119:5
154+
--> $DIR/no_effect.rs:124:5
155155
|
156156
LL | FooString { s: s };
157157
| ^^^^^^^^^^^^^^^^^^^
158158

159159
error: binding to `_` prefixed variable with no side-effect
160-
--> $DIR/no_effect.rs:120:5
160+
--> $DIR/no_effect.rs:125:5
161161
|
162162
LL | let _unused = 1;
163163
| ^^^^^^^^^^^^^^^^
164164
|
165165
= note: `-D clippy::no-effect-underscore-binding` implied by `-D warnings`
166166

167167
error: binding to `_` prefixed variable with no side-effect
168-
--> $DIR/no_effect.rs:121:5
168+
--> $DIR/no_effect.rs:126:5
169169
|
170170
LL | let _penguin = || println!("Some helpful closure");
171171
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
172172

173173
error: binding to `_` prefixed variable with no side-effect
174-
--> $DIR/no_effect.rs:122:5
174+
--> $DIR/no_effect.rs:127:5
175175
|
176176
LL | let _duck = Struct { field: 0 };
177177
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
178178

179179
error: binding to `_` prefixed variable with no side-effect
180-
--> $DIR/no_effect.rs:123:5
180+
--> $DIR/no_effect.rs:128:5
181181
|
182182
LL | let _cat = [2, 4, 6, 8][2];
183183
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

‎tests/ui/unnecessary_operation.fixed

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
// run-rustfix
22

33
#![feature(box_syntax)]
4-
#![allow(clippy::deref_addrof, dead_code, unused, clippy::no_effect)]
4+
#![allow(
5+
clippy::deref_addrof,
6+
dead_code,
7+
unused,
8+
clippy::no_effect,
9+
clippy::unnecessary_struct
10+
)]
511
#![warn(clippy::unnecessary_operation)]
612

713
struct Tuple(i32);

‎tests/ui/unnecessary_operation.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
// run-rustfix
22

33
#![feature(box_syntax)]
4-
#![allow(clippy::deref_addrof, dead_code, unused, clippy::no_effect)]
4+
#![allow(
5+
clippy::deref_addrof,
6+
dead_code,
7+
unused,
8+
clippy::no_effect,
9+
clippy::unnecessary_struct
10+
)]
511
#![warn(clippy::unnecessary_operation)]
612

713
struct Tuple(i32);

0 commit comments

Comments
 (0)