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

added recipe to collapse consecutive AssertThat statements (#373) #392

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

srmalkan
Copy link

@srmalkan srmalkan commented Aug 1, 2023

Closes #373

Hi All,

I have added a recipe to collapse if consecutive AssertThat statements on the same parameter are found. It ignores cases where the parameter is a Method Invocation or lambda or existing chained assertThat statement.

I am facing a bit of difficulty in formatting the chained assertThat statement, can someone guide me on that?

@timtebeek timtebeek self-requested a review August 1, 2023 20:10
@timtebeek timtebeek added the recipe Recipe request label Aug 1, 2023
@timtebeek
Copy link
Contributor

Really nice to see you picked this up @srmalkan! Impressed with how much of the recipe already works. I've explored locally if I could get the indentation to work, but no such luck yet either. I'd expected the fix to be in setting a prefix on each of the method invocations where we change the select, but I've not yet found a working combination. Will try to explore later again.

@srmalkan
Copy link
Author

srmalkan commented Aug 3, 2023

Yes, i thought so too. After looking at few objects, now i feel its related to setting up 'after' correctly in JRightPadded.

@srmalkan
Copy link
Author

srmalkan commented Aug 6, 2023

Hi @timtebeek,

I was able to fix formatting by setting after in the JRightPadded element. However, facing difficulty to set whitespace correctly.

@timtebeek
Copy link
Contributor

Glad to see how far this has come! I think we can just adjust the after values to include and additional level of indentation; That's the only remaining issue right?

@srmalkan
Copy link
Author

srmalkan commented Aug 9, 2023

Glad to see how far this has come! I think we can just adjust the after values to include and additional level of indentation; That's the only remaining issue right?

That's correct.

@timtebeek timtebeek requested a review from joanvr August 9, 2023 12:54
Comment on lines +82 to +84
int statementListSize = statements.size();
List<J.MethodInvocation> currList = new ArrayList<>();
for (int currIndex = 0; currIndex < statementListSize; currIndex++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
int statementListSize = statements.size();
List<J.MethodInvocation> currList = new ArrayList<>();
for (int currIndex = 0; currIndex < statementListSize; currIndex++) {
List<J.MethodInvocation> currList = new ArrayList<>();
for (int currIndex = 0; currIndex < statements.size(); currIndex++) {

Optional<J.MethodInvocation> assertThatMi = getAssertThatMi(statement);
if (assertThatMi.isPresent()) {
if (isAssertThatValid(statement)) {
if (!getFirstArgumentName(assertThatMi.get()).equals(prevArg)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to use org.openrewrite.java.search.SemanticallyEqual here instead of just checking the argument name?

@timtebeek timtebeek removed their request for review March 15, 2024 10:26
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…secutiveAssertThatStatements.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@openrewrite openrewrite deleted a comment from github-actions bot Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Recipe request
Projects
Status: Ready to Review
Development

Successfully merging this pull request may close these issues.

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