Skip to content

Commit 92fe3a9

Browse files
committed
don't change parameters referring to generics used elsewhere
1 parent dafdad7 commit 92fe3a9

File tree

2 files changed

+24
-7
lines changed

2 files changed

+24
-7
lines changed

clippy_lints/src/needless_path_new.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,31 @@ fn check_arguments<'tcx>(
8888

8989
let implements_asref_path = |arg| implements_trait(cx, arg, asref_def_id, &[path_ty.into()]);
9090

91-
if let ty::FnDef(def_id, ..) = type_definition.kind()
91+
if let ty::FnDef(def_id, generic_args) = type_definition.kind()
9292
// if there are any bound vars, just give up... we might be able to be smarter here
9393
&& let Some(fn_sig) = type_definition.fn_sig(tcx).no_bound_vars()
9494
{
9595
let parameters = fn_sig.inputs();
9696

9797
let bounds = tcx.param_env(def_id).caller_bounds();
9898
dbg!(bounds);
99+
let generic_args_we_can_change: Vec<_> = generic_args
100+
.iter()
101+
.filter_map(|g| g.as_type())
102+
// if a generic is used in multiple places, we should better not touch it,
103+
// since we'd need to suggest changing both parameters that using it at once,
104+
// which might not be possible
105+
.filter(|g| {
106+
let inputs_and_output = fn_sig.inputs().iter().copied().chain([fn_sig.output()]);
107+
inputs_and_output.filter(|i| i.contains(*g)).count() < 2
108+
})
109+
.collect();
110+
dbg!(&generic_args_we_can_change);
111+
112+
if generic_args_we_can_change.is_empty() {
113+
// can't change anything
114+
return;
115+
}
99116

100117
for (argument, parameter) in iter::zip(arguments, parameters) {
101118
// we want `argument` to be `Path::new(x)`, which has one arg, x

tests/ui/needless_path_new.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ fn takes_impl_path(_: impl AsRef<Path>) {}
1010
fn takes_path_and_impl_path(_: &Path, _: impl AsRef<Path>) {}
1111

1212
fn takes_two_impl_paths_with_the_same_generic<P: AsRef<Path>>(_: P, _: P) {}
13+
fn takes_two_impl_paths_with_different_generics<P: AsRef<Path>>(_: P, _: P) {}
1314

1415
struct Foo;
1516

@@ -37,8 +38,8 @@ fn main() {
3738
Path::new("bar"), //~ needless_path_new
3839
);
3940

40-
// we can and should change both at the same time
41-
takes_two_impl_paths_with_the_same_generic(
41+
// we can and should change both independently
42+
takes_two_impl_paths_with_different_generics(
4243
Path::new("foo"), //~ needless_path_new
4344
Path::new("bar"), //~ needless_path_new
4445
);
@@ -58,8 +59,7 @@ fn main() {
5859
Foo::takes_path_and_impl_path(Path::new("foo"), "bar");
5960
f.takes_self_and_path_and_impl_path(Path::new("foo"), "bar");
6061

61-
fn foo() -> Option<&'static Path> {
62-
// Some(...) is `ExprKind::Call`, but we don't consider it
63-
Some(Path::new("foo.txt"))
64-
}
62+
// we are conservative and don't suggest changing a parameter
63+
// if it contains a generic type used elsewhere in the function
64+
takes_two_impl_paths_with_the_same_generic(Path::new("foo"), Path::new("bar"));
6565
}

0 commit comments

Comments
 (0)