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

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

Closed
1 of 2 tasks
oxkitsune opened this issue Nov 8, 2022 · 5 comments
Closed
1 of 2 tasks
Labels
Milestone

Comments

@oxkitsune
Copy link
Contributor

oxkitsune commented Nov 8, 2022

Problem

Declaring a Matcher using Matchers.{allOf,anyOf}(Matcher...) containing just a single Matcher, is redundant.

Description of the proposed new feature

  • Support a stylistic preference.
  • Avoid a common gotcha, or potential problem.

We can rewrite

allOf(annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource")));
anyOf(annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource")));

to:

annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource"));
annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource"));

Considerations

This should support the method overloads Matchers.{allOf,anyOf}(Iterable<? extends Matcher>).

Participation

This is a nice opportunity to get your hands dirty with BugCheckers 🚀

@rickie
Copy link
Member

rickie commented Nov 8, 2022

A related example of what this check would flag: #335 (comment).

@EvgheniiShipilov
Copy link
Contributor

EvgheniiShipilov commented Dec 1, 2022

I'd like to pick this one up. Anything I should be aware of?

Update: I see something similar already existing 🤔

@rickie
Copy link
Member

rickie commented Dec 1, 2022

I'd like to pick this one up.

Nice!

Ah I think you found the most similar RefasterAnyOfUsage check already. Another one would be the IdentityConversion check. As that check also flags enclosing method invocations that can be dropped.

Well, actually now that I'm thinking of that and looking into the tests of that check; maybe this would be as simple as adding these exact methods to IS_CONVERSION_METHOD 🤔.

(Not sure if the naming would still be correct for IS_CONVERSION_METHOD but that's another concern).

Currently not able to try it out, but might be later / tomorrow.

@rickie
Copy link
Member

rickie commented Dec 4, 2022

Sorry, a little later than expected, but I tried it out.

Indeed, we can use the IS_CONVERSION_METHOD and the IdentityConversion check to solve this problem.

With this information, do you have enough context to proceed with opening a PR 😉?

@EvgheniiShipilov
Copy link
Contributor

I think this should be enough, thanks for looking into it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants