Subject
: Remove named()
, actual()
, and type parameters
Note: The decisions proposed in this document have already been implemented. We are publishing the document for any users who are interested in our thought process.
Background
Subject
has 2 type parameters
Truth’s Subject
class defines 2 type parameters:
/**
* ...
*
* @param <S> the self-type, allowing {@code this}-returning methods to avoid needing subclassing
* @param <T> the type of the object being tested by this {@code Subject} [i.e., the actual value]
* ...
*/
public class Subject<S extends Subject<S, T>, T> {
(See a footnote note on <S>
1.)
Each type parameter has one purpose:
S
is the return type ofnamed(...)
, our onlythis
-returning method. Test writers use it like this:assertThat(username).named("username").isNotEmpty();
T
is the return type ofactual()
, aprotected
method used bySubject
implementations to retrieve the value under test. Subclasses use it in operations like this:if (!actual().containsEntry(key, value)) { fail(...); }
This proposal (eventually, a few pages from now…) is to remove those type parameters and so, necessarily, to remove the 2 methods that use them. (OK, it’s not strictly necessary to remove the 2 methods, but we’ll discuss that later.)
Self-type parameters make subclassing hard
When you implement a Subject
, you have to decide what to supply for the type
parameters. Your options:
1. Specify concrete values for both type parameters
class ThrowableSubject extends Subject<ThrowableSubject, Throwable>
This is the most popular and convenient option. The problem arises when someone wants to subclass your subject:
a. Calling named
on the subclass will return plain ThrowableSubject
instead
of the subtype. The subclass can override named, but few do.
b. It’s impossible to declare an appropriate Subject.Factory
for the subtype.
At best, you can declare a factory that accepts any Throwable
(which might or
might not be what you want) and returns a plain ThrowableSubject
(unlikely to
be what you want). Both of these are problems that we have inside Truth, and so
do at least some other users. (And likely some other users wanted to make this
work but couldn’t figure it out: The users who are defining such a factory are
mostly doing so because I personally edited their code to define it.) Note that
defining a factory and casting isn’t a convenient solution for users2, as any
non-assert_()
users of the subject (like check()
or expectFailure()
) have
to ensure they pass the right argument type and cast the result.
I think we could half solve (b) by loosening the generics of Subject.Factory
from:
<SubjectT extends Subject<SubjectT, ActualT>, ActualT>
to:
<SubjectT extends Subject<SubjectT, ?>, ActualT>
This would permit declaring a Subject.Factory
that requires a more specific
type for the actual value (e.g., Multiset
instead of Iterable
). However, it
would still return the original subject type (IterableSubject
, rather than
MultisetSubject
). And the subclass would also need to override named
to
solve (a).
2. Declare your own <S, T>
parameters
class ComparableSubject<S extends ComparableSubject<S, T>, T extends Comparable>
extends Subject<S, T>
This is the most flexible option (arguably, the correct option for extensible subjects), but:
a. It’s a mouthful. And keep in mind that subjects may have their own type parameters:
public abstract class TransformedValueSubject<
S extends TransformedValueSubject<S, D, V, O>,
D extends OriginalValueSubject.ValueDescription,
V extends Value,
O extends OriginalValueSubject<O, D, V>>
extends TransformedSubject<S, D, V, O> {
Compare that to a version without <S, D>
(and, as a result, also without
<V>
):
public abstract class TransformedValueSubject<O extends OriginalValueSubject>
extends TransformedSubject<O> {
b. It’s not possible to define a Subject.Factory
for the type itself. If you
want users to be able to create a plain instance of your type, say,
FooSubject
, then you need to declare an
AbstractFooSubject
with the type parameters plus a
FooSubject
that extends
AbstractFooSubject<FooSubject, Foo>
. If you do, the users see two
types, with the assertions defined on a different type than the one they have an
instance of3.
3. A hybrid option, where you specify a concrete actual-value type but declare a self-type parameter
class AbstractCheckedFutureSubject
<S extends [AbstractCheckedFuture]Subject<S, CheckedFuture<?, ?>>>
extends Subject<S, CheckedFuture<?, ?>>
This works well if all your subclasses are happy to accept a plain
CheckedFuture
(rather than having some subclasses that need to require a
specific subclass of that); otherwise, not. Of course, it’s a mouthful, too. And
you still need a non-abstract
subclass if you want users to be able to create
an instance of this plain type, as in 2(b).
4. Give up on extension
Maybe you give up on extending IterableSubject
(or letting people extend your
custom subject) entirely, or you ask for advice, or you realize on your own that
you can delegate to IterableSubject
instead of extending it, implementing
methods wholesale or as needed.
Delegation is especially common with ThrowableSubject
.
We see the IterableSubject
problem in our own ProtoTruth (though the situation
there is more complex).
Now, normally we favor composition over inheritance. However, this is a bit of a
different case: Just as MyException
extends Throwable
, it’s generally
reasonable for MyExceptionSubject
to extend ThrowableSubject
(if it weren’t
blocked by these generics issues). And extension has advantages: Custom subjects
pick up new methods from the superclass automatically, and they’re covered by
any static analysis that finds issues like type mismatches. Plus, any custom
methods added by the custom subject stand out from the default IterableSubject
methods, which are defined in another file. Additionally, it’s (usually; there
are other edge cases) possible to import a new assertThat
method without
breaking existing code, since assertThat(SubFoo)
is likely to expose all the
same assertions as assertThat(Foo)
.
(It is still reasonable for some subjects to choose not to extend an existing
subject type, perhaps to limit the number of assertions they expose to a more
tractable set. (For example, ProtoTruth doesn’t want to expose the no-arg
isInOrder
, since proto classes don’t define a natural order.) I’d just like
for them to have a choice.)
Truth offers multiple ways to add to failure messages
There are a few, and there are likely to be more someday.
Truth currently provides 2 ways for the caller of an assertion to add to the assertion’s default failure message:
assertThat(username).named("username").isNotEmpty();
assertWithMessage("username").that(username).isNotEmpty();
Under the old, prose-style failure messages, the messages these produce looked
significantly different. Under the new, key-value-style failure messages, they
look almost the same: Both put “username” on a new line at the beginning of the
message. The only difference is that named
prefixes it with “name:
.”
In addition to those 2 ways, we recently added another way tailor-made for the specific case in which one assertion is being implemented in terms of another:
check("username()").that(actual().username()).isNotEmpty();
(Note that implementations of Subject
classes naturally
have influence over the failure message in other ways, thanks to other methods
they can call and implement. I mention only check("username")
here
because it’s the most similar to named
and
assertWithMessage
, and in fact people used named
in
place of check("username")
before the latter existed. But keep in
mind that there are plenty of existing options and future possibilities here,
too4.)
On top of the existing ways for callers to add to the failure message, it’s
likely that we’ll add some others in the future. For example, we’ve had several
requests for adding context or scoping assertions. We’ve also speculated about a
Fact
-based method.
You could even argue that we have some other ways of supplying messages, like
assert_().fail(...)
and maybe someday
Truth.fail(...)
– and maybe even an assertThrows
someday. And hopefully we’ll soon automatically infer a good description.
Some subjects fail to include the name passed to named
in their failure messages
About half(!) of custom assertion methods omit it, and so do some assertions in Truth itself. The usual cause is a call to the no-arg check() method. These should someday be fixed, but I have automation for only about half the work, and we may need to add new APIs to support some callers, so the rest won’t happen anytime soon. (Other assertions drop all context, but that is easier to fix.)
Also note that, for most subjects that have subclasses, named
doesn’t return
the right type on the subclasses, thanks to the generics issues described above.
Issue A: Remove named
Users would use assertWithMessage
(or withMessage
) instead.
- + Removing it is (almost) a prerequisite to simplifying type parameters.
- To be clear: This is my primary motivation (though the next bullet,
about how
named
often doesn’t work with custom subjects, is fairly embarrassing, too). - For more on the advantages of simplifying type parameters, see Issue C.
- I say “almost” a prerequisite because we could keep
named
but have it return a plainSubject
.- That would mean that you couldn’t write things like
assertThat(string).named(...).contains(...)
, only things likeassertThat(string).named(...).isEqualTo(...)
. - We could then “fix” that by overriding
named
in all ourSubject
subclasses, but it seems very unlikely that most custom subjects would do that (since presumably they don’t viewnamed
as an essential feature). - We could try to force custom subjects to do it by making
named
beabstract
, but:- The boilerplate would annoy people.
- That wouldn’t solve the problem for subclasses of custom subjects.
- That would mean that you couldn’t write things like
- To be clear: This is my primary motivation (though the next bullet,
about how
- + Some subjects fail to include the name passed to named in their failure
messages.
- To be fair, most assertions use the built-in subjects (or extensions that we own), which we can ensure get this right (though we don’t always get it right currently).
- And we’re phasing no-arg
check()
calls out, anyway. - However, it’s possible (though far from certain) that we’ll someday
introduce some kind of “check, replacing this value in the chain with
the new value.”
named
would require special handling there, though we could probably make it work. - I think
named
is also lost if it’s called in the middle of a chain, even if it uses the overload ofcheck
that accepts arguments. Possibly we could avoid that, but if the user hasfoo.bar.baz
and callsnamed
on the subjects for bothfoo
andfoo.bar.baz
, what should the failure message be? “value of: the baz, aka the foo.bar.baz?”
- + All else equal, 1 way to add context is better than 2 (or n is better
than n+1).
- It’s less for users to understand and less for us to document. (In
particular, it’s one less method on
Subject
, a type that’s already crowded with methods for both users and subclasses.) - It’s less for static analysis to handle. (For example,
TruthSelfEquals
missed some bugs because ofnamed
. For more examples, see the next bullet aboutnamed
.) - It’s less to maintain (bug fixes, feature requests).
- And
assertWithMessage
has 4x as many users asnamed
, so (again, all else being equal),named
is the one to get rid of. It also has faster growth, even in relative terms (>6% vs. <3% over the last ~4 months.)
- It’s less for users to understand and less for us to document. (In
particular, it’s one less method on
- + We have just a shred of evidence that users sometimes think of “named” as
an assertion, so they write “assertions” like
assertThat(description).named("expected-description");
.- A similar problem appears to be with “assertions” of the form
assertThat(someBoolean).named("something");
. - Or, occasionally, users appear to just forget to write the assertion
after filling in the name. (Perhaps that’s easier to do when you’ve
written
assertThat(foo).named(...)
than when you’ve writtenassertWithMessage(...)
(which doesn’t even include the actual value), and perhaps it stands out less, too?) - But both of these ought to be caught by
@CheckReturnValue
(like a plainassertThat(someBoolean);
“assertion” is); we just never got around to removing@CanIgnoreReturnValue
fromnamed
. - Actually, wait, that’s not quite true: Because
named
is a mutator, it’s actually “fine” to callnamed
on an object and ignore the return value. For example, we have several callers who call[this.]named(...)
in their constructors. We also see it called on delegate subjects. - Still, this is at least a small negative, especially for users without
@CheckReturnValue
enforcement.
- A similar problem appears to be with “assertions” of the form
- + This also allows us to get rid of
actualAsString()
andinternalCustomName()
. - + It’s the one thing that makes
Subject
instances mutable.- Admittedly, I don’t know of any concrete problems that this has
caused – except arguably for discouraging us from enabling
CheckReturnValue
enforcement, as described in an earlier bullet. - Because some
Subject
implementations callnamed
internally (eitherthis.named(...)
orcheck(....).that(...).named(...)
(orassertThat(...).named(...)
if they haven’t been moved tocheck
:)), it’s possible that their name gets overwritten by a later user-specified name. I haven’t seen this come up, but then I haven’t looked at all users of the possibly affectedSubject
implementations. We could in principle fix this by “stacking” multiplenamed
calls (like what we do with multiplewithMessage
calls) (so this bullet isn’t really about mutability per se; it just didn’t feel important enough for a top-level bullet), but it’s more work we should do if we keep things as they are.
- Admittedly, I don’t know of any concrete problems that this has
caused – except arguably for discouraging us from enabling
- ~ The policy for when to include
named
in the failure message has changed over time and may be confusing.- The old
fail*
methods mostly included it, including (perhaps surprisingly)failWithoutActual
/failWithoutSubject
but not (probably _un_surprisingly but still likely to bite someone)failWithRawMessage
. (failWithRawMessage
used to be another very common case in whichnamed
was ignored.) However, we changedfailWithRawMessage
(and its modern equivalent) to include the name. This is probably good overall, but it also means thatnamed
(andassertWithMessage
, if we migratenamed
users to that) may be duplicating information that was manually added by the call tofailWithRawMessage
.
- The old
- ~ I see at least some evidence that people are filling in
named
even in cases in which Truth already includes the information they’re providing.- As an example, several people are passing names like “Exception message” to assertions that will already explain that.
- ~ It’s possible that
named
andwithMessage
encourage different kinds of messages.- Maybe users are more willing to use a short description of the actual
value for
named
than forwithMessage
? (I haven’t tried to investigate.)- Maybe it’s good to encourage short names?
- There’s no sense in writing “The foo was expected to be null” when you can communicate the same information by writing “foo.”
- If engineers don’t feel obligated to write long descriptions, maybe they’ll write more?
- Maybe it’s bad?
- Messages that give additional information (like the values of variables or the reason that the test exists) are more useful than names that just save you a trip to the test source file.
- Maybe it’s good to encourage short names?
- Maybe users are more willing to use a short description of the actual
value for
- ~ Some people may find that some assertion statements read more naturally with the name inline – or, conversely, with the message out-of-line.
- ~ AssertJ has a method on its
Subject
-like class like this.- Though it’s displayed slightly more like our
withMessage
(which is a method onStandardSubjectBuilder
, not onSubject
) than ournamed
. (Their withFailMessage replaces the failure message, rather than adding to it.) - - So, if we leave it out, this may slightly complicate migration from AssertJ to Truth.
- But we don’t have to follow AssertJ (nor FEST, which was probably our inspiration).
- Note also that AssertJ more strongly encourages proper self-typing by
(a) providing both
FooSubject
andAbstractFooSubject
in all(?) cases and (b) accepting aClass<S>
parameter in their Subject-like class’s constructor.
- Though it’s displayed slightly more like our
- ~ If a particular custom
Subject
wants to support this, it can add its own.- Of course we would feel bad if users chose to do this often, but if it were particularly important somewhere, it’s an option (and maybe there’s even a way they can make it better for their specific use case??).
- ~ Since we’d migrate calls from
named
toassertWithMessage
, it’s possible that we’ll produce some less than ideally phrased failure messages.- However, the real damage here was already done when we changed the format of our failure messages: That’s what moved the given name from inline (“Not true that my object (<…>) was true”) to a separate line, prefixed by “named:.” The main thing we’re doing now is removing that “named” prefix.
- - Requires an LSC to migrate existing users to
assertWithMessage
(or, if we’re up for it, sometimes tocheck(String, Object...)
).- This can be done with Refaster for the common cases but requires Error Prone for full generality. It’s doable.
- This includes some Kotlin.
- - Will break external users’ code.
- We’ll release Error-Prone-powered migration tools, but users would need to run them, and that requires some up-front investment to set up Error Prone.
- (Or, as an easy “fix,” users can just remove
named
calls.)
- -
named
is more convenient for users of custom subjects.- Large caveat: That’s only true when it works. (And see also the cases in
which
named
doesn’t return the original subject type.) - But, when it does work, compare:
assertThat(foo).named("foo").hasBar(bar);
assertWithMessage("foo").about(foos()).that(foo).hasBar(bar);
- (And that’s assuming that the author of
FooSubject
exposed aSubject.Factory
– which all should (since the lack of one causes problems other than this) but not all do. (I don’t have numbers, but I’d guess that a majority do. We should write Error Prone checks for this.))- This makes editing an existing assertion to add a message more complex.
- It may discourage users from adding messages at all.
- Large caveat: That’s only true when it works. (And see also the cases in
which
- - Maybe people like the appearance of having every assertion begin with
assertThat(
?- But that’s already not the case:
assertWithMessage
exists (and is a better fit thannamed
in some cases, asnamed
is intended only for naming a specific value, not for giving additional background like the values of other variables or the reason that the test exists).- So do
expect
and other alternative failure strategies, which sometimes requireexpect.about(...).that(...)
instead ofexpect.that(...)
.
- But that’s already not the case:
Another way that people may be interested in looking at this: What has changed
since we originally added named
? named
was added in
version 0.17 in 2014 before Truth became a Google project developed in our
depot, so it didn’t go through API Review, but it’s still useful to consider
what’s changed:
- We’ve added
assertWithMessage
andwithMessage
(originally namedwithFailureMessage
but subsequently shortened), which offer similar functionality.- (Note again that
named
behaves almost identically withwithMessage
under the new failure message style. That wasn’t the case with the failure-message style we were using whenwithMessage
was first added.)
- (Note again that
- We’ve added
check(String, Object...)
, which is superior tocheck().that(...).named(...)
. - We should soon automatically infer a description of the actual value in some
cases. Hopefully this (and other changes, like to always include the
(now-trimmed) stack trace when using
Expect
) will address common use cases for names and custom messages, including boolean assertions. - We have more static analysis (which
named
complicates), more custom subjects (which often don’t work right withnamed
), and more users (to be bit by the preceding problems). - Additionally, the
named
functionality seems to have come from FEST), which, besides not offeringwithMessage
, already has a self-type parameter for chaining multiple assertions on the same value, as inassertThat(x).isNotNull().isNotEqualTo(other).contains(x)
. So even if FEST dropped its equivalent tonamed
, that wouldn’t permit it to remove its self-type parameter.
Issue B: Remove actual
Each subclass would have to declare a field of the appropriate type and store
the actual value during its constructor. (It’s legal Java for every class in a
hierarchy to declare a private
field named actual
.)
+ @Nullable private final Integer actual;
+
protected IntegerSubject(FailureMetadata metadata, @Nullable Integer integer) {
super(metadata, integer);
+ this.actual = integer;
}
- + Removing it is (almost) a prerequisite to simplifying type parameters.
- This is my primary motivation (like in Issue A, only more so).
- For more on the advantages of simplifying type parameters, see Issue C.
- I say “almost” a prerequisite because, if we really wanted to remove the
type parameters but keep
actual
, we could do it by changingactual
to returnObject
.- Subclasses could then use it by casting to the appropriate type.
- Casts are probably more error-prone (and disliked) than fields.
- The casts would be unchecked in some cases.
- We could also make it overridable so that subclasses can redefine it
to return the appropriate type.
- But they might think they can do weird things like effectively
change the actual value by overriding the method, which might or
might not work, depending on the implementation of each any
individual assertion.
- We saw something like this when a user overrode one of the
fail*
methods.
- We saw something like this when a user overrode one of the
- And an override makes
actual
visible to users of the subject in the same package.
- But they might think they can do weird things like effectively
change the actual value by overriding the method, which might or
might not work, depending on the implementation of each any
individual assertion.
- I don’t think anyone really wants this; the value of
actual
is primarily as a convenient way to return a typed value.- (This does save a field, if anyone cares about that. But Truth
already does more inefficient things than have multiple
actual
fields.) - We could look into how many subclasses actually need the typed
actual value, not just
Object
. We suspect that most do.
- (This does save a field, if anyone cares about that. But Truth
already does more inefficient things than have multiple
- Another note: Removing only one type parameter from
Subject
is harder than removing both.- That’s because removing one would have to be done atomically,
while removing both can be done by gradually making all subjects
extend raw
Subject
and later removing the type parameters. - So, if we want to remove
named
and the pseudo-self-type parameter, then it’s simplest to removeactual
and its type parameter, too. - If not for that, I’d consider keeping this method and type
parameter in place (along with loosening the type parameters of
Subject.Factory
, as discussed above).
- That’s because removing one would have to be done atomically,
while removing both can be done by gradually making all subjects
extend raw
- Subclasses could then use it by casting to the appropriate type.
- + It removes one of the subclass-facing methods from the crowded
Subject
type, which users look at primarily for its assertion methods.- To be fair, it’s a
protected
method, anyway, so, while users see it in source and Javadoc, they don’t see it in autocomplete. - And there are several other methods like this, including others beginning with the word actual.
- To be fair, it’s a
- + This may help static analysis, like nullness analysis, which can
recognize that a
final
field won’t change between reads but has a harder time recognizing that a method (declared in a separate file) won’t return different values between calls.- Example:
if (actual() == null || actual().doubleValue() != 0.0) {
- (Note that nullness in the type system doesn’t necessarily avoid this
problem: We might well want to let users write
assertThat(possiblyNullDouble).isZero()
without first performing their own null check. Or maybe not; it’s not clear that users will have a universal preference here, nor is it clear whether we can support any given behavior, let alone a configurable one. It’s possible that full support for nullabililty (whatever that means) would require including the actual-value type parameter on all classes, evenfinal
ones, and on allSubject.Factory
andassertThat
methods, to distinguish between “aSubject
for aFoo
” and “aSubject
for aFoo
ornull
.”)
- Example:
- + Each usage of the actual value is a whole two characters shorter :)
- ~ Subclasses could choose a different name than
actual
for their fields.- + This could be helpful if users need to distinguish between one
view/part of the actual value and another (like how our
StreamSubject
has anactualList
that it uses for itsstream
-specific operations). - - This could be less clear to readers than sticking with the convention.
- + This could be helpful if users need to distinguish between one
view/part of the actual value and another (like how our
- ~ We recently heard from some Kotlin users of Truth who are reflectively
calling
actual()
as part of implementing extension methods on subjects. They would have to reflectively access theSubject.actual
field that will still exist. Downsides for them:actual()
, whileprotected
and notpublic
(hence the need for reflection), is at least an API exposed to other packages. Theactual
field, by contrast, would beprivate
.- This might not matter in practice – would we really rename or remove the field? – but would at least feel icky.
- But I expect that we’ll provide a real, public method for this
someday – say,
ExtensionMethods.getActualValue(Subject)
, backed by a package-privateactual()
accessor.
- The field type would be plain
Object
, rather thanT
.- (Strictly speaking, that’s more a result of removing the type
parameters (Issue C), not of removing
actual
.) - The same would be true of the return type of the hypothetical
ExtensionMethods.getActualValue(Subject)
. - So callers would have to cast, and sometimes the cast would be unchecked.
- That’s probably tolerable for the unusual case of extension methods, but it’s not ideal.
- (Crazy workaround that I’m not sure I’d actually want: Let callers
pass a
Subject.Factory
for theSubject
itself as a type hint. But not all subjects have one (e.g., ProtoTruth has aCustomSubjectBuilder.Factory
), and anyway, it’s kind of weird to use the factory as a typing hint – especially when another factory may well be involved for creating a derived subject.)
- (Strictly speaking, that’s more a result of removing the type
parameters (Issue C), not of removing
- ~ It looks a little weird to some of us nowadays to see a constructor that
assigns a parameter to a field without calling
checkNotNull
first :) - ~ For whatever reason, ~10% of
Subject
implementations already callactual()
and assign the result to a local variable inside an assertion method. - - Without
actual
, subclasses need 2 lines of boilerplate to manually store the actual value.- The hope is that this is offset somewhat by removing the type parameters. In rare cases, removing the type parameters could even help more than it hurts. But on average, boilerplate would increase.
- Comparison: Consider that AssertJ (like Truth currently)
doesn’t require those 2 lines, as it has a
protected actual field
. (It does require the type parameters.)
- - It also just feels weird for the subclass, which knows
that the superclass is storing exactly the value that it needs, to
refuse to expose it.
- Counterargument from the similar case of injecting dependencies into a
constructor that calls a super-constructor: Some people argue that, if
the subclass needs a value that the superclass also needs, it’s still
preferable to both pass the value to
super()
and also store a local copy. This avoids tying the two classes closely together. So, for example, if the superclass no longer needs a parameter, it doesn’t need to continue to store it and expose an accessor for it (or, alternatively, force the subclass to migrate off the accessor when it removes it).
- Counterargument from the similar case of injecting dependencies into a
constructor that calls a super-constructor: Some people argue that, if
the subclass needs a value that the superclass also needs, it’s still
preferable to both pass the value to
- - Static analysis that looks for operations on the actual value will need
more logic to detect each class’s
actual
field (rather than the standardactual()
method). - - Requires an LSC to migrate existing users to declare and use a field.
- This again requires Error Prone, but it’s straightforward.
- This again includes some Kotlin.
- - Will break external users’ code.
- We’ll release Error-Prone-powered migration tools, but users would need to run them, and that requires some up-front investment to set up Error Prone.
- It’s also pretty straightforward to do by hand.
- “Hopefully” not too many people are writing custom subjects externally.
- We are planning to make a 1.0, after all.
Issue C: Remove type parameters
I propose to remove both parameters. (Removing only one is much more difficult, as discussed above.)
To re-reiterate: This is the primary goal of all the proposals in this doc.
- + Self-type parameters make subclassing hard.
- Removing the type parameters also addresses the issue that actual-type parameters make subclassing hard, too, though that one could be solved in other ways.
- (Subclassing difficulties could be particularly bad if we ever explore
codegen for subjects (example), which would likely be implemented with
subclassing, like
AutoValue
.)
- + Simpler for users of subjects.
- That’s true in various situations:
- reading the code
- looking at the Javadoc
- interpreting compilation errors
- See again an extreme example above.
- This is another area where we can differentiate ourselves from AssertJ, which users have told us is more complex than Truth.
- That’s true in various situations:
- + Opens the door to re-adding element-type parameters to
IterableSubject
, etc.- (Adding a type parameter now, when we already have type parameters, is
hard, as discussed above. Plus, 3 type parameters (maybe more for types
like
MultimapSubject
andTableSubject
) looks especially scary, particularly when they don’t map directly to the type parameters of the underlying actual-value type.) - Benefits:
- This can support static analysis, like how Error Prone looks for
type mismatches in calls to
Collections.contains
. - This could provide better compile-time type-checking for users of
isInOrder
. - This could provide better compile-time type-checking for users of Fuzzy Truth.
- This could provide better type inference for users of Fuzzy Truth.
- This can support static analysis, like how Error Prone looks for
type mismatches in calls to
- However, we’re not deciding on this yet:
- It’s not a 1.0 blocker, as the parameters are safe to add later.
- (The possible
isInOrder
and Fuzzy Truth changes are binary-compatible but not necessarily source-compatible. We’d have to take that into account. But even if we don’t make those changes, we can still benefit from improved static analysis.)
- (The possible
- We’d want to check whether it would still be safe to static import
both
Truth.assertThat(Iterable)
andProtoTruth.assertThat(Iterable)
. I think we currently get away with statically importing both because only ProtoTruth has a type parameter? Or maybe it would still work as long as ProtoTruth’s parameter is more specific (which it will be)?
- It’s not a 1.0 blocker, as the parameters are safe to add later.
- (Adding a type parameter now, when we already have type parameters, is
hard, as discussed above. Plus, 3 type parameters (maybe more for types
like
- + Makes it a no-brainer to remove
DefaultSubject
. - + Should let us eliminate
LiteProtoSubject.Factory
as a public type. - + Should make it possible to create an API like
assertThat(future).value(strings()).startsWith("foo")
, should we want that someday. - ~ (Almost) requires removing
named
andactual
, as discussed in previous issues. - - Requires an LSC to remove the type parameters from existing subclasses.
- This should mostly be doable even with sed.
- For extra expediency, we could initially keep any self-type and
actual-type parameters declared in custom subjects. (For example,
class MySubject<S extends MySubject<S, A>, A> extends Subject<S, A>
could continue to declare<S ..., A>
, even though we’d switch it to extend unparameterizedSubject
. We could clean up the unused type parameters later.) - This again includes some Kotlin.
- - Will break external users’ code.
- “Hopefully” not too many people are writing custom subjects externally.
- And this one is mostly fixable with
sed
. - This change is at least binary-compatible, just not source-compatible.
- - Cuts off other potential uses of self types.
- For example, AssertJ offers
inHexadecimal()
andinBinary()
.- However, I don’t think we’ve seen demand for features like this in our years of maintaining Truth. (I didn’t find any feature requests for hex or binary specifically, though I could believe they’ve come up.)
- AssertJ also defines most assertion methods to return the
Subject
, enabling chains likeassertThat(x).isNotNull().isNotEqualTo(other).contains(x)
, which we’ve decided not to support. - AssertJ also offers
Predicate
-accepting methods, which can be type-safe with a self type.- We have seen requests for this, and we plan not to implement it. However, it’s possible that we’d consider something based on Correspondence someday.
- We have also seen requests for
Comparator
-based comparisons, which would be only half type-safe without the actual-value parameter. - Even if there are problems that self types can solve, we may be able to
find alternative solutions:
- We could still add
IntegerSubject.inHexadecimal()
, though it would return a plainIntegerSubject
. That could be awkward if someone subclassed it, but:- No one does currently internally.
- The
IntegerSubject
assertions will still be available, even if the subclass ones aren’t, and that might often be good enough. - A subclass could override
inHexadecimal()
to declare a more specific return type.
- We could automatically display values in hexadecimal/binary for any assertions we add for which that would be appropriate, like bitwise assertions. (For bitwise assertions, maybe we could do even better and figure out the names of the constants in some cases??)
- We can offer features in non-
Subject
classes, likeCaseInsensitiveStringComparison
. This doesn’t really solve the problem, but it makes clear that, if you define a subclass, theignoringCase()
method is still going to return a plainCaseInsensitiveStringComparison
, not some subtype of yours, unless you explicitly override it.
- We could still add
- For example, AssertJ offers
Again, it may be useful to review what has changed in the past several years.
That includes considering how Truth is different from FEST (which seems to have
been our inspiration for the type parameters). FEST uses the self type for
chaining multiple assertions on the same value, as in
assertThat(x).isNotNull().isNotEqualTo(other).contains(x)
; Truth
does not. So even if FEST dropped its equivalent to named
, that
wouldn’t permit it to remove its self-type parameter. Truth has a better case
for dropping it, and one of the original authors had considered doing so (though
dropped the effort for reasons I’m unsure of – perhaps just that we had more
pressing issues).
-
“Avoid needing subclassing” might not be the best way to put this. The point is that we can declare a method that returns
S
, and we need to implement it only a single time inSubject
itself. ↩ -
self-nitpicking: OK, it’s also the type of the constructor parameter that subclasses call to pass that actual value to
Subject
. However,Subject
uses the actual value only for assertions likeisNotNull()
, where it doesn’t need to know that it’s specifically aT
. TheT
type specifically is in service ofactual()
, which is in service of subclasses. ↩ -
It turns out that a full (OK, almost full) solution does exist. It hadn’t occurred to me, but some users found it. The solution is to use
CustomSubjectBuilder
. This lets you force the input to be of whatever type you want and lets you return whatever subject type you want. In short, because you can’t write:public static Factory<FooSubject, Foo> foos() { return FooSubject::new; }
You instead write:
public static CustomSubjectBuilder.Factory<FooSubjectBuilder> foos() { return FooSubjectBuilder::new; } public static final class FooSubjectBuilder extends CustomSubjectBuilder { FooSubjectBuilder(FailureMetadata metadata) { super(metadata); } public FooSubject that(Foo actual) { return new FooSubject(metadata(), actual); } }
I am simultaneously horrified that this is necessary, impressed that some people found it, and tickled that
CustomSubjectBuilder
ended up satisfying this unforeseen use case. ↩ -
You could also make
ViewSubject
the abstract type. Then you’d create a privateConcreteViewSubject
subtype and a factory for the subtype. I think this will work, but you’ll have to expose the privateConcreteViewSubject
type in the publicviews()
method that exposes your factory, and then yourassertThat
method will either have to expose it again or else declare a return type ofViewSubject<?, ?>
, which is a little weird in its own way. (And it gets worse if you have other type parameters that you want to survive a call tonamed
, like what we used to have on IterableSubject.) ↩