Skip to content

Commit

Permalink
8189591: No way to locally suppress doclint warnings
Browse files Browse the repository at this point in the history
Reviewed-by: hannesw, prappo
  • Loading branch information
jonathan-gibbons committed Oct 16, 2021
1 parent 7fc3a8d commit 96fef40
Show file tree
Hide file tree
Showing 4 changed files with 257 additions and 10 deletions.
109 changes: 107 additions & 2 deletions src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Env.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -26,21 +26,31 @@
package jdk.javadoc.internal.doclint;


import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.AnnotationValue;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.Elements;
import javax.lang.model.util.Types;
import javax.tools.Diagnostic.Kind;

import com.sun.source.doctree.DocCommentTree;
import com.sun.source.tree.CompilationUnitTree;
Expand Down Expand Up @@ -106,6 +116,7 @@ else if (mods.contains(Modifier.PRIVATE))
// Types used when analysing doc comments.
TypeMirror java_lang_Error;
TypeMirror java_lang_RuntimeException;
TypeMirror java_lang_SuppressWarnings;
TypeMirror java_lang_Throwable;
TypeMirror java_lang_Void;

Expand All @@ -125,6 +136,9 @@ else if (mods.contains(Modifier.PRIVATE))
/** The set of methods, if any, that the current declaration overrides. */
Set<? extends ExecutableElement> currOverriddenMethods;

/** A map containing the info derived from {@code @SuppressWarnings} for an element. */
Map<Element, Set<Messages.Group>> suppressWarnings = new HashMap<>();

Env() {
messages = new Messages(this);
}
Expand All @@ -145,6 +159,7 @@ void initTypes() {

java_lang_Error = elements.getTypeElement("java.lang.Error").asType();
java_lang_RuntimeException = elements.getTypeElement("java.lang.RuntimeException").asType();
java_lang_SuppressWarnings = elements.getTypeElement("java.lang.SuppressWarnings").asType();
java_lang_Throwable = elements.getTypeElement("java.lang.Throwable").asType();
java_lang_Void = elements.getTypeElement("java.lang.Void").asType();
}
Expand Down Expand Up @@ -247,6 +262,96 @@ boolean shouldCheck(CompilationUnitTree unit) {
return true;
}

/**
* {@return whether or not warnings in a group are suppressed for the current element}
* @param g the group
*/
boolean suppressWarnings(Messages.Group g) {
return suppressWarnings(currElement, g);
}

/**
* {@return whether or not warnings in a group are suppressed for a given element}
* @param e the element
* @param g the group
*/
boolean suppressWarnings(Element e, Messages.Group g) {
// check if warnings are suppressed in any enclosing classes
Element encl = e.getEnclosingElement();
if (encl != null && encl.asType().getKind() == TypeKind.DECLARED) {
if (suppressWarnings(encl, g)) {
return true;
}
}

// check the local @SuppressWarnings annotation, caching the results
return suppressWarnings.computeIfAbsent(e, this::getSuppressedGroups).contains(g);
}

/**
* Returns the set of groups for an element for which messages should be suppressed.
* The set is determined by examining the arguments for any {@code @SuppressWarnings}
* annotation that may be present on the element.
* The supported strings are: "doclint" and "doclint:GROUP,..." for each GROUP
*
* @param e the element
* @return the set
*/
private Set<Messages.Group> getSuppressedGroups(Element e) {
var gMap = Arrays.stream(Messages.Group.values())
.collect(Collectors.toMap(Messages.Group::optName, Function.identity()));
var set = EnumSet.noneOf(Messages.Group.class);
for (String arg : getSuppressWarningsValue(e)) {
if (arg.equals("doclint")) {
set = EnumSet.allOf(Messages.Group.class);
break;
} else if (arg.startsWith("doclint:")) {
final int len = "doclint:".length();
for (String a : arg.substring(len).split(",")) {
Messages.Group argGroup = gMap.get(a);
if (argGroup != null) {
set.add(argGroup);
}
}
}
}
return set;
}

/**
* Returns the list of values given to an instance of {@code @SuppressWarnings} for an element,
* or an empty list if there is no annotation.
*
* @param e the element
* @return the list
*/
private List<String> getSuppressWarningsValue(Element e) {
for (AnnotationMirror am : e.getAnnotationMirrors()) {
DeclaredType dt = am.getAnnotationType();
if (types.isSameType(dt, java_lang_SuppressWarnings)) {
var values = am.getElementValues();
for (var entry : values.entrySet()) {
if (entry.getKey().getSimpleName().contentEquals("value")) {
AnnotationValue av = entry.getValue();
if (av.getValue() instanceof List<?> list) {
List<String> result = new ArrayList<>();
for (var item : list) {
if (item instanceof AnnotationValue avItem
&& avItem.getValue() instanceof String s) {
result.add(s);
}
}
return result;
}
}
}

}
}
return Collections.emptyList();
}


private <T extends Comparable<T>> T min(T item1, T item2) {
return (item1 == null) ? item2
: (item2 == null) ? item1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -115,6 +115,9 @@ void reportStats(PrintWriter out) {

protected void report(Group group, Diagnostic.Kind dkind, DocTree tree, String code, Object... args) {
if (options.isEnabled(group, env.currAccess)) {
if (dkind == Diagnostic.Kind.WARNING && env.suppressWarnings(group)) {
return;
}
String msg = (code == null) ? (String) args[0] : localize(code, args);
env.trees.printMessage(dkind, msg, tree,
env.currDocComment, env.currPath.getCompilationUnit());
Expand All @@ -125,6 +128,9 @@ protected void report(Group group, Diagnostic.Kind dkind, DocTree tree, String c

protected void report(Group group, Diagnostic.Kind dkind, Tree tree, String code, Object... args) {
if (options.isEnabled(group, env.currAccess)) {
if (dkind == Diagnostic.Kind.WARNING && env.suppressWarnings(group)) {
return;
}
String msg = localize(code, args);
env.trees.printMessage(dkind, msg, tree, env.currPath.getCompilationUnit());

Expand Down Expand Up @@ -315,18 +321,14 @@ void report(PrintWriter out) {
*/
private static class Table {

private static final Comparator<Integer> DECREASING = (o1, o2) -> o2.compareTo(o1);
private static final Comparator<Integer> DECREASING = Comparator.reverseOrder();
private final TreeMap<Integer, Set<String>> map = new TreeMap<>(DECREASING);

void put(String label, int n) {
if (n == 0) {
return;
}
Set<String> labels = map.get(n);
if (labels == null) {
map.put(n, labels = new TreeSet<>());
}
labels.add(label);
map.computeIfAbsent(n, k -> new TreeSet<>()).add(label);
}

void print(PrintWriter out) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ public class BooleanProperty { }
checkExit(Exit.OK);

checkOutput(Output.OUT, false,
"warning",
": warning:",
"no comment");
}
}
140 changes: 140 additions & 0 deletions test/langtools/tools/doclint/SuppressWarningsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8189591
* @summary No way to locally suppress doclint warnings
* @library /tools/lib
* @modules jdk.javadoc/jdk.javadoc.internal.doclint
* @build toolbox.ToolBox SuppressWarningsTest
* @run main SuppressWarningsTest
*/

import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;

import jdk.javadoc.internal.doclint.DocLint;
import toolbox.ToolBox;

public class SuppressWarningsTest {
public static void main(String... args) throws Exception {
new SuppressWarningsTest().run();
}

enum Kind {
NONE("// no annotation"),
MISSING("@SuppressWarnings(\"doclint:missing\")"),
OTHER("@SuppressWarnings(\"doclint:html\")"),
ALL("@SuppressWarnings(\"doclint\")");
final String anno;
Kind(String anno) {
this.anno = anno;
}
}

ToolBox tb = new ToolBox();
PrintStream log = System.err;

void run() throws Exception {
for (Kind ok : Kind.values()) {
for (Kind ik : Kind.values()) {
for (Kind mk : Kind.values()) {
test(ok, ik, mk);
}
}
}

if (errorCount == 0) {
log.println("No errors");
} else {
log.println(errorCount + " errors");
throw new Exception(errorCount + " errors");
}
}

void test(Kind outerKind, Kind innerKind, Kind memberKind) throws Exception {
log.println("Test: outer:" + outerKind + " inner: " + innerKind + " member:" + memberKind);
Path base = Path.of(outerKind + "-" + innerKind + "-" + memberKind);
Files.createDirectories(base);
Path src = base.resolve("src");
tb.writeJavaFiles(src, """
/** . */
##OUTER##
public class Outer {
/** . */
private Outer() { }
/** . */
##INNER##
public class Inner {
/** . */
private Inner() { }
##MEMBER##
public void m() { }
}
}
"""
.replace("##OUTER##", outerKind.anno)
.replace("##INNER##", innerKind.anno)
.replace("##MEMBER##", memberKind.anno));

DocLint dl = new DocLint();
StringWriter sw = new StringWriter();
try (PrintWriter pw = new PrintWriter(sw)) {
dl.run(pw, src.resolve("Outer.java").toString());
}
String out = sw.toString();
out.lines().forEach(System.err::println);

boolean expectSuppressed = false;
for (Kind k : List.of(outerKind, innerKind, memberKind)) {
expectSuppressed |= k == Kind.ALL || k == Kind.MISSING;
}
boolean foundWarning = out.contains("warning: no comment");
if (expectSuppressed) {
if (foundWarning) {
error("found unexpected warning");
} else {
log.println("Passed: no warning, as expected");
}
} else {
if (foundWarning) {
log.println("Passed: found warning, as expected");
} else {
error("no warning");
}
}
log.println();
}

int errorCount;

void error(String message) {
log.println("Error: " + message);
errorCount++;
}
}

1 comment on commit 96fef40

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.