Skip to content

Preserve comments before collapsed AssertJ chains - #1096

Draft
martinfrancois wants to merge 1 commit into
openrewrite:mainfrom
martinfrancois:agent/repro-collapse-assertj-comment-boundary
Draft

Preserve comments before collapsed AssertJ chains#1096
martinfrancois wants to merge 1 commit into
openrewrite:mainfrom
martinfrancois:agent/repro-collapse-assertj-comment-boundary

Conversation

@martinfrancois

@martinfrancois martinfrancois commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Suggested review order: 25 of 52 (Score: 5)
Review first: openrewrite/rewrite-static-analysis#980

What's changed?

Keeps the first assertion's prefix and comments on the collapsed assertThat(...) call. Comments between later compatible assertions remain attached to their corresponding chained assertion.

What's your motivation?

Recipe: org.openrewrite.java.testing.assertj.CollapseConsecutiveAssertThatStatements.

I found this by running org.openrewrite.java.testing.assertj.CollapseConsecutiveAssertThatStatements from org.openrewrite.recipe:rewrite-testing-frameworks:3.42.0 on TestConventionTest.java in Symphony-Trello at a8013f27. I reproduced the same result with the latest released recipe artifact, org.openrewrite.recipe:rewrite-testing-frameworks:3.44.0.

Before

// then
assertThat(pom).contains("<jspecify.version>1.0.0</jspecify.version>");
assertThat(pom).contains(dependency);
assertThat(pom).doesNotContain("<artifactId>jspecify</artifactId>");

Actual after the recipe

assertThat(pom)
        // then
        .contains("<jspecify.version>1.0.0</jspecify.version>")
        .contains(dependency)
        .doesNotContain("<artifactId>jspecify</artifactId>");

Expected after the recipe

// then
assertThat(pom)
        .contains("<jspecify.version>1.0.0</jspecify.version>")
        .contains(dependency)
        .doesNotContain("<artifactId>jspecify</artifactId>");

The // then comment marks the start of the assertion phase. Moving it inside the fluent chain attaches it to the first .contains(...) call instead of the assertion block.

Confirmed real-world execution

Anything in particular you'd like reviewers to focus on?

Please review comment ownership at the collapse boundary and the behavior when later assertions also have comments.

Have you considered any alternatives or workarounds?

Leaving all commented sequences uncollapsed would avoid movement but would reject safe cases where only later assertion comments need to remain on their calls.

Any additional context

Pre-existing tests changed: CollapseConsecutiveAssertThatStatementsTest.java.collapseIfMultipleConsecutiveAssertThatPresent (updated), CollapseConsecutiveAssertThatStatementsTest.java.preserveCommentsWhenCollapsingAssertions (updated).

This change was prepared with AI assistance. I reviewed the target execution evidence, implementation, tests, and contribution text.

Checklist

@martinfrancois
martinfrancois force-pushed the agent/repro-collapse-assertj-comment-boundary branch from 11c96d2 to 3f4d043 Compare August 15, 2026 23:11
@martinfrancois martinfrancois changed the title Preserve standalone comments between AssertJ statements Preserve comments before collapsed AssertJ chains Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant