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

Have IdentityConversion flag com.google.errorprone.matchers.Matchers#{allOf,anyOf} #420

Merged
merged 4 commits into from
Dec 20, 2022

Conversation

EvgheniiShipilov
Copy link
Contributor

@EvgheniiShipilov EvgheniiShipilov commented Dec 19, 2022

Check for cases when allOf and anyOf Matchers have a single parameter. Such cases can be rewritten by simply omitting the respective method.

Suggested commit message:

Drop redundant Matchers.{allOf,anyOf} containing a single Matcher

@github-actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@EvgheniiShipilov EvgheniiShipilov force-pushed the EvgheniiShipilov/redundant-all-any-of branch from bac4154 to ef9d1ba Compare December 19, 2022 14:57
@github-actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

1 similar comment
@github-actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Added a commit.

Alternative suggested commit message:

Have `IdentityConversion` flag `com.google.errorprone.matchers.Matchers#{allOf,anyOf}` (#420)

Thanks for opening a PR @EvgheniiShipilov , it looks good 🚀!

@github-actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 force-pushed the EvgheniiShipilov/redundant-all-any-of branch from 59212f0 to 9a2dc67 Compare December 20, 2022 08:26
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Rebased and added a commit. Expanding on @rickie's suggested commit message:

Have `IdentityConversion` flag `com.google.errorprone.matchers.Matchers#{allOf,anyOf}` (#420)

While there, sort and rename some (test) code.

Fixes #340.

Comment on lines 166 to 170
"",
" // BUG: Diagnostic contains:",
" Matcher matcher1 = Matchers.allOf(annotations(AT_LEAST_ONE, isType(\"foo\")));",
" // BUG: Diagnostic contains:",
" Matcher matcher2 = Matchers.anyOf(annotations(AT_LEAST_ONE, isType(\"bar\")));",
Copy link
Member

Choose a reason for hiding this comment

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

We can also move these up a bit, in line with the main code order. (Edit: seems there are other inconsistencies. Will move some stuff around.)

Next to that:

  1. We can use simpler expressions for this test.
  2. We should do this more generally, but since it's cheap for this case: let's also add some non-matching cases.

@github-actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 added this to the 0.7.0 milestone Dec 20, 2022
@rickie
Copy link
Member

rickie commented Dec 20, 2022

Changes LGTM @Stephan202!

@rickie rickie changed the title Check for redundant allOf, anyOf error-prone matchers Have IdentityConversion flag com.google.errorprone.matchers.Matchers#{allOf,anyOf} Dec 20, 2022
@rickie rickie merged commit d0a89da into master Dec 20, 2022
@rickie rickie deleted the EvgheniiShipilov/redundant-all-any-of branch December 20, 2022 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants