On Fri, 8 Oct 2021 18:32:53 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
> Please review a moderately simple change, to have DocLint check for relevant
> `@SuppressWarnings` annotations before reporting any messages.
Looks good; consider the trivial suggestions below.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Env.java line 298:
> 296: set = EnumSet.allOf(Messages.Group.class);
> 297: break;
> 298: } else if (arg.startsWith("doclint:")){
Prepend `{` with whitespace:
Suggestion:
} else if (arg.startsWith("doclint:")) {
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Env.java line 300:
> 298: } else if (arg.startsWith("doclint:")){
> 299: final int len = "doclint:".length();
> 300: for (String a: arg.substring(len).split(",")) {
Add whitespace after `String a` for consistency with other enhanced for-loops
in this file:
Suggestion:
for (String a : arg.substring(len).split(",")) {
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Env.java line 310:
> 308: suppressWarnings.put(e, set);
> 309: }
> 310: return set.contains(g);
Refactor as `suppressWarnings.computeIfAbsent(...).contains(g)` with a method
reference for readability.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Env.java line 332:
> 330: for (var item : list) {
> 331: if (item instanceof AnnotationValue
> avItem) {
> 332: if (avItem.getValue() instanceof
> String s) {
Suggestion:
if (item instanceof AnnotationValue avItem
&& avItems.getValue() instanceof Srting
s) {
test/langtools/tools/doclint/SuppressWarningsTest.java line 141:
> 139: }
> 140:
> 141: }
Add trailing newline.
-------------
Marked as reviewed by prappo (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5870
