Skip to content

Commit 1499021

Browse files
committed
Enhance needless_borrow to consider trait implementations
1 parent 7c8e1bf commit 1499021

File tree

8 files changed

+582
-74
lines changed

8 files changed

+582
-74
lines changed

clippy_lints/src/dereference.rs

Lines changed: 270 additions & 25 deletions
Large diffs are not rendered by default.

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
807807
store.register_late_pass(|| Box::new(verbose_file_reads::VerboseFileReads));
808808
store.register_late_pass(|| Box::new(redundant_pub_crate::RedundantPubCrate::default()));
809809
store.register_late_pass(|| Box::new(unnamed_address::UnnamedAddress));
810-
store.register_late_pass(|| Box::new(dereference::Dereferencing::default()));
810+
store.register_late_pass(move || Box::new(dereference::Dereferencing::new(msrv)));
811811
store.register_late_pass(|| Box::new(option_if_let_else::OptionIfLetElse));
812812
store.register_late_pass(|| Box::new(future_not_send::FutureNotSend));
813813
store.register_late_pass(|| Box::new(if_let_mutex::IfLetMutex));

clippy_lints/src/methods/unnecessary_to_owned.rs

Lines changed: 4 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,18 @@ use super::unnecessary_iter_cloned::{self, is_into_iter};
33
use clippy_utils::diagnostics::span_lint_and_sugg;
44
use clippy_utils::source::snippet_opt;
55
use clippy_utils::ty::{
6-
contains_ty, get_associated_type, get_iterator_item_ty, implements_trait, is_copy, peel_mid_ty_refs,
6+
contains_ty, get_associated_type, get_input_traits_and_projections, get_iterator_item_ty, implements_trait,
7+
is_copy, peel_mid_ty_refs,
78
};
8-
use clippy_utils::{meets_msrv, msrvs};
9-
109
use clippy_utils::{fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item};
10+
use clippy_utils::{meets_msrv, msrvs};
1111
use rustc_errors::Applicability;
1212
use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind};
1313
use rustc_lint::LateContext;
1414
use rustc_middle::mir::Mutability;
15+
use rustc_middle::ty;
1516
use rustc_middle::ty::adjustment::{Adjust, Adjustment, OverloadedDeref};
1617
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, SubstsRef};
17-
use rustc_middle::ty::{self, PredicateKind, ProjectionPredicate, TraitPredicate, Ty};
1818
use rustc_semver::RustcVersion;
1919
use rustc_span::{sym, Symbol};
2020
use std::cmp::max;
@@ -352,42 +352,6 @@ fn get_callee_substs_and_args<'tcx>(
352352
None
353353
}
354354

355-
/// Returns the `TraitPredicate`s and `ProjectionPredicate`s for a function's input type.
356-
fn get_input_traits_and_projections<'tcx>(
357-
cx: &LateContext<'tcx>,
358-
callee_def_id: DefId,
359-
input: Ty<'tcx>,
360-
) -> (Vec<TraitPredicate<'tcx>>, Vec<ProjectionPredicate<'tcx>>) {
361-
let mut trait_predicates = Vec::new();
362-
let mut projection_predicates = Vec::new();
363-
for (predicate, _) in cx.tcx.predicates_of(callee_def_id).predicates.iter() {
364-
// `substs` should have 1 + n elements. The first is the type on the left hand side of an
365-
// `as`. The remaining n are trait parameters.
366-
let is_input_substs = |substs: SubstsRef<'tcx>| {
367-
if_chain! {
368-
if let Some(arg) = substs.iter().next();
369-
if let GenericArgKind::Type(arg_ty) = arg.unpack();
370-
if arg_ty == input;
371-
then { true } else { false }
372-
}
373-
};
374-
match predicate.kind().skip_binder() {
375-
PredicateKind::Trait(trait_predicate) => {
376-
if is_input_substs(trait_predicate.trait_ref.substs) {
377-
trait_predicates.push(trait_predicate);
378-
}
379-
},
380-
PredicateKind::Projection(projection_predicate) => {
381-
if is_input_substs(projection_predicate.projection_ty.substs) {
382-
projection_predicates.push(projection_predicate);
383-
}
384-
},
385-
_ => {},
386-
}
387-
}
388-
(trait_predicates, projection_predicates)
389-
}
390-
391355
/// Composes two substitutions by applying the latter to the types of the former.
392356
fn compose_substs<'tcx>(
393357
cx: &LateContext<'tcx>,

clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ macro_rules! msrv_aliases {
1313
// names may refer to stabilized feature flags or library items
1414
msrv_aliases! {
1515
1,62,0 { BOOL_THEN_SOME }
16-
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN }
16+
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR }
1717
1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
1818
1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS }
1919
1,50,0 { BOOL_THEN }

clippy_utils/src/ty.rs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ use rustc_lint::LateContext;
1414
use rustc_middle::mir::interpret::{ConstValue, Scalar};
1515
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst};
1616
use rustc_middle::ty::{
17-
self, AdtDef, Binder, BoundRegion, DefIdTree, FnSig, IntTy, ParamEnv, Predicate, PredicateKind, ProjectionTy,
18-
Region, RegionKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor, UintTy, VariantDef, VariantDiscr,
17+
self, AdtDef, Binder, BoundRegion, DefIdTree, FnSig, IntTy, ParamEnv, Predicate, PredicateKind,
18+
ProjectionPredicate, ProjectionTy, Region, RegionKind, TraitPredicate, Ty, TyCtxt, TypeSuperVisitable,
19+
TypeVisitable, TypeVisitor, UintTy, VariantDef, VariantDiscr,
1920
};
2021
use rustc_span::symbol::Ident;
2122
use rustc_span::{sym, Span, Symbol, DUMMY_SP};
@@ -85,6 +86,32 @@ pub fn get_associated_type<'tcx>(
8586
})
8687
}
8788

89+
/// Returns the `TraitPredicate`s and `ProjectionPredicate`s for a function's input type.
90+
pub fn get_input_traits_and_projections<'tcx>(
91+
cx: &LateContext<'tcx>,
92+
callee_def_id: DefId,
93+
input: Ty<'tcx>,
94+
) -> (Vec<TraitPredicate<'tcx>>, Vec<ProjectionPredicate<'tcx>>) {
95+
let mut trait_predicates = Vec::new();
96+
let mut projection_predicates = Vec::new();
97+
for predicate in cx.tcx.param_env(callee_def_id).caller_bounds() {
98+
match predicate.kind().skip_binder() {
99+
PredicateKind::Trait(trait_predicate) => {
100+
if trait_predicate.trait_ref.self_ty() == input {
101+
trait_predicates.push(trait_predicate);
102+
}
103+
},
104+
PredicateKind::Projection(projection_predicate) => {
105+
if projection_predicate.projection_ty.self_ty() == input {
106+
projection_predicates.push(projection_predicate);
107+
}
108+
},
109+
_ => {},
110+
}
111+
}
112+
(trait_predicates, projection_predicates)
113+
}
114+
88115
/// Get the diagnostic name of a type, e.g. `sym::HashMap`. To check if a type
89116
/// implements a trait marked with a diagnostic item use [`implements_trait`].
90117
///

tests/ui/needless_borrow.fixed

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// run-rustfix
22

3-
#![feature(lint_reasons)]
3+
#![feature(custom_inner_attributes, lint_reasons)]
44

55
#[warn(clippy::all, clippy::needless_borrow)]
66
#[allow(unused_variables, clippy::unnecessary_mut_passed)]
@@ -127,6 +127,20 @@ fn main() {
127127
0
128128
}
129129
}
130+
131+
let _ = std::process::Command::new("ls").args(["-a", "-l"]).status().unwrap();
132+
let _ = std::path::Path::new(".").join(".");
133+
deref_target_is_x(X);
134+
multiple_constraints([[""]]);
135+
multiple_constraints_normalizes_to_same(X, X);
136+
let _ = Some("").unwrap_or("");
137+
138+
only_sized(&""); // Don't lint. `Sized` is only bound
139+
let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
140+
let _ = Box::new(&""); // Don't lint. Type parameter appears in return type
141+
ref_as_ref_path(&""); // Don't lint. Argument type is not a type parameter
142+
refs_only(&()); // Don't lint. `&T` implements trait, but `T` doesn't
143+
multiple_constraints_normalizes_to_different(&[[""]], &[""]); // Don't lint. Projected type appears in arguments
130144
}
131145

132146
#[allow(clippy::needless_borrowed_reference)]
@@ -183,3 +197,104 @@ mod issue9160 {
183197
}
184198
}
185199
}
200+
201+
#[derive(Clone, Copy)]
202+
struct X;
203+
204+
impl std::ops::Deref for X {
205+
type Target = X;
206+
fn deref(&self) -> &Self::Target {
207+
self
208+
}
209+
}
210+
211+
fn deref_target_is_x<T>(_: T)
212+
where
213+
T: std::ops::Deref<Target = X>,
214+
{
215+
}
216+
217+
fn multiple_constraints<T, U, V, X, Y>(_: T)
218+
where
219+
T: IntoIterator<Item = U> + IntoIterator<Item = X>,
220+
U: IntoIterator<Item = V>,
221+
V: AsRef<str>,
222+
X: IntoIterator<Item = Y>,
223+
Y: AsRef<std::ffi::OsStr>,
224+
{
225+
}
226+
227+
fn multiple_constraints_normalizes_to_same<T, U, V>(_: T, _: V)
228+
where
229+
T: std::ops::Deref<Target = U>,
230+
U: std::ops::Deref<Target = V>,
231+
{
232+
}
233+
234+
fn only_sized<T>(_: T) {}
235+
236+
fn ref_as_ref_path<T: 'static>(_: &'static T)
237+
where
238+
&'static T: AsRef<std::path::Path>,
239+
{
240+
}
241+
242+
trait RefsOnly {
243+
type Referent;
244+
}
245+
246+
impl<T> RefsOnly for &T {
247+
type Referent = T;
248+
}
249+
250+
fn refs_only<T, U>(_: T)
251+
where
252+
T: RefsOnly<Referent = U>,
253+
{
254+
}
255+
256+
fn multiple_constraints_normalizes_to_different<T, U, V>(_: T, _: U)
257+
where
258+
T: IntoIterator<Item = U>,
259+
U: IntoIterator<Item = V>,
260+
V: AsRef<str>,
261+
{
262+
}
263+
264+
// https://github.com/rust-lang/rust-clippy/pull/9136#pullrequestreview-1037379321
265+
#[allow(dead_code)]
266+
mod copyable_iterator {
267+
#[derive(Clone, Copy)]
268+
struct Iter;
269+
impl Iterator for Iter {
270+
type Item = ();
271+
fn next(&mut self) -> Option<Self::Item> {
272+
None
273+
}
274+
}
275+
fn takes_iter(_: impl Iterator) {}
276+
fn dont_warn(mut x: Iter) {
277+
takes_iter(&mut x);
278+
}
279+
fn warn(mut x: &mut Iter) {
280+
takes_iter(&mut x)
281+
}
282+
}
283+
284+
mod under_msrv {
285+
#![allow(dead_code)]
286+
#![clippy::msrv = "1.52.0"]
287+
288+
fn foo() {
289+
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
290+
}
291+
}
292+
293+
mod meets_msrv {
294+
#![allow(dead_code)]
295+
#![clippy::msrv = "1.53.0"]
296+
297+
fn foo() {
298+
let _ = std::process::Command::new("ls").args(["-a", "-l"]).status().unwrap();
299+
}
300+
}

tests/ui/needless_borrow.rs

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// run-rustfix
22

3-
#![feature(lint_reasons)]
3+
#![feature(custom_inner_attributes, lint_reasons)]
44

55
#[warn(clippy::all, clippy::needless_borrow)]
66
#[allow(unused_variables, clippy::unnecessary_mut_passed)]
@@ -127,6 +127,20 @@ fn main() {
127127
0
128128
}
129129
}
130+
131+
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
132+
let _ = std::path::Path::new(".").join(&&".");
133+
deref_target_is_x(&X);
134+
multiple_constraints(&[[""]]);
135+
multiple_constraints_normalizes_to_same(&X, X);
136+
let _ = Some("").unwrap_or(&"");
137+
138+
only_sized(&""); // Don't lint. `Sized` is only bound
139+
let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
140+
let _ = Box::new(&""); // Don't lint. Type parameter appears in return type
141+
ref_as_ref_path(&""); // Don't lint. Argument type is not a type parameter
142+
refs_only(&()); // Don't lint. `&T` implements trait, but `T` doesn't
143+
multiple_constraints_normalizes_to_different(&[[""]], &[""]); // Don't lint. Projected type appears in arguments
130144
}
131145

132146
#[allow(clippy::needless_borrowed_reference)]
@@ -183,3 +197,104 @@ mod issue9160 {
183197
}
184198
}
185199
}
200+
201+
#[derive(Clone, Copy)]
202+
struct X;
203+
204+
impl std::ops::Deref for X {
205+
type Target = X;
206+
fn deref(&self) -> &Self::Target {
207+
self
208+
}
209+
}
210+
211+
fn deref_target_is_x<T>(_: T)
212+
where
213+
T: std::ops::Deref<Target = X>,
214+
{
215+
}
216+
217+
fn multiple_constraints<T, U, V, X, Y>(_: T)
218+
where
219+
T: IntoIterator<Item = U> + IntoIterator<Item = X>,
220+
U: IntoIterator<Item = V>,
221+
V: AsRef<str>,
222+
X: IntoIterator<Item = Y>,
223+
Y: AsRef<std::ffi::OsStr>,
224+
{
225+
}
226+
227+
fn multiple_constraints_normalizes_to_same<T, U, V>(_: T, _: V)
228+
where
229+
T: std::ops::Deref<Target = U>,
230+
U: std::ops::Deref<Target = V>,
231+
{
232+
}
233+
234+
fn only_sized<T>(_: T) {}
235+
236+
fn ref_as_ref_path<T: 'static>(_: &'static T)
237+
where
238+
&'static T: AsRef<std::path::Path>,
239+
{
240+
}
241+
242+
trait RefsOnly {
243+
type Referent;
244+
}
245+
246+
impl<T> RefsOnly for &T {
247+
type Referent = T;
248+
}
249+
250+
fn refs_only<T, U>(_: T)
251+
where
252+
T: RefsOnly<Referent = U>,
253+
{
254+
}
255+
256+
fn multiple_constraints_normalizes_to_different<T, U, V>(_: T, _: U)
257+
where
258+
T: IntoIterator<Item = U>,
259+
U: IntoIterator<Item = V>,
260+
V: AsRef<str>,
261+
{
262+
}
263+
264+
// https://github.com/rust-lang/rust-clippy/pull/9136#pullrequestreview-1037379321
265+
#[allow(dead_code)]
266+
mod copyable_iterator {
267+
#[derive(Clone, Copy)]
268+
struct Iter;
269+
impl Iterator for Iter {
270+
type Item = ();
271+
fn next(&mut self) -> Option<Self::Item> {
272+
None
273+
}
274+
}
275+
fn takes_iter(_: impl Iterator) {}
276+
fn dont_warn(mut x: Iter) {
277+
takes_iter(&mut x);
278+
}
279+
fn warn(mut x: &mut Iter) {
280+
takes_iter(&mut x)
281+
}
282+
}
283+
284+
mod under_msrv {
285+
#![allow(dead_code)]
286+
#![clippy::msrv = "1.52.0"]
287+
288+
fn foo() {
289+
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
290+
}
291+
}
292+
293+
mod meets_msrv {
294+
#![allow(dead_code)]
295+
#![clippy::msrv = "1.53.0"]
296+
297+
fn foo() {
298+
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
299+
}
300+
}

0 commit comments

Comments
 (0)