From 37f716a1143879a1d8cd94ecb8e976eee593d43f Mon Sep 17 00:00:00 2001 From: google-java-format Team Date: Tue, 24 Aug 2021 17:42:10 -0700 Subject: [PATCH] Automated rollback of commit 1a875792f6a074f3f0a504ddb8c7cd932ba6b073. *** Reason for rollback *** TAP shows this is broke Voice Access (and perhaps other projects) [] *** Original change description *** 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... *** PiperOrigin-RevId: 392785771 --- core/pom.xml | 6 +- .../google/googlejavaformat/OpsBuilder.java | 24 -- .../java/JavaInputAstVisitor.java | 340 +++--------------- .../java/java14/Java14InputAstVisitor.java | 13 +- .../java/testdata/TypeAnnotations.input | 33 -- .../java/testdata/TypeAnnotations.output | 39 -- pom.xml | 11 - 7 files changed, 68 insertions(+), 398 deletions(-) delete mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.input delete mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.output diff --git a/core/pom.xml b/core/pom.xml index e8e9bf249..74b7504ac 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -51,11 +51,7 @@ 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 db431c040..36e038adf 100644 --- a/core/src/main/java/com/google/googlejavaformat/OpsBuilder.java +++ b/core/src/main/java/com/google/googlejavaformat/OpsBuilder.java @@ -18,8 +18,6 @@ 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; @@ -283,28 +281,6 @@ 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 d0cd7976f..372f3bb59 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java @@ -14,8 +14,6 @@ package com.google.googlejavaformat.java; -import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; import static com.google.common.collect.Iterables.getLast; import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.googlejavaformat.Doc.FillMode.INDEPENDENT; @@ -45,26 +43,19 @@ 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.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; @@ -148,9 +139,7 @@ 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; @@ -178,7 +167,7 @@ boolean isVertical() { } /** Whether to break or not. */ - protected enum BreakOrNot { + enum BreakOrNot { YES, NO; @@ -279,13 +268,6 @@ boolean isYes() { } } - // TODO(cushon): generalize this - private static final ImmutableMultimap TYPE_ANNOTATIONS = - Stream.of( - "org.jspecify.nullness.Nullable", - "org.checkerframework.checker.nullness.qual.Nullable") - .collect(toImmutableSetMultimap(x -> x.substring(x.lastIndexOf('.') + 1), x -> x)); - protected final OpsBuilder builder; protected static final Indent.Const ZERO = Indent.Const.ZERO; @@ -295,8 +277,6 @@ 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)); } @@ -312,6 +292,8 @@ 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. @@ -435,7 +417,10 @@ public Void visitClass(ClassTree tree, Void unused) { public void visitAnnotationType(ClassTree node) { sync(node); builder.open(ZERO); - typeDeclarationModifiers(node.getModifiers()); + visitAndBreakModifiers( + node.getModifiers(), + Direction.VERTICAL, + /* declarationAnnotationBreak= */ Optional.empty()); builder.open(ZERO); token("@"); token("interface"); @@ -694,10 +679,9 @@ public Void visitNewClass(NewClassTree node, Void unused) { builder.space(); addTypeArguments(node.getTypeArguments(), plusFour); if (node.getClassBody() != null) { - List annotations = + builder.addAll( visitModifiers( - node.getClassBody().getModifiers(), Direction.HORIZONTAL, Optional.empty()); - visitAnnotations(annotations, BreakOrNot.NO, BreakOrNot.YES); + node.getClassBody().getModifiers(), Direction.HORIZONTAL, Optional.empty())); } scan(node.getIdentifier(), null); addArguments(node.getArguments(), plusFour); @@ -818,7 +802,10 @@ private void visitEnumConstantDeclaration(VariableTree enumConstant) { public boolean visitEnumDeclaration(ClassTree node) { sync(node); builder.open(ZERO); - typeDeclarationModifiers(node.getModifiers()); + visitAndBreakModifiers( + node.getModifiers(), + Direction.VERTICAL, + /* declarationAnnotationBreak= */ Optional.empty()); builder.open(plusFour); token("enum"); builder.breakOp(" "); @@ -982,7 +969,7 @@ void visitVariables( } } - private static TypeWithDims variableFragmentDims(boolean first, int leadingDims, Tree type) { + private TypeWithDims variableFragmentDims(boolean first, int leadingDims, Tree type) { if (type == null) { return null; } @@ -1127,7 +1114,6 @@ public Void visitIf(IfTree node, Void unused) { @Override public Void visitImport(ImportTree node, Void unused) { - checkForTypeAnnotation(node); sync(node); token("import"); builder.space(); @@ -1142,21 +1128,6 @@ 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); @@ -1389,12 +1360,9 @@ public Void visitMethod(MethodTree node, Void unused) { } } } - List typeAnnotations = + builder.addAll( visitModifiers( - node.getModifiers(), - annotations, - Direction.VERTICAL, - /* declarationAnnotationBreak= */ Optional.empty()); + annotations, Direction.VERTICAL, /* declarationAnnotationBreak= */ Optional.empty())); Tree baseReturnType = null; Deque> dims = null; @@ -1403,9 +1371,6 @@ 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); @@ -1414,14 +1379,7 @@ 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()) { @@ -1998,11 +1956,16 @@ public Void visitTry(TryTree node, Void unused) { public void visitClassDeclaration(ClassTree node) { sync(node); - typeDeclarationModifiers(node.getModifiers()); + List breaks = + visitModifiers( + node.getModifiers(), + Direction.VERTICAL, + /* declarationAnnotationBreak= */ Optional.empty()); 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()); @@ -2106,7 +2069,7 @@ public Void visitWildcard(WildcardTree node, Void unused) { // Helper methods. /** Helper method for annotations. */ - protected void visitAnnotations( + void visitAnnotations( List annotations, BreakOrNot breakBefore, BreakOrNot breakAfter) { if (!annotations.isEmpty()) { if (breakBefore.isYes()) { @@ -2126,14 +2089,6 @@ protected 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, @@ -2221,21 +2176,12 @@ 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) { - List typeAnnotations = - visitModifiers(modifiers, annotationDirection, declarationAnnotationBreak); - visitAnnotations(typeAnnotations, BreakOrNot.NO, BreakOrNot.YES); + builder.addAll(visitModifiers(modifiers, annotationDirection, declarationAnnotationBreak)); } @Override @@ -2244,50 +2190,36 @@ public Void visitModifiers(ModifiersTree node, Void unused) { } /** Output combined modifiers and annotations and returns the trailing break. */ - @CheckReturnValue - protected ImmutableList visitModifiers( + protected List visitModifiers( ModifiersTree modifiersTree, Direction annotationsDirection, Optional declarationAnnotationBreak) { return visitModifiers( - modifiersTree, - modifiersTree.getAnnotations(), - annotationsDirection, - declarationAnnotationBreak); + modifiersTree.getAnnotations(), annotationsDirection, declarationAnnotationBreak); } - @CheckReturnValue - protected ImmutableList visitModifiers( - ModifiersTree modifiersTree, + protected List visitModifiers( List annotationTrees, Direction annotationsDirection, Optional declarationAnnotationBreak) { - 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(); + if (annotationTrees.isEmpty() && !nextIsModifier()) { + return EMPTY_LIST; } - Deque declarationModifiers = - new ArrayDeque<>(splitModifiers.declarationModifiers()); + Deque annotations = new ArrayDeque<>(annotationTrees); builder.open(ZERO); boolean first = true; boolean lastWasAnnotation = false; - while (!declarationModifiers.isEmpty() && !declarationModifiers.peekFirst().isModifier()) { + while (!annotations.isEmpty()) { + if (nextIsModifier()) { + break; + } if (!first) { builder.addAll( annotationsDirection.isVertical() ? forceBreakList(declarationAnnotationBreak) : breakList(declarationAnnotationBreak)); } - formatAnnotationOrModifier(declarationModifiers.removeFirst()); + scan(annotations.removeFirst(), null); first = false; lastWasAnnotation = true; } @@ -2296,9 +2228,8 @@ private ImmutableList visitModifiers( annotationsDirection.isVertical() ? forceBreakList(declarationAnnotationBreak) : breakList(declarationAnnotationBreak); - if (declarationModifiers.isEmpty()) { - builder.addAll(trailingBreak); - return splitModifiers.typeAnnotations(); + if (annotations.isEmpty() && !nextIsModifier()) { + return trailingBreak; } if (lastWasAnnotation) { builder.addAll(trailingBreak); @@ -2306,169 +2237,24 @@ private ImmutableList visitModifiers( builder.open(ZERO); first = true; - while (!declarationModifiers.isEmpty()) { + while (nextIsModifier() || !annotations.isEmpty()) { if (!first) { builder.addAll(breakFillList(Optional.empty())); } - formatAnnotationOrModifier(declarationModifiers.removeFirst()); + if (nextIsModifier()) { + token(builder.peekToken().get()); + } else { + scan(annotations.removeFirst(), null); + lastWasAnnotation = true; + } first = false; } builder.close(); - 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 = - 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(toImmutableList()); - // 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()); + return breakFillList(Optional.empty()); } boolean nextIsModifier() { - return isModifier(builder.peekToken().get()); - } - - private static boolean isModifier(String token) { - switch (token) { + switch (builder.peekToken().get()) { case "public": case "protected": case "private": @@ -3091,7 +2877,7 @@ private void visitDotWithPrefix( } /** Returns the simple names of expressions in a "." chain. */ - private static ImmutableList simpleNames(Deque stack) { + private List simpleNames(Deque stack) { ImmutableList.Builder simpleNames = ImmutableList.builder(); OUTER: for (ExpressionTree expression : stack) { @@ -3148,14 +2934,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 static ExpressionTree getArrayBase(ExpressionTree node) { + private ExpressionTree getArrayBase(ExpressionTree node) { while (node instanceof ArrayAccessTree) { node = ((ArrayAccessTree) node).getExpression(); } return node; } - private static ExpressionTree getMethodReceiver(MethodInvocationTree methodInvocation) { + private ExpressionTree getMethodReceiver(MethodInvocationTree methodInvocation) { ExpressionTree select = methodInvocation.getMethodSelect(); return select instanceof MemberSelectTree ? ((MemberSelectTree) select).getExpression() : null; } @@ -3196,7 +2982,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 static Deque getArrayIndices(ExpressionTree expression) { + private Deque getArrayIndices(ExpressionTree expression) { Deque indices = new ArrayDeque<>(); while (expression instanceof ArrayAccessTree) { ArrayAccessTree array = (ArrayAccessTree) expression; @@ -3496,22 +3282,16 @@ 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 && declarationAndTypeModifiers.hasDeclarationAnnotation() + kind == DeclarationKind.PARAMETER + && (modifiers.isPresent() && !modifiers.get().getAnnotations().isEmpty()) ? plusFour : ZERO); { - List annotations = - visitModifiers( - declarationAndTypeModifiers, - annotationsDirection, - Optional.of(verticalAnnotationBreak)); + if (modifiers.isPresent()) { + visitAndBreakModifiers( + modifiers.get(), annotationsDirection, Optional.of(verticalAnnotationBreak)); + } boolean isVar = builder.peekToken().get().equals("var") && (!name.contentEquals("var") || builder.peekToken(1).get().equals("var")); @@ -3522,7 +3302,6 @@ 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(); @@ -3794,8 +3573,7 @@ private void classDeclarationTypeList(String token, List types) * *

e.g. {@code int x, y;} is parsed as {@code int x; int y;}. */ - private static List variableFragments( - PeekingIterator it, Tree first) { + private List variableFragments(PeekingIterator it, Tree first) { List fragments = new ArrayList<>(); if (first.getKind() == VARIABLE) { int start = getStartPosition(first); @@ -3845,7 +3623,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 static Direction canLocalHaveHorizontalAnnotations(ModifiersTree modifiers) { + private Direction canLocalHaveHorizontalAnnotations(ModifiersTree modifiers) { int parameterlessAnnotations = 0; for (AnnotationTree annotation : modifiers.getAnnotations()) { if (annotation.getArguments().isEmpty()) { @@ -3862,7 +3640,7 @@ private static Direction canLocalHaveHorizontalAnnotations(ModifiersTree modifie * Should a field with a set of modifiers be declared with horizontal annotations? This is * currently true if all annotations are parameterless annotations. */ - private static Direction fieldAnnotationDirection(ModifiersTree modifiers) { + private 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 b0d2a7675..e5227da4b 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,9 +112,7 @@ public Void visitBindingPattern(BindingPatternTree node, Void unused) { private void visitBindingPattern(ModifiersTree modifiers, Tree type, Name name) { if (modifiers != null) { - List annotations = - visitModifiers(modifiers, Direction.HORIZONTAL, Optional.empty()); - visitAnnotations(annotations, BreakOrNot.NO, BreakOrNot.YES); + builder.addAll(visitModifiers(modifiers, Direction.HORIZONTAL, Optional.empty())); } scan(type, null); builder.breakOp(" "); @@ -162,9 +160,14 @@ public Void visitClass(ClassTree tree, Void unused) { public void visitRecordDeclaration(ClassTree node) { sync(node); - typeDeclarationModifiers(node.getModifiers()); + List breaks = + visitModifiers( + node.getModifiers(), + Direction.VERTICAL, + /* declarationAnnotationBreak= */ Optional.empty()); 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 deleted file mode 100644 index ddaa8f1ad..000000000 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.input +++ /dev/null @@ -1,33 +0,0 @@ -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 deleted file mode 100644 index 8dd5d4efc..000000000 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/TypeAnnotations.output +++ /dev/null @@ -1,39 +0,0 @@ -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 c66ed2f23..aa85ac7c2 100644 --- a/pom.xml +++ b/pom.xml @@ -94,7 +94,6 @@ 1.0 3.6.1 2.7.1 - 1.8.2 3.1.0 3.2.1 @@ -119,11 +118,6 @@ error_prone_annotations ${errorprone.version} - - com.google.auto.value - auto-value-annotations - ${auto-value.version} - @@ -215,11 +209,6 @@ error_prone_core ${errorprone.version} - - com.google.auto.value - auto-value - ${auto-value.version} -