AssertionsForClassTypes import is replaced by Assertions

[barbulescu]

What's changed?

Add test to reproduce the issue.

What's your motivation?

Code does not compile after execution.

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

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[timtebeek]

Thanks @barbulescu ! I'd missed your PR as I don't get notified about drafts; would you want to continue to work on this? Would you need any guidance as you do so?

[barbulescu]

Yes, I would like to continue but I need some guidance.

[timtebeek]

Had a brief look here; looks like we get back a different assertion object from the Assertions class then we do from the AssertionsForClassTypes

Looks like AbstractCollectionAssert extends AbstractAssert, and not the AbstractObjectAssert we get from AssertionsForClassTypes.

This to me is a bit of a surprise on the part of AssertJ more than about the recipe change we're making here; not sure if that was intentional on their end but perhaps good to check as well.

[timtebeek]

@scordio I hope you don't mind me tagging you here; don't feel pressured to respond if you're busy! For context, we have some recipes that nudge folks to use the Assertions class for their imports instead of any more specific AssertionsForXyz classes:

rewrite-testing-frameworks/src/main/resources/META-INF/rewrite/assertj.yml

Lines 61 to 69 in fc9d8b4

	- org.openrewrite.java.ChangeMethodTargetToStatic:
	methodPattern: "org.assertj.core.api.AssertionsForClassTypes assertThat(..)"
	fullyQualifiedTargetTypeName: "org.assertj.core.api.Assertions"
	- org.openrewrite.java.ChangeMethodTargetToStatic:
	methodPattern: "org.assertj.core.api.AssertionsForInterfaceTypes assertThat(..)"
	fullyQualifiedTargetTypeName: "org.assertj.core.api.Assertions"
	- org.openrewrite.java.ChangeMethodTargetToStatic:
	methodPattern: "org.assertj.core.api.Fail fail(..)"
	fullyQualifiedTargetTypeName: "org.assertj.core.api.Assertions"

Up to now that had mostly worked quite well for us; it's just in this case that we've encountered a difference in the returned types (AbstractObjectAssert vs AbstractCollectionAssert) between AssertionsForClassTypes and Assertions when passing in a collection, and what assertion methods we can call on those return types (hasNoNullFieldsOrProperties not on AbstractCollectionAssert, as that extends AbstractAssert, not AbstractObjectAssert).

Is my expectation just off that folks should be able to use Assertions wherever?
Any conscious choice to have AbstractCollectionAssert not offer hasNoNullFieldsOrProperties?

[scordio]

At first glance, it looks like an oversight on our end. I'll have a deeper look and get back to you!

[scordio]

Some thoughts added at #664 (comment).

Is my expectation just off that folks should be able to use Assertions wherever?

Your expectation makes sense, although there can be corner cases that require a more specific strategy (more in my other comment).

Any conscious choice to have AbstractCollectionAssert not offer hasNoNullFieldsOrProperties?

Yes – in this context, hasNoNullFieldsOrProperties makes little sense as you usually don't want to validate the internal state of a Collection instance.

[timtebeek]

Thanks for chiming in @scordio ! With more details added to the associated issue let's track the suggested next steps there.

• Code does not compile after AssertionsForClassTypes import is replaced by Assertions #664

If it turns out we do want to make code changes to accommodate we can already reopen this pull request.