Skip to content
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

Add more cases to the useless_conversion lint #13756

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

samueltardieu
Copy link
Contributor

@samueltardieu samueltardieu commented Nov 29, 2024

The new cases are the application of Into::into or From::from through the following functions with no effect:

  • Option::map()
  • Result::map() and Result::map_err()
  • ControlFlow::map_break() and ControlFlow::map_err()
  • Iterator::map()

changelog: [useless_conversion]: detect useless calls to Into::into and From::from application through map* methods

@rustbot
Copy link
Collaborator

rustbot commented Nov 29, 2024

r? @blyxyas

rustbot has assigned @blyxyas.
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 Nov 29, 2024
@samueltardieu
Copy link
Contributor Author

The hits in lintcheck are true positives.

@samueltardieu samueltardieu force-pushed the push-qryupxlomuwt branch 3 times, most recently from 159742f to eac1e50 Compare November 30, 2024 19:11
@samueltardieu
Copy link
Contributor Author

Lookup code will be simplified when rust-lang/rust#133686 is merged and synchronized with Clippy.

The new cases are `x.map(f)` and `x.map_err(f)` when `f` is `Into::into`
or `From::from` with the same input and output types.
@samueltardieu samueltardieu force-pushed the push-qryupxlomuwt branch 2 times, most recently from 723b9a6 to 856aa03 Compare November 30, 2024 21:00
return true;
}

// Necessary for `core::ops::control_flow::ControlFlow` until a diagnostic item
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add FIXME: Add ControlFlow diagnostic item, so that in the future we can add necessary diagnostic items in masse.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. My PR has been merged into rust-lang so we should get it at the next sync.

return true;
}
}
if is_trait_method(cx, expr, sym::Iterator) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to lint for any methods of Iterator? What would the use case for this branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only activates for the recognized methods (map_*), no other methods of Iterator will trigger that.

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! ❤️
Could you squash that third commit into the second one?

- `ControlFlow::map_break()`
- `ControlFlow::map_continue()`
- `Iterator::map()`
@samueltardieu
Copy link
Contributor Author

Could you squash that third commit into the second one?

Done

@blyxyas blyxyas added this pull request to the merge queue Dec 2, 2024
Merged via the queue into rust-lang:master with commit 2ddfc92 Dec 2, 2024
9 checks passed
@samueltardieu samueltardieu deleted the push-qryupxlomuwt branch December 30, 2024 23:49
gsson pushed a commit to gsson/scylla-rust-driver that referenced this pull request Feb 22, 2025
gsson pushed a commit to gsson/scylla-rust-driver that referenced this pull request Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants