From 8d026d9abec0b0d1a5bd0613bf31eff0ba667a11 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 19 Jun 2017 16:27:07 -0600 Subject: Fix searching for overridden methods. If the superclass is an abstract class, we want to search it for possibly overridden methods; we don't want to search ourselves. This bug resulted in (incorrectly) thinking that overridden "default" methods from interfaces were standalone methods on the class. (The expected behavior has always been to omit any overridden methods from the public API surface area to avoid redundant noise.) Also fix another bug that was quietly letting overridden methods change return types or exceptions without calling them out as API changes. Test: make update-api Bug: 62675475 Change-Id: Ib789fcd6820a34208bb08e1391026c90a2891a37 --- src/com/google/doclava/MethodInfo.java | 18 +++++------ src/com/google/doclava/Stubs.java | 57 ++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/com/google/doclava/MethodInfo.java b/src/com/google/doclava/MethodInfo.java index 33197a3..e5761c1 100644 --- a/src/com/google/doclava/MethodInfo.java +++ b/src/com/google/doclava/MethodInfo.java @@ -28,6 +28,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Queue; +import java.util.function.Predicate; public class MethodInfo extends MemberInfo implements AbstractMethodInfo, Resolvable { public static final Comparator comparator = new Comparator() { @@ -108,7 +109,7 @@ public class MethodInfo extends MemberInfo implements AbstractMethodInfo, Resolv ArrayList queue = new ArrayList(); if (containingClass().realSuperclass() != null && containingClass().realSuperclass().isAbstract()) { - queue.add(containingClass()); + queue.add(containingClass().realSuperclass()); } addInterfaces(containingClass().realInterfaces(), queue); for (ClassInfo iface : queue) { @@ -123,17 +124,13 @@ public class MethodInfo extends MemberInfo implements AbstractMethodInfo, Resolv return null; } - public MethodInfo findSuperclassImplementation(HashSet notStrippable) { + public MethodInfo findPredicateOverriddenMethod(Predicate predicate) { if (mReturnType == null) { // ctor return null; } if (mOverriddenMethod != null) { - // Even if we're told outright that this was the overridden method, we want to - // be conservative and ignore mismatches of parameter types -- they arise from - // extending generic specializations, and we want to consider the derived-class - // method to be a non-override. - if (this.signature().equals(mOverriddenMethod.signature())) { + if (predicate.test(mOverriddenMethod)) { return mOverriddenMethod; } } @@ -141,13 +138,12 @@ public class MethodInfo extends MemberInfo implements AbstractMethodInfo, Resolv ArrayList queue = new ArrayList(); if (containingClass().realSuperclass() != null && containingClass().realSuperclass().isAbstract()) { - queue.add(containingClass()); + queue.add(containingClass().realSuperclass()); } addInterfaces(containingClass().realInterfaces(), queue); for (ClassInfo iface : queue) { for (MethodInfo me : iface.methods()) { - if (me.name().equals(this.name()) && me.signature().equals(this.signature()) - && notStrippable.contains(me.containingClass())) { + if (predicate.test(me)) { return me; } } @@ -167,7 +163,7 @@ public class MethodInfo extends MemberInfo implements AbstractMethodInfo, Resolv ArrayList queue = new ArrayList(); if (containingClass().realSuperclass() != null && containingClass().realSuperclass().isAbstract()) { - queue.add(containingClass()); + queue.add(containingClass().realSuperclass()); } addInterfaces(containingClass().realInterfaces(), queue); for (ClassInfo iface : queue) { diff --git a/src/com/google/doclava/Stubs.java b/src/com/google/doclava/Stubs.java index fbcff97..fdd9d0d 100644 --- a/src/com/google/doclava/Stubs.java +++ b/src/com/google/doclava/Stubs.java @@ -18,6 +18,7 @@ package com.google.doclava; import java.io.BufferedOutputStream; import java.io.BufferedReader; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -811,40 +812,32 @@ public class Stubs { || !field.type().dimension().equals("") || field.containingClass().isInterface(); } - // Returns 'true' if the method is an @Override of a visible parent - // method implementation, and thus does not affect the API. + /** + * Test if the given method has a concrete implementation in a superclass or + * interface that has no differences in its public API representation. + * + * @return {@code true} if the tested method can be safely elided from the + * public API to conserve space. + */ static boolean methodIsOverride(HashSet notStrippable, MethodInfo mi) { // Abstract/static/final methods are always listed in the API description if (mi.isAbstract() || mi.isStatic() || mi.isFinal()) { return false; } - // Find any relevant ancestor declaration and inspect it - MethodInfo om = mi; - do { - MethodInfo superMethod = om.findSuperclassImplementation(notStrippable); - if (om.equals(superMethod)) { - break; - } - om = superMethod; - } while (om != null && (om.isHiddenOrRemoved() || om.containingClass().isHiddenOrRemoved())); - if (om != null) { - // Visibility mismatch is an API change, so check for it - if (mi.mIsPrivate == om.mIsPrivate && mi.mIsPublic == om.mIsPublic - && mi.mIsProtected == om.mIsProtected) { - // Look only for overrides of an ancestor class implementation, - // not of e.g. an abstract or interface method declaration - if (!om.isAbstract()) { - // If the only "override" turns out to be in our own class - // (which sometimes happens in concrete subclasses of - // abstract base classes), it's not really an override - if (!mi.mContainingClass.equals(om.mContainingClass)) { - return true; - } + final String api = writeMethodApiWithoutDefault(mi); + final MethodInfo overridden = mi.findPredicateOverriddenMethod(new Predicate() { + @Override + public boolean test(MethodInfo test) { + if (test.isHiddenOrRemoved() || test.containingClass().isHiddenOrRemoved()) { + return false; } + + final String testApi = writeMethodApiWithoutDefault(test); + return api.equals(testApi); } - } - return false; + }); + return (overridden != null); } static boolean canCallMethod(ClassInfo from, MethodInfo m) { @@ -1573,10 +1566,20 @@ public class Stubs { apiWriter.print(";\n"); } + static String writeMethodApiWithoutDefault(MethodInfo mi) { + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + writeMethodApi(new PrintStream(out), mi, false); + return out.toString(); + } + static void writeMethodApi(PrintStream apiWriter, MethodInfo mi) { + writeMethodApi(apiWriter, mi, true); + } + + static void writeMethodApi(PrintStream apiWriter, MethodInfo mi, boolean withDefault) { apiWriter.print(" method "); apiWriter.print(mi.scope()); - if (mi.isDefault()) { + if (mi.isDefault() && withDefault) { apiWriter.print(" default"); } if (mi.isStatic()) { -- cgit v1.2.3