Skip to content

Commit

Permalink
8223607: --override-methods=summary ignores some signature changes
Browse files Browse the repository at this point in the history 
Reviewed-by: jjg
  • Loading branch information
@hns
hns committed Dec 18, 2020
1 parent 59ae054 commit 45bd3b9
Show file tree
Hide file tree
Showing 7 changed files with 457 additions and 29 deletions.
9 changes: 0 additions & 9 deletions src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java
View file
Expand Up @@ -25,13 +25,10 @@

package jdk.javadoc.internal.doclets.toolkit;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;

Expand All @@ -48,14 +45,10 @@
import javax.tools.FileObject;
import javax.tools.JavaFileManager.Location;

import com.sun.source.tree.CompilationUnitTree;
import com.sun.source.util.JavacTask;
import com.sun.source.util.TreePath;
import com.sun.tools.javac.api.BasicJavacTask;
import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Flags;
import com.sun.tools.javac.code.Scope;
import com.sun.tools.javac.code.Source.Feature;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
Expand All @@ -66,11 +59,9 @@
import com.sun.tools.javac.comp.AttrContext;
import com.sun.tools.javac.comp.Env;
import com.sun.tools.javac.model.JavacElements;
import com.sun.tools.javac.model.JavacTypes;
import com.sun.tools.javac.util.Names;

import jdk.javadoc.internal.doclets.toolkit.util.Utils;
import jdk.javadoc.internal.doclint.DocLint;
import jdk.javadoc.internal.tool.ToolEnvironment;
import jdk.javadoc.internal.tool.DocEnvImpl;

Expand Down
104 changes: 96 additions & 8 deletions ...k.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java
View file
Expand Up @@ -25,20 +25,28 @@

package jdk.javadoc.internal.doclets.toolkit.util;

import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.ArrayType;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.ExecutableType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.type.WildcardType;
import javax.lang.model.util.Elements;
import javax.lang.model.util.SimpleElementVisitor14;
import javax.lang.model.util.SimpleTypeVisitor14;
import java.lang.ref.SoftReference;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumMap;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -582,14 +590,10 @@ boolean allowInheritedMethods(ExecutableElement inheritedMethod,
return false;
}

TypeMirror inheritedMethodReturn = inheritedMethod.getReturnType();
TypeMirror lMethodReturn = lMethod.getReturnType();
boolean covariantReturn =
lMethodReturn.getKind() == TypeKind.DECLARED
&& inheritedMethodReturn.getKind() == TypeKind.DECLARED
&& !utils.typeUtils.isSameType(lMethodReturn, inheritedMethodReturn)
&& utils.typeUtils.isSubtype(lMethodReturn, inheritedMethodReturn);
boolean simpleOverride = covariantReturn ? false : utils.isSimpleOverride(lMethod);
// Even with --override-methods=summary we want to include details of
// overriding method if something noteworthy has been added or changed.
boolean simpleOverride = utils.isSimpleOverride(lMethod)
&& !overridingSignatureChanged(lMethod, inheritedMethod);
overriddenMethodTable.computeIfAbsent(lMethod,
l -> new OverridingMethodInfo(inheritedMethod, simpleOverride));
return simpleOverride;
Expand All @@ -598,6 +602,90 @@ boolean allowInheritedMethods(ExecutableElement inheritedMethod,
return true;
}

// Check whether the signature of an overriding method has any changes worth
// being documented compared to the overridden method.
private boolean overridingSignatureChanged(ExecutableElement method, ExecutableElement overriddenMethod) {
// Covariant return type
TypeMirror overriddenMethodReturn = overriddenMethod.getReturnType();
TypeMirror methodReturn = method.getReturnType();
if (methodReturn.getKind() == TypeKind.DECLARED
&& overriddenMethodReturn.getKind() == TypeKind.DECLARED
&& !utils.typeUtils.isSameType(methodReturn, overriddenMethodReturn)
&& utils.typeUtils.isSubtype(methodReturn, overriddenMethodReturn)) {
return true;
}
// Modifiers changed from protected to public, non-final to final, or change in abstractness
Set<Modifier> modifiers = method.getModifiers();
Set<Modifier> overriddenModifiers = overriddenMethod.getModifiers();
if ((modifiers.contains(Modifier.PUBLIC) && overriddenModifiers.contains(Modifier.PROTECTED))
|| modifiers.contains(Modifier.FINAL)
|| modifiers.contains(Modifier.ABSTRACT) != overriddenModifiers.contains(Modifier.ABSTRACT)) {
return true;
}
// Change in thrown types
if (!method.getThrownTypes().equals(overriddenMethod.getThrownTypes())) {
return true;
}
// Documented annotations added anywhere in the method signature
return !getDocumentedAnnotations(method).equals(getDocumentedAnnotations(overriddenMethod));
}

private Set<AnnotationMirror> getDocumentedAnnotations(ExecutableElement element) {
Set<AnnotationMirror> annotations = new HashSet<>();
addDocumentedAnnotations(annotations, element.getAnnotationMirrors());

new SimpleTypeVisitor14<Void, Void>() {
@Override
protected Void defaultAction(TypeMirror e, Void v) {
addDocumentedAnnotations(annotations, e.getAnnotationMirrors());
return null;
}

@Override
public Void visitArray(ArrayType t, Void unused) {
if (t.getComponentType() != null) {
visit(t.getComponentType());
}
return super.visitArray(t, unused);
}

@Override
public Void visitDeclared(DeclaredType t, Void unused) {
t.getTypeArguments().forEach(this::visit);
return super.visitDeclared(t, unused);
}

@Override
public Void visitWildcard(WildcardType t, Void unused) {
if (t.getExtendsBound() != null) {
visit(t.getExtendsBound());
}
if (t.getSuperBound() != null) {
visit(t.getSuperBound());
}
return super.visitWildcard(t, unused);
}

@Override
public Void visitExecutable(ExecutableType t, Void unused) {
t.getParameterTypes().forEach(this::visit);
t.getTypeVariables().forEach(this::visit);
visit(t.getReturnType());
return super.visitExecutable(t, unused);
}
}.visit(element.asType());

return annotations;
}

private void addDocumentedAnnotations(Set<AnnotationMirror> annotations, List<? extends AnnotationMirror> annotationMirrors) {
annotationMirrors.forEach(annotation -> {
if (utils.isDocumentedAnnotation((TypeElement) annotation.getAnnotationType().asElement())) {
annotations.add(annotation);
}
});
}

/*
* This class encapsulates the details of local members, orderedMembers
* contains the members in the declaration order, additionally a
Expand Down

0 comments on commit 45bd3b9

Please sign in to comment.