Skip to content

New lint no_effect_replace #8747

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

Closed
wants to merge 0 commits into from
Closed

New lint no_effect_replace #8747

wants to merge 0 commits into from

Conversation

guerinoni
Copy link
Contributor

Closes #1595

Signed-off-by: Federico Guerinoni [email protected]

Thank you for making Clippy better!

We're collecting our changelog from pull request descriptions.
If your PR only includes internal changes, you can just write
changelog: none. Otherwise, please write a short comment
explaining your change. Also, it's helpful for us that
the lint name is put into brackets [] and backticks ` `,
e.g. [`lint_name`].

If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

  • Followed lint naming conventions
  • Added passing UI tests (including committed .stderr file)
  • cargo test passes locally
  • Executed cargo dev update_lints
  • Added lint documentation
  • Run cargo dev fmt

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

Delete this line and everything above before opening your PR.


Please write a short comment explaining your change (or "none" for internal only changes)

changelog:
Add no_effect_replace lint.

@rust-highfive
Copy link

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 25, 2022
@bors
Copy link
Contributor

bors commented Apr 25, 2022

☔ The latest upstream changes (presumably #8727) made this pull request unmergeable. Please resolve the merge conflicts.

@xFrednet
Copy link
Member

r? @dswij 🙃

@xFrednet xFrednet self-assigned this Apr 25, 2022
@matthiaskrgr
Copy link
Member

Might be nice to also have tests where we do something like let x=...; foo.replace(x,x); and foo.replace(x.f(),x.f()) where x is a fn f(&mut self)

@guerinoni guerinoni closed this Apr 27, 2022
@guerinoni
Copy link
Contributor Author

Moved to #8754 because changing PC I messed up something :)

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.

Spot where str::replace is used with the same argument
7 participants