Skip to content

Commit 3cee98d

Browse files
borsbors
committed
Auto merge of #11019 - Centri3:ptr_arg, r=llogiq
[`ptr_arg`]: Don't lint when return type uses `Cow`'s lifetime Closes #9218 changelog: [`ptr_arg`]: Don't lint when return type uses `Cow`'s lifetime
2 parents ce0a48a + db4efe3 commit 3cee98d

File tree

3 files changed

+141
-57
lines changed

3 files changed

+141
-57
lines changed

‎clippy_lints/src/ptr.rs‎

Lines changed: 70 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use clippy_utils::source::snippet_opt;
55
use clippy_utils::ty::expr_sig;
66
use clippy_utils::visitors::contains_unsafe_block;
77
use clippy_utils::{get_expr_use_or_unification_node, is_lint_allowed, path_def_id, path_to_local, paths};
8+
use hir::LifetimeName;
89
use if_chain::if_chain;
910
use rustc_errors::{Applicability, MultiSpan};
1011
use rustc_hir::def_id::DefId;
@@ -15,6 +16,7 @@ use rustc_hir::{
1516
ImplItemKind, ItemKind, Lifetime, Mutability, Node, Param, PatKind, QPath, TraitFn, TraitItem, TraitItemKind,
1617
TyKind, Unsafety,
1718
};
19+
use rustc_hir_analysis::hir_ty_to_ty;
1820
use rustc_infer::infer::TyCtxtInferExt;
1921
use rustc_infer::traits::{Obligation, ObligationCause};
2022
use rustc_lint::{LateContext, LateLintPass};
@@ -166,6 +168,7 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
166168
cx,
167169
cx.tcx.fn_sig(item.owner_id).subst_identity().skip_binder().inputs(),
168170
sig.decl.inputs,
171+
&sig.decl.output,
169172
&[],
170173
)
171174
.filter(|arg| arg.mutability() == Mutability::Not)
@@ -218,7 +221,7 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
218221
check_mut_from_ref(cx, sig, Some(body));
219222
let decl = sig.decl;
220223
let sig = cx.tcx.fn_sig(item_id).subst_identity().skip_binder();
221-
let lint_args: Vec<_> = check_fn_args(cx, sig.inputs(), decl.inputs, body.params)
224+
let lint_args: Vec<_> = check_fn_args(cx, sig.inputs(), decl.inputs, &decl.output, body.params)
222225
.filter(|arg| !is_trait_item || arg.mutability() == Mutability::Not)
223226
.collect();
224227
let results = check_ptr_arg_usage(cx, body, &lint_args);
@@ -407,29 +410,27 @@ impl<'tcx> DerefTy<'tcx> {
407410
}
408411
}
409412

413+
#[expect(clippy::too_many_lines)]
410414
fn check_fn_args<'cx, 'tcx: 'cx>(
411415
cx: &'cx LateContext<'tcx>,
412416
tys: &'tcx [Ty<'tcx>],
413417
hir_tys: &'tcx [hir::Ty<'tcx>],
418+
ret_ty: &'tcx FnRetTy<'tcx>,
414419
params: &'tcx [Param<'tcx>],
415420
) -> impl Iterator<Item = PtrArg<'tcx>> + 'cx {
416421
tys.iter()
417422
.zip(hir_tys.iter())
418423
.enumerate()
419-
.filter_map(|(i, (ty, hir_ty))| {
420-
if_chain! {
421-
if let ty::Ref(_, ty, mutability) = *ty.kind();
422-
if let ty::Adt(adt, substs) = *ty.kind();
423-
424-
if let TyKind::Ref(lt, ref ty) = hir_ty.kind;
425-
if let TyKind::Path(QPath::Resolved(None, path)) = ty.ty.kind;
426-
424+
.filter_map(move |(i, (ty, hir_ty))| {
425+
if let ty::Ref(_, ty, mutability) = *ty.kind()
426+
&& let ty::Adt(adt, substs) = *ty.kind()
427+
&& let TyKind::Ref(lt, ref ty) = hir_ty.kind
428+
&& let TyKind::Path(QPath::Resolved(None, path)) = ty.ty.kind
427429
// Check that the name as typed matches the actual name of the type.
428430
// e.g. `fn foo(_: &Foo)` shouldn't trigger the lint when `Foo` is an alias for `Vec`
429-
if let [.., name] = path.segments;
430-
if cx.tcx.item_name(adt.did()) == name.ident.name;
431-
432-
then {
431+
&& let [.., name] = path.segments
432+
&& cx.tcx.item_name(adt.did()) == name.ident.name
433+
{
433434
let emission_id = params.get(i).map_or(hir_ty.hir_id, |param| param.hir_id);
434435
let (method_renames, deref_ty) = match cx.tcx.get_diagnostic_name(adt.did()) {
435436
Some(sym::Vec) => (
@@ -454,30 +455,65 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
454455
DerefTy::Path,
455456
),
456457
Some(sym::Cow) if mutability == Mutability::Not => {
457-
let ty_name = name.args
458+
if let Some((lifetime, ty)) = name.args
458459
.and_then(|args| {
459-
args.args.iter().find_map(|a| match a {
460-
GenericArg::Type(x) => Some(x),
461-
_ => None,
462-
})
460+
if let [GenericArg::Lifetime(lifetime), ty] = args.args {
461+
return Some((lifetime, ty));
462+
}
463+
None
463464
})
464-
.and_then(|arg| snippet_opt(cx, arg.span))
465-
.unwrap_or_else(|| substs.type_at(1).to_string());
466-
span_lint_hir_and_then(
467-
cx,
468-
PTR_ARG,
469-
emission_id,
470-
hir_ty.span,
471-
"using a reference to `Cow` is not recommended",
472-
|diag| {
473-
diag.span_suggestion(
474-
hir_ty.span,
475-
"change this to",
476-
format!("&{}{ty_name}", mutability.prefix_str()),
477-
Applicability::Unspecified,
478-
);
465+
{
466+
if !lifetime.is_anonymous()
467+
&& let FnRetTy::Return(ret_ty) = ret_ty
468+
&& let ret_ty = hir_ty_to_ty(cx.tcx, ret_ty)
469+
&& ret_ty
470+
.walk()
471+
.filter_map(|arg| {
472+
arg.as_region().and_then(|lifetime| {
473+
match lifetime.kind() {
474+
ty::ReEarlyBound(r) => Some(r.def_id),
475+
ty::ReLateBound(_, r) => r.kind.get_id(),
476+
ty::ReFree(r) => r.bound_region.get_id(),
477+
ty::ReStatic
478+
| ty::ReVar(_)
479+
| ty::RePlaceholder(_)
480+
| ty::ReErased
481+
| ty::ReError(_) => None,
482+
}
483+
})
484+
})
485+
.any(|def_id| {
486+
matches!(
487+
lifetime.res,
488+
LifetimeName::Param(param_def_id) if def_id
489+
.as_local()
490+
.is_some_and(|def_id| def_id == param_def_id),
491+
)
492+
})
493+
{
494+
// `&Cow<'a, T>` when the return type uses 'a is okay
495+
return None;
479496
}
480-
);
497+
498+
let ty_name =
499+
snippet_opt(cx, ty.span()).unwrap_or_else(|| substs.type_at(1).to_string());
500+
501+
span_lint_hir_and_then(
502+
cx,
503+
PTR_ARG,
504+
emission_id,
505+
hir_ty.span,
506+
"using a reference to `Cow` is not recommended",
507+
|diag| {
508+
diag.span_suggestion(
509+
hir_ty.span,
510+
"change this to",
511+
format!("&{}{ty_name}", mutability.prefix_str()),
512+
Applicability::Unspecified,
513+
);
514+
}
515+
);
516+
}
481517
return None;
482518
},
483519
_ => return None,
@@ -495,7 +531,6 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
495531
},
496532
deref_ty,
497533
});
498-
}
499534
}
500535
None
501536
})

‎tests/ui/ptr_arg.rs‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
#![feature(lint_reasons)]
2-
#![allow(unused, clippy::many_single_char_names, clippy::redundant_clone)]
2+
#![allow(
3+
unused,
4+
clippy::many_single_char_names,
5+
clippy::needless_lifetimes,
6+
clippy::redundant_clone
7+
)]
38
#![warn(clippy::ptr_arg)]
49

510
use std::borrow::Cow;
@@ -235,3 +240,29 @@ fn dyn_trait(a: &mut Vec<u32>, b: &mut String, c: &mut PathBuf) {
235240
takes_dyn(b);
236241
takes_dyn(c);
237242
}
243+
244+
mod issue_9218 {
245+
use std::borrow::Cow;
246+
247+
fn cow_non_elided_lifetime<'a>(input: &Cow<'a, str>) -> &'a str {
248+
todo!()
249+
}
250+
251+
// This one has an anonymous lifetime so it's not okay
252+
fn cow_elided_lifetime<'a>(input: &'a Cow<str>) -> &'a str {
253+
todo!()
254+
}
255+
256+
// These two's return types don't use use 'a so it's not okay
257+
fn cow_bad_ret_ty_1<'a>(input: &'a Cow<'a, str>) -> &'static str {
258+
todo!()
259+
}
260+
fn cow_bad_ret_ty_2<'a, 'b>(input: &'a Cow<'a, str>) -> &'b str {
261+
todo!()
262+
}
263+
264+
// Inferred to be `&'a str`, afaik.
265+
fn cow_good_ret_ty<'a>(input: &'a Cow<'a, str>) -> &str {
266+
todo!()
267+
}
268+
}

‎tests/ui/ptr_arg.stderr‎

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,49 @@
11
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
2-
--> $DIR/ptr_arg.rs:8:14
2+
--> $DIR/ptr_arg.rs:13:14
33
|
44
LL | fn do_vec(x: &Vec<i64>) {
55
| ^^^^^^^^^ help: change this to: `&[i64]`
66
|
77
= note: `-D clippy::ptr-arg` implied by `-D warnings`
88

99
error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
10-
--> $DIR/ptr_arg.rs:12:18
10+
--> $DIR/ptr_arg.rs:17:18
1111
|
1212
LL | fn do_vec_mut(x: &mut Vec<i64>) {
1313
| ^^^^^^^^^^^^^ help: change this to: `&mut [i64]`
1414

1515
error: writing `&String` instead of `&str` involves a new object where a slice will do
16-
--> $DIR/ptr_arg.rs:16:14
16+
--> $DIR/ptr_arg.rs:21:14
1717
|
1818
LL | fn do_str(x: &String) {
1919
| ^^^^^^^ help: change this to: `&str`
2020

2121
error: writing `&mut String` instead of `&mut str` involves a new object where a slice will do
22-
--> $DIR/ptr_arg.rs:20:18
22+
--> $DIR/ptr_arg.rs:25:18
2323
|
2424
LL | fn do_str_mut(x: &mut String) {
2525
| ^^^^^^^^^^^ help: change this to: `&mut str`
2626

2727
error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
28-
--> $DIR/ptr_arg.rs:24:15
28+
--> $DIR/ptr_arg.rs:29:15
2929
|
3030
LL | fn do_path(x: &PathBuf) {
3131
| ^^^^^^^^ help: change this to: `&Path`
3232

3333
error: writing `&mut PathBuf` instead of `&mut Path` involves a new object where a slice will do
34-
--> $DIR/ptr_arg.rs:28:19
34+
--> $DIR/ptr_arg.rs:33:19
3535
|
3636
LL | fn do_path_mut(x: &mut PathBuf) {
3737
| ^^^^^^^^^^^^ help: change this to: `&mut Path`
3838

3939
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
40-
--> $DIR/ptr_arg.rs:36:18
40+
--> $DIR/ptr_arg.rs:41:18
4141
|
4242
LL | fn do_vec(x: &Vec<i64>);
4343
| ^^^^^^^^^ help: change this to: `&[i64]`
4444

4545
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
46-
--> $DIR/ptr_arg.rs:49:14
46+
--> $DIR/ptr_arg.rs:54:14
4747
|
4848
LL | fn cloned(x: &Vec<u8>) -> Vec<u8> {
4949
| ^^^^^^^^
@@ -60,7 +60,7 @@ LL ~ x.to_owned()
6060
|
6161

6262
error: writing `&String` instead of `&str` involves a new object where a slice will do
63-
--> $DIR/ptr_arg.rs:58:18
63+
--> $DIR/ptr_arg.rs:63:18
6464
|
6565
LL | fn str_cloned(x: &String) -> String {
6666
| ^^^^^^^
@@ -76,7 +76,7 @@ LL ~ x.to_owned()
7676
|
7777

7878
error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
79-
--> $DIR/ptr_arg.rs:66:19
79+
--> $DIR/ptr_arg.rs:71:19
8080
|
8181
LL | fn path_cloned(x: &PathBuf) -> PathBuf {
8282
| ^^^^^^^^
@@ -92,7 +92,7 @@ LL ~ x.to_path_buf()
9292
|
9393

9494
error: writing `&String` instead of `&str` involves a new object where a slice will do
95-
--> $DIR/ptr_arg.rs:74:44
95+
--> $DIR/ptr_arg.rs:79:44
9696
|
9797
LL | fn false_positive_capacity(x: &Vec<u8>, y: &String) {
9898
| ^^^^^^^
@@ -106,19 +106,19 @@ LL ~ let c = y;
106106
|
107107

108108
error: using a reference to `Cow` is not recommended
109-
--> $DIR/ptr_arg.rs:88:25
109+
--> $DIR/ptr_arg.rs:93:25
110110
|
111111
LL | fn test_cow_with_ref(c: &Cow<[i32]>) {}
112112
| ^^^^^^^^^^^ help: change this to: `&[i32]`
113113

114114
error: writing `&String` instead of `&str` involves a new object where a slice will do
115-
--> $DIR/ptr_arg.rs:117:66
115+
--> $DIR/ptr_arg.rs:122:66
116116
|
117117
LL | fn some_allowed(#[allow(clippy::ptr_arg)] _v: &Vec<u32>, _s: &String) {}
118118
| ^^^^^^^ help: change this to: `&str`
119119

120120
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
121-
--> $DIR/ptr_arg.rs:146:21
121+
--> $DIR/ptr_arg.rs:151:21
122122
|
123123
LL | fn foo_vec(vec: &Vec<u8>) {
124124
| ^^^^^^^^
@@ -131,7 +131,7 @@ LL ~ let _ = vec.to_owned().clone();
131131
|
132132

133133
error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
134-
--> $DIR/ptr_arg.rs:151:23
134+
--> $DIR/ptr_arg.rs:156:23
135135
|
136136
LL | fn foo_path(path: &PathBuf) {
137137
| ^^^^^^^^
@@ -144,7 +144,7 @@ LL ~ let _ = path.to_path_buf().clone();
144144
|
145145

146146
error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
147-
--> $DIR/ptr_arg.rs:156:21
147+
--> $DIR/ptr_arg.rs:161:21
148148
|
149149
LL | fn foo_str(str: &PathBuf) {
150150
| ^^^^^^^^
@@ -157,28 +157,46 @@ LL ~ let _ = str.to_path_buf().clone();
157157
|
158158

159159
error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
160-
--> $DIR/ptr_arg.rs:162:29
160+
--> $DIR/ptr_arg.rs:167:29
161161
|
162162
LL | fn mut_vec_slice_methods(v: &mut Vec<u32>) {
163163
| ^^^^^^^^^^^^^ help: change this to: `&mut [u32]`
164164

165165
error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
166-
--> $DIR/ptr_arg.rs:224:17
166+
--> $DIR/ptr_arg.rs:229:17
167167
|
168168
LL | fn dyn_trait(a: &mut Vec<u32>, b: &mut String, c: &mut PathBuf) {
169169
| ^^^^^^^^^^^^^ help: change this to: `&mut [u32]`
170170

171171
error: writing `&mut String` instead of `&mut str` involves a new object where a slice will do
172-
--> $DIR/ptr_arg.rs:224:35
172+
--> $DIR/ptr_arg.rs:229:35
173173
|
174174
LL | fn dyn_trait(a: &mut Vec<u32>, b: &mut String, c: &mut PathBuf) {
175175
| ^^^^^^^^^^^ help: change this to: `&mut str`
176176

177177
error: writing `&mut PathBuf` instead of `&mut Path` involves a new object where a slice will do
178-
--> $DIR/ptr_arg.rs:224:51
178+
--> $DIR/ptr_arg.rs:229:51
179179
|
180180
LL | fn dyn_trait(a: &mut Vec<u32>, b: &mut String, c: &mut PathBuf) {
181181
| ^^^^^^^^^^^^ help: change this to: `&mut Path`
182182

183-
error: aborting due to 20 previous errors
183+
error: using a reference to `Cow` is not recommended
184+
--> $DIR/ptr_arg.rs:252:39
185+
|
186+
LL | fn cow_elided_lifetime<'a>(input: &'a Cow<str>) -> &'a str {
187+
| ^^^^^^^^^^^^ help: change this to: `&str`
188+
189+
error: using a reference to `Cow` is not recommended
190+
--> $DIR/ptr_arg.rs:257:36
191+
|
192+
LL | fn cow_bad_ret_ty_1<'a>(input: &'a Cow<'a, str>) -> &'static str {
193+
| ^^^^^^^^^^^^^^^^ help: change this to: `&str`
194+
195+
error: using a reference to `Cow` is not recommended
196+
--> $DIR/ptr_arg.rs:260:40
197+
|
198+
LL | fn cow_bad_ret_ty_2<'a, 'b>(input: &'a Cow<'a, str>) -> &'b str {
199+
| ^^^^^^^^^^^^^^^^ help: change this to: `&str`
200+
201+
error: aborting due to 23 previous errors
184202

0 commit comments

Comments
 (0)