This makes jdk.javadoc compare javax.lang.model primitives correctly: elements
with `Object.equals` and type mirrors with `Types.isSameType`.
jdk.javadoc sometimes compares elements using identity comparison (`==`) and
type mirrors using equality comparison (`equals`). While this is generally
wrong, we likely get away with it because of the implementation specifics. I've
heard that javac is very forgiving in the sense that there's no difference
between `==` and `equals` for elements. In addition to that, I noticed that
more often than not there's no difference between `equals` and `isSameType` for
type mirrors in the context of jdk.javadoc.
Since there's no guarantee that a different compiler implementation or the
future javac itself will be implemented similarly, it's better to abandon these
bad practices while we are still lucky.
----
With a change like this, there are two main concerns. The first is nullity.
When it's not immediately obvious if `a`, `b`, or both can be null, it's not
clear how to check for equality. To avoid introducing bugs to a code that I
don't know that much, I use the `java.util.Objects.equals` method, which takes
care of nullity the same way the existing code does: if any of `a` or `b` are
null, `Objects.equals(a, b)` behaves exactly as `a == b`, provided `equals` is
well-defined and returns false for the null argument.
The second concern is identity. If equality does not imply identity, it might
be important to distinguish between equal elements and identical elements.
There are a few cases in jdk.javadoc where it's identity that is required; for
example, consider:
SortedSet<ExecutableElement> members =
utils.serializationMethods(currentTypeElement);
for (ExecutableElement member : members) {
currentMember = member;
Content methodsContent = methodWriter.getMethodsContentHeader(
currentMember == members.last());
...
Here's the loop computes a boolean indicating the last iteration. If `members`
contained identical elements, then that check could misjudge the last iteration
and exit prematurely. Thankfully, in that and the similar cases, iteration
happens on a (sorted) set, which by definition cannot have duplicates, provided
the comparator used by the set is consistent with `equals`, which is also
consistent with `==`. So in this case we can safely change `==` to `equals`.
However, when I fixed those iterations to use `equals`, it didn't feel right;
it felt marginally less brittle than what there was before. I then restuctured
the loops to avoid equality comparison altogether, which had a side-benefit of
widening abstractions from `SortedSet` to just `Iterable`.
-------------
Commit messages:
- Initial commit
Changes: https://git.openjdk.org/jdk/pull/12369/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12369&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8301618
Stats: 27 lines in 5 files changed: 6 ins; 0 del; 21 mod
Patch: https://git.openjdk.org/jdk/pull/12369.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12369/head:pull/12369
PR: https://git.openjdk.org/jdk/pull/12369
