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

Use match_function_call wherever possible #4649

Merged
merged 2 commits into from
Oct 12, 2019

Conversation

Lythenas
Copy link
Contributor

@Lythenas Lythenas commented Oct 10, 2019

Move match_function_call to utils and use it wherever possible as discussed in #4635.

changelog: none

@Lythenas
Copy link
Contributor Author

I wasn't able to use match_function_call everywhere.

  • clippy_lints/src/explicit_write.rs:44 is a little more inefficient than before because it matches ExprKind::Call twice now.
  • For a couple other places something that only matches the ExprKind::Path could be used.
  • For some other places something a little more generic would be better. E.g. fn match_function_call_map(cx, expr, cb: Fn(&[&str], &[Expr]) -> b) -> Option<b> would be better. E.g. at clippy_lints/src/explicit_write.rs:44 we could match the path in the callback

But then there might be to many util function that do almost the same thing. Maybe just one generalized version of match_function_call would be better. But I'm not sure how that would look. And to be honest I think it's ok the way it is now.

Also what should I put in changelog?

@phansch
Copy link
Member

phansch commented Oct 11, 2019

@bors r+ thanks!

too many util functions that do almost the same thing

I think that's fine as long as they are all documented. They could possibly be split up into separate modules if it gets too confusing? (In general I don't like the notion of utils but I think it's fine for now)

Also what should I put in changelog?

changelog: none is fine for refactorings; it's only meant for user-facing changes that are listed in the changelog

@bors
Copy link
Contributor

bors commented Oct 11, 2019

📌 Commit 15b433a has been approved by phansch

@bors
Copy link
Contributor

bors commented Oct 11, 2019

⌛ Testing commit 15b433a with merge 9ee6eb1...

bors added a commit that referenced this pull request Oct 11, 2019
Use match_function_call wherever possible

Move `match_function_call` to `utils` and use it wherever possible as discussed in #4635.

changelog: none
@bors
Copy link
Contributor

bors commented Oct 11, 2019

💔 Test failed - status-appveyor

@phansch
Copy link
Member

phansch commented Oct 12, 2019

@bors retry

@bors
Copy link
Contributor

bors commented Oct 12, 2019

⌛ Testing commit 15b433a with merge a865d0a...

bors added a commit that referenced this pull request Oct 12, 2019
Use match_function_call wherever possible

Move `match_function_call` to `utils` and use it wherever possible as discussed in #4635.

changelog: none
@bors
Copy link
Contributor

bors commented Oct 12, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: phansch
Pushing a865d0a to master...

@bors bors merged commit 15b433a into rust-lang:master Oct 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants