Provide support for Kotlin extension methods on Subjects

[vRallev]

The title might be a little off. We recently converted the Point of Sale Android codebase at Square from AssertJ to Truth and used Kotlin for our custom Subjects. I try to walk you through the issues I ran into, maybe it is beneficial for your future API considerations.

Kotlin supports extension function, what allows "extending" existing Subjects easily and in a fluent way. My goal was to extend the StringSubject with checks that compresses multiple whitespaces into one similar ignoreCase().

assertThat("hello   world").compressWhitespaces().isEqualTo("hello world")

From Java's point of view compressWhitespaces() is like a static method that has StringSubject as argument.

The first problem I ran into is that actual() is protected final in Subject. There wasn't a way for me to get the actual value from the StringSubject that I'm "extending". (My solution is to use a reflective call for now)

Similar to StringSubject.CaseInsensitiveStringComparison I planned to implement the checks in a class, that doesn't extend Subject. This way I can add methods like isEqualTo() when I actually implement them. There were multiple problems with this approach:

1. CaseInsensitiveStringComparison is an inner class, it gets access to actual() and other methods through StringSubject. My class doesn't have access to these methods. My biggest problem was that I couldn't construct failure messages easily. I have the impression that Subject has too many concerns: It provides the API for checks, but also gives you access to methods to create error messages. It would be helpful, if both things were separated.
2. So my class had to extend Subject. I could hide the implementation behind an interface, but this wouldn't allow providing a factory method that creates the Subject.Factory, what is necessary for assertAbout(..) and quite useful in Java.

assertAbout(compressWhitespaces()).that("string   string").isEqualTo("string string")

3. I had to expose that my class extends Subject, meaning every caller sees all the methods, that I might not have implemented. In fact, my class implements isEqualTo() right now, but not isNotEqualTo(). You could treat it like a not finished implementation for the API and I could throw an exception / error when calling this method, but from an API point of view the caller doesn't know that. They shouldn't see the method at all. This leads to false positives at the moment.

assertAbout(compressWhitespaces()).that("string   string").isNotEqualTo("string string") // doesn't throw at the moment

To summarize those are the things I wish would be available or different.

• Get access to the actual() value from outside.
• Make creating error messages possible without extending Subject.
• Split the default check methods into separate classes or interfaces and provide default implementations in a composable way.

[cgruber]

This sort of relates to #204 a bit.

[cpovirk]

Thanks very much for all the detail. I was intrigued when you mentioned using extension methods like this, so it's sad to hear that they don't quite work as is.

One initial question to buy me time to think about this more: In this particular case, how much does it help to put compressWhitespaces() on String instead of StringSubject? That of course doesn't help with the ignoreCase() use case, where you have to transform both the expected value and the actual -- and maybe you need to transform both values here, too?

(I'm wondering if this is going to lead us to create a separate ExtensionMethods class that provides the access you need.)

[vRallev]

I need to transform both values, so the API wouldn't be nice if compressWhitespaces() is on the String. My goal was also that people type assertThat(string). and get a list of all options with auto-complete to improve discoverability.

[lowasser]

Copying some internal discussion and thoughts. I was actually writing some Kotlin Truth subjects just as this issue got posted, so I had a couple.

I think @cgruber is right that the linked issue, about reorganizing subjects, would help. Having isEqualTo on Subject definitely kind of bakes in an assumption that there's exactly one Subject per type, and they'll all e.g. have the same notion of equality.

I'd personally be hesitant to expose actual() directly, but I could see providing a method on Subject that makes it possible for an extension function to return a new or modified Subject. I find myself wondering about exposing something that derives a new Subject from the existing actual value, preserving existing metadata about failure. But there's already an API that accepts a value and metadata about failure, and returns a Subject about that value -- it's Subject.Factory. So maybe some API accepting a "continuation" Subject.Factory...?

To be clear, writing vanilla Subjects in Kotlin is quite doable and pleasant, and the ability to have top-level assertThat is nice. But you can't really write useful extension functions on Subjects with the current API (other than those which just completely delegate to another assertion). That includes both terminal assertions and nonterminal extensions which have to propagate information.

[ronshapiro]

Shall we merge this with #404?

[cpovirk]

Let's keep this one open, since it's more about extension methods. I just closed #404 along with the other requests for Predicate-accepting assertion methods. I'll update the title of this one to be more specific.

[cpovirk]

Two notes:

• We're going to somewhat regress here in the next couple Truth releases as we remove actual(). I've been assuming that you can reflectively access private Object actual just as well as you can access protected A actual(), though I grant that the private access is a little dirtier, since it could theoretically be removed or renamed at any time. We don't intend to remove it without providing you with something better (or remove it ever, in all likelihood, but no promises on that :)). Let me know if that will be a big problem, and we can look into expediting some other solution.

• We might like something like "Make creating error messages possible without extending Subject" / SubjectProtectedMethods inside Truth itself for IterableSubject.UsingCorrespondence (especially now that it's a static class for weird Java-7-compatibility reasons). However, I haven't thought this through.

[cgruber]

I admit to some anxiety about relying on reflective access, but I can see why you're doing it. I would suggest that in the process, that reflective access actually be through a utility which has a consistent API, to avoid having subject implementers rolling their own reflection, and insulating internal changes a bit.

[cpovirk]

Agreed, we should provide a non-reflective API for this. (We should be able to avoid reflection even in the implementation, since there's still a package-private actual field on Subject, not private as I'd claimed above.)

[efemoney]

Running into this issue (somewhat). My use case is also very similar:

I need to assert that certain lines of a multi-line string (coming from an existing subject chain) starting with a specific token, end with another token

Something like:

assertThat(gradleBuild()) // 3rd party truth extension
  .output() // multi line sting -> StringSubject
  .lines() // extension I want to add -> 'LinesSubject'
  .startingWith("my-start-token") // Filtered 'LinesSubject'
  .endsWith(expected)