Skip to content
This repository was archived by the owner on Sep 2, 2022. It is now read-only.

Commit 45bd3b9

Browse files
hnshns
committed
8223607: --override-methods=summary ignores some signature changes
Reviewed-by: jjg
1 parent 59ae054 commit 45bd3b9

File tree

7 files changed

+457
-29
lines changed

7 files changed

+457
-29
lines changed

‎src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,10 @@
2525

2626
package jdk.javadoc.internal.doclets.toolkit;
2727

28-
import java.util.ArrayList;
2928
import java.util.Arrays;
3029
import java.util.HashMap;
31-
import java.util.HashSet;
3230
import java.util.List;
3331
import java.util.Map;
34-
import java.util.Set;
3532
import java.util.SortedSet;
3633
import java.util.TreeSet;
3734

@@ -48,14 +45,10 @@
4845
import javax.tools.FileObject;
4946
import javax.tools.JavaFileManager.Location;
5047

51-
import com.sun.source.tree.CompilationUnitTree;
52-
import com.sun.source.util.JavacTask;
5348
import com.sun.source.util.TreePath;
54-
import com.sun.tools.javac.api.BasicJavacTask;
5549
import com.sun.tools.javac.code.Attribute;
5650
import com.sun.tools.javac.code.Flags;
5751
import com.sun.tools.javac.code.Scope;
58-
import com.sun.tools.javac.code.Source.Feature;
5952
import com.sun.tools.javac.code.Symbol;
6053
import com.sun.tools.javac.code.Symbol.ClassSymbol;
6154
import com.sun.tools.javac.code.Symbol.MethodSymbol;
@@ -66,11 +59,9 @@
6659
import com.sun.tools.javac.comp.AttrContext;
6760
import com.sun.tools.javac.comp.Env;
6861
import com.sun.tools.javac.model.JavacElements;
69-
import com.sun.tools.javac.model.JavacTypes;
7062
import com.sun.tools.javac.util.Names;
7163

7264
import jdk.javadoc.internal.doclets.toolkit.util.Utils;
73-
import jdk.javadoc.internal.doclint.DocLint;
7465
import jdk.javadoc.internal.tool.ToolEnvironment;
7566
import jdk.javadoc.internal.tool.DocEnvImpl;
7667

‎src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java

Lines changed: 96 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,28 @@
2525

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

28+
import javax.lang.model.element.AnnotationMirror;
2829
import javax.lang.model.element.Element;
2930
import javax.lang.model.element.ExecutableElement;
31+
import javax.lang.model.element.Modifier;
3032
import javax.lang.model.element.TypeElement;
3133
import javax.lang.model.element.VariableElement;
34+
import javax.lang.model.type.ArrayType;
35+
import javax.lang.model.type.DeclaredType;
36+
import javax.lang.model.type.ExecutableType;
3237
import javax.lang.model.type.TypeKind;
3338
import javax.lang.model.type.TypeMirror;
39+
import javax.lang.model.type.WildcardType;
3440
import javax.lang.model.util.Elements;
3541
import javax.lang.model.util.SimpleElementVisitor14;
42+
import javax.lang.model.util.SimpleTypeVisitor14;
3643
import java.lang.ref.SoftReference;
3744
import java.util.ArrayList;
3845
import java.util.Collections;
3946
import java.util.EnumMap;
4047
import java.util.EnumSet;
4148
import java.util.HashMap;
49+
import java.util.HashSet;
4250
import java.util.LinkedHashMap;
4351
import java.util.LinkedHashSet;
4452
import java.util.List;
@@ -582,14 +590,10 @@ boolean allowInheritedMethods(ExecutableElement inheritedMethod,
582590
return false;
583591
}
584592

585-
TypeMirror inheritedMethodReturn = inheritedMethod.getReturnType();
586-
TypeMirror lMethodReturn = lMethod.getReturnType();
587-
boolean covariantReturn =
588-
lMethodReturn.getKind() == TypeKind.DECLARED
589-
&& inheritedMethodReturn.getKind() == TypeKind.DECLARED
590-
&& !utils.typeUtils.isSameType(lMethodReturn, inheritedMethodReturn)
591-
&& utils.typeUtils.isSubtype(lMethodReturn, inheritedMethodReturn);
592-
boolean simpleOverride = covariantReturn ? false : utils.isSimpleOverride(lMethod);
593+
// Even with --override-methods=summary we want to include details of
594+
// overriding method if something noteworthy has been added or changed.
595+
boolean simpleOverride = utils.isSimpleOverride(lMethod)
596+
&& !overridingSignatureChanged(lMethod, inheritedMethod);
593597
overriddenMethodTable.computeIfAbsent(lMethod,
594598
l -> new OverridingMethodInfo(inheritedMethod, simpleOverride));
595599
return simpleOverride;
@@ -598,6 +602,90 @@ boolean allowInheritedMethods(ExecutableElement inheritedMethod,
598602
return true;
599603
}
600604

605+
// Check whether the signature of an overriding method has any changes worth
606+
// being documented compared to the overridden method.
607+
private boolean overridingSignatureChanged(ExecutableElement method, ExecutableElement overriddenMethod) {
608+
// Covariant return type
609+
TypeMirror overriddenMethodReturn = overriddenMethod.getReturnType();
610+
TypeMirror methodReturn = method.getReturnType();
611+
if (methodReturn.getKind() == TypeKind.DECLARED
612+
&& overriddenMethodReturn.getKind() == TypeKind.DECLARED
613+
&& !utils.typeUtils.isSameType(methodReturn, overriddenMethodReturn)
614+
&& utils.typeUtils.isSubtype(methodReturn, overriddenMethodReturn)) {
615+
return true;
616+
}
617+
// Modifiers changed from protected to public, non-final to final, or change in abstractness
618+
Set<Modifier> modifiers = method.getModifiers();
619+
Set<Modifier> overriddenModifiers = overriddenMethod.getModifiers();
620+
if ((modifiers.contains(Modifier.PUBLIC) && overriddenModifiers.contains(Modifier.PROTECTED))
621+
|| modifiers.contains(Modifier.FINAL)
622+
|| modifiers.contains(Modifier.ABSTRACT) != overriddenModifiers.contains(Modifier.ABSTRACT)) {
623+
return true;
624+
}
625+
// Change in thrown types
626+
if (!method.getThrownTypes().equals(overriddenMethod.getThrownTypes())) {
627+
return true;
628+
}
629+
// Documented annotations added anywhere in the method signature
630+
return !getDocumentedAnnotations(method).equals(getDocumentedAnnotations(overriddenMethod));
631+
}
632+
633+
private Set<AnnotationMirror> getDocumentedAnnotations(ExecutableElement element) {
634+
Set<AnnotationMirror> annotations = new HashSet<>();
635+
addDocumentedAnnotations(annotations, element.getAnnotationMirrors());
636+
637+
new SimpleTypeVisitor14<Void, Void>() {
638+
@Override
639+
protected Void defaultAction(TypeMirror e, Void v) {
640+
addDocumentedAnnotations(annotations, e.getAnnotationMirrors());
641+
return null;
642+
}
643+
644+
@Override
645+
public Void visitArray(ArrayType t, Void unused) {
646+
if (t.getComponentType() != null) {
647+
visit(t.getComponentType());
648+
}
649+
return super.visitArray(t, unused);
650+
}
651+
652+
@Override
653+
public Void visitDeclared(DeclaredType t, Void unused) {
654+
t.getTypeArguments().forEach(this::visit);
655+
return super.visitDeclared(t, unused);
656+
}
657+
658+
@Override
659+
public Void visitWildcard(WildcardType t, Void unused) {
660+
if (t.getExtendsBound() != null) {
661+
visit(t.getExtendsBound());
662+
}
663+
if (t.getSuperBound() != null) {
664+
visit(t.getSuperBound());
665+
}
666+
return super.visitWildcard(t, unused);
667+
}
668+
669+
@Override
670+
public Void visitExecutable(ExecutableType t, Void unused) {
671+
t.getParameterTypes().forEach(this::visit);
672+
t.getTypeVariables().forEach(this::visit);
673+
visit(t.getReturnType());
674+
return super.visitExecutable(t, unused);
675+
}
676+
}.visit(element.asType());
677+
678+
return annotations;
679+
}
680+
681+
private void addDocumentedAnnotations(Set<AnnotationMirror> annotations, List<? extends AnnotationMirror> annotationMirrors) {
682+
annotationMirrors.forEach(annotation -> {
683+
if (utils.isDocumentedAnnotation((TypeElement) annotation.getAnnotationType().asElement())) {
684+
annotations.add(annotation);
685+
}
686+
});
687+
}
688+
601689
/*
602690
* This class encapsulates the details of local members, orderedMembers
603691
* contains the members in the declaration order, additionally a

0 commit comments

Comments
 (0)