7904082: The UI for API Diff could be improved

Reviewed-by: cstein

Author: lahodaj

src/share/classes/jdk/codetools/apidiff/Options.java

@@ -171,6 +171,7 @@ public String toString() {
     Boolean compareDocComments;
     Boolean compareApiDescriptions;
     Boolean compareApiDescriptionsAsText;
+    Boolean showUnchanged;
     String jdkDocs;
 
     // output options
@@ -181,7 +182,6 @@ public String toString() {
     Path mainStylesheet;
     List<Path> extraStylesheets;
     List<Path> resourceFiles;
-    boolean showEqual;
 
     /**
      * The position of additional text to be included in the report.
@@ -336,6 +336,16 @@ void process(String opt, String arg, Options options) throws BadOption {
             }
         },
 
+        /**
+         * {@code --show-unchanged} <var>boolean-value</var>
+         */
+        SHOW_UNCHANGED("--show-unchanged", "opt.arg.boolean") {
+            @Override
+            void process(String opt, String arg, Options options) throws BadOption {
+                options.showUnchanged = asBoolean(arg);
+            }
+        },
+
         /**
          * {@code --enable-preview}.
          *
@@ -909,6 +919,15 @@ public boolean compareApiDescriptionsAsText() {
         return compareApiDescriptionsAsText;
     }
 
+    /**
+     * Returns whether unchanged API elements should be unconditionally shown.
+     *
+     * @return {@code true} if unchanged API elements should be unconditionally shown
+     */
+    public boolean showUnchanged() {
+        return showUnchanged;
+    }
+
     /**
      * Returns whether documentation comments should be compared.
      *
@@ -981,10 +1000,6 @@ public List<Path> getResourceFiles() {
         return (resourceFiles == null) ? Collections.emptyList() : resourceFiles;
     }
 
-    public boolean showEqual() {
-        return showEqual;
-    }
-
     /**
      * Returns the value of a "hidden" option, set by {@code -XD<name>} or
      * {@code -XD<name>=<value>}.
@@ -1082,6 +1097,10 @@ void validate() {
             compareDocComments = !compareApiDescriptions;
         }
 
+        if (showUnchanged == null) {
+            showUnchanged = false;
+        }
+
         if (resourceFiles != null) {
             for (var resFile : resourceFiles) {
                 if (resFile.isAbsolute() && !Files.exists(resFile)) {

src/share/classes/jdk/codetools/apidiff/report/html/ModulePageReporter.java

@@ -30,13 +30,15 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ModuleElement;
 import javax.lang.model.element.ModuleElement.Directive;
@@ -163,7 +165,7 @@ protected List<Content> buildEnclosedElements() {
     private <T> void addDirectives(List<Content> contents,
                                String headingKey,
                                Predicate<RelativePosition<?>> filter,
-                               BiFunction<RelativePosition<?>, APIMap<T>, Content> f) {
+                               BiFunction<RelativePosition<?>, APIMap<T>, ContentAndResultKind> f) {
         Map<RelativePosition<?>, APIMap<T>> dMaps = new TreeMap<>(RelativePosition.elementKeyIndexComparator);
         // apiMaps will only contain maps for directives which should be compared and displayed;
         // i.e. they have already been filtered according to accessKind.allDirectiveDetails
@@ -179,27 +181,44 @@ private <T> void addDirectives(List<Content> contents,
             }
         }
 
-        if (!dMaps.isEmpty()) {
+        List<ContentAndResultKind> converted =
+                dMaps.entrySet()
+                     .stream()
+                     .map(e -> f.apply(e.getKey(), e.getValue()))
+                     .toList();
+
+        if (!converted.isEmpty()) {
+            boolean allUnchanged = converted.stream()
+                                            .allMatch(c -> c.resultKind() == ResultKind.SAME);
             HtmlTree section = HtmlTree.SECTION().setClass("enclosed");
             section.add(HtmlTree.H2(Text.of(msgs.getString(headingKey))));
             HtmlTree ul = HtmlTree.UL();
-            dMaps.forEach((rp, apiMap) -> ul.add(HtmlTree.LI(f.apply(rp, apiMap))));
+            for (ContentAndResultKind c : converted) {
+                HtmlTree item = HtmlTree.LI(c.content());
+                if (!allUnchanged && c.resultKind() == ResultKind.SAME) {
+                    item.setClass("unchanged");
+                }
+                ul.add(item);
+            }
             section.add(ul);
+            if (allUnchanged) {
+                section = HtmlTree.DIV(section).setClass("unchanged");
+            }
             contents.add(section);
         }
     }
 
-    private Content buildExports(RelativePosition<?> rPos, APIMap<ExportsDirective> apiMap) {
+    private ContentAndResultKind buildExports(RelativePosition<?> rPos, APIMap<ExportsDirective> apiMap) {
         return buildExportsOpensProvides(rPos, apiMap, Keywords.EXPORTS, Keywords.TO,
                 ExportsDirective::getPackage, ExportsDirective::getTargetModules);
     }
 
-    private Content buildOpens(RelativePosition<?> rPos, APIMap<OpensDirective> apiMap) {
+    private ContentAndResultKind buildOpens(RelativePosition<?> rPos, APIMap<OpensDirective> apiMap) {
         return buildExportsOpensProvides(rPos, apiMap, Keywords.OPENS, Keywords.TO,
                 OpensDirective::getPackage, OpensDirective::getTargetModules);
     }
 
-    private Content buildProvides(RelativePosition<?> rPos, APIMap<ProvidesDirective> apiMap) {
+    private ContentAndResultKind buildProvides(RelativePosition<?> rPos, APIMap<ProvidesDirective> apiMap) {
         // ProvidesDirective is unusual in that part of it (i.e. the implementations)
         // is not part of the public API, and should only be displayed if allDirectiveDetails
         // is true.
@@ -208,13 +227,13 @@ private Content buildProvides(RelativePosition<?> rPos, APIMap<ProvidesDirective
                 pd -> allDirectiveDetails ? pd.getImplementations() : Collections.emptyList());
     }
 
-    private Content buildUses(RelativePosition<?> rPos, APIMap<UsesDirective> apiMap) {
+    private ContentAndResultKind buildUses(RelativePosition<?> rPos, APIMap<UsesDirective> apiMap) {
         return buildExportsOpensProvides(rPos, apiMap, Keywords.USES, Content.empty,
                 UsesDirective::getService, d -> Collections.emptyList());
     }
 
     private <T extends Directive, U extends Element, V extends Element>
-    Content buildExportsOpensProvides(RelativePosition<?> rPos, APIMap<T> apiMap,
+    ContentAndResultKind buildExportsOpensProvides(RelativePosition<?> rPos, APIMap<T> apiMap,
                                       Content directiveKeyword, Content sublistKeyword,
                                       Function<T, U> getPrimaryElement,
                                       Function<T, List<V>> getSecondaryElements) {
@@ -261,13 +280,14 @@ Content buildExportsOpensProvides(RelativePosition<?> rPos, APIMap<T> apiMap,
             }
         }
 
-
         // TODO: for now, this is stylistically similar to buildEnclosedElement,
         //       but arguably a better way would be to move code for the check or cross into
         //       the enclosing loop that builds the list.
-        return HtmlTree.SPAN(getResultGlyph(rPos), Text.SPACE)
-                .add(HtmlTree.SPAN(contents).setClass("signature"));
 
+        ResultKind result = getResultKind(rPos);
+
+        return new ContentAndResultKind(HtmlTree.SPAN(result.getContent(), Text.SPACE)
+                .add(HtmlTree.SPAN(contents).setClass("signature")), result);
     }
 
     private Content getName(ElementKey ek) {
@@ -288,7 +308,7 @@ private CharSequence getQualifiedName(ElementKey ek) {
         }
     }
 
-    private Content buildRequires(RelativePosition<?> rPos, APIMap<RequiresDirective> apiMap) {
+    private ContentAndResultKind buildRequires(RelativePosition<?> rPos, APIMap<RequiresDirective> apiMap) {
         List<Content> contents = new ArrayList<>();
 
         contents.add(Keywords.REQUIRES);
@@ -337,11 +357,12 @@ private Content buildRequires(RelativePosition<?> rPos, APIMap<RequiresDirective
         // TODO: would be nice to link to the module page if it can be determined to be available.
         contents.add(Text.of(archetype.getDependency().getQualifiedName()));
 
-
         // TODO: for now, this is stylistically similar to buildEnclosedElement,
         //       but arguably a better way would be to move code for the check or cross into
         //       the enclosing loop that builds the list.
-        return HtmlTree.SPAN(getResultGlyph(rPos), Text.SPACE)
-                .add(HtmlTree.SPAN(contents).setClass("signature"));
+        ResultKind result = getResultKind(rPos);
+
+        return new ContentAndResultKind(HtmlTree.SPAN(result.getContent(), Text.SPACE)
+                .add(HtmlTree.SPAN(contents).setClass("signature")), result);
     }
 }