diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationPosition.java b/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationPosition.java index 0a1ca982d0b..a6ba9a3ce66 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationPosition.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationPosition.java @@ -18,8 +18,9 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; -import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Iterables.concat; +import static com.google.common.collect.Sets.toImmutableEnumSet; +import static com.google.common.collect.Streams.concat; import static com.google.common.collect.Streams.stream; import static com.google.errorprone.BugPattern.LinkType.CUSTOM; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; @@ -28,6 +29,7 @@ import static com.google.errorprone.util.ASTHelpers.getStartPosition; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static java.util.Comparator.naturalOrder; +import static java.util.EnumSet.allOf; import static java.util.stream.Collectors.joining; import com.google.common.collect.ImmutableList; @@ -54,7 +56,6 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.TypeAnnotations.AnnotationType; import com.sun.tools.javac.parser.Tokens.TokenKind; -import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.stream.Stream; @@ -75,21 +76,6 @@ link = "https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations") public final class AnnotationPosition extends BugChecker implements ClassTreeMatcher, MethodTreeMatcher, VariableTreeMatcher { - - private static final ImmutableMap TOKEN_KIND_BY_NAME = - Arrays.stream(TokenKind.values()).collect(toImmutableMap(tk -> tk.name(), tk -> tk)); - - private static final ImmutableSet MODIFIERS = - Streams.concat( - Arrays.stream(Modifier.values()) - .map(m -> TOKEN_KIND_BY_NAME.get(m.name())) - // TODO(b/168625474): sealed doesn't have a token kind in Java 15 - .filter(Objects::nonNull), - // Pretend that "<" and ">" are modifiers, so that type arguments wind up grouped with - // modifiers. - Stream.of(TokenKind.LT, TokenKind.GT, TokenKind.GTGT)) - .collect(toImmutableSet()); - @Override public Description matchClass(ClassTree tree, VisitorState state) { return handle(tree, tree.getSimpleName(), tree.getModifiers(), state); @@ -112,11 +98,11 @@ private Description handle(Tree tree, Name name, ModifiersTree modifiers, Visito } int treePos = getStartPosition(tree); - List tokens = annotationTokens(tree, state, treePos); + ImmutableList tokens = annotationTokens(tree, state, treePos); ErrorProneComment danglingJavadoc = findOrphanedJavadoc(name, tokens); ImmutableList modifierTokens = - tokens.stream().filter(t -> MODIFIERS.contains(t.kind())).collect(toImmutableList()); + tokens.stream().filter(t -> isModifier(t)).collect(toImmutableList()); int firstModifierPos = modifierTokens.stream().findFirst().map(x -> x.pos()).orElse(Integer.MAX_VALUE); @@ -212,7 +198,7 @@ private Description checkAnnotations( .collect(toImmutableList()); int lastNonTypeAnnotationOrModifierPosition = - Streams.concat( + concat( Stream.of(lastModifierPos), shouldBeBefore.stream().map(ASTHelpers::getStartPosition)) .max(naturalOrder()) @@ -337,4 +323,26 @@ private enum Position { AFTER, EITHER } + + private static final ImmutableMap TOKEN_KIND_BY_NAME = + allOf(TokenKind.class).stream().collect(toImmutableMap(tk -> tk.name(), tk -> tk)); + + private static final ImmutableSet MODIFIERS = + concat( + allOf(Modifier.class).stream() + .map(m -> TOKEN_KIND_BY_NAME.get(m.name())) + // "sealed" does not have a TokenKind. + .filter(Objects::nonNull), + // Pretend that "<" and ">" are modifiers, so that type arguments wind up grouped with + // modifiers. + Stream.of(TokenKind.LT, TokenKind.GT, TokenKind.GTGT)) + .collect(toImmutableEnumSet()); + + private static boolean isModifier(ErrorProneToken t) { + return MODIFIERS.contains(t.kind()) + || (t.kind().equals(TokenKind.IDENTIFIER) + // It suffices here to consider "non" to be a modifier by itself, given we only use this + // method to find the start/end of the modifiers. + && (t.name().contentEquals("sealed") || t.name().contentEquals("non"))); + } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AnnotationPositionTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/AnnotationPositionTest.java index ef4661932c3..c692979e2ce 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/AnnotationPositionTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/AnnotationPositionTest.java @@ -475,7 +475,6 @@ interface Test { .doTest(); } - // TODO(b/168625474): 'sealed' doesn't have a TokenKind @Test public void sealedInterface() { refactoringHelper @@ -491,11 +490,35 @@ final class A implements Test {} "Test.java", """ /** Javadoc! */ - sealed @Deprecated interface Test { + @Deprecated + sealed interface Test { final class A implements Test {} } """) - .setArgs("--enable-preview", "--release", Integer.toString(Runtime.version().feature())) + .doTest(TEXT_MATCH); + } + + @Test + public void nonSealedInterface() { + refactoringHelper + .addInputLines( + "Test.java", + """ + /** Javadoc! */ + sealed @Deprecated interface Test { + non-sealed @Deprecated class A implements Test {} + } + """) + .addOutputLines( + "Test.java", + """ + /** Javadoc! */ + @Deprecated + sealed interface Test { + @Deprecated + non-sealed class A implements Test {} + } + """) .doTest(TEXT_MATCH); }