Unable to construct iterable assertions across a specific type easily

[ascopes]

Within the new AssertJ 3.26.0 release, I can see that the factory based navigable assertion for iterables has been deprecated internally, along with other mechanisms to do the same thing.

This raises an issue for me, as I extend the AssertJ API and provide additional custom assertions that wrap the assertj ones.

In this case, I have a requirement to be able to return an assertion object across a list of strings for the user to be able to perform further assertions on. The current implementation looks like this: https://github.com/ascopes/java-compiler-testing/blob/1a92ef59df49cf2f45ae83a109569be006b08df2/java-compiler-testing/src/main/java/io/github/ascopes/jct/assertions/JctCompilationAssert.java#L70.

Unfortunately with these APIs being deprecated, I can not find a way to return an iterable assert across strings that automatically expand to string assertions. I can see new API methods such as .first(AssertFactory) and last(AssertFactory), but no way to instruct AssertJ how to expose assertions on internal objects anymore such that the user does not need to explicitly cast these assertions with an explicit InstanceOfAssertFactory object.

Is there any advice for how to migrate off of this, or is this something that will need to be a breaking API change on my side?

[scordio]

Thanks for your feedback, @ascopes!

Please give me a bit of time to get familiar with your use case – I'll get back to you.

[scordio]

Although it's possible to obtain the previous behavior, similar to how you did it with TypeAwareListAssert, I believe we should provide better support for such use cases.

However, I wouldn't reintroduce what we've been deprecating as it's less discoverable.

What if we would add an API to configure the assertions for the available navigation methods?

Something like:

assertThat(List.of("a", "b", "c"))
  .withElementAssert(StringAssert::new)
  // further assertions on the list
  .hasSize(3)
  // navigation method
  .first() // returns StringAssert instead of ObjectAssert<String>
  // further String assertions on the element
  ...

Then, your example would become:

public AbstractListAssert<?, List<? extends String>, String, ? extends AbstractStringAssert<?>> arguments() {
  isNotNull();

  var arguments = actual.getArguments();

  return Assertions.assertThat(arguments)
      .withElementAssert(StringAssert::new)
      .as("Compiler arguments %s", StringUtils.quotedIterable(arguments));
}

[ascopes]

This sounds like a very good idea, and would be very helpful! I struggled with getting generics to work consistently for this so any help would be greatly appreciated like this on AssertJ's side to simplify this.

[scordio]

Would it be okay if we introduce such a change in 3.27.0?

I'd prefer to have enough time to digest it properly, and the deprecations will only be removed in version 4.

[ascopes]

Sounds sensible to me! I'd need to release a new version to change to a new API anyway so there is no rush

[Adrodoc55]

I also heavily use FactoryBasedNavigableListAssert and love the proposal for withElementAssert. I actually wanted to open an issue for improving the generics, but then saw this issue about it beeing deprecated. So I'll post here.

Currently FactoryBasedNavigableListAssert and by extension the deprecated assertThat method requires the ELEMENT_ASSERT to extend AbstractAssert<ELEMENT_ASSERT, ELEMENT>. This sounds reasonable at first and works fine for concrete assert classes, but the bound is too tight for generic assert classes such as abstract ones.

Consider the following example:

import java.util.List;
import org.assertj.core.api.AbstractAssert;
import org.assertj.core.api.AbstractIntegerAssert;
import org.assertj.core.api.AssertFactory;
import org.assertj.core.api.Assertions;
import org.assertj.core.api.ClassAssert;
import org.assertj.core.api.FactoryBasedNavigableListAssert;

public class Test {
  public static void main(String[] args) {
    List<Class<?>> actualClasses = List.of(Integer.class);
    ClassAssert classAsserter1 = Assertions.assertThat(actualClasses.get(0));
    ClassAssert classAsserter2 = Assertions.assertThat(actualClasses, Assertions::assertThat).element(0);
    ClassAssert classAsserter3 = assertThat2(actualClasses, Assertions::assertThat).element(0);

    List<Integer> actualIntegers = List.of(0);
    AbstractIntegerAssert<?> integerAsserter1 = Assertions.assertThat(actualIntegers.get(0));
    AbstractIntegerAssert<?> integerAsserter2 =
        Assertions.assertThat(actualIntegers, Assertions::assertThat).element(0);
    AbstractIntegerAssert<?> integerAsserter3 = assertThat2(actualIntegers, Assertions::assertThat).element(0);
  }

  public static <ACTUAL extends List<? extends ELEMENT>, ELEMENT, ELEMENT_ASSERT extends AbstractAssert<? extends ELEMENT_ASSERT, ELEMENT>> FactoryBasedNavigableListAssert<?, ACTUAL, ELEMENT, ELEMENT_ASSERT> assertThat2(
      List<? extends ELEMENT> actual, AssertFactory<ELEMENT, ELEMENT_ASSERT> assertFactory) {
    return null;
  }
}

For the List<Class<?>> everything works fine, because assertThat(Class<?> actual) returns the non-generic type ClassAssert, but for integers it does not compile. The reason is that AbstractIntegerAssert<?> which is the return type of assertThat(Integer actual) does not satisfy the bound i mentioned above: AbstractAssert<ELEMENT_ASSERT, ELEMENT>. The bound should really be AbstractAssert<? extends ELEMENT_ASSERT, ELEMENT>. To prove this change makes it work I added the method assertThat2 in the example. This method doesn't compile because the incorrect upper bound also appears in the declaration of FactoryBasedNavigableListAssert and its superclasses, but the call to assertThat2 does compile.

[scordio]

Thanks a lot for your feedback, @Adrodoc55. I'll keep that in mind!