Skip to content

Commit f66a723

Browse files
ada4aada4a
committed
new lint: needless_path_new 
1 parent 8a5dc7c commit f66a723

File tree

7 files changed

+308
-0
lines changed

7 files changed

+308
-0
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6434,6 +6434,7 @@ Released 2018-09-13
64346434
[`needless_parens_on_range_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_parens_on_range_literals
64356435
[`needless_pass_by_ref_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
64366436
[`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
6437+
[`needless_path_new`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_path_new
64376438
[`needless_pub_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pub_self
64386439
[`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
64396440
[`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
553553
crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,
554554
crate::needless_pass_by_ref_mut::NEEDLESS_PASS_BY_REF_MUT_INFO,
555555
crate::needless_pass_by_value::NEEDLESS_PASS_BY_VALUE_INFO,
556+
crate::needless_path_new::NEEDLESS_PATH_NEW_INFO,
556557
crate::needless_question_mark::NEEDLESS_QUESTION_MARK_INFO,
557558
crate::needless_update::NEEDLESS_UPDATE_INFO,
558559
crate::neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD_INFO,

‎clippy_lints/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ mod needless_maybe_sized;
269269
mod needless_parens_on_range_literals;
270270
mod needless_pass_by_ref_mut;
271271
mod needless_pass_by_value;
272+
mod needless_path_new;
272273
mod needless_question_mark;
273274
mod needless_update;
274275
mod neg_cmp_op_on_partial_ord;
@@ -831,5 +832,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
831832
store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
832833
store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
833834
store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
835+
store.register_late_pass(|_| Box::new(needless_path_new::NeedlessPathNew));
834836
// add lints here, do not remove this comment, it's used in `new_lint`
835837
}

‎clippy_lints/src/needless_path_new.rs‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::sugg::Sugg;
3+
use clippy_utils::{expr_or_init, is_path_diagnostic_item, path_res};
4+
use rustc_errors::Applicability;
5+
use rustc_hir::def::{CtorKind, DefKind, Res};
6+
use rustc_hir::{Expr, ExprKind, QPath};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_middle::ty::{self, GenericPredicates, ParamTy, Ty};
9+
use rustc_session::declare_lint_pass;
10+
use rustc_span::sym;
11+
use std::iter;
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Detects expressions being enclosed in `Path::new` when passed to a function which would
16+
/// accept the enclosed expression directly.
17+
///
18+
/// ### Why is this bad?
19+
/// It is unnecessarily verbose
20+
///
21+
/// ### Example
22+
/// ```no_run
23+
/// # use std::{fs, path::Path};
24+
/// fs::write(Path::new("foo.txt"), "foo");
25+
/// ```
26+
/// Use instead:
27+
/// ```no_run
28+
/// # use std::{fs, path::Path};
29+
/// fs::write("foo.txt", "foo");
30+
/// ```
31+
#[clippy::version = "1.92.0"]
32+
pub NEEDLESS_PATH_NEW,
33+
nursery,
34+
"`Path::new(x)` passed as an argument where `x` would suffice"
35+
}
36+
37+
declare_lint_pass!(NeedlessPathNew => [NEEDLESS_PATH_NEW]);
38+
39+
impl<'tcx> LateLintPass<'tcx> for NeedlessPathNew {
40+
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) {
41+
let tcx = cx.tcx;
42+
43+
let (fn_did, args) = match e.kind {
44+
ExprKind::Call(callee, args)
45+
if let Res::Def(DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(_, CtorKind::Fn), did) =
46+
// re: `expr_or_init`: `callee` might be a variable storing a fn ptr, for example,
47+
// so we need to get to the actual initializer
48+
path_res(cx, expr_or_init(cx, callee)) =>
49+
{
50+
(did, args)
51+
},
52+
ExprKind::MethodCall(_, _, args, _)
53+
if let Some(did) = cx.typeck_results().type_dependent_def_id(e.hir_id) =>
54+
{
55+
(did, args)
56+
},
57+
_ => return,
58+
};
59+
60+
let sig = tcx.fn_sig(fn_did).skip_binder().skip_binder();
61+
62+
let has_required_preds = |_param_ty: &ParamTy, _preds: GenericPredicates<'_>| -> bool {
63+
// TODO
64+
true
65+
};
66+
67+
// `ExprKind::MethodCall` doesn't include the receiver in `args`, but does in `sig.inputs()`
68+
// -- so we iterate over both in `rev`erse in order to line them up starting from the _end_
69+
//
70+
// and for `ExprKind::Call` this is basically a no-op
71+
iter::zip(sig.inputs().iter().rev(), args.iter().rev())
72+
.enumerate()
73+
.for_each(|(arg_idx, (arg_ty, arg))| {
74+
// we want `arg` to be `Path::new(x)`
75+
if let ExprKind::Call(path_new, [x]) = arg.kind
76+
&& let ExprKind::Path(QPath::TypeRelative(path, new)) = path_new.kind
77+
&& is_path_diagnostic_item(cx, path, sym::Path)
78+
&& new.ident.name == sym::new
79+
&& let ty::Param(arg_param_ty) = arg_ty.kind()
80+
&& !is_used_anywhere_else(
81+
*arg_param_ty,
82+
sig.inputs()
83+
.iter()
84+
// `arg_idx` is based on the reversed order, so we need to reverse as well for the
85+
// `enumerate` indices to work
86+
.rev()
87+
.enumerate()
88+
.filter_map(|(i, input)| (i != arg_idx).then_some(*input)),
89+
)
90+
&& has_required_preds(arg_param_ty, cx.tcx.predicates_of(fn_did))
91+
{
92+
let mut applicability = Applicability::MachineApplicable;
93+
let sugg = Sugg::hir_with_applicability(cx, x, "_", &mut applicability);
94+
span_lint_and_sugg(
95+
cx,
96+
NEEDLESS_PATH_NEW,
97+
arg.span,
98+
"the expression enclosed in `Path::new` can be passed directly",
99+
"try",
100+
sugg.to_string(),
101+
applicability,
102+
);
103+
}
104+
});
105+
}
106+
}
107+
108+
fn is_used_anywhere_else<'tcx>(param_ty: ParamTy, mut other_sig_tys: impl Iterator<Item = Ty<'tcx>>) -> bool {
109+
other_sig_tys.any(|sig_ty| {
110+
sig_ty.walk().any(|generic_arg| {
111+
if let Some(ty) = generic_arg.as_type()
112+
&& let ty::Param(pt) = ty.kind()
113+
&& *pt == param_ty
114+
{
115+
true
116+
} else {
117+
false
118+
}
119+
})
120+
})
121+
}

‎tests/ui/needless_path_new.fixed‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
#![warn(clippy::needless_path_new)]
2+
3+
use std::fs;
4+
use std::path::Path;
5+
6+
fn takes_path(_: &Path) {}
7+
fn takes_impl_path(_: impl AsRef<Path>) {}
8+
fn takes_path_and_impl_path(_: &Path, _: impl AsRef<Path>) {}
9+
fn takes_two_impl_paths_with_the_same_generic<P: AsRef<Path>>(_: P, _: P) {}
10+
fn takes_two_impl_paths_with_different_generics<P: AsRef<Path>, Q: AsRef<Path>>(_: P, _: Q) {}
11+
12+
struct Foo;
13+
14+
impl Foo {
15+
fn takes_path(_: &Path) {}
16+
fn takes_self_and_path(&self, _: &Path) {}
17+
fn takes_path_and_impl_path(_: &Path, _: impl AsRef<Path>) {}
18+
fn takes_self_and_path_and_impl_path(&self, _: &Path, _: impl AsRef<Path>) {}
19+
}
20+
21+
fn main() {
22+
let f = Foo;
23+
24+
fs::write("foo.txt", "foo"); //~ needless_path_new
25+
26+
fs::copy(
27+
"foo", //~ needless_path_new
28+
"bar", //~ needless_path_new
29+
);
30+
31+
Foo::takes_path(Path::new("foo"));
32+
33+
f.takes_self_and_path_and_impl_path(
34+
Path::new("foo"),
35+
"bar", //~ needless_path_new
36+
);
37+
38+
// We can and should change both independently
39+
takes_two_impl_paths_with_different_generics(
40+
"foo", //~ needless_path_new
41+
"bar", //~ needless_path_new
42+
);
43+
44+
let a = takes_impl_path;
45+
a("foo.txt"); //~ needless_path_new
46+
47+
// Don't lint
48+
49+
takes_path(Path::new("foo"));
50+
51+
// The paramater that _could_ be passed directly, was;
52+
// The parameter that could't, wasn't
53+
takes_path_and_impl_path(Path::new("foo"), "bar");
54+
Foo::takes_path_and_impl_path(Path::new("foo"), "bar");
55+
f.takes_self_and_path_and_impl_path(Path::new("foo"), "bar");
56+
57+
// We are conservative and don't suggest changing a parameter
58+
// if it contains a generic type used elsewhere in the function
59+
takes_two_impl_paths_with_the_same_generic(Path::new("foo"), Path::new("bar"));
60+
61+
// The type ascription specifies `Path`, not just `impl AsRef<Path>`
62+
let _: Option<&Path> = Some(Path::new("foo"));
63+
64+
// The return type requires `Path`, not just `impl AsRef<Path>`
65+
fn foo() -> Option<&'static Path> {
66+
Some(Path::new("foo.txt"))
67+
}
68+
}

‎tests/ui/needless_path_new.rs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
#![warn(clippy::needless_path_new)]
2+
3+
use std::fs;
4+
use std::path::Path;
5+
6+
fn takes_path(_: &Path) {}
7+
fn takes_impl_path(_: impl AsRef<Path>) {}
8+
fn takes_path_and_impl_path(_: &Path, _: impl AsRef<Path>) {}
9+
fn takes_two_impl_paths_with_the_same_generic<P: AsRef<Path>>(_: P, _: P) {}
10+
fn takes_two_impl_paths_with_different_generics<P: AsRef<Path>, Q: AsRef<Path>>(_: P, _: Q) {}
11+
12+
struct Foo;
13+
14+
impl Foo {
15+
fn takes_path(_: &Path) {}
16+
fn takes_self_and_path(&self, _: &Path) {}
17+
fn takes_path_and_impl_path(_: &Path, _: impl AsRef<Path>) {}
18+
fn takes_self_and_path_and_impl_path(&self, _: &Path, _: impl AsRef<Path>) {}
19+
}
20+
21+
fn main() {
22+
let f = Foo;
23+
24+
fs::write(Path::new("foo.txt"), "foo"); //~ needless_path_new
25+
26+
fs::copy(
27+
Path::new("foo"), //~ needless_path_new
28+
Path::new("bar"), //~ needless_path_new
29+
);
30+
31+
Foo::takes_path(Path::new("foo"));
32+
33+
f.takes_self_and_path_and_impl_path(
34+
Path::new("foo"),
35+
Path::new("bar"), //~ needless_path_new
36+
);
37+
38+
// We can and should change both independently
39+
takes_two_impl_paths_with_different_generics(
40+
Path::new("foo"), //~ needless_path_new
41+
Path::new("bar"), //~ needless_path_new
42+
);
43+
44+
let a = takes_impl_path;
45+
a(Path::new("foo.txt")); //~ needless_path_new
46+
47+
// Don't lint
48+
49+
takes_path(Path::new("foo"));
50+
51+
// The paramater that _could_ be passed directly, was;
52+
// The parameter that could't, wasn't
53+
takes_path_and_impl_path(Path::new("foo"), "bar");
54+
Foo::takes_path_and_impl_path(Path::new("foo"), "bar");
55+
f.takes_self_and_path_and_impl_path(Path::new("foo"), "bar");
56+
57+
// We are conservative and don't suggest changing a parameter
58+
// if it contains a generic type used elsewhere in the function
59+
takes_two_impl_paths_with_the_same_generic(Path::new("foo"), Path::new("bar"));
60+
61+
// The type ascription specifies `Path`, not just `impl AsRef<Path>`
62+
let _: Option<&Path> = Some(Path::new("foo"));
63+
64+
// The return type requires `Path`, not just `impl AsRef<Path>`
65+
fn foo() -> Option<&'static Path> {
66+
Some(Path::new("foo.txt"))
67+
}
68+
}

‎tests/ui/needless_path_new.stderr‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
error: the expression enclosed in `Path::new` implements `AsRef<Path>`
2+
--> tests/ui/needless_path_new.rs:24:15
3+
|
4+
LL | fs::write(Path::new("foo.txt"), "foo");
5+
| ^^^^^^^^^^^^^^^^^^^^ help: remove the enclosing `Path::new`: `"foo.txt"`
6+
|
7+
= note: `-D clippy::needless-path-new` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::needless_path_new)]`
9+
10+
error: the expression enclosed in `Path::new` implements `AsRef<Path>`
11+
--> tests/ui/needless_path_new.rs:28:9
12+
|
13+
LL | Path::new("bar"),
14+
| ^^^^^^^^^^^^^^^^ help: remove the enclosing `Path::new`: `"bar"`
15+
16+
error: the expression enclosed in `Path::new` implements `AsRef<Path>`
17+
--> tests/ui/needless_path_new.rs:27:9
18+
|
19+
LL | Path::new("foo"),
20+
| ^^^^^^^^^^^^^^^^ help: remove the enclosing `Path::new`: `"foo"`
21+
22+
error: the expression enclosed in `Path::new` implements `AsRef<Path>`
23+
--> tests/ui/needless_path_new.rs:35:9
24+
|
25+
LL | Path::new("bar"),
26+
| ^^^^^^^^^^^^^^^^ help: remove the enclosing `Path::new`: `"bar"`
27+
28+
error: the expression enclosed in `Path::new` implements `AsRef<Path>`
29+
--> tests/ui/needless_path_new.rs:41:9
30+
|
31+
LL | Path::new("bar"),
32+
| ^^^^^^^^^^^^^^^^ help: remove the enclosing `Path::new`: `"bar"`
33+
34+
error: the expression enclosed in `Path::new` implements `AsRef<Path>`
35+
--> tests/ui/needless_path_new.rs:40:9
36+
|
37+
LL | Path::new("foo"),
38+
| ^^^^^^^^^^^^^^^^ help: remove the enclosing `Path::new`: `"foo"`
39+
40+
error: the expression enclosed in `Path::new` implements `AsRef<Path>`
41+
--> tests/ui/needless_path_new.rs:45:7
42+
|
43+
LL | a(Path::new("foo.txt"));
44+
| ^^^^^^^^^^^^^^^^^^^^ help: remove the enclosing `Path::new`: `"foo.txt"`
45+
46+
error: aborting due to 7 previous errors
47+

0 commit comments

Comments
 (0)