new lint: [`useless_pathbuf_conversion`] #15640

dbaranov34 · 2025-09-09T10:29:19Z

changelog: new lint: [useless_pathbuf_conversion]

resolves
#15637

rustbot · 2025-09-09T10:29:24Z

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

github-actions · 2025-09-09T10:29:27Z

Lintcheck changes for 5412590

Lint	Added	Removed	Changed
`clippy::useless_pathbuf_conversion`	1	0	0

This comment will be updated if you push new changes

ada4a

Thank you very much for working on this -- this is a lint I wanted to propose myself, alongside #14668. So here are some of the tips I've learned while working on that lint -- hope they'll be useful:)

View changes since this review

ada4a · 2025-09-23T22:51:04Z

clippy_lints/src/useless_pathbuf_conversion.rs

+        {
+            // check that the function is `from`
+            if cx.tcx.item_name(def_id) != sym::from {
+                return;
+            }
+
+            // get the type of the function's return value
+            let ty = cx.typeck_results().expr_ty(inner);
+
+            if is_type_diagnostic_item(cx, ty, sym::PathBuf)
+                && let Some(arg) = args.first()


nit: I think pulling all of this into the let-chain wouldn't hurt

Suggested change

{

// check that the function is `from`

if cx.tcx.item_name(def_id) != sym::from {

return;

}

// get the type of the function's return value

let ty = cx.typeck_results().expr_ty(inner);

if is_type_diagnostic_item(cx, ty, sym::PathBuf)

&& let Some(arg) = args.first()

// check that the function is `from`

&& cx.tcx.item_name(def_id) == sym::from {

// get the type of the function's return value

&& let ty = cx.typeck_results().expr_ty(inner)

&& is_type_diagnostic_item(cx, ty, sym::PathBuf)

&& let Some(arg) = args.first()

Yep, that looks more accurate, fixed

ada4a · 2025-09-23T22:53:22Z

clippy_lints/src/useless_pathbuf_conversion.rs

+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        // Only care about &PathBuf::from(...)
+        if let ExprKind::AddrOf(_, _, inner) = &expr.kind
+            && let ExprKind::Call(func, args) = &inner.kind


You could ensure PathBuf::from having only one argument already at this line:

Suggested change

&& let ExprKind::Call(func, args) = &inner.kind

&& let ExprKind::Call(func, [arg]) = &inner.kind

This avoids the need for && let Some(arg) = args.first() below

ada4a · 2025-09-23T22:53:38Z

clippy_lints/src/useless_pathbuf_conversion.rs

+    /// fn use_path(p: &std::path::Path) {}
+    /// use_path(std::path::Path::new("abc"));
+    /// ```
+   #[clippy::version = "1.91.0"]


Suggested change

#[clippy::version = "1.91.0"]

#[clippy::version = "1.92.0"]

ada4a · 2025-09-23T22:54:19Z

clippy_lints/src/useless_pathbuf_conversion.rs

+   #[clippy::version = "1.91.0"]
+    pub USELESS_PATHBUF_CONVERSION,
+    complexity,
+    "creating a PathBuf only to take a reference, where Path::new would suffice"


Suggested change

"creating a PathBuf only to take a reference, where Path::new would suffice"

"creating a `PathBuf` only to take a reference, where `Path::new` would suffice"

ada4a · 2025-09-23T23:08:29Z

clippy_lints/src/useless_pathbuf_conversion.rs

+    ///
+    /// **Example:**
+    /// ```rust
+    /// fn use_path(p: &std::path::Path) {}


Most functions working with Paths will have the parameter specified as impl AsRef<Path>, which this lint unfortunately wouldn't catch right now.

This is very similar to a lint I suggested myself: #14668, and I'm not sure what the interplay between them should be. That lint basically checks two things:

that the implicit generic in impl AsRef<Path> isn't used elsewhere in the function

that the x in Path::new(x) implements all the trait bounds of that generic (notably AsRef, but often also Sized), and therefore could be used directly

So this lint would probably need to copy the first check from that lint; and at this point one could argue that it could go the extra mile and also check the second thing, which would allow it to replace &PathBuf::from(x) directly with x -- otherwise, one would first create Path::new(x), and then need to repeat the entirety of the first check on that.

Now that I've written this out though, the first check isn't actually all that big, and also it's preferable to let the lints compose, so this is probably fine after all.

Thanks for breaking it down. I agree, let the lints complement each other.

ada4a · 2025-09-23T23:21:52Z

clippy_lints/src/useless_pathbuf_conversion.rs

+            && let ExprKind::Path(ref qpath) = func.kind
+            && let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id()
+        {
+            // check that the function is `from`
+            if cx.tcx.item_name(def_id) != sym::from {
+                return;
+            }
+
+            // get the type of the function's return value
+            let ty = cx.typeck_results().expr_ty(inner);
+
+            if is_type_diagnostic_item(cx, ty, sym::PathBuf)


I think you should be able to check that the call is PathBuf::from with something like this:

&& let ExprKind::Path(ref qpath) = func.kind // pre-existing logic && let QPath::TypeRelative(pathbuf, from) = qpath && from.ident.name == sym::from && is_path_diagnostic_item(cx, pathbuf, sym::PathBuf)

This avoids the need to resolve/type-check everything for the second time¹, since QPath already contains the whole resolution

Footnotes

I'm pretty sure it's all cached in the compiler, but still ↩

#[clippy::dump] is quite useful for finding the right incantations like this. For this case, I used the following code:

fn main() { #[clippy::dump] std::path::PathBuf::from("x"); }

Nice trick with clipy_dump, I didn`t know it, thanks.

dbaranov34 · 2025-09-30T13:25:19Z

Thanks for the comments, @ada4a . PR is updated now.

ada4a · 2025-09-30T13:50:36Z

@rustbot note remove Feature-freeze

ada4a · 2025-09-30T13:55:33Z

tests/ui/unnecessary_to_owned.rs

    //~^ redundant_clone
    require_os_str(&OsString::from("x").to_os_string());
    //~^ redundant_clone
-    require_path(&std::path::PathBuf::from("x").to_path_buf());


redundant_clone fires here not because of the &PathBuf, but because of the .to_path_buf() (which is effectively a .clone()), so this test case is important to retain. ~~You should probably allow useless_pathbuf_conversion in this file to avoid the extraneous warning.~~ Actually, even that's not necessary, since useless_pathbuf_conversion no longer lints here

ada4a · 2025-09-30T14:24:13Z

clippy_lints/src/useless_pathbuf_conversion.rs

+            && let QPath::TypeRelative(pathbuf, _) = qpath
+            && is_path_diagnostic_item(cx, *pathbuf, sym::PathBuf)
+        {
+            let sugg = format!("Path::new({})", snippet(cx, arg.span, ".."));


Please prefer snippet_with_applicability where possible. If you start with Applicabiliy::MachineApplicable, but a snippet for a given span isn't available, then snippet_with_applicability will reduce the applicability to Applicability::MaybeIncorrect. The whole pattern usually looks like this:

let mut app = Applicability::MachineApplicable; let sugg = snippet_with_applicability(cx, <span>, <default>, &mut app); span_lint_and_sugg(.., sugg, app);

Note to self: this really ought to be in the documentation

And it might even be necessary to use snippet_with_context -- the argument to PathBuf::from might be a macro call, in which case snippet_with_applicability would incorrectly expand it. You could use the following as a test case:

macro_rules! str { () => { "expands to a `&str`" } } let _ = PathBuf::from(str!());

ada4a · 2025-09-30T14:38:53Z

clippy_lints/src/useless_pathbuf_conversion.rs

+                cx,
+                USELESS_PATHBUF_CONVERSION,
+                expr.span,
+                "unnecessary `PathBuf::from` when a `&Path` is enough",


Currently, this lint doesn't actually check whether a &Path is enough -- for example, it would falsely warn in a case like:

fn takes_ref_pathbuf(_: &PathBuf) {} fn main() { takes_ref_pathbuf(&PathBuf::from("path")); }

ada4a · 2025-09-30T14:51:49Z

clippy_lints/src/useless_pathbuf_conversion.rs

+            && let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id()
+            && cx.tcx.item_name(def_id) == sym::from
+            && let QPath::TypeRelative(pathbuf, _) = qpath
+            && is_path_diagnostic_item(cx, *pathbuf, sym::PathBuf)


Suggested change

&& let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id()

&& cx.tcx.item_name(def_id) == sym::from

&& let QPath::TypeRelative(pathbuf, _) = qpath

&& is_path_diagnostic_item(cx, *pathbuf, sym::PathBuf)

&& let QPath::TypeRelative(pathbuf, from) = qpath

&& is_path_diagnostic_item(cx, *pathbuf, sym::PathBuf)

&& from.ident.name == sym::from

ada4a · 2025-09-30T16:33:59Z

clippy_lints/src/useless_pathbuf_conversion.rs

+            && is_type_diagnostic_item(cx, *inner_ty, sym::Path)
+        {
+            let mut app = Applicability::MachineApplicable;
+            let arg_snippet = snippet_with_context(cx, arg.span, arg.span.ctxt(), "..", &mut app).0;


Oh, sorry for not explaining it fully. In snippet_with_context, the span you use to get the ctxt should be of a node that is the parent of the node for which you're creating the snippet. That was convoluted, so here's what that'd look like in your case:

You have &PathBuf::from(arg), and you want to get the snippet for arg. Therefore, you get the ctxt from the parent expression of arg, which is PathBuf::from(arg), which you call inner. With that, the right invocation would be:

Suggested change

let arg_snippet = snippet_with_context(cx, arg.span, arg.span.ctxt(), "..", &mut app).0;

let arg_snippet = snippet_with_context(cx, arg.span, inner.span.ctxt(), "..", &mut app).0;

The best way I can explain the logic behind this is that you specifically want to avoid viewing arg from "inside" -- fully expanded -- but rather from the "outside", and the most obvious thing that's looking at arg from outside is its parent expression. I'm pretty sure a grandparent or something would work as well here, but the parent is the easiest thing to get to.

No problem, thank you for the clarification. I have also added a note in a comment to snippet_with_context declaration.

I have also added a note in a comment to snippet_with_context declaration.

The two paragraphs before your addition are trying to explain basically the same as what you added I think, so imo it would make sense to clarify them. One could imagine a more full-fledged example showing a code snippet, and what snippet_with_context would return with different combinations of nodes. If you want to take a stab at that, feel free, otherwise I'll try to do that in a separate PR

rustbot · 2025-09-30T17:45:57Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Jarcho · 2025-10-01T19:33:51Z

This is mostly implemented already as unnecessary_to_owned. e.g.

use std::path::Path;

fn f(_: impl AsRef<Path>) {}

fn f2(x: &Path) {
    f(x.to_path_buf())
}

It doesn't currently recognize into and from as a conversion, but that's all that needs to change.

rustbot assigned Jarcho Sep 9, 2025

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 9, 2025

rustbot added S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work A-lint Area: New lints and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 9, 2025

samueltardieu added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work labels Sep 17, 2025

ada4a reviewed Sep 23, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Sep 30, 2025

dbaranov34 force-pushed the a-lint-useless_pathbuf_conversion branch from 1b9ec32 to 19d7324 Compare September 30, 2025 13:26

rustbot removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) has-merge-commits PR has merge commits, merge with caution. labels Sep 30, 2025

ada4a suggested changes Sep 30, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Sep 30, 2025

dbaranov34 added 4 commits September 30, 2025 21:56

implemeted lint, but the other lint now fails expectedly

9665f66

rm useless pathbuf check from unnecessary_to_owned_check

99e9718

cleanup and clippy

4380e83

update lints in changelog

e735110

dbaranov34 added 4 commits September 30, 2025 21:56

addressed pr comments

a43202b

addressed pr comments

54211e6

addressed pr comments

e3ec8c0

correct usage of snippet_with_context

aaa4e16

dbaranov34 force-pushed the a-lint-useless_pathbuf_conversion branch from b41b15f to aaa4e16 Compare September 30, 2025 17:56

rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Sep 30, 2025

fmt

5412590

samueltardieu added the needs-fcp label Oct 17, 2025

	&& let ExprKind::Call(func, args) = &inner.kind
	&& let ExprKind::Call(func, [arg]) = &inner.kind

	"creating a PathBuf only to take a reference, where Path::new would suffice"
	"creating a `PathBuf` only to take a reference, where `Path::new` would suffice"

	let arg_snippet = snippet_with_context(cx, arg.span, arg.span.ctxt(), "..", &mut app).0;
	let arg_snippet = snippet_with_context(cx, arg.span, inner.span.ctxt(), "..", &mut app).0;

new lint: [useless_pathbuf_conversion] #15640

Are you sure you want to change the base?

new lint: [useless_pathbuf_conversion] #15640

Uh oh!

Conversation

dbaranov34 commented Sep 9, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 9, 2025

Uh oh!

github-actions bot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ada4a left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

dbaranov34 commented Sep 30, 2025

Uh oh!

ada4a commented Sep 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Sep 30, 2025

Uh oh!

This comment has been minimized.

Jarcho commented Oct 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

new lint: [`useless_pathbuf_conversion`] #15640

new lint: [`useless_pathbuf_conversion`] #15640

dbaranov34 commented Sep 9, 2025 •

edited by rustbot

Loading

github-actions bot commented Sep 9, 2025 •

edited

Loading

ada4a left a comment •

edited by rustbot

Loading