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

Collapse consecutive AssertJ assertThat(..) statements that each use the same object to a single statement #373

Open
timtebeek opened this issue Jul 6, 2023 · 2 comments · May be fixed by #392
Labels
good first issue Good for newcomers recipe Recipe request

Comments

@timtebeek
Copy link
Contributor

timtebeek commented Jul 6, 2023

What problem are you trying to solve?

What precondition(s) should be checked before applying this recipe?

Only consecutive calls; not when interleaved with others.
Only when the asserted object does not change; so for instance no calls to extracting("baz"), as that would change the semantics.

Describe the situation before applying the recipe

assertThat(listA).isNotNull();
assertThat(listA).hasSize(3);
assertThat(listA).containsExactly("a", "b", "c");

Describe the situation after applying the recipe

assertThat(listA)
    .isNotNull()
    .hasSize(3)
    .containsExactly("a", "b", "c");

Any additional context

Would help clean up after the migration from Hamcrest allOf(Matcher...) to individual statements.

@timtebeek timtebeek added the recipe Recipe request label Jul 6, 2023
@timtebeek
Copy link
Contributor Author

Perhaps @pstreef can provide some pointers on the implementation here if he's willing to share, as I know he's done some similar work recently in an unrelated project. To be sure: I don't expect him to pick this up, but the general approach of collapsing subsequent calls to a more fluent invocation seems familiar. :)

@timtebeek timtebeek added the good first issue Good for newcomers label Jul 6, 2023
@pstreef
Copy link

pstreef commented Jul 7, 2023

Sure thing @timtebeek!

In most of the cases these assertions happen in the same method (maybe only work on @Test methods) and on either on a local variable, global variable or method invocation.

I would only focus on local variables to start with, and maybe extend to global if it makes sense. Method invocation should not be considered as the return values do not have to always be the same for subsequent calls.

I would only take into account calls that are consecutive, because if there is any other statement in between that could change the variable being asserted:

String a = "a";
assertThat(a).isNotNull();
a = "b";
assertThat(a).isEqualTo("a");

To get starter you will either have to start from a visitBlock(..) or visitMethodDeclaration(..) (if you want to filter on test methods, the later is helpful), itterate over all statements in the block/body and find any statement where a J.MethodInvocation has a select that is itself a J.MethodInvocation to assertThat(..)

Alternatively you can start from visitMethodInvocation(..) and find any J.MethodInvocation that has a select that is a J.MethodInvocation to assertThat(..). However that makes it harder to determine if there is only 1 call after assertThat(x) which might cause trouble.

Make sure to:

  • Only collect them if they are consecutive and the assertThat(x) call have exactly the same parameters.
  • Only collect if the chained calls do not change semantics
  • Dont collect calls that already are chaining more than once

Then chain those calls by appending each chained call to the next one and ending with the asserThat(x) using withSelect(..)

Edge cases to test for:

  • non local variable arguments
  • non consecutive calls
  • calls with different arguments
  • calls with methodInvocations as argument
  • calls where the semantics change ( for instance extracting)
  • multiple already chained calls
  • calls made in nested blocks

timtebeek referenced this issue in bhavyasripenumarthi/rewrite-logging-frameworks Jul 26, 2023
srmalkan added a commit to srmalkan/rewrite-testing-frameworks that referenced this issue Aug 1, 2023
srmalkan added a commit to srmalkan/rewrite-testing-frameworks that referenced this issue Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers recipe Recipe request
Projects
Status: Recipes Wanted
2 participants