Skip to content

Add needless_conversion_for_trait lint #15451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

smoelius
Copy link
Contributor

@smoelius smoelius commented Aug 10, 2025

Adds a lint to flag trait-behavior preserving calls where the receiver/argument could be used directly. For example, the lint flags the call to as_path in the following code, because &lint_file_path could be used instead:

write_file(lint_file_path.as_path(), lint_file_contents)?;

Organization

The PR is currently organized as five commits:

  • DisallowedPath -> ConfPath; REPLACEMENTS_ALLOWED -> REPLACEABLE: Shorten some configuration-related names for types used by this lint.
  • Move some functions into clippy_utils::ty: Move some functions from unnecessary_to_owned and needless_conversion_for_generic_args (cc: @Jarcho) into the clippy_utils::ty module.
  • Expand comment in replace_types: Emphasize that replace_types cannot change a function's output type (see below).
  • Add needless_conversion_for_trait lint; get uitest to pass: The PR's main commit.
  • Fix adjacent code: Fix Clippy code flagged by the lint.

Relationship to unnecessary_to_owned

Some of unnecessary_to_owned's functionality is subsumed by this lint. However, replace_types, which needless_conversion_for_trait uses, does not allow a function's output type to change. unnecessary_to_owned does not have this restriction. So, to ensure the two lints do not flag the same code, unnecessary_to_owned now requires that a function's output type contain the type to be replaced. Note that some unnecessary_to_owned tests contain a function whose output type is changed by the lint's suggestion:

Also, unnecessary_to_owned will not eliminate a call to ToString::to_string unless the receiver implements Deref<Target = str> or AsRef<str>. needless_conversion_for_trait incorporates this restriction as well.

check_inherent_functions

The lint includes a check_crate_post check that runs when Clippy is compiled in debug mode. The check looks for functions the lint should either warn about, or should explicitly ignore. For example, while preparing this PR, the check flagged Vec::into_chunks, which was added to the list of explicitly ignored functions.

False positive in clippy_utils

There are three false positives in clippy_utils that I do not know how best to resolve. They concern the implementations of the three functions for DiagExt:

fn suggest_item_with_attr<D: Display + ?Sized>(
&mut self,
cx: &T,
item: Span,
msg: &str,
attr: &D,
applicability: Applicability,
) {
if let Some(indent) = indentation(cx, item) {
let span = item.with_hi(item.lo());
self.span_suggestion(span, msg.to_string(), format!("{attr}\n{indent}"), applicability);
}
}

needless_conversion_for_trait reports that msg.to_string() could be replaced with msg. However, making this change causes the borrow checker to think msg's reference flows into &mut self. I have allowed the lint for the trait implementation, but I am open to suggestions for eliminating this class of false positives.

changelog: Add needless_conversion_for_trait lint

Summary Notes

Managed by @rustbot—see help for details

@rustbot
Copy link
Collaborator

rustbot commented Aug 10, 2025

r? @y21

rustbot has assigned @y21.
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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 10, 2025
Copy link

github-actions bot commented Aug 10, 2025

Lintcheck changes for 867968f

Lint Added Removed Changed
clippy::needless_conversion_for_trait 42 0 0
clippy::unnecessary_to_owned 0 2 0

This comment will be updated if you push new changes

@rustbot 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 Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants