@@ -17,12 +17,12 @@ use rustc_hir::{
17
17
use rustc_lint:: { LateContext , LateLintPass , LintContext } ;
18
18
use rustc_middle:: hir:: map:: Map ;
19
19
use rustc_middle:: lint:: in_external_macro;
20
- use rustc_middle:: ty:: layout:: LayoutOf ;
21
20
use rustc_middle:: ty:: { self , InferTy , Ty , TyCtxt , TypeckTables } ;
22
21
use rustc_session:: { declare_lint_pass, declare_tool_lint, impl_lint_pass} ;
23
22
use rustc_span:: hygiene:: { ExpnKind , MacroKind } ;
24
23
use rustc_span:: source_map:: Span ;
25
24
use rustc_span:: symbol:: { sym, Symbol } ;
25
+ use rustc_target:: abi:: LayoutOf ;
26
26
use rustc_target:: spec:: abi:: Abi ;
27
27
use rustc_typeck:: hir_ty_to_ty;
28
28
@@ -99,16 +99,33 @@ declare_clippy_lint! {
99
99
/// represents an optional optional value which is logically the same thing as an optional
100
100
/// value but has an unneeded extra level of wrapping.
101
101
///
102
+ /// If you have a case where `Some(Some(_))`, `Some(None)` and `None` are distinct cases,
103
+ /// consider a custom `enum` instead, with clear names for each case.
104
+ ///
102
105
/// **Known problems:** None.
103
106
///
104
107
/// **Example**
105
108
/// ```rust
106
- /// fn x () -> Option<Option<u32>> {
109
+ /// fn get_data () -> Option<Option<u32>> {
107
110
/// None
108
111
/// }
109
112
/// ```
113
+ ///
114
+ /// Better:
115
+ ///
116
+ /// ```rust
117
+ /// pub enum Contents {
118
+ /// Data(Vec<u8>), // Was Some(Some(Vec<u8>))
119
+ /// NotYetFetched, // Was Some(None)
120
+ /// None, // Was None
121
+ /// }
122
+ ///
123
+ /// fn get_data() -> Contents {
124
+ /// Contents::None
125
+ /// }
126
+ /// ```
110
127
pub OPTION_OPTION ,
111
- complexity ,
128
+ pedantic ,
112
129
"usage of `Option<Option<T>>`"
113
130
}
114
131
@@ -171,11 +188,35 @@ declare_clippy_lint! {
171
188
"a borrow of a boxed type"
172
189
}
173
190
191
+ declare_clippy_lint ! {
192
+ /// **What it does:** Checks for use of redundant allocations anywhere in the code.
193
+ ///
194
+ /// **Why is this bad?** Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Box<T>>`, `Box<&T>`
195
+ /// add an unnecessary level of indirection.
196
+ ///
197
+ /// **Known problems:** None.
198
+ ///
199
+ /// **Example:**
200
+ /// ```rust
201
+ /// # use std::rc::Rc;
202
+ /// fn foo(bar: Rc<&usize>) {}
203
+ /// ```
204
+ ///
205
+ /// Better:
206
+ ///
207
+ /// ```rust
208
+ /// fn foo(bar: &usize) {}
209
+ /// ```
210
+ pub REDUNDANT_ALLOCATION ,
211
+ perf,
212
+ "redundant allocation"
213
+ }
214
+
174
215
pub struct Types {
175
216
vec_box_size_threshold : u64 ,
176
217
}
177
218
178
- impl_lint_pass ! ( Types => [ BOX_VEC , VEC_BOX , OPTION_OPTION , LINKEDLIST , BORROWED_BOX ] ) ;
219
+ impl_lint_pass ! ( Types => [ BOX_VEC , VEC_BOX , OPTION_OPTION , LINKEDLIST , BORROWED_BOX , REDUNDANT_ALLOCATION ] ) ;
179
220
180
221
impl < ' a , ' tcx > LateLintPass < ' a , ' tcx > for Types {
181
222
fn check_fn (
@@ -217,7 +258,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types {
217
258
}
218
259
219
260
/// Checks if `qpath` has last segment with type parameter matching `path`
220
- fn match_type_parameter ( cx : & LateContext < ' _ , ' _ > , qpath : & QPath < ' _ > , path : & [ & str ] ) -> bool {
261
+ fn match_type_parameter ( cx : & LateContext < ' _ , ' _ > , qpath : & QPath < ' _ > , path : & [ & str ] ) -> Option < Span > {
221
262
let last = last_path_segment ( qpath) ;
222
263
if_chain ! {
223
264
if let Some ( ref params) = last. args;
@@ -230,10 +271,27 @@ fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath<'_>, path: &[&st
230
271
if let Some ( did) = qpath_res( cx, qpath, ty. hir_id) . opt_def_id( ) ;
231
272
if match_def_path( cx, did, path) ;
232
273
then {
233
- return true ;
274
+ return Some ( ty . span ) ;
234
275
}
235
276
}
236
- false
277
+ None
278
+ }
279
+
280
+ fn match_borrows_parameter ( _cx : & LateContext < ' _ , ' _ > , qpath : & QPath < ' _ > ) -> Option < Span > {
281
+ let last = last_path_segment ( qpath) ;
282
+ if_chain ! {
283
+ if let Some ( ref params) = last. args;
284
+ if !params. parenthesized;
285
+ if let Some ( ty) = params. args. iter( ) . find_map( |arg| match arg {
286
+ GenericArg :: Type ( ty) => Some ( ty) ,
287
+ _ => None ,
288
+ } ) ;
289
+ if let TyKind :: Rptr ( ..) = ty. kind;
290
+ then {
291
+ return Some ( ty. span) ;
292
+ }
293
+ }
294
+ None
237
295
}
238
296
239
297
impl Types {
@@ -257,6 +315,7 @@ impl Types {
257
315
/// The parameter `is_local` distinguishes the context of the type; types from
258
316
/// local bindings should only be checked for the `BORROWED_BOX` lint.
259
317
#[ allow( clippy:: too_many_lines) ]
318
+ #[ allow( clippy:: cognitive_complexity) ]
260
319
fn check_ty ( & mut self , cx : & LateContext < ' _ , ' _ > , hir_ty : & hir:: Ty < ' _ > , is_local : bool ) {
261
320
if hir_ty. span . from_expansion ( ) {
262
321
return ;
@@ -267,7 +326,19 @@ impl Types {
267
326
let res = qpath_res ( cx, qpath, hir_id) ;
268
327
if let Some ( def_id) = res. opt_def_id ( ) {
269
328
if Some ( def_id) == cx. tcx . lang_items ( ) . owned_box ( ) {
270
- if match_type_parameter ( cx, qpath, & paths:: VEC ) {
329
+ if let Some ( span) = match_borrows_parameter ( cx, qpath) {
330
+ span_lint_and_sugg (
331
+ cx,
332
+ REDUNDANT_ALLOCATION ,
333
+ hir_ty. span ,
334
+ "usage of `Box<&T>`" ,
335
+ "try" ,
336
+ snippet ( cx, span, ".." ) . to_string ( ) ,
337
+ Applicability :: MachineApplicable ,
338
+ ) ;
339
+ return ; // don't recurse into the type
340
+ }
341
+ if match_type_parameter ( cx, qpath, & paths:: VEC ) . is_some ( ) {
271
342
span_lint_and_help (
272
343
cx,
273
344
BOX_VEC ,
@@ -277,6 +348,43 @@ impl Types {
277
348
) ;
278
349
return ; // don't recurse into the type
279
350
}
351
+ } else if Some ( def_id) == cx. tcx . lang_items ( ) . rc ( ) {
352
+ if let Some ( span) = match_type_parameter ( cx, qpath, & paths:: RC ) {
353
+ span_lint_and_sugg (
354
+ cx,
355
+ REDUNDANT_ALLOCATION ,
356
+ hir_ty. span ,
357
+ "usage of `Rc<Rc<T>>`" ,
358
+ "try" ,
359
+ snippet ( cx, span, ".." ) . to_string ( ) ,
360
+ Applicability :: MachineApplicable ,
361
+ ) ;
362
+ return ; // don't recurse into the type
363
+ }
364
+ if let Some ( span) = match_type_parameter ( cx, qpath, & paths:: BOX ) {
365
+ span_lint_and_sugg (
366
+ cx,
367
+ REDUNDANT_ALLOCATION ,
368
+ hir_ty. span ,
369
+ "usage of `Rc<Box<T>>`" ,
370
+ "try" ,
371
+ snippet ( cx, span, ".." ) . to_string ( ) ,
372
+ Applicability :: MachineApplicable ,
373
+ ) ;
374
+ return ; // don't recurse into the type
375
+ }
376
+ if let Some ( span) = match_borrows_parameter ( cx, qpath) {
377
+ span_lint_and_sugg (
378
+ cx,
379
+ REDUNDANT_ALLOCATION ,
380
+ hir_ty. span ,
381
+ "usage of `Rc<&T>`" ,
382
+ "try" ,
383
+ snippet ( cx, span, ".." ) . to_string ( ) ,
384
+ Applicability :: MachineApplicable ,
385
+ ) ;
386
+ return ; // don't recurse into the type
387
+ }
280
388
} else if cx. tcx . is_diagnostic_item ( Symbol :: intern ( "vec_type" ) , def_id) {
281
389
if_chain ! {
282
390
// Get the _ part of Vec<_>
@@ -314,7 +422,7 @@ impl Types {
314
422
}
315
423
}
316
424
} else if match_def_path ( cx, def_id, & paths:: OPTION ) {
317
- if match_type_parameter ( cx, qpath, & paths:: OPTION ) {
425
+ if match_type_parameter ( cx, qpath, & paths:: OPTION ) . is_some ( ) {
318
426
span_lint (
319
427
cx,
320
428
OPTION_OPTION ,
0 commit comments