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

Assert instance of migration #372

Merged
merged 3 commits into from
Jul 11, 2023

Conversation

matusmatokpt
Copy link
Contributor

@matusmatokpt matusmatokpt commented Jul 6, 2023

Added recipe which migrates JUnit4 (or JUnit5) assertTrue(x instanceof y) into JUnit5 assertInstanceOf(y.class, x)

What's changed?

Added recipe which migrates JUnit4 (or JUnit5) assertTrue(x instanceof y) into JUnit5 assertInstanceOf(y.class, x)

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've added the license header to any new files through ./gradlew licenseFormat
  • I've used the IntelliJ auto-formatter on affected files
  • I've updated the documentation (if applicable)

Added recipe which migrates JUnit4 (or JUnit5) assertTrue(x instanceof y)
into JUnit5 assertInstanceOf(y.class, x)

Signed-off-by: matus.matok <matus.matok@pantheon.tech>
Added recipe which migrates JUnit4 (or JUnit5) assertTrue(x instanceof y)
into JUnit5 assertInstanceOf(y.class, x)

Signed-off-by: matus.matok <matus.matok@pantheon.tech>
@matusmatokpt
Copy link
Contributor Author

Firstly, if this is something that is already covered in the project, shut me down.

Secondly, I am kinda desparate about the method matcher:
MethodMatcher junit5Matcher = new MethodMatcher("org.junit.jupiter.api.Assertions assertTrue(* instanceof *,String)");
which only matches
assertTrue(list instanceof Iterable);
but not
assertTrue(list instanceof Iterable, "Not instance of Iterable");

@timtebeek timtebeek added the recipe Recipe request label Jul 6, 2023
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
J.MethodInvocation mi = super.visitMethodInvocation(method, executionContext);
MethodMatcher junit5Matcher = new MethodMatcher("org.junit.jupiter.api.Assertions assertTrue(* instanceof *,String)");
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see what you are trying to do here. Unfortunately MethodMatcher can't do that, but since the rest of your code already properly checks the parameter for J.InstanceOf, all you have to do is this:

Suggested change
MethodMatcher junit5Matcher = new MethodMatcher("org.junit.jupiter.api.Assertions assertTrue(* instanceof *,String)");
MethodMatcher junit5Matcher = new MethodMatcher("org.junit.jupiter.api.Assertions assertTrue(boolean, ..)");

public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
J.MethodInvocation mi = super.visitMethodInvocation(method, executionContext);
MethodMatcher junit5Matcher = new MethodMatcher("org.junit.jupiter.api.Assertions assertTrue(* instanceof *,String)");
MethodMatcher junit4Matcher = new MethodMatcher("org.junit.Assert assertTrue(..,* instanceof *)");
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly here:

Suggested change
MethodMatcher junit4Matcher = new MethodMatcher("org.junit.Assert assertTrue(..,* instanceof *)");
MethodMatcher junit4Matcher = new MethodMatcher("org.junit.Assert assertTrue(.., boolean)");

Fixed method matchers to work properly.

Signed-off-by: matus.matok <matus.matok@pantheon.tech>
@sambsnyd sambsnyd merged commit a428327 into openrewrite:main Jul 11, 2023
1 check passed
@matusmatokpt matusmatokpt deleted the AssertInstanceOfMigration branch July 12, 2023 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Recipe request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants