<T as Into<T>>::into lint#129249
Open
estebank wants to merge 6 commits into
Open
Conversation
Collaborator
|
r? @chenyukang rustbot has assigned @chenyukang. Use |
rustbot
added
O-unix
Collaborator
|
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
Contributor
Author
|
@bors try @craterbot check |
Contributor
Author
|
@bors try |
This comment was marked as outdated.
This comment was marked as outdated.
estebank
commented
Aug 18, 2024
Contributor
Author
|
@bors try |
Collaborator
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 18, 2024
[Experimental] `<T as Into<T>>::into` lint Running crater to see how common that pattern is. The Lint would have to be at most warn-by-default because there are a handful of cases detected that are actually perfectly reasonable (`type` aliases with per-platform `cfg`, or macros) which are now at best half-heartedly handled. I've detected a handful of cases where we're calling `.into()` unnecessarily in the `rustc` codebase as well, and changed those.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Collaborator
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-author
Member
|
🤔 looks like you are reimplementing a subset of https://rust-lang.github.io/rust-clippy/master/index.html#/useless_conversion ? edit: spoiler |
Member
|
looks a bit like a mix of |
This comment has been minimized.
This comment has been minimized.
traviscross
added
the
S-experimental
Contributor
Author
|
@matthiaskrgr I'm trying to gauge how common the root cause of the recent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
@bors try |
This comment has been minimized.
This comment has been minimized.
rust-bors Bot
pushed a commit
that referenced
this pull request
Jun 1, 2026
[Experimental] `<T as Into<T>>::into` lint
estebank
added a commit
to estebank/rust
that referenced
this pull request
Jun 1, 2026
While working on rust-lang#129249, encountered these cases in the codebase of calling `.into()` unecessarily. Splitting them out so that they can land independently of the lint.
This comment has been minimized.
This comment has been minimized.
Contributor
Contributor
Author
|
@craterbot check |
Collaborator
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
Jun 3, 2026
…ochenkov [tiny] remove unecessary `.into()` calls While working on rust-lang#129249, encountered these cases in the codebase of calling `.into()` unecessarily. Splitting them out so that they can land independently of the lint.
This comment has been minimized.
This comment has been minimized.
estebank
changed the title
<T as Into<T>>::into lint<T as Into<T>>::into lint
Contributor
Author
|
This PR is rebased on top of #157218 for the I am still waiting on the crater results, but I am satisfied with the current state of the patch. This is now ready for review. As an update to #129249 (comment), these two open questions for t-lang:
The impact of this pattern is significant enough that I feel like it deserves to be warn-by-default in rustc itself. I also split the lint into two, one for the expected case and another for this occurring withing a macro expansion. I was under the initial plan of detecting whether the macro was local and only warning if that was the case, but even in local macros there are cases where calling More complex scenarios (like glob imports or two step imports where intermediary steps are conditionally compiled) are not handled, but I only encountered 1 such case. We'll see how much that happens in the wild, but expect it to be low enough to be able to move forward. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
When annotating an item with `#[cfg]` we track both the item that got annotated (with an inert attr) and the names of items that got directly cfg'd out. Extend this mechanism to also work for items within a `cfg_select!`.
Add two lints, very similar to `clippy::useless_conversion`, detecting when `.into()` is being called unnecessarily. We present two separate lints, one that triggers inside of macros, because it is common for macros to have `.into()` calls to make the caller side easier to use. In those cases people should still use the fully-qualified path instead, but allow them to silence that lint without silencing the more general, more likely to be problematic case. This lint allows us to protect developers from the semver-hazard that time 0.3.34 encountered, where a std change caused a valid method chain to start producing inference errors. This is explicitly allowed by the Rust project's backwards compatibility guarantees (inference is not included in them), which means that relying on the blanket `impl Into` is a problem. The lint as implemented has false negatives: because of the way type aliases are handled by the type system, we can't know whether `let _: i32 = 0i32.into()` corresponds to a call on `i32` *or* a call on a type alias that is `i32` only on some platforms (like `#[cfg(..)] type Int = i32;`). To avoid false positives, we keep track of type aliases that have been imported in the local crate and mark their types for exclusion. This means that calling `let _: i32 = Int::into(0i32);` will not be linted against even though it should.
Collaborator
|
This PR was rebased onto a different main 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
View all comments
Add two lints, very similar to
clippy::useless_conversion, detecting when.into()is being called unnecessarily. We present two separate lints, one that triggers inside of macros, because it is common for macros to have.into()calls to make the caller side easier to use. In those cases people should still use the fully-qualified path instead,but allow them to silence that lint without silencing the more general, more likely to be problematic case.
This lint allows us to protect developers from the semver-hazard that time 0.3.34 encountered, where a std change caused a valid method chain to start producing inference errors. This is explicitly allowed by the Rust project's backwards compatibility guarantees (inference is not included in them), which means that relying on the blanket
impl Intois a problem.The lint as implemented has false negatives: because of the way type aliases are handled by the type system, we can't know whether
let _: i32 = 0i32.into()corresponds to a call oni32or a call on a type alias that isi32only on some platforms (like#[cfg(..)] type Int = i32;). To avoid false positives, we keep track of type aliases that have been imported in the local crate and mark their types for exclusion. This means that callinglet _: i32 = Int::into(0i32);will not be linted against even though it should.Running crater to see how common that pattern is. The Lint would have to be at most warn-by-default because there are a handful of cases detected that are actually perfectly reasonable (
typealiases with per-platformcfg, or macros) which are now at best half-heartedly handled.I've detected a handful of cases where we're calling
.into()unnecessarily in therustccodebase as well, and changed those.CC #127343.