mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-23 21:23:56 +00:00
Don't lint ptr_arg when return type uses Cow's lifetime
This commit is contained in:
Catherine
parent
ce0a48a9b6
commit
db4efe3f6d
3 changed files with 141 additions and 57 deletions
|
|
@ -5,6 +5,7 @@ use clippy_utils::source::snippet_opt;
|
|||
use clippy_utils::ty::expr_sig;
|
||||
use clippy_utils::visitors::contains_unsafe_block;
|
||||
use clippy_utils::{get_expr_use_or_unification_node, is_lint_allowed, path_def_id, path_to_local, paths};
|
||||
use hir::LifetimeName;
|
||||
use if_chain::if_chain;
|
||||
use rustc_errors::{Applicability, MultiSpan};
|
||||
use rustc_hir::def_id::DefId;
|
||||
|
|
@ -15,6 +16,7 @@ use rustc_hir::{
|
|||
ImplItemKind, ItemKind, Lifetime, Mutability, Node, Param, PatKind, QPath, TraitFn, TraitItem, TraitItemKind,
|
||||
TyKind, Unsafety,
|
||||
};
|
||||
use rustc_hir_analysis::hir_ty_to_ty;
|
||||
use rustc_infer::infer::TyCtxtInferExt;
|
||||
use rustc_infer::traits::{Obligation, ObligationCause};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
|
|
@ -166,6 +168,7 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
|
|||
cx,
|
||||
cx.tcx.fn_sig(item.owner_id).subst_identity().skip_binder().inputs(),
|
||||
sig.decl.inputs,
|
||||
&sig.decl.output,
|
||||
&[],
|
||||
)
|
||||
.filter(|arg| arg.mutability() == Mutability::Not)
|
||||
|
|
@ -218,7 +221,7 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
|
|||
check_mut_from_ref(cx, sig, Some(body));
|
||||
let decl = sig.decl;
|
||||
let sig = cx.tcx.fn_sig(item_id).subst_identity().skip_binder();
|
||||
let lint_args: Vec<_> = check_fn_args(cx, sig.inputs(), decl.inputs, body.params)
|
||||
let lint_args: Vec<_> = check_fn_args(cx, sig.inputs(), decl.inputs, &decl.output, body.params)
|
||||
.filter(|arg| !is_trait_item || arg.mutability() == Mutability::Not)
|
||||
.collect();
|
||||
let results = check_ptr_arg_usage(cx, body, &lint_args);
|
||||
|
|
@ -407,29 +410,27 @@ impl<'tcx> DerefTy<'tcx> {
|
|||
}
|
||||
}
|
||||
|
||||
#[expect(clippy::too_many_lines)]
|
||||
fn check_fn_args<'cx, 'tcx: 'cx>(
|
||||
cx: &'cx LateContext<'tcx>,
|
||||
tys: &'tcx [Ty<'tcx>],
|
||||
hir_tys: &'tcx [hir::Ty<'tcx>],
|
||||
ret_ty: &'tcx FnRetTy<'tcx>,
|
||||
params: &'tcx [Param<'tcx>],
|
||||
) -> impl Iterator<Item = PtrArg<'tcx>> + 'cx {
|
||||
tys.iter()
|
||||
.zip(hir_tys.iter())
|
||||
.enumerate()
|
||||
.filter_map(|(i, (ty, hir_ty))| {
|
||||
if_chain! {
|
||||
if let ty::Ref(_, ty, mutability) = *ty.kind();
|
||||
if let ty::Adt(adt, substs) = *ty.kind();
|
||||
|
||||
if let TyKind::Ref(lt, ref ty) = hir_ty.kind;
|
||||
if let TyKind::Path(QPath::Resolved(None, path)) = ty.ty.kind;
|
||||
|
||||
.filter_map(move |(i, (ty, hir_ty))| {
|
||||
if let ty::Ref(_, ty, mutability) = *ty.kind()
|
||||
&& let ty::Adt(adt, substs) = *ty.kind()
|
||||
&& let TyKind::Ref(lt, ref ty) = hir_ty.kind
|
||||
&& let TyKind::Path(QPath::Resolved(None, path)) = ty.ty.kind
|
||||
// Check that the name as typed matches the actual name of the type.
|
||||
// e.g. `fn foo(_: &Foo)` shouldn't trigger the lint when `Foo` is an alias for `Vec`
|
||||
if let [.., name] = path.segments;
|
||||
if cx.tcx.item_name(adt.did()) == name.ident.name;
|
||||
|
||||
then {
|
||||
&& let [.., name] = path.segments
|
||||
&& cx.tcx.item_name(adt.did()) == name.ident.name
|
||||
{
|
||||
let emission_id = params.get(i).map_or(hir_ty.hir_id, |param| param.hir_id);
|
||||
let (method_renames, deref_ty) = match cx.tcx.get_diagnostic_name(adt.did()) {
|
||||
Some(sym::Vec) => (
|
||||
|
|
@ -454,30 +455,65 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
|
|||
DerefTy::Path,
|
||||
),
|
||||
Some(sym::Cow) if mutability == Mutability::Not => {
|
||||
let ty_name = name.args
|
||||
if let Some((lifetime, ty)) = name.args
|
||||
.and_then(|args| {
|
||||
args.args.iter().find_map(|a| match a {
|
||||
GenericArg::Type(x) => Some(x),
|
||||
_ => None,
|
||||
})
|
||||
if let [GenericArg::Lifetime(lifetime), ty] = args.args {
|
||||
return Some((lifetime, ty));
|
||||
}
|
||||
None
|
||||
})
|
||||
.and_then(|arg| snippet_opt(cx, arg.span))
|
||||
.unwrap_or_else(|| substs.type_at(1).to_string());
|
||||
span_lint_hir_and_then(
|
||||
cx,
|
||||
PTR_ARG,
|
||||
emission_id,
|
||||
hir_ty.span,
|
||||
"using a reference to `Cow` is not recommended",
|
||||
|diag| {
|
||||
diag.span_suggestion(
|
||||
hir_ty.span,
|
||||
"change this to",
|
||||
format!("&{}{ty_name}", mutability.prefix_str()),
|
||||
Applicability::Unspecified,
|
||||
);
|
||||
{
|
||||
if !lifetime.is_anonymous()
|
||||
&& let FnRetTy::Return(ret_ty) = ret_ty
|
||||
&& let ret_ty = hir_ty_to_ty(cx.tcx, ret_ty)
|
||||
&& ret_ty
|
||||
.walk()
|
||||
.filter_map(|arg| {
|
||||
arg.as_region().and_then(|lifetime| {
|
||||
match lifetime.kind() {
|
||||
ty::ReEarlyBound(r) => Some(r.def_id),
|
||||
ty::ReLateBound(_, r) => r.kind.get_id(),
|
||||
ty::ReFree(r) => r.bound_region.get_id(),
|
||||
ty::ReStatic
|
||||
| ty::ReVar(_)
|
||||
| ty::RePlaceholder(_)
|
||||
| ty::ReErased
|
||||
| ty::ReError(_) => None,
|
||||
}
|
||||
})
|
||||
})
|
||||
.any(|def_id| {
|
||||
matches!(
|
||||
lifetime.res,
|
||||
LifetimeName::Param(param_def_id) if def_id
|
||||
.as_local()
|
||||
.is_some_and(|def_id| def_id == param_def_id),
|
||||
)
|
||||
})
|
||||
{
|
||||
// `&Cow<'a, T>` when the return type uses 'a is okay
|
||||
return None;
|
||||
}
|
||||
);
|
||||
|
||||
let ty_name =
|
||||
snippet_opt(cx, ty.span()).unwrap_or_else(|| substs.type_at(1).to_string());
|
||||
|
||||
span_lint_hir_and_then(
|
||||
cx,
|
||||
PTR_ARG,
|
||||
emission_id,
|
||||
hir_ty.span,
|
||||
"using a reference to `Cow` is not recommended",
|
||||
|diag| {
|
||||
diag.span_suggestion(
|
||||
hir_ty.span,
|
||||
"change this to",
|
||||
format!("&{}{ty_name}", mutability.prefix_str()),
|
||||
Applicability::Unspecified,
|
||||
);
|
||||
}
|
||||
);
|
||||
}
|
||||
return None;
|
||||
},
|
||||
_ => return None,
|
||||
|
|
@ -495,7 +531,6 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
|
|||
},
|
||||
deref_ty,
|
||||
});
|
||||
}
|
||||
}
|
||||
None
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,5 +1,10 @@
|
|||
#![feature(lint_reasons)]
|
||||
#![allow(unused, clippy::many_single_char_names, clippy::redundant_clone)]
|
||||
#![allow(
|
||||
unused,
|
||||
clippy::many_single_char_names,
|
||||
clippy::needless_lifetimes,
|
||||
clippy::redundant_clone
|
||||
)]
|
||||
#![warn(clippy::ptr_arg)]
|
||||
|
||||
use std::borrow::Cow;
|
||||
|
|
@ -235,3 +240,29 @@ fn dyn_trait(a: &mut Vec<u32>, b: &mut String, c: &mut PathBuf) {
|
|||
takes_dyn(b);
|
||||
takes_dyn(c);
|
||||
}
|
||||
|
||||
mod issue_9218 {
|
||||
use std::borrow::Cow;
|
||||
|
||||
fn cow_non_elided_lifetime<'a>(input: &Cow<'a, str>) -> &'a str {
|
||||
todo!()
|
||||
}
|
||||
|
||||
// This one has an anonymous lifetime so it's not okay
|
||||
fn cow_elided_lifetime<'a>(input: &'a Cow<str>) -> &'a str {
|
||||
todo!()
|
||||
}
|
||||
|
||||
// These two's return types don't use use 'a so it's not okay
|
||||
fn cow_bad_ret_ty_1<'a>(input: &'a Cow<'a, str>) -> &'static str {
|
||||
todo!()
|
||||
}
|
||||
fn cow_bad_ret_ty_2<'a, 'b>(input: &'a Cow<'a, str>) -> &'b str {
|
||||
todo!()
|
||||
}
|
||||
|
||||
// Inferred to be `&'a str`, afaik.
|
||||
fn cow_good_ret_ty<'a>(input: &'a Cow<'a, str>) -> &str {
|
||||
todo!()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:8:14
|
||||
--> $DIR/ptr_arg.rs:13:14
|
||||
|
|
||||
LL | fn do_vec(x: &Vec<i64>) {
|
||||
| ^^^^^^^^^ help: change this to: `&[i64]`
|
||||
|
|
@ -7,43 +7,43 @@ LL | fn do_vec(x: &Vec<i64>) {
|
|||
= note: `-D clippy::ptr-arg` implied by `-D warnings`
|
||||
|
||||
error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:12:18
|
||||
--> $DIR/ptr_arg.rs:17:18
|
||||
|
|
||||
LL | fn do_vec_mut(x: &mut Vec<i64>) {
|
||||
| ^^^^^^^^^^^^^ help: change this to: `&mut [i64]`
|
||||
|
||||
error: writing `&String` instead of `&str` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:16:14
|
||||
--> $DIR/ptr_arg.rs:21:14
|
||||
|
|
||||
LL | fn do_str(x: &String) {
|
||||
| ^^^^^^^ help: change this to: `&str`
|
||||
|
||||
error: writing `&mut String` instead of `&mut str` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:20:18
|
||||
--> $DIR/ptr_arg.rs:25:18
|
||||
|
|
||||
LL | fn do_str_mut(x: &mut String) {
|
||||
| ^^^^^^^^^^^ help: change this to: `&mut str`
|
||||
|
||||
error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:24:15
|
||||
--> $DIR/ptr_arg.rs:29:15
|
||||
|
|
||||
LL | fn do_path(x: &PathBuf) {
|
||||
| ^^^^^^^^ help: change this to: `&Path`
|
||||
|
||||
error: writing `&mut PathBuf` instead of `&mut Path` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:28:19
|
||||
--> $DIR/ptr_arg.rs:33:19
|
||||
|
|
||||
LL | fn do_path_mut(x: &mut PathBuf) {
|
||||
| ^^^^^^^^^^^^ help: change this to: `&mut Path`
|
||||
|
||||
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:36:18
|
||||
--> $DIR/ptr_arg.rs:41:18
|
||||
|
|
||||
LL | fn do_vec(x: &Vec<i64>);
|
||||
| ^^^^^^^^^ help: change this to: `&[i64]`
|
||||
|
||||
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:49:14
|
||||
--> $DIR/ptr_arg.rs:54:14
|
||||
|
|
||||
LL | fn cloned(x: &Vec<u8>) -> Vec<u8> {
|
||||
| ^^^^^^^^
|
||||
|
|
@ -60,7 +60,7 @@ LL ~ x.to_owned()
|
|||
|
|
||||
|
||||
error: writing `&String` instead of `&str` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:58:18
|
||||
--> $DIR/ptr_arg.rs:63:18
|
||||
|
|
||||
LL | fn str_cloned(x: &String) -> String {
|
||||
| ^^^^^^^
|
||||
|
|
@ -76,7 +76,7 @@ LL ~ x.to_owned()
|
|||
|
|
||||
|
||||
error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:66:19
|
||||
--> $DIR/ptr_arg.rs:71:19
|
||||
|
|
||||
LL | fn path_cloned(x: &PathBuf) -> PathBuf {
|
||||
| ^^^^^^^^
|
||||
|
|
@ -92,7 +92,7 @@ LL ~ x.to_path_buf()
|
|||
|
|
||||
|
||||
error: writing `&String` instead of `&str` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:74:44
|
||||
--> $DIR/ptr_arg.rs:79:44
|
||||
|
|
||||
LL | fn false_positive_capacity(x: &Vec<u8>, y: &String) {
|
||||
| ^^^^^^^
|
||||
|
|
@ -106,19 +106,19 @@ LL ~ let c = y;
|
|||
|
|
||||
|
||||
error: using a reference to `Cow` is not recommended
|
||||
--> $DIR/ptr_arg.rs:88:25
|
||||
--> $DIR/ptr_arg.rs:93:25
|
||||
|
|
||||
LL | fn test_cow_with_ref(c: &Cow<[i32]>) {}
|
||||
| ^^^^^^^^^^^ help: change this to: `&[i32]`
|
||||
|
||||
error: writing `&String` instead of `&str` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:117:66
|
||||
--> $DIR/ptr_arg.rs:122:66
|
||||
|
|
||||
LL | fn some_allowed(#[allow(clippy::ptr_arg)] _v: &Vec<u32>, _s: &String) {}
|
||||
| ^^^^^^^ help: change this to: `&str`
|
||||
|
||||
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:146:21
|
||||
--> $DIR/ptr_arg.rs:151:21
|
||||
|
|
||||
LL | fn foo_vec(vec: &Vec<u8>) {
|
||||
| ^^^^^^^^
|
||||
|
|
@ -131,7 +131,7 @@ LL ~ let _ = vec.to_owned().clone();
|
|||
|
|
||||
|
||||
error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:151:23
|
||||
--> $DIR/ptr_arg.rs:156:23
|
||||
|
|
||||
LL | fn foo_path(path: &PathBuf) {
|
||||
| ^^^^^^^^
|
||||
|
|
@ -144,7 +144,7 @@ LL ~ let _ = path.to_path_buf().clone();
|
|||
|
|
||||
|
||||
error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:156:21
|
||||
--> $DIR/ptr_arg.rs:161:21
|
||||
|
|
||||
LL | fn foo_str(str: &PathBuf) {
|
||||
| ^^^^^^^^
|
||||
|
|
@ -157,28 +157,46 @@ LL ~ let _ = str.to_path_buf().clone();
|
|||
|
|
||||
|
||||
error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:162:29
|
||||
--> $DIR/ptr_arg.rs:167:29
|
||||
|
|
||||
LL | fn mut_vec_slice_methods(v: &mut Vec<u32>) {
|
||||
| ^^^^^^^^^^^^^ help: change this to: `&mut [u32]`
|
||||
|
||||
error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:224:17
|
||||
--> $DIR/ptr_arg.rs:229:17
|
||||
|
|
||||
LL | fn dyn_trait(a: &mut Vec<u32>, b: &mut String, c: &mut PathBuf) {
|
||||
| ^^^^^^^^^^^^^ help: change this to: `&mut [u32]`
|
||||
|
||||
error: writing `&mut String` instead of `&mut str` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:224:35
|
||||
--> $DIR/ptr_arg.rs:229:35
|
||||
|
|
||||
LL | fn dyn_trait(a: &mut Vec<u32>, b: &mut String, c: &mut PathBuf) {
|
||||
| ^^^^^^^^^^^ help: change this to: `&mut str`
|
||||
|
||||
error: writing `&mut PathBuf` instead of `&mut Path` involves a new object where a slice will do
|
||||
--> $DIR/ptr_arg.rs:224:51
|
||||
--> $DIR/ptr_arg.rs:229:51
|
||||
|
|
||||
LL | fn dyn_trait(a: &mut Vec<u32>, b: &mut String, c: &mut PathBuf) {
|
||||
| ^^^^^^^^^^^^ help: change this to: `&mut Path`
|
||||
|
||||
error: aborting due to 20 previous errors
|
||||
error: using a reference to `Cow` is not recommended
|
||||
--> $DIR/ptr_arg.rs:252:39
|
||||
|
|
||||
LL | fn cow_elided_lifetime<'a>(input: &'a Cow<str>) -> &'a str {
|
||||
| ^^^^^^^^^^^^ help: change this to: `&str`
|
||||
|
||||
error: using a reference to `Cow` is not recommended
|
||||
--> $DIR/ptr_arg.rs:257:36
|
||||
|
|
||||
LL | fn cow_bad_ret_ty_1<'a>(input: &'a Cow<'a, str>) -> &'static str {
|
||||
| ^^^^^^^^^^^^^^^^ help: change this to: `&str`
|
||||
|
||||
error: using a reference to `Cow` is not recommended
|
||||
--> $DIR/ptr_arg.rs:260:40
|
||||
|
|
||||
LL | fn cow_bad_ret_ty_2<'a, 'b>(input: &'a Cow<'a, str>) -> &'b str {
|
||||
| ^^^^^^^^^^^^^^^^ help: change this to: `&str`
|
||||
|
||||
error: aborting due to 23 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue