Moe Sync#575
Conversation
…tosSubject. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=252055248
…m things. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=252693716
Thoughts: - Now that the main page addresses the comparison to AssertJ (and links to the more detailed comparison), I figure that's less important. - FAQ seems like a good thing to put first after Home. - Known Types is more of a reference, and the Javadoc is perhaps a better reference. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=252696923
RELNOTES=n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=252699810
| Under Truth, `assertThat(listOfStrings).doesNotContain(integer)` passes, even | ||
| though your test is probably buggy. Under AssertJ, it doesn't compile. | ||
| though your test is probably buggy. Under AssertJ, it doesn't compile. We plan | ||
| to add static analysis to [Error Prone] to catch such bugs. |
There was a problem hiding this comment.
@cpovirk I'm curious to know why this something you need static analysis for rather than having the compiler check it for you. Would you mind explaining the rationale behind this? :)
There was a problem hiding this comment.
It's a similar argument to https://errorprone.info/bugpattern/CollectionIncompatibleType
Plus, when type inference fails on a call that should be permitted, the error messages are ugly:
javatests/com/google/common/collect/SetsTest.java:779:
containsExactly(java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>...)
in com.google.common.truth.IterableSubject<capture#106 of ?
extends com.google.common.truth.CollectionSubject<?,java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>,java.util.Collection<java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>>>,java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>,java.util.Collection<java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>>>
cannot be applied to
(java.util.List<java.lang.Object>,java.util.List<java.lang.Object>,java.util.List<java.lang.Object>,java.util.List<java.lang.Object>)
assertThat(cartesianProduct(x, y)).containsExactly(exp1,
exp2, exp3, exp4);
I'm still not 100% convinced that we made the right choice, but that's the theory. I'll note that in the docs. Thanks.
There was a problem hiding this comment.
Hmm, actually, that's an old message from the days in which Subject and its subclasses had type parameters. Let me see what it would look like today.
There was a problem hiding this comment.
It looks like probably even from an older javac, too.
There was a problem hiding this comment.
(Back in the day, we didn't have @SafeVarargs, either, but that's now a non-issue.)
There was a problem hiding this comment.
And interestingly, that particular error might not happen with the current javac.
So, while the logic of CollectionIncompatibleType still applies, E... has certainly gotten more attractive from all the other changes.
There was a problem hiding this comment.
No, wait, I messed up my change to IterableSubject. Everything I said above is wrong :)
There was a problem hiding this comment.
OK, trying again: Truth-that-requires-T would still fail on the cartesianProduct example, even under a new javac. The error is now:
core/src/test/java/com/google/common/truth/IterableSubjectTest.java:56: error: method containsExactly in class IterableSubject<E> cannot be applied to given types;
.containsExactly(exp1, exp2, exp3, exp4);
^
required: List<INT#1>[]
found: List<Object>,List<Object>,List<Object>,List<Object>
reason: varargs mismatch; List<Object> cannot be converted to List<INT#1>
where E is a type-variable:
E extends Object declared in class IterableSubject
where INT#1,INT#2 are intersection types:
INT#1 extends Object,Serializable,Comparable<? extends INT#2>
INT#2 extends Object,Serializable,Comparable<?>
And, as a digression here into something else relevant to my interests, the error message is just slightly longer if IterableSubject has self-type and actual-type parameters:
core/src/test/java/com/google/common/truth/IterableSubjectTest.java:56: error: method containsExactly in class IterableSubject<S,A,E> cannot be applied to given types;
.containsExactly(exp1, exp2, exp3, exp4);
^
required: List<INT#1>[]
found: List<Object>,List<Object>,List<Object>,List<Object>
reason: varargs mismatch; List<Object> cannot be converted to List<INT#1>
where S,A,E are type-variables:
S extends IterableSubject<S,A,E> declared in class IterableSubject
A extends Iterable<E> declared in class IterableSubject
E extends Object declared in class IterableSubject
where INT#1,INT#2 are intersection types:
INT#1 extends Object,Serializable,Comparable<? extends INT#2>
INT#2 extends Object,Serializable,Comparable<?>
[edit years later: One case in which it's easy to see how the looser types are useful is if your input is a List<?>.]
There was a problem hiding this comment.
It's a similar argument to https://errorprone.info/bugpattern/CollectionIncompatibleType
Ahh, okay. I'm reasonably happy to accept that, then. :)
…or worse. #575 (comment) RELNOTES=n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=253032775
…or worse. #575 (comment) RELNOTES=n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=253032775
4 participants
This code has been reviewed and submitted internally. Feel free to discuss on the PR and we can submit follow-up changes as necessary.
Commits:
Update docs for upcoming removal of type parameter from IterableOfProtosSubject.
9260675
Update comparison, especially about failure messages, and other random things.
a9fbb70
Somewhat arbitrarily shuffle the order of the nav bar.
Thoughts:
06287e1
Point links to truth.dev.
44257d2