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

INSP: add remove parameter quick fix to liveness inspection #5561

Merged
merged 1 commit into from Jun 22, 2020

Conversation

Kobzol
Copy link
Member

@Kobzol Kobzol commented Jun 12, 2020

This PR adds a quick fix to remove an unused parameter, along with its usages in call sites. The call site arguments are removed without any checks for side effects, same as in the Kotlin plugin.

Should we ignore call sites with an incorrect number of arguments?

This PR is based on #5557 which introduced some common changes. I opened this one for discussion, but #5557 should be merged first, I'll rebase this one after/if it gets merged.

Part of #3163

@Kobzol
Copy link
Member Author

Kobzol commented Jun 18, 2020

Rebased onto master with #5557.

@Kobzol Kobzol added this to In Progress in To test Jun 18, 2020
Copy link
Member

@artemmukhin artemmukhin left a comment

Choose a reason for hiding this comment

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

Should we ignore call sites with an incorrect number of arguments?

Yes, I think that's OK.

Generally LGTM

@artemmukhin artemmukhin added this to the v125 milestone Jun 19, 2020
@Kobzol Kobzol force-pushed the insp-liveness-remove-parameter branch from a1e16e7 to 7f8f3b2 Compare June 21, 2020 11:45
@Kobzol
Copy link
Member Author

Kobzol commented Jun 22, 2020

I changed comma handling in such a way that only one comma is always removed, either the preceding or the following, based on which one is present. This handles parameter removal at the beginning, middle and end of parameter lists, and it also keeps trailing commas.

foo(p0, /*caret*/p1) -> foo(p0)
foo(p0, /*caret*/p1, ) -> foo(p0, )

@Kobzol
Copy link
Member Author

Kobzol commented Jun 22, 2020

bors r=ortem

@bors
Copy link
Contributor

bors bot commented Jun 22, 2020

Build succeeded:

@bors bors bot merged commit 465ff7c into intellij-rust:master Jun 22, 2020
To test automation moved this from In Progress to Test Jun 22, 2020
@Kobzol Kobzol deleted the insp-liveness-remove-parameter branch June 22, 2020 13:53
@lancelote lancelote moved this from Test to Done in To test Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
To test
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants