From 865cff01c1bd5b7393f0dd88dbb92a499289e376 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 25 Aug 2021 09:53:59 -0700 Subject: [PATCH] Format type annotation as part of the type, not part of the modifiers list Given e.g. `@Deprecated @Nullable Object foo() {}`, prefer this: ``` @Deprecated @Nullable Object foo() {} ``` instead of: ``` @Deprecated @Nullable Object foo() {} ``` The implementation is complicated by the fact that the AST doesn't store source position information for modifiers, and there's no requirement that declaration annotations, modifiers, and type annotations appear in any particular order in source. To work around this, we examine the token stream to figure out the ordering of the modifiers and annotations. https://github.com/google/google-java-format/issues/5 Roll forward of https://github.com/google/google-java-format/commit/1a875792f6a074f3f0a504ddb8c7cd932ba6b073 without a use of a server-only Guava API. PiperOrigin-RevId: 392919024 --- core/pom.xml | 6 +- .../google/googlejavaformat/OpsBuilder.java | 24 ++ .../java/JavaInputAstVisitor.java | 348 +++++++++++++++--- .../java/java14/Java14InputAstVisitor.java | 13 +- .../java/testdata/TypeAnnotations.input | 33 ++ .../java/testdata/TypeAnnotations.output | 39 ++ pom.xml | 11 + 7 files changed, 406 insertions(+), 68 deletions(-) create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.input create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.output diff --git a/core/pom.xml b/core/pom.xml index 74b7504ac..e8e9bf249 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -51,7 +51,11 @@ error_prone_annotations true - + + com.google.auto.value + auto-value-annotations + true + diff --git a/core/src/main/java/com/google/googlejavaformat/OpsBuilder.java b/core/src/main/java/com/google/googlejavaformat/OpsBuilder.java index 36e038adf..db431c040 100644 --- a/core/src/main/java/com/google/googlejavaformat/OpsBuilder.java +++ b/core/src/main/java/com/google/googlejavaformat/OpsBuilder.java @@ -18,6 +18,8 @@ import static java.lang.Math.min; import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; +import com.google.common.base.Predicate; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; @@ -281,6 +283,28 @@ public final Optional peekToken(int skip) { : Optional.empty(); } + /** + * Returns the {@link Input.Tok}s starting at the current source position, which are satisfied by + * the given predicate. + */ + public ImmutableList peekTokens(int startPosition, Predicate predicate) { + ImmutableList tokens = input.getTokens(); + Preconditions.checkState( + tokens.get(tokenI).getTok().getPosition() == startPosition, + "Expected the current token to be at position %s, found: %s", + startPosition, + tokens.get(tokenI)); + ImmutableList.Builder result = ImmutableList.builder(); + for (int idx = tokenI; idx < tokens.size(); idx++) { + Tok tok = tokens.get(idx).getTok(); + if (!predicate.apply(tok)) { + break; + } + result.add(tok); + } + return result.build(); + } + /** * Emit an optional token iff it exists on the input. This is used to emit tokens whose existence * has been lost in the AST. diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java index 372f3bb59..8da719564 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java @@ -43,19 +43,27 @@ import static com.sun.source.tree.Tree.Kind.VARIABLE; import static java.util.stream.Collectors.toList; +import com.google.auto.value.AutoOneOf; +import com.google.auto.value.AutoValue; import com.google.common.base.MoreObjects; import com.google.common.base.Predicate; import com.google.common.base.Throwables; import com.google.common.base.Verify; import com.google.common.collect.HashMultiset; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Iterators; import com.google.common.collect.Multiset; import com.google.common.collect.PeekingIterator; +import com.google.common.collect.Range; +import com.google.common.collect.RangeSet; import com.google.common.collect.Streams; +import com.google.common.collect.TreeRangeSet; +import com.google.errorprone.annotations.CheckReturnValue; import com.google.googlejavaformat.CloseOp; import com.google.googlejavaformat.Doc; import com.google.googlejavaformat.Doc.FillMode; @@ -139,7 +147,9 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; import java.util.Deque; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -167,7 +177,7 @@ boolean isVertical() { } /** Whether to break or not. */ - enum BreakOrNot { + protected enum BreakOrNot { YES, NO; @@ -268,6 +278,21 @@ boolean isYes() { } } + // TODO(cushon): generalize this + private static final ImmutableMultimap TYPE_ANNOTATIONS = typeAnnotations(); + + private static ImmutableSetMultimap typeAnnotations() { + ImmutableSetMultimap.Builder result = ImmutableSetMultimap.builder(); + for (String annotation : + ImmutableList.of( + "org.jspecify.nullness.Nullable", + "org.checkerframework.checker.nullness.qual.Nullable")) { + String simpleName = annotation.substring(annotation.lastIndexOf('.') + 1); + result.put(simpleName, annotation); + } + return result.build(); + } + protected final OpsBuilder builder; protected static final Indent.Const ZERO = Indent.Const.ZERO; @@ -277,6 +302,8 @@ boolean isYes() { protected final Indent.Const plusTwo; protected final Indent.Const plusFour; + private final Set typeAnnotationSimpleNames = new HashSet<>(); + private static final ImmutableList breakList(Optional breakTag) { return ImmutableList.of(Doc.Break.make(Doc.FillMode.UNIFIED, " ", ZERO, breakTag)); } @@ -292,8 +319,6 @@ private static final ImmutableList forceBreakList(Optional breakTa return ImmutableList.of(Doc.Break.make(FillMode.FORCED, "", Indent.Const.ZERO, breakTag)); } - private static final ImmutableList EMPTY_LIST = ImmutableList.of(); - /** * Allow multi-line filling (of array initializers, argument lists, and boolean expressions) for * items with length less than or equal to this threshold. @@ -417,10 +442,7 @@ public Void visitClass(ClassTree tree, Void unused) { public void visitAnnotationType(ClassTree node) { sync(node); builder.open(ZERO); - visitAndBreakModifiers( - node.getModifiers(), - Direction.VERTICAL, - /* declarationAnnotationBreak= */ Optional.empty()); + typeDeclarationModifiers(node.getModifiers()); builder.open(ZERO); token("@"); token("interface"); @@ -679,9 +701,10 @@ public Void visitNewClass(NewClassTree node, Void unused) { builder.space(); addTypeArguments(node.getTypeArguments(), plusFour); if (node.getClassBody() != null) { - builder.addAll( + List annotations = visitModifiers( - node.getClassBody().getModifiers(), Direction.HORIZONTAL, Optional.empty())); + node.getClassBody().getModifiers(), Direction.HORIZONTAL, Optional.empty()); + visitAnnotations(annotations, BreakOrNot.NO, BreakOrNot.YES); } scan(node.getIdentifier(), null); addArguments(node.getArguments(), plusFour); @@ -802,10 +825,7 @@ private void visitEnumConstantDeclaration(VariableTree enumConstant) { public boolean visitEnumDeclaration(ClassTree node) { sync(node); builder.open(ZERO); - visitAndBreakModifiers( - node.getModifiers(), - Direction.VERTICAL, - /* declarationAnnotationBreak= */ Optional.empty()); + typeDeclarationModifiers(node.getModifiers()); builder.open(plusFour); token("enum"); builder.breakOp(" "); @@ -969,7 +989,7 @@ void visitVariables( } } - private TypeWithDims variableFragmentDims(boolean first, int leadingDims, Tree type) { + private static TypeWithDims variableFragmentDims(boolean first, int leadingDims, Tree type) { if (type == null) { return null; } @@ -1114,6 +1134,7 @@ public Void visitIf(IfTree node, Void unused) { @Override public Void visitImport(ImportTree node, Void unused) { + checkForTypeAnnotation(node); sync(node); token("import"); builder.space(); @@ -1128,6 +1149,21 @@ public Void visitImport(ImportTree node, Void unused) { return null; } + private void checkForTypeAnnotation(ImportTree node) { + Name simpleName = getSimpleName(node); + Collection wellKnownAnnotations = TYPE_ANNOTATIONS.get(simpleName.toString()); + if (!wellKnownAnnotations.isEmpty() + && wellKnownAnnotations.contains(node.getQualifiedIdentifier().toString())) { + typeAnnotationSimpleNames.add(simpleName); + } + } + + private static Name getSimpleName(ImportTree importTree) { + return importTree.getQualifiedIdentifier() instanceof IdentifierTree + ? ((IdentifierTree) importTree.getQualifiedIdentifier()).getName() + : ((MemberSelectTree) importTree.getQualifiedIdentifier()).getIdentifier(); + } + @Override public Void visitBinary(BinaryTree node, Void unused) { sync(node); @@ -1360,9 +1396,12 @@ public Void visitMethod(MethodTree node, Void unused) { } } } - builder.addAll( + List typeAnnotations = visitModifiers( - annotations, Direction.VERTICAL, /* declarationAnnotationBreak= */ Optional.empty())); + node.getModifiers(), + annotations, + Direction.VERTICAL, + /* declarationAnnotationBreak= */ Optional.empty()); Tree baseReturnType = null; Deque> dims = null; @@ -1371,6 +1410,9 @@ public Void visitMethod(MethodTree node, Void unused) { DimensionHelpers.extractDims(node.getReturnType(), SortedDims.YES); baseReturnType = extractedDims.node; dims = new ArrayDeque<>(extractedDims.dims); + } else { + verticalAnnotations(typeAnnotations); + typeAnnotations = ImmutableList.of(); } builder.open(plusFour); @@ -1379,7 +1421,14 @@ public Void visitMethod(MethodTree node, Void unused) { builder.open(ZERO); { boolean first = true; + if (!typeAnnotations.isEmpty()) { + visitAnnotations(typeAnnotations, BreakOrNot.NO, BreakOrNot.NO); + first = false; + } if (!node.getTypeParameters().isEmpty()) { + if (!first) { + builder.breakToFill(" "); + } token("<"); typeParametersRest(node.getTypeParameters(), plusFour); if (!returnTypeAnnotations.isEmpty()) { @@ -1956,16 +2005,11 @@ public Void visitTry(TryTree node, Void unused) { public void visitClassDeclaration(ClassTree node) { sync(node); - List breaks = - visitModifiers( - node.getModifiers(), - Direction.VERTICAL, - /* declarationAnnotationBreak= */ Optional.empty()); + typeDeclarationModifiers(node.getModifiers()); List permitsTypes = getPermitsClause(node); boolean hasSuperclassType = node.getExtendsClause() != null; boolean hasSuperInterfaceTypes = !node.getImplementsClause().isEmpty(); boolean hasPermitsTypes = !permitsTypes.isEmpty(); - builder.addAll(breaks); token(node.getKind() == Tree.Kind.INTERFACE ? "interface" : "class"); builder.space(); visit(node.getSimpleName()); @@ -2069,7 +2113,7 @@ public Void visitWildcard(WildcardTree node, Void unused) { // Helper methods. /** Helper method for annotations. */ - void visitAnnotations( + protected void visitAnnotations( List annotations, BreakOrNot breakBefore, BreakOrNot breakAfter) { if (!annotations.isEmpty()) { if (breakBefore.isYes()) { @@ -2089,6 +2133,14 @@ void visitAnnotations( } } + void verticalAnnotations(List annotations) { + for (AnnotationTree annotation : annotations) { + builder.forcedBreak(); + scan(annotation, null); + builder.forcedBreak(); + } + } + /** Helper method for blocks. */ protected void visitBlock( BlockTree node, @@ -2176,12 +2228,21 @@ protected void visitStatements(List statements) { } } + protected void typeDeclarationModifiers(ModifiersTree modifiers) { + List typeAnnotations = + visitModifiers( + modifiers, Direction.VERTICAL, /* declarationAnnotationBreak= */ Optional.empty()); + verticalAnnotations(typeAnnotations); + } + /** Output combined modifiers and annotations and the trailing break. */ void visitAndBreakModifiers( ModifiersTree modifiers, Direction annotationDirection, Optional declarationAnnotationBreak) { - builder.addAll(visitModifiers(modifiers, annotationDirection, declarationAnnotationBreak)); + List typeAnnotations = + visitModifiers(modifiers, annotationDirection, declarationAnnotationBreak); + visitAnnotations(typeAnnotations, BreakOrNot.NO, BreakOrNot.YES); } @Override @@ -2190,36 +2251,50 @@ public Void visitModifiers(ModifiersTree node, Void unused) { } /** Output combined modifiers and annotations and returns the trailing break. */ - protected List visitModifiers( + @CheckReturnValue + protected ImmutableList visitModifiers( ModifiersTree modifiersTree, Direction annotationsDirection, Optional declarationAnnotationBreak) { return visitModifiers( - modifiersTree.getAnnotations(), annotationsDirection, declarationAnnotationBreak); + modifiersTree, + modifiersTree.getAnnotations(), + annotationsDirection, + declarationAnnotationBreak); } - protected List visitModifiers( + @CheckReturnValue + protected ImmutableList visitModifiers( + ModifiersTree modifiersTree, List annotationTrees, Direction annotationsDirection, Optional declarationAnnotationBreak) { - if (annotationTrees.isEmpty() && !nextIsModifier()) { - return EMPTY_LIST; + DeclarationModifiersAndTypeAnnotations splitModifiers = + splitModifiers(modifiersTree, annotationTrees); + return visitModifiers(splitModifiers, annotationsDirection, declarationAnnotationBreak); + } + + @CheckReturnValue + private ImmutableList visitModifiers( + DeclarationModifiersAndTypeAnnotations splitModifiers, + Direction annotationsDirection, + Optional declarationAnnotationBreak) { + if (splitModifiers.declarationModifiers().isEmpty()) { + return splitModifiers.typeAnnotations(); } - Deque annotations = new ArrayDeque<>(annotationTrees); + Deque declarationModifiers = + new ArrayDeque<>(splitModifiers.declarationModifiers()); builder.open(ZERO); boolean first = true; boolean lastWasAnnotation = false; - while (!annotations.isEmpty()) { - if (nextIsModifier()) { - break; - } + while (!declarationModifiers.isEmpty() && !declarationModifiers.peekFirst().isModifier()) { if (!first) { builder.addAll( annotationsDirection.isVertical() ? forceBreakList(declarationAnnotationBreak) : breakList(declarationAnnotationBreak)); } - scan(annotations.removeFirst(), null); + formatAnnotationOrModifier(declarationModifiers.removeFirst()); first = false; lastWasAnnotation = true; } @@ -2228,8 +2303,9 @@ protected List visitModifiers( annotationsDirection.isVertical() ? forceBreakList(declarationAnnotationBreak) : breakList(declarationAnnotationBreak); - if (annotations.isEmpty() && !nextIsModifier()) { - return trailingBreak; + if (declarationModifiers.isEmpty()) { + builder.addAll(trailingBreak); + return splitModifiers.typeAnnotations(); } if (lastWasAnnotation) { builder.addAll(trailingBreak); @@ -2237,24 +2313,170 @@ protected List visitModifiers( builder.open(ZERO); first = true; - while (nextIsModifier() || !annotations.isEmpty()) { + while (!declarationModifiers.isEmpty()) { if (!first) { builder.addAll(breakFillList(Optional.empty())); } - if (nextIsModifier()) { - token(builder.peekToken().get()); - } else { - scan(annotations.removeFirst(), null); - lastWasAnnotation = true; - } + formatAnnotationOrModifier(declarationModifiers.removeFirst()); first = false; } builder.close(); - return breakFillList(Optional.empty()); + builder.addAll(breakFillList(Optional.empty())); + return splitModifiers.typeAnnotations(); + } + + /** Represents an annotation or a modifier in a {@link ModifiersTree}. */ + @AutoOneOf(AnnotationOrModifier.Kind.class) + abstract static class AnnotationOrModifier implements Comparable { + enum Kind { + MODIFIER, + ANNOTATION + } + + abstract Kind getKind(); + + abstract AnnotationTree annotation(); + + abstract Input.Tok modifier(); + + static AnnotationOrModifier ofModifier(Input.Tok m) { + return AutoOneOf_JavaInputAstVisitor_AnnotationOrModifier.modifier(m); + } + + static AnnotationOrModifier ofAnnotation(AnnotationTree a) { + return AutoOneOf_JavaInputAstVisitor_AnnotationOrModifier.annotation(a); + } + + boolean isModifier() { + return getKind().equals(Kind.MODIFIER); + } + + boolean isAnnotation() { + return getKind().equals(Kind.ANNOTATION); + } + + int position() { + switch (getKind()) { + case MODIFIER: + return modifier().getPosition(); + case ANNOTATION: + return getStartPosition(annotation()); + } + throw new AssertionError(); + } + + private static final Comparator COMPARATOR = + Comparator.comparingInt(AnnotationOrModifier::position); + + @Override + public int compareTo(AnnotationOrModifier o) { + return COMPARATOR.compare(this, o); + } + } + + /** + * The modifiers annotations for a declaration, grouped in to a prefix that contains all of the + * declaration annotations and modifiers, and a suffix of type annotations. + * + *

For examples like {@code @Deprecated public @Nullable Foo foo();}, this allows us to format + * {@code @Deprecated public} as declaration modifiers, and {@code @Nullable} as a type annotation + * on the return type. + */ + @AutoValue + abstract static class DeclarationModifiersAndTypeAnnotations { + abstract ImmutableList declarationModifiers(); + + abstract ImmutableList typeAnnotations(); + + static DeclarationModifiersAndTypeAnnotations create( + ImmutableList declarationModifiers, + ImmutableList typeAnnotations) { + return new AutoValue_JavaInputAstVisitor_DeclarationModifiersAndTypeAnnotations( + declarationModifiers, typeAnnotations); + } + + static DeclarationModifiersAndTypeAnnotations empty() { + return create(ImmutableList.of(), ImmutableList.of()); + } + + boolean hasDeclarationAnnotation() { + return declarationModifiers().stream().anyMatch(AnnotationOrModifier::isAnnotation); + } + } + + /** + * Examines the token stream to convert the modifiers for a declaration into a {@link + * DeclarationModifiersAndTypeAnnotations}. + */ + DeclarationModifiersAndTypeAnnotations splitModifiers( + ModifiersTree modifiersTree, List annotations) { + if (annotations.isEmpty() && !isModifier(builder.peekToken().get())) { + return DeclarationModifiersAndTypeAnnotations.empty(); + } + RangeSet annotationRanges = TreeRangeSet.create(); + for (AnnotationTree annotationTree : annotations) { + annotationRanges.add( + Range.closedOpen( + getStartPosition(annotationTree), getEndPosition(annotationTree, getCurrentPath()))); + } + ImmutableList toks = + builder.peekTokens( + getStartPosition(modifiersTree), + (Input.Tok tok) -> + // ModifiersTree end position information isn't reliable, so scan tokens as long as + // we're seeing annotations or modifiers + annotationRanges.contains(tok.getPosition()) || isModifier(tok.getText())); + ImmutableList modifiers = + ImmutableList.copyOf( + Streams.concat( + toks.stream() + // reject tokens from inside AnnotationTrees, we only want modifiers + .filter(t -> !annotationRanges.contains(t.getPosition())) + .map(AnnotationOrModifier::ofModifier), + annotations.stream().map(AnnotationOrModifier::ofAnnotation)) + .sorted() + .collect(toList())); + // Take a suffix of annotations that are well-known type annotations, and which appear after any + // declaration annotations or modifiers + ImmutableList.Builder typeAnnotations = ImmutableList.builder(); + int idx = modifiers.size() - 1; + while (idx >= 0) { + AnnotationOrModifier modifier = modifiers.get(idx); + if (!modifier.isAnnotation() || !isTypeAnnotation(modifier.annotation())) { + break; + } + typeAnnotations.add(modifier.annotation()); + idx--; + } + return DeclarationModifiersAndTypeAnnotations.create( + modifiers.subList(0, idx + 1), typeAnnotations.build().reverse()); + } + + private void formatAnnotationOrModifier(AnnotationOrModifier modifier) { + switch (modifier.getKind()) { + case MODIFIER: + token(modifier.modifier().getText()); + break; + case ANNOTATION: + scan(modifier.annotation(), null); + break; + } + } + + boolean isTypeAnnotation(AnnotationTree annotationTree) { + Tree annotationType = annotationTree.getAnnotationType(); + if (!(annotationType instanceof IdentifierTree)) { + return false; + } + return typeAnnotationSimpleNames.contains(((IdentifierTree) annotationType).getName()); } boolean nextIsModifier() { - switch (builder.peekToken().get()) { + return isModifier(builder.peekToken().get()); + } + + private static boolean isModifier(String token) { + switch (token) { case "public": case "protected": case "private": @@ -2877,7 +3099,7 @@ private void visitDotWithPrefix( } /** Returns the simple names of expressions in a "." chain. */ - private List simpleNames(Deque stack) { + private static ImmutableList simpleNames(Deque stack) { ImmutableList.Builder simpleNames = ImmutableList.builder(); OUTER: for (ExpressionTree expression : stack) { @@ -2934,14 +3156,14 @@ private void dotExpressionUpToArgs(ExpressionTree expression, Optional * Returns the base expression of an erray access, e.g. given {@code foo[0][0]} returns {@code * foo}. */ - private ExpressionTree getArrayBase(ExpressionTree node) { + private static ExpressionTree getArrayBase(ExpressionTree node) { while (node instanceof ArrayAccessTree) { node = ((ArrayAccessTree) node).getExpression(); } return node; } - private ExpressionTree getMethodReceiver(MethodInvocationTree methodInvocation) { + private static ExpressionTree getMethodReceiver(MethodInvocationTree methodInvocation) { ExpressionTree select = methodInvocation.getMethodSelect(); return select instanceof MemberSelectTree ? ((MemberSelectTree) select).getExpression() : null; } @@ -2982,7 +3204,7 @@ private void formatArrayIndices(Deque indices) { * Returns all array indices for the given expression, e.g. given {@code foo[0][0]} returns the * expressions for {@code [0][0]}. */ - private Deque getArrayIndices(ExpressionTree expression) { + private static Deque getArrayIndices(ExpressionTree expression) { Deque indices = new ArrayDeque<>(); while (expression instanceof ArrayAccessTree) { ArrayAccessTree array = (ArrayAccessTree) expression; @@ -3282,16 +3504,22 @@ int declareOne( new ArrayDeque<>(typeWithDims.isPresent() ? typeWithDims.get().dims : ImmutableList.of()); int baseDims = 0; + // preprocess to separate declaration annotations + modifiers, type annotations + + DeclarationModifiersAndTypeAnnotations declarationAndTypeModifiers = + modifiers + .map(m -> splitModifiers(m, m.getAnnotations())) + .orElse(DeclarationModifiersAndTypeAnnotations.empty()); builder.open( - kind == DeclarationKind.PARAMETER - && (modifiers.isPresent() && !modifiers.get().getAnnotations().isEmpty()) + kind == DeclarationKind.PARAMETER && declarationAndTypeModifiers.hasDeclarationAnnotation() ? plusFour : ZERO); { - if (modifiers.isPresent()) { - visitAndBreakModifiers( - modifiers.get(), annotationsDirection, Optional.of(verticalAnnotationBreak)); - } + List annotations = + visitModifiers( + declarationAndTypeModifiers, + annotationsDirection, + Optional.of(verticalAnnotationBreak)); boolean isVar = builder.peekToken().get().equals("var") && (!name.contentEquals("var") || builder.peekToken(1).get().equals("var")); @@ -3302,6 +3530,7 @@ int declareOne( { builder.open(ZERO); { + visitAnnotations(annotations, BreakOrNot.NO, BreakOrNot.YES); if (typeWithDims.isPresent() && typeWithDims.get().node != null) { scan(typeWithDims.get().node, null); int totalDims = dims.size(); @@ -3573,7 +3802,8 @@ private void classDeclarationTypeList(String token, List types) * *

e.g. {@code int x, y;} is parsed as {@code int x; int y;}. */ - private List variableFragments(PeekingIterator it, Tree first) { + private static List variableFragments( + PeekingIterator it, Tree first) { List fragments = new ArrayList<>(); if (first.getKind() == VARIABLE) { int start = getStartPosition(first); @@ -3623,7 +3853,7 @@ private boolean hasTrailingToken(Input input, List nodes, String * @param modifiers the list of {@link ModifiersTree}s * @return whether the local can be declared with horizontal annotations */ - private Direction canLocalHaveHorizontalAnnotations(ModifiersTree modifiers) { + private static Direction canLocalHaveHorizontalAnnotations(ModifiersTree modifiers) { int parameterlessAnnotations = 0; for (AnnotationTree annotation : modifiers.getAnnotations()) { if (annotation.getArguments().isEmpty()) { @@ -3640,7 +3870,7 @@ private Direction canLocalHaveHorizontalAnnotations(ModifiersTree modifiers) { * Should a field with a set of modifiers be declared with horizontal annotations? This is * currently true if all annotations are parameterless annotations. */ - private Direction fieldAnnotationDirection(ModifiersTree modifiers) { + private static Direction fieldAnnotationDirection(ModifiersTree modifiers) { for (AnnotationTree annotation : modifiers.getAnnotations()) { if (!annotation.getArguments().isEmpty()) { return Direction.VERTICAL; diff --git a/core/src/main/java/com/google/googlejavaformat/java/java14/Java14InputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/java14/Java14InputAstVisitor.java index e5227da4b..b0d2a7675 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/java14/Java14InputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/java14/Java14InputAstVisitor.java @@ -18,10 +18,10 @@ import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; -import com.google.googlejavaformat.Op; import com.google.googlejavaformat.OpsBuilder; import com.google.googlejavaformat.OpsBuilder.BlankLineWanted; import com.google.googlejavaformat.java.JavaInputAstVisitor; +import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.BindingPatternTree; import com.sun.source.tree.BlockTree; import com.sun.source.tree.CaseTree; @@ -112,7 +112,9 @@ public Void visitBindingPattern(BindingPatternTree node, Void unused) { private void visitBindingPattern(ModifiersTree modifiers, Tree type, Name name) { if (modifiers != null) { - builder.addAll(visitModifiers(modifiers, Direction.HORIZONTAL, Optional.empty())); + List annotations = + visitModifiers(modifiers, Direction.HORIZONTAL, Optional.empty()); + visitAnnotations(annotations, BreakOrNot.NO, BreakOrNot.YES); } scan(type, null); builder.breakOp(" "); @@ -160,14 +162,9 @@ public Void visitClass(ClassTree tree, Void unused) { public void visitRecordDeclaration(ClassTree node) { sync(node); - List breaks = - visitModifiers( - node.getModifiers(), - Direction.VERTICAL, - /* declarationAnnotationBreak= */ Optional.empty()); + typeDeclarationModifiers(node.getModifiers()); Verify.verify(node.getExtendsClause() == null); boolean hasSuperInterfaceTypes = !node.getImplementsClause().isEmpty(); - builder.addAll(breaks); token("record"); builder.space(); visit(node.getSimpleName()); diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.input new file mode 100644 index 000000000..ddaa8f1ad --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.input @@ -0,0 +1,33 @@ +import org.checkerframework.checker.nullness.qual.Nullable; + +class TypeAnnotations { + + @Deprecated + public @Nullable Object foo() {} + + public @Deprecated Object foo() {} + + @Nullable Foo handle() { + @Nullable Bar bar = bar(); + try (@Nullable Baz baz = baz()) {} + } + + Foo( + @Nullable Bar // + param1, // + Baz // + param2) {} + + void g( + @Deprecated @Nullable ImmutableList veryVeryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, + @Deprecated @Nullable ImmutableList veryVeryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooong) {} + + @Deprecated @Nullable TypeAnnotations() {} + + enum Foo { + @Nullable + BAR; + } + + @Nullable @Nullable Object doubleTrouble() {} +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.output new file mode 100644 index 000000000..8dd5d4efc --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.output @@ -0,0 +1,39 @@ +import org.checkerframework.checker.nullness.qual.Nullable; + +class TypeAnnotations { + + @Deprecated + public @Nullable Object foo() {} + + public @Deprecated Object foo() {} + + @Nullable Foo handle() { + @Nullable Bar bar = bar(); + try (@Nullable Baz baz = baz()) {} + } + + Foo( + @Nullable Bar // + param1, // + Baz // + param2) {} + + void g( + @Deprecated + @Nullable ImmutableList + veryVeryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong, + @Deprecated + @Nullable ImmutableList + veryVeryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooong) {} + + @Deprecated + @Nullable + TypeAnnotations() {} + + enum Foo { + @Nullable + BAR; + } + + @Nullable @Nullable Object doubleTrouble() {} +} diff --git a/pom.xml b/pom.xml index aa85ac7c2..c66ed2f23 100644 --- a/pom.xml +++ b/pom.xml @@ -94,6 +94,7 @@ 1.0 3.6.1 2.7.1 + 1.8.2 3.1.0 3.2.1 @@ -118,6 +119,11 @@ error_prone_annotations ${errorprone.version} + + com.google.auto.value + auto-value-annotations + ${auto-value.version} + @@ -209,6 +215,11 @@ error_prone_core ${errorprone.version} + + com.google.auto.value + auto-value + ${auto-value.version} +