Skip to content

Commit 048297b

Browse files
JarchoJarcho
committed
ptr_arg cleanup
1 parent 7ed86bf commit 048297b

File tree

2 files changed

+89
-85
lines changed

2 files changed

+89
-85
lines changed

‎clippy_lints/src/ptr.rs

Lines changed: 83 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -385,87 +385,91 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
385385
hir_tys: &'tcx [hir::Ty<'_>],
386386
params: &'tcx [Param<'_>],
387387
) -> impl Iterator<Item = PtrArg<'tcx>> + 'cx {
388-
tys.iter().enumerate().filter_map(|(i, ty)| {
389-
if_chain! {
390-
if let ty::Ref(_, ty, mutability) = *ty.kind();
391-
if let ty::Adt(adt, substs) = *ty.kind();
392-
393-
let hir_ty = &hir_tys[i];
394-
if let TyKind::Rptr(lt, ref ty) = hir_ty.kind;
395-
if let TyKind::Path(QPath::Resolved(None, path)) = ty.ty.kind;
396-
397-
if let [.., name] = path.segments;
398-
if cx.tcx.item_name(adt.did) == name.ident.name;
399-
400-
if !is_lint_allowed(cx, PTR_ARG, hir_ty.hir_id);
401-
if params.get(i).map_or(true, |p| !is_lint_allowed(cx, PTR_ARG, p.hir_id));
402-
403-
then {
404-
let (method_renames, deref_ty, deref_impl_id) = match cx.tcx.get_diagnostic_name(adt.did) {
405-
Some(sym::Vec) => (
406-
[("clone", ".to_owned()")].as_slice(),
407-
DerefTy::Slice(
408-
name.args
409-
.and_then(|args| args.args.first())
410-
.and_then(|arg| if let GenericArg::Type(ty) = arg {
411-
Some(ty.span)
412-
} else {
413-
None
414-
}),
415-
substs.type_at(0),
388+
tys.iter()
389+
.zip(hir_tys.iter())
390+
.enumerate()
391+
.filter_map(|(i, (ty, hir_ty))| {
392+
if_chain! {
393+
if let ty::Ref(_, ty, mutability) = *ty.kind();
394+
if let ty::Adt(adt, substs) = *ty.kind();
395+
396+
if let TyKind::Rptr(lt, ref ty) = hir_ty.kind;
397+
if let TyKind::Path(QPath::Resolved(None, path)) = ty.ty.kind;
398+
399+
// Check that the name as typed matches the actual name of the type.
400+
// e.g. `fn foo(_: &Foo)` shouldn't trigger the lint when `Foo` is an alias for `Vec`
401+
if let [.., name] = path.segments;
402+
if cx.tcx.item_name(adt.did) == name.ident.name;
403+
404+
if !is_lint_allowed(cx, PTR_ARG, hir_ty.hir_id);
405+
if params.get(i).map_or(true, |p| !is_lint_allowed(cx, PTR_ARG, p.hir_id));
406+
407+
then {
408+
let (method_renames, deref_ty, deref_impl_id) = match cx.tcx.get_diagnostic_name(adt.did) {
409+
Some(sym::Vec) => (
410+
[("clone", ".to_owned()")].as_slice(),
411+
DerefTy::Slice(
412+
name.args
413+
.and_then(|args| args.args.first())
414+
.and_then(|arg| if let GenericArg::Type(ty) = arg {
415+
Some(ty.span)
416+
} else {
417+
None
418+
}),
419+
substs.type_at(0),
420+
),
421+
cx.tcx.lang_items().slice_impl()
416422
),
417-
cx.tcx.lang_items().slice_impl()
418-
),
419-
Some(sym::String) => (
420-
[("clone", ".to_owned()"), ("as_str", "")].as_slice(),
421-
DerefTy::Str,
422-
cx.tcx.lang_items().str_impl()
423-
),
424-
Some(sym::PathBuf) => (
425-
[("clone", ".to_path_buf()"), ("as_path", "")].as_slice(),
426-
DerefTy::Path,
427-
None,
428-
),
429-
Some(sym::Cow) => {
430-
let ty_name = name.args
431-
.and_then(|args| {
432-
args.args.iter().find_map(|a| match a {
433-
GenericArg::Type(x) => Some(x),
434-
_ => None,
423+
Some(sym::String) => (
424+
[("clone", ".to_owned()"), ("as_str", "")].as_slice(),
425+
DerefTy::Str,
426+
cx.tcx.lang_items().str_impl()
427+
),
428+
Some(sym::PathBuf) => (
429+
[("clone", ".to_path_buf()"), ("as_path", "")].as_slice(),
430+
DerefTy::Path,
431+
None,
432+
),
433+
Some(sym::Cow) => {
434+
let ty_name = name.args
435+
.and_then(|args| {
436+
args.args.iter().find_map(|a| match a {
437+
GenericArg::Type(x) => Some(x),
438+
_ => None,
439+
})
435440
})
436-
})
437-
.and_then(|arg| snippet_opt(cx, arg.span))
438-
.unwrap_or_else(|| substs.type_at(1).to_string());
439-
span_lint_and_sugg(
440-
cx,
441-
PTR_ARG,
442-
hir_ty.span,
443-
"using a reference to `Cow` is not recommended",
444-
"change this to",
445-
format!("&{}{}", mutability.prefix_str(), ty_name),
446-
Applicability::Unspecified,
447-
);
448-
return None;
449-
},
450-
_ => return None,
451-
};
452-
return Some(PtrArg {
453-
idx: i,
454-
span: hir_ty.span,
455-
ty_did: adt.did,
456-
ty_name: name.ident.name,
457-
method_renames,
458-
ref_prefix: RefPrefix {
459-
lt: lt.name,
460-
mutability,
461-
},
462-
deref_ty,
463-
deref_assoc_items: deref_impl_id.map(|id| (id, cx.tcx.associated_items(id))),
464-
});
441+
.and_then(|arg| snippet_opt(cx, arg.span))
442+
.unwrap_or_else(|| substs.type_at(1).to_string());
443+
span_lint_and_sugg(
444+
cx,
445+
PTR_ARG,
446+
hir_ty.span,
447+
"using a reference to `Cow` is not recommended",
448+
"change this to",
449+
format!("&{}{}", mutability.prefix_str(), ty_name),
450+
Applicability::Unspecified,
451+
);
452+
return None;
453+
},
454+
_ => return None,
455+
};
456+
return Some(PtrArg {
457+
idx: i,
458+
span: hir_ty.span,
459+
ty_did: adt.did,
460+
ty_name: name.ident.name,
461+
method_renames,
462+
ref_prefix: RefPrefix {
463+
lt: lt.name,
464+
mutability,
465+
},
466+
deref_ty,
467+
deref_assoc_items: deref_impl_id.map(|id| (id, cx.tcx.associated_items(id))),
468+
});
469+
}
465470
}
466-
}
467-
None
468-
})
471+
None
472+
})
469473
}
470474

471475
fn check_mut_from_ref(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
@@ -504,7 +508,7 @@ fn check_mut_from_ref(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
504508
fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &'tcx Body<'_>, args: &[PtrArg<'tcx>]) -> Vec<PtrArgResult> {
505509
struct V<'cx, 'tcx> {
506510
cx: &'cx LateContext<'tcx>,
507-
/// Map from a local id to which argument it cam from (index into `Self::args` and
511+
/// Map from a local id to which argument it came from (index into `Self::args` and
508512
/// `Self::results`)
509513
bindings: HirIdMap<usize>,
510514
/// The arguments being checked.

‎tests/ui/ptr_arg.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,13 @@ fn fn_requires_vec(v: &Vec<u32>) -> bool {
169169
vec_contains(v)
170170
}
171171

172-
// fn impl_fn_requires_vec(v: &Vec<u32>, f: impl Fn(&Vec<u32>)) {
173-
// f(v);
174-
// }
172+
fn impl_fn_requires_vec(v: &Vec<u32>, f: impl Fn(&Vec<u32>)) {
173+
f(v);
174+
}
175175

176-
// fn dyn_fn_requires_vec(v: &Vec<u32>, f: &dyn Fn(&Vec<u32>)) {
177-
// f(v);
178-
// }
176+
fn dyn_fn_requires_vec(v: &Vec<u32>, f: &dyn Fn(&Vec<u32>)) {
177+
f(v);
178+
}
179179

180180
// No error for types behind an alias (#7699)
181181
type A = Vec<u8>;

0 commit comments

Comments
 (0)