@@ -2,14 +2,15 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
22use clippy_utils:: source:: snippet;
33use clippy_utils:: ty:: is_copy;
44use clippy_utils:: { get_parent_expr, path_to_local} ;
5- use rustc_hir:: { BindingMode , Expr , ExprKind , Node , PatKind , UnOp } ;
5+ use rustc_hir:: { BindingMode , Expr , ExprField , ExprKind , Node , PatKind , Path , QPath , UnOp } ;
66use rustc_lint:: { LateContext , LateLintPass } ;
77use rustc_session:: declare_lint_pass;
88
99declare_clippy_lint ! {
1010 /// ### What it does
11- /// Checks for initialization of a `struct` by copying a base without setting
12- /// any field.
11+ /// Checks for initialization of an identical `struct` from another instance
12+ /// of the type, either by copying a base without setting any field or by
13+ /// moving all fields individually.
1314///
1415/// ### Why is this bad?
1516/// Readability suffers from unnecessary struct building.
@@ -29,9 +30,14 @@ declare_clippy_lint! {
2930/// let b = a;
3031/// ```
3132///
33+ /// The struct literal ``S { ..a }`` in the assignment to ``b`` could be replaced
34+ /// with just ``a``.
35+ ///
3236/// ### Known Problems
3337/// Has false positives when the base is a place expression that cannot be
3438/// moved out of, see [#10547](https://github.com/rust-lang/rust-clippy/issues/10547).
39+ ///
40+ /// Empty structs are ignored by the lint.
3541[ clippy:: version = "1.70.0" ]
3642 pub UNNECESSARY_STRUCT_INITIALIZATION ,
3743 nursery,
@@ -41,43 +47,113 @@ declare_lint_pass!(UnnecessaryStruct => [UNNECESSARY_STRUCT_INITIALIZATION]);
4147
4248impl LateLintPass < ' _ > for UnnecessaryStruct {
4349 fn check_expr ( & mut self , cx : & LateContext < ' _ > , expr : & Expr < ' _ > ) {
44- if let ExprKind :: Struct ( _, & [ ] , Some ( base) ) = expr. kind {
45- if let Some ( parent) = get_parent_expr ( cx, expr)
46- && let parent_ty = cx. typeck_results ( ) . expr_ty_adjusted ( parent)
47- && parent_ty. is_any_ptr ( )
48- {
49- if is_copy ( cx, cx. typeck_results ( ) . expr_ty ( expr) ) && path_to_local ( base) . is_some ( ) {
50- // When the type implements `Copy`, a reference to the new struct works on the
51- // copy. Using the original would borrow it.
52- return ;
53- }
54-
55- if parent_ty. is_mutable_ptr ( ) && !is_mutable ( cx, base) {
56- // The original can be used in a mutable reference context only if it is mutable.
57- return ;
58- }
59- }
50+ let ExprKind :: Struct ( _, fields, base) = expr. kind else {
51+ return ;
52+ } ;
6053
61- // TODO: do not propose to replace *XX if XX is not Copy
62- if let ExprKind :: Unary ( UnOp :: Deref , target) = base. kind
63- && matches ! ( target. kind, ExprKind :: Path ( ..) )
64- && !is_copy ( cx, cx. typeck_results ( ) . expr_ty ( expr) )
65- {
66- // `*base` cannot be used instead of the struct in the general case if it is not Copy.
67- return ;
68- }
54+ if expr. span . from_expansion ( ) {
55+ // Prevent lint from hitting inside macro code
56+ return ;
57+ }
58+
59+ // Conditions that must be satisfied to trigger this variant of the lint:
60+ // - source of the assignment must be a struct
61+ // - type of struct in expr must match
62+ // - of destination struct fields must match the names of the source
63+ // - all fields musts be assigned (no Default)
64+
65+ let field_path = same_path_in_all_fields ( cx, expr, fields) ;
66+
67+ let sugg = match ( field_path, base) {
68+ ( Some ( & path) , None ) => {
69+ // all fields match, no base given
70+ path. span
71+ } ,
72+ /*
73+ (Some(&path), Some(&Expr{ kind: ExprKind::Struct(&base_path, _, _), .. }))
74+ //if ExprKind::Path(base_path) == path => {
75+ if base_path == path => {
76+ // all fields match, has base: ensure that the path of the base matches
77+ path.span
78+ },
79+ */
80+ ( Some ( path) , Some ( base) ) if base_is_suitable ( cx, expr, base) && path_matches_base ( path, base) => {
81+ eprintln ! ( "[phg] »»» path: {:?}" , path) ;
82+ eprintln ! ( "[phg] »»» base: {:?}" , base) ;
83+
84+ // all fields match, has base: ensure that the path of the base matches
85+ base. span
86+ } ,
87+ ( None , Some ( base) ) if fields. is_empty ( ) && base_is_suitable ( cx, expr, base) => {
88+ // just the base, no explicit fields
89+
90+ base. span
91+ } ,
92+ _ => return ,
93+ } ;
94+
95+ span_lint_and_sugg (
96+ cx,
97+ UNNECESSARY_STRUCT_INITIALIZATION ,
98+ expr. span ,
99+ "unnecessary struct building" ,
100+ "replace with" ,
101+ snippet ( cx, sugg, ".." ) . into_owned ( ) ,
102+ rustc_errors:: Applicability :: MachineApplicable ,
103+ ) ;
104+ }
105+ }
106+
107+ fn base_is_suitable ( cx : & LateContext < ' _ > , expr : & Expr < ' _ > , base : & Expr < ' _ > ) -> bool {
108+ if !mutability_matches ( cx, expr, base) {
109+ return false ;
110+ }
111+ // TODO: do not propose to replace *XX if XX is not Copy
112+ if let ExprKind :: Unary ( UnOp :: Deref , target) = base. kind
113+ && matches ! ( target. kind, ExprKind :: Path ( ..) )
114+ && !is_copy ( cx, cx. typeck_results ( ) . expr_ty ( expr) )
115+ {
116+ // `*base` cannot be used instead of the struct in the general case if it is not Copy.
117+ return false ;
118+ }
119+ true
120+ }
121+
122+ fn same_path_in_all_fields < ' tcx > (
123+ cx : & LateContext < ' _ > ,
124+ expr : & Expr < ' _ > ,
125+ fields : & [ ExprField < ' tcx > ] ,
126+ ) -> Option < & ' tcx Path < ' tcx > > {
127+ let ty = cx. typeck_results ( ) . expr_ty ( expr) ;
128+
129+ let mut seen_path = None ;
69130
70- span_lint_and_sugg (
71- cx,
72- UNNECESSARY_STRUCT_INITIALIZATION ,
73- expr. span ,
74- "unnecessary struct building" ,
75- "replace with" ,
76- snippet ( cx, base. span , ".." ) . into_owned ( ) ,
77- rustc_errors:: Applicability :: MachineApplicable ,
78- ) ;
131+ for f in fields. iter ( ) . filter ( |f| !f. is_shorthand ) {
132+ // fields are assigned from expression
133+ if let ExprKind :: Field ( expr, ident) = f. expr . kind
134+ // expression type matches
135+ && ty == cx. typeck_results ( ) . expr_ty ( expr)
136+ // field name matches
137+ && f. ident == ident
138+ // assigned from a path expression
139+ && let ExprKind :: Path ( QPath :: Resolved ( None , path) ) = expr. kind
140+ {
141+ let Some ( p) = seen_path else {
142+ // this is the first field assignment in the list
143+ seen_path = Some ( path) ;
144+ continue ;
145+ } ;
146+
147+ if p. res == path. res {
148+ // subsequent field assignment with same origin struct as before
149+ continue ;
150+ }
79151 }
152+ // source of field assignment doesn’t qualify
153+ return None ;
80154 }
155+
156+ seen_path
81157}
82158
83159fn is_mutable ( cx : & LateContext < ' _ > , expr : & Expr < ' _ > ) -> bool {
@@ -89,3 +165,43 @@ fn is_mutable(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
89165 true
90166 }
91167}
168+
169+ fn mutability_matches ( cx : & LateContext < ' _ > , expr_a : & Expr < ' _ > , expr_b : & Expr < ' _ > ) -> bool {
170+ if let Some ( parent) = get_parent_expr ( cx, expr_a)
171+ && let parent_ty = cx. typeck_results ( ) . expr_ty_adjusted ( parent)
172+ && parent_ty. is_any_ptr ( )
173+ {
174+ if is_copy ( cx, cx. typeck_results ( ) . expr_ty ( expr_a) ) && path_to_local ( expr_b) . is_some ( ) {
175+ // When the type implements `Copy`, a reference to the new struct works on the
176+ // copy. Using the original would borrow it.
177+ return false ;
178+ }
179+
180+ if parent_ty. is_mutable_ptr ( ) && !is_mutable ( cx, expr_b) {
181+ // The original can be used in a mutable reference context only if it is mutable.
182+ return false ;
183+ }
184+ }
185+
186+ true
187+ }
188+
189+ /// When some fields are assigned from a base struct and others individually
190+ /// the lint applies only if the source of the field is the same as the base.
191+ /// This is enforced here by comparing the path of the base expression;
192+ /// needless to say the link only applies if it (or whatever expression it is
193+ /// a reference of) actually has a path.
194+ fn path_matches_base ( path : & Path < ' _ > , base : & Expr < ' _ > ) -> bool {
195+ let base_path = match base. kind {
196+ ExprKind :: Unary ( UnOp :: Deref , base_expr) => {
197+ if let ExprKind :: Path ( QPath :: Resolved ( _, base_path) ) = base_expr. kind {
198+ base_path
199+ } else {
200+ return false ;
201+ }
202+ } ,
203+ ExprKind :: Path ( QPath :: Resolved ( _, base_path) ) => base_path,
204+ _ => return false ,
205+ } ;
206+ path. res == base_path. res
207+ }
0 commit comments