From ec0849e26ccccee06016469a2e44a23cee542636 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Fri, 30 Jul 2021 09:14:55 -0700 Subject: [PATCH 01/16] Update version in README.md PiperOrigin-RevId: 387819445 --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 41ab0806f..9802ebaaf 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ and run it with: ``` -java -jar /path/to/google-java-format-1.10.0-all-deps.jar [files...] +java -jar /path/to/google-java-format-1.11.0-all-deps.jar [files...] ``` The formatter can act on whole files, on limited lines (`--lines`), on specific @@ -39,7 +39,7 @@ java \ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED \ - -jar google-java-format-1.10.0-all-deps.jar [files...] + -jar google-java-format-1.11.0-all-deps.jar [files...] ``` ### IntelliJ, Android Studio, and other JetBrains IDEs @@ -113,7 +113,7 @@ configuration. com.google.googlejavaformat google-java-format - 1.10.0 + 1.11.0 ``` @@ -121,7 +121,7 @@ configuration. ```groovy dependencies { - implementation 'com.google.googlejavaformat:google-java-format:1.10.0' + implementation 'com.google.googlejavaformat:google-java-format:1.11.0' } ``` From 184a5adc4774ae636f6dab4859490753d3d2ae0a Mon Sep 17 00:00:00 2001 From: Amit Mendapara Date: Fri, 30 Jul 2021 09:15:45 -0700 Subject: [PATCH 02/16] Improve version handling for eclipse plugin Use same versions for plugin as google-java-format and strip versions from copied jar to avoid updating build.properties and MANIFEST.MF. Fixes #635 COPYBARA_INTEGRATE_REVIEW=https://github.com/google/google-java-format/pull/635 from cristatus:eclipse-plugin b4f56be226c67375491bf40fb032e139923fe56f PiperOrigin-RevId: 387819617 --- eclipse_plugin/META-INF/MANIFEST.MF | 6 +++--- eclipse_plugin/README.md | 16 +++++---------- eclipse_plugin/build.properties | 4 ++-- eclipse_plugin/pom.xml | 30 ++++++++++++++--------------- 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/eclipse_plugin/META-INF/MANIFEST.MF b/eclipse_plugin/META-INF/MANIFEST.MF index b5583d602..34b1e8c88 100644 --- a/eclipse_plugin/META-INF/MANIFEST.MF +++ b/eclipse_plugin/META-INF/MANIFEST.MF @@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2 Bundle-Name: google-java-format Bundle-SymbolicName: google-java-format-eclipse-plugin;singleton:=true Bundle-Vendor: Google -Bundle-Version: 1.9.0 +Bundle-Version: 1.11.0 Bundle-RequiredExecutionEnvironment: JavaSE-11 Require-Bundle: org.eclipse.jdt.core;bundle-version="3.10.0", org.eclipse.jface, @@ -11,5 +11,5 @@ Require-Bundle: org.eclipse.jdt.core;bundle-version="3.10.0", org.eclipse.ui, org.eclipse.equinox.common Bundle-ClassPath: ., - lib/guava-30.1.1-jre.jar, - lib/google-java-format-1.9.jar + lib/guava.jar, + lib/google-java-format.jar diff --git a/eclipse_plugin/README.md b/eclipse_plugin/README.md index a45b46538..395a368dc 100644 --- a/eclipse_plugin/README.md +++ b/eclipse_plugin/README.md @@ -8,14 +8,9 @@ See https://github.com/google/google-java-format#eclipse ### Prerequisites -Make sure that the `build.properties` and `META-INF/MANIFEST.MF` contain all -necessary dependencies for the build. Furthermore, make sure that the -dependencies declared in the `pom.xml` match the entries in `build.properties` -and `META-INF/MANIFEST.MF`. - -If the used google java format core version is a 'SNAPSHOT' release, the version -for the Eclipse plugin in the `pom.xml` must end in '-SNAPSHOT' as well and the -bundle version specified in `META-INF/MANIFEST.MF` must end in '.qualifier'. +Before building the plugin, make sure to run `mvn +tycho-versions:update-eclipse-metadata` to update the bundle version in +`META-INF/MANIFEST.MF`. ### Building the Plugin @@ -39,9 +34,8 @@ information on this issue is given #### Building against a local (snapshot) release of the core With the current build setup, the Eclipse plugin build pulls the needed build -artifacts of the google java format core specified in the property -`google-java-format.version` and copies it into the `eclipse_plugin/lib/` -directory. +artifacts of the google java format core from the maven repository and copies it +into the `eclipse_plugin/lib/` directory. If you instead want to build against a local (snapshot) build of the core which is not available in a maven repository (local or otherwise), you will have to diff --git a/eclipse_plugin/build.properties b/eclipse_plugin/build.properties index de8466a3f..dd1d835c5 100644 --- a/eclipse_plugin/build.properties +++ b/eclipse_plugin/build.properties @@ -3,5 +3,5 @@ output.. = target/classes bin.includes = META-INF/,\ .,\ plugin.xml,\ - lib/guava-30.1.1-jre.jar,\ - lib/google-java-format-1.9.jar + lib/guava.jar,\ + lib/google-java-format.jar diff --git a/eclipse_plugin/pom.xml b/eclipse_plugin/pom.xml index 600810687..6b49c901a 100644 --- a/eclipse_plugin/pom.xml +++ b/eclipse_plugin/pom.xml @@ -22,7 +22,7 @@ com.google.googlejavaformat google-java-format-eclipse-plugin eclipse-plugin - 1.9.0 + 1.11.0 Google Java Format Plugin for Eclipse 4.5+ @@ -32,26 +32,14 @@ UTF-8 - 1.7.0 - - - 1.9 - 30.1.1-jre com.google.googlejavaformat google-java-format - ${google-java-format.version} - jar - - - com.google.guava - guava - ${guava.version} - jar + ${project.version} @@ -81,8 +69,10 @@ lib runtime - true + true + true true + guava,google-java-format @@ -92,6 +82,16 @@ ${tycho-version} true + + + org.eclipse.tycho + tycho-versions-plugin + ${tycho-version} + + ${project.version} + + + org.eclipse.tycho target-platform-configuration From f2eabaa9a7a2fe1ccd17c30124d2041fe97a6313 Mon Sep 17 00:00:00 2001 From: Avi Mimoun <36456709+av1m@users.noreply.github.com> Date: Mon, 2 Aug 2021 10:25:57 -0700 Subject: [PATCH 03/16] Updates the eclipse plugin link Upgrade from version 1.6 to version 1.11 (according #465) Fixes #639 COPYBARA_INTEGRATE_REVIEW=https://github.com/google/google-java-format/pull/639 from av1m:patch-1 2c5c2b05a9b0257ac96fdc616772c1e0a9ed451c PiperOrigin-RevId: 388249150 --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9802ebaaf..202e69d16 100644 --- a/README.md +++ b/README.md @@ -70,8 +70,8 @@ and import it into File→Settings→Editor→Code Style. ### Eclipse -Version 1.6 of the -[google-java-format Eclipse plugin](https://github.com/google/google-java-format/releases/download/google-java-format-1.6/google-java-format-eclipse-plugin_1.6.0.jar) +Version 1.11 of the +[google-java-format Eclipse plugin](https://github.com/google/google-java-format/releases/download/v1.11.0/google-java-format-eclipse-plugin-1.11.0.jar) can be downloaded from the releases page. Drop it into the Eclipse [drop-ins folder](http://help.eclipse.org/neon/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fmisc%2Fp2_dropins_format.html) to activate the plugin. From 00511536792cd6b72d3cfb83d426d25521874e80 Mon Sep 17 00:00:00 2001 From: nakulj <3213360+nakulj@users.noreply.github.com> Date: Mon, 2 Aug 2021 13:33:12 -0700 Subject: [PATCH 04/16] Run 2to3 on google-java-format-diff.py Fixes #640 COPYBARA_INTEGRATE_REVIEW=https://github.com/google/google-java-format/pull/640 from nakulj:py2to3 8cf03bcdf5a62aea2bee0e5d9fb05ea88644c253 PiperOrigin-RevId: 388292852 --- scripts/google-java-format-diff.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/google-java-format-diff.py b/scripts/google-java-format-diff.py index 63685aac3..cf2ded9ec 100755 --- a/scripts/google-java-format-diff.py +++ b/scripts/google-java-format-diff.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python2.7 +#!/usr/bin/env python3 # #===- google-java-format-diff.py - google-java-format Diff Reformatter -----===# # @@ -31,7 +31,7 @@ import re import string import subprocess -import StringIO +import io import sys from distutils.spawn import find_executable @@ -109,9 +109,9 @@ def main(): base_command = [binary] # Reformat files containing changes in place. - for filename, lines in lines_by_file.iteritems(): + for filename, lines in lines_by_file.items(): if args.i and args.verbose: - print 'Formatting', filename + print('Formatting', filename) command = base_command[:] if args.i: command.append('-i') @@ -134,7 +134,7 @@ def main(): if not args.i: with open(filename) as f: code = f.readlines() - formatted_code = StringIO.StringIO(stdout).readlines() + formatted_code = io.StringIO(stdout).readlines() diff = difflib.unified_diff(code, formatted_code, filename, filename, '(before formatting)', '(after formatting)') From 1a875792f6a074f3f0a504ddb8c7cd932ba6b073 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Tue, 24 Aug 2021 16:11:35 -0700 Subject: [PATCH 05/16] 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 PiperOrigin-RevId: 392769609 --- 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, 398 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..d0cd7976f 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java @@ -14,6 +14,8 @@ 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; @@ -43,19 +45,26 @@ 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; @@ -139,7 +148,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 +178,7 @@ boolean isVertical() { } /** Whether to break or not. */ - enum BreakOrNot { + protected enum BreakOrNot { YES, NO; @@ -268,6 +279,13 @@ 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; @@ -277,6 +295,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 +312,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 +435,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 +694,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 +818,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 +982,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 +1127,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 +1142,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 +1389,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 +1403,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 +1414,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 +1998,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 +2106,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 +2126,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 +2221,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 +2244,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 +2296,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 +2306,169 @@ 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 = + 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()); } 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 +3091,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 +3148,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 +3196,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 +3496,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 +3522,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 +3794,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 +3845,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 +3862,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} + 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 06/16] 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} - From 865cff01c1bd5b7393f0dd88dbb92a499289e376 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 25 Aug 2021 09:53:59 -0700 Subject: [PATCH 07/16] 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} + From 79257edbf26894102793b00a19fe4b68d83e5622 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 25 Aug 2021 13:24:47 -0700 Subject: [PATCH 08/16] Fix indentation of case statements on JDK 17 Fixes https://github.com/google/google-java-format/issues/643 PiperOrigin-RevId: 392966720 --- .../java/java14/Java14InputAstVisitor.java | 2 +- .../googlejavaformat/java/testdata/I643.input | 14 ++++++++++++++ .../googlejavaformat/java/testdata/I643.output | 16 ++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/I643.input create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/I643.output 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..3facbd711 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 @@ -251,7 +251,7 @@ public Void visitCase(CaseTree node, Void unused) { token("default", plusTwo); } else { token("case", plusTwo); - builder.open(plusFour); + builder.open(node.getExpressions().size() > 1 ? plusFour : ZERO); builder.space(); boolean first = true; for (ExpressionTree expression : node.getExpressions()) { diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I643.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I643.input new file mode 100644 index 000000000..a31a230be --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I643.input @@ -0,0 +1,14 @@ +public class Foo { + static final int VERBOSE_WORDY_AND_LENGTHY_ONE = 1; + static final int VERBOSE_WORDY_AND_LENGTHY_TWO = 2; + static final int VERBOSE_WORDY_AND_LENGTHY_FOUR = 4; + + public static int fn(int x) { + switch (x) { + case VERBOSE_WORDY_AND_LENGTHY_ONE | VERBOSE_WORDY_AND_LENGTHY_TWO | VERBOSE_WORDY_AND_LENGTHY_FOUR: + return 0; + default: + return 1; + } + } +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I643.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I643.output new file mode 100644 index 000000000..945cbea14 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I643.output @@ -0,0 +1,16 @@ +public class Foo { + static final int VERBOSE_WORDY_AND_LENGTHY_ONE = 1; + static final int VERBOSE_WORDY_AND_LENGTHY_TWO = 2; + static final int VERBOSE_WORDY_AND_LENGTHY_FOUR = 4; + + public static int fn(int x) { + switch (x) { + case VERBOSE_WORDY_AND_LENGTHY_ONE + | VERBOSE_WORDY_AND_LENGTHY_TWO + | VERBOSE_WORDY_AND_LENGTHY_FOUR: + return 0; + default: + return 1; + } + } +} From 3fc15f7c5de79cf74ef95343aa8562a9c3b2dd2c Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 25 Aug 2021 15:36:46 -0700 Subject: [PATCH 09/16] Clean up some redundancies PiperOrigin-RevId: 392995117 --- core/src/main/java/com/google/googlejavaformat/java/Main.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/Main.java b/core/src/main/java/com/google/googlejavaformat/java/Main.java index 451bc697c..4056f2ae3 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Main.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Main.java @@ -41,7 +41,7 @@ public final class Main { private static final int MAX_THREADS = 20; private static final String STDIN_FILENAME = ""; - static final String versionString() { + static String versionString() { return "google-java-format: Version " + GoogleJavaFormatVersion.version(); } From 86db2b1609031532780af159a59dd1814227ea2b Mon Sep 17 00:00:00 2001 From: nlupien <68999381+nlupien@users.noreply.github.com> Date: Thu, 26 Aug 2021 07:51:10 -0700 Subject: [PATCH 10/16] Make the formatJavadoc option public. Right now the `formatJavadoc` option in the `JavaFormatterOptions` isn't tag as either public or private so it gets the default package private scope. This change makes it public so other projects using this as a dependency can change this option. In regards to tests, I'm not sure how to add the relevant test to validate the scope change but since we don't generally test scope, I assume we can skip them?. Fixes #512 COPYBARA_INTEGRATE_REVIEW=https://github.com/google/google-java-format/pull/512 from nlupien:make_format_javadoc_option_public 77e01d177b97934250d97a51ff9de569c3e5a436 PiperOrigin-RevId: 393123739 --- .../google/googlejavaformat/java/JavaFormatterOptions.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java b/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java index 4d3d30d4c..fbb6fe7e4 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java @@ -60,7 +60,7 @@ public int indentationMultiplier() { return style.indentationMultiplier(); } - boolean formatJavadoc() { + public boolean formatJavadoc() { return formatJavadoc; } @@ -91,7 +91,7 @@ public Builder style(Style style) { return this; } - Builder formatJavadoc(boolean formatJavadoc) { + public Builder formatJavadoc(boolean formatJavadoc) { this.formatJavadoc = formatJavadoc; return this; } From 129a70dd911566f3089796568a975afa98899b73 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Thu, 26 Aug 2021 13:55:47 -0700 Subject: [PATCH 11/16] Correct package name of javax.annotation.Nullable. It doesn't really matter, since we don't look things up in a classpath. But as we begin experimenting with recognizing specific fully qualified annotation names, there's an 0.1% change that it will matter somehow. And I just stumbled across this typo while doing research, so I figured I'd make sure it never matters. PiperOrigin-RevId: 393200270 --- .../java/com/google/googlejavaformat/java/FormatterTest.java | 4 ++-- .../com/google/googlejavaformat/java/ImportOrdererTest.java | 4 ++-- .../java/testimports/A.formatting-and-import-sorting | 2 +- .../java/testimports/A.formatting-and-unused-import-removal | 2 +- .../googlejavaformat/java/testimports/A.formatting-only | 2 +- .../java/testimports/A.imports-and-formatting | 2 +- .../google/googlejavaformat/java/testimports/A.imports-only | 2 +- .../com/google/googlejavaformat/java/testimports/A.input | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java index 9859245c7..1653e5645 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java @@ -238,7 +238,7 @@ public void voidMethod() throws FormatterException { "", "import java.util.List;", "", - "import javax.annotations.Nullable;"); + "import javax.annotation.Nullable;"); @Test public void importsNotReorderedByDefault() throws FormatterException { @@ -262,7 +262,7 @@ public void importsFixedIfRequested() throws FormatterException { String expect = "package com.google.example;\n\n" + "import java.util.List;\n" - + "import javax.annotations.Nullable;\n\n" + + "import javax.annotation.Nullable;\n\n" + "public class ExampleTest {\n" + " @Nullable List xs;\n" + "}\n"; diff --git a/core/src/test/java/com/google/googlejavaformat/java/ImportOrdererTest.java b/core/src/test/java/com/google/googlejavaformat/java/ImportOrdererTest.java index f40fa3821..0b9dab26f 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/ImportOrdererTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/ImportOrdererTest.java @@ -313,7 +313,7 @@ public static Collection parameters() { "", "import java.util.List;", "", - "import javax.annotations.Nullable;", + "import javax.annotation.Nullable;", "", "import static org.junit.Assert.fail;", "import static com.google.truth.Truth.assertThat;", @@ -329,7 +329,7 @@ public static Collection parameters() { "", "import com.google.common.base.Preconditions;", "import java.util.List;", - "import javax.annotations.Nullable;", + "import javax.annotation.Nullable;", "import org.junit.runner.RunWith;", "import org.junit.runners.JUnit4;", "", diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-and-import-sorting b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-and-import-sorting index e6994f7fd..8d144c2c7 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-and-import-sorting +++ b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-and-import-sorting @@ -6,7 +6,7 @@ import static org.junit.Assert.fail; import com.google.common.base.Preconditions; import java.util.List; import java.util.Set; -import javax.annotations.Nullable; +import javax.annotation.Nullable; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-and-unused-import-removal b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-and-unused-import-removal index 7d5df53cb..9b1d01f3f 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-and-unused-import-removal +++ b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-and-unused-import-removal @@ -7,7 +7,7 @@ import org.junit.runners.JUnit4; import java.util.List; -import javax.annotations.Nullable; +import javax.annotation.Nullable; import static org.junit.Assert.fail; import static com.google.truth.Truth.assertThat; diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-only b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-only index 7d5df53cb..9b1d01f3f 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-only +++ b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.formatting-only @@ -7,7 +7,7 @@ import org.junit.runners.JUnit4; import java.util.List; -import javax.annotations.Nullable; +import javax.annotation.Nullable; import static org.junit.Assert.fail; import static com.google.truth.Truth.assertThat; diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-and-formatting b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-and-formatting index 887d43c6e..d7bcd6d9b 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-and-formatting +++ b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-and-formatting @@ -5,7 +5,7 @@ import static org.junit.Assert.fail; import com.google.common.base.Preconditions; import java.util.List; -import javax.annotations.Nullable; +import javax.annotation.Nullable; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-only b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-only index 88f83f187..a50a83ec1 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-only +++ b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-only @@ -5,7 +5,7 @@ import static org.junit.Assert.fail; import com.google.common.base.Preconditions; import java.util.List; -import javax.annotations.Nullable; +import javax.annotation.Nullable; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.input b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.input index be1eacda1..dd992a32f 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.input +++ b/core/src/test/resources/com/google/googlejavaformat/java/testimports/A.input @@ -8,7 +8,7 @@ import org.junit.runners.JUnit4; import java.util.List; import java.util.Set; -import javax.annotations.Nullable; +import javax.annotation.Nullable; import static org.junit.Assert.fail; import static com.google.truth.Truth.assertThat; From 67f75cb5a5377346266a8ae93526c6bc5a7cb36c Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Thu, 2 Sep 2021 15:35:02 -0700 Subject: [PATCH 12/16] Migrate from adopt https://github.com/actions/setup-java#supported-distributions https://blog.adoptopenjdk.net/2021/08/goodbye-adoptopenjdk-hello-adoptium/ PiperOrigin-RevId: 394558213 --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cfd9cf37b..7f4080a67 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,7 +63,7 @@ jobs: uses: actions/setup-java@v2 with: java-version: ${{ matrix.java }} - distribution: 'adopt' + distribution: 'zulu' - name: 'Install' shell: bash run: mvn install -DskipTests=true -Dmaven.javadoc.skip=true -B -V @@ -90,7 +90,7 @@ jobs: uses: actions/setup-java@v2 with: java-version: 15 - distribution: 'adopt' + distribution: 'zulu' server-id: sonatype-nexus-snapshots server-username: CI_DEPLOY_USERNAME server-password: CI_DEPLOY_PASSWORD From 8490fce70fc6182e1b298fe08ec9eda2961c525e Mon Sep 17 00:00:00 2001 From: Daniel Kraus Date: Thu, 9 Sep 2021 04:49:52 -0700 Subject: [PATCH 13/16] Cache local Maven repository via actions/setup-java As of August 30, ["`setup-java` supports caching for both Gradle and Maven projects"](https://github.blog/changelog/2021-08-30-github-actions-setup-java-now-supports-dependency-caching/). This helps to reduce the amount of boilerplate code in the workflow file. More details can be found in the [corresponding docs](https://github.com/actions/setup-java#caching-packages-dependencies). (And if you wonder what's the used cache key format: https://github.com/actions/setup-java/pull/215/) Fixes #660 COPYBARA_INTEGRATE_REVIEW=https://github.com/google/google-java-format/pull/660 from beatngu13:patch-1 47e262b489e0f3ce795413bd10b7fe50a22ca2c6 PiperOrigin-RevId: 395674252 --- .github/workflows/ci.yml | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7f4080a67..0386892e9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,18 +52,12 @@ jobs: access_token: ${{ github.token }} - name: 'Check out repository' uses: actions/checkout@v2 - - name: 'Cache local Maven repository' - uses: actions/cache@v2 - with: - path: ~/.m2/repository - key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }} - restore-keys: | - ${{ runner.os }}-maven- - name: 'Set up JDK ${{ matrix.java }}' uses: actions/setup-java@v2 with: java-version: ${{ matrix.java }} distribution: 'zulu' + cache: 'maven' - name: 'Install' shell: bash run: mvn install -DskipTests=true -Dmaven.javadoc.skip=true -B -V @@ -79,18 +73,12 @@ jobs: steps: - name: 'Check out repository' uses: actions/checkout@v2 - - name: 'Cache local Maven repository' - uses: actions/cache@v2 - with: - path: ~/.m2/repository - key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }} - restore-keys: | - ${{ runner.os }}-maven- - name: 'Set up JDK 15' uses: actions/setup-java@v2 with: java-version: 15 distribution: 'zulu' + cache: 'maven' server-id: sonatype-nexus-snapshots server-username: CI_DEPLOY_USERNAME server-password: CI_DEPLOY_PASSWORD From 2c5d5fd85bebbcb7e3b146d983e3027930aee8ef Mon Sep 17 00:00:00 2001 From: google-java-format Team Date: Wed, 15 Sep 2021 15:19:58 -0700 Subject: [PATCH 14/16] Automatic code cleanup. COPYBARA_INTEGRATE_REVIEW=https://github.com/google/google-java-format/pull/656 from sormuras:issues/561-toolprovider 82130f3bb3b4cb9d8bdd62e29c78f818f3108de5 PiperOrigin-RevId: 396936217 --- core/pom.xml | 5 ++ .../java/GoogleJavaFormatToolProvider.java | 38 +++++++++++++ .../google/googlejavaformat/java/Main.java | 15 +++-- .../java/javadoc/JavadocWriter.java | 3 +- .../GoogleJavaFormatToolProviderTest.java | 57 +++++++++++++++++++ pom.xml | 11 ++++ 6 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 core/src/main/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProvider.java create mode 100644 core/src/test/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProviderTest.java diff --git a/core/pom.xml b/core/pom.xml index e8e9bf249..5ebdb591b 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -56,6 +56,11 @@ auto-value-annotations true + + com.google.auto.service + auto-service-annotations + true + diff --git a/core/src/main/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProvider.java b/core/src/main/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProvider.java new file mode 100644 index 000000000..7bcad4cc5 --- /dev/null +++ b/core/src/main/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProvider.java @@ -0,0 +1,38 @@ +/* + * Copyright 2021 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package com.google.googlejavaformat.java; + +import com.google.auto.service.AutoService; +import java.io.PrintWriter; +import java.util.spi.ToolProvider; + +/** Provide a way to be invoked without necessarily starting a new VM. */ +@AutoService(ToolProvider.class) +public class GoogleJavaFormatToolProvider implements ToolProvider { + @Override + public String name() { + return "google-java-format"; + } + + @Override + public int run(PrintWriter out, PrintWriter err, String... args) { + try { + return Main.main(out, err, args); + } catch (RuntimeException e) { + err.print(e.getMessage()); + return -1; // pass non-zero value back indicating an error has happened + } + } +} diff --git a/core/src/main/java/com/google/googlejavaformat/java/Main.java b/core/src/main/java/com/google/googlejavaformat/java/Main.java index 4056f2ae3..953ca5860 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Main.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Main.java @@ -63,20 +63,27 @@ public Main(PrintWriter outWriter, PrintWriter errWriter, InputStream inStream) * @param args the command-line arguments */ public static void main(String[] args) { - int result; PrintWriter out = new PrintWriter(new OutputStreamWriter(System.out, UTF_8)); PrintWriter err = new PrintWriter(new OutputStreamWriter(System.err, UTF_8)); + int result = main(out, err, args); + System.exit(result); + } + + /** + * Package-private main entry point used this CLI program and the java.util.spi.ToolProvider + * implementation in the same package as this Main class. + */ + static int main(PrintWriter out, PrintWriter err, String... args) { try { Main formatter = new Main(out, err, System.in); - result = formatter.format(args); + return formatter.format(args); } catch (UsageException e) { err.print(e.getMessage()); - result = 0; + return 0; } finally { err.flush(); out.flush(); } - System.exit(result); } /** diff --git a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java index 9c5838aeb..0361415a1 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java @@ -27,7 +27,6 @@ import static com.google.googlejavaformat.java.javadoc.Token.Type.LIST_ITEM_OPEN_TAG; import static com.google.googlejavaformat.java.javadoc.Token.Type.PARAGRAPH_OPEN_TAG; -import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import com.google.googlejavaformat.java.javadoc.Token.Type; @@ -395,7 +394,7 @@ private int innerIndent() { // If this is a hotspot, keep a String of many spaces around, and call append(string, start, end). private void appendSpaces(int count) { - output.append(Strings.repeat(" ", count)); + output.append(" ".repeat(count)); } /** diff --git a/core/src/test/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProviderTest.java b/core/src/test/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProviderTest.java new file mode 100644 index 000000000..15e452297 --- /dev/null +++ b/core/src/test/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProviderTest.java @@ -0,0 +1,57 @@ +/* + * Copyright 2021 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package com.google.googlejavaformat.java; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.ServiceLoader; +import java.util.spi.ToolProvider; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link GoogleJavaFormatToolProvider}. */ +@RunWith(JUnit4.class) +public class GoogleJavaFormatToolProviderTest { + + @Test + public void testUsageOutputAfterLoadingViaToolName() { + String name = "google-java-format"; + + assertThat( + ServiceLoader.load(ToolProvider.class).stream() + .map(ServiceLoader.Provider::get) + .map(ToolProvider::name)) + .contains(name); + + ToolProvider format = ToolProvider.findFirst(name).get(); + + StringWriter out = new StringWriter(); + StringWriter err = new StringWriter(); + + int result = format.run(new PrintWriter(out, true), new PrintWriter(err, true), "--help"); + + assertThat(result).isEqualTo(0); + + String usage = err.toString(); + + // Check that doc links are included. + assertThat(usage).containsMatch("http.*/google-java-format"); + assertThat(usage).contains("Usage: google-java-format"); + } +} diff --git a/pom.xml b/pom.xml index c66ed2f23..75a18293d 100644 --- a/pom.xml +++ b/pom.xml @@ -95,6 +95,7 @@ 3.6.1 2.7.1 1.8.2 + 1.0 3.1.0 3.2.1 @@ -124,6 +125,11 @@ auto-value-annotations ${auto-value.version} + + com.google.auto.service + auto-service-annotations + ${auto-service.version} + @@ -220,6 +226,11 @@ auto-value ${auto-value.version} + + com.google.auto.service + auto-service + ${auto-service.version} + From 0eb12a1d824204f5d1e4e00fd09149cf6debf783 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Tue, 28 Sep 2021 10:15:52 -0700 Subject: [PATCH 15/16] Update to non-EA JDK 17 builds and add JDK 8 to the matrix. PiperOrigin-RevId: 399469056 --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0386892e9..7d3dbc75e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,19 +29,19 @@ jobs: fail-fast: false matrix: os: [ ubuntu-latest ] - java: [ 16, 14, 11 ] + java: [ 17, 14, 11 ] experimental: [ false ] include: # Only test on macos and windows with a single recent JDK to avoid a # combinatorial explosion of test configurations. - os: macos-latest - java: 16 + java: 17 experimental: false - os: windows-latest - java: 16 + java: 17 experimental: false - os: ubuntu-latest - java: 17-ea + java: 18-ea experimental: true runs-on: ${{ matrix.os }} continue-on-error: ${{ matrix.experimental }} From e563df88de07e78f2623aad64d25bbcc4ef956f6 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 20 Oct 2021 12:56:14 -0700 Subject: [PATCH 16/16] Release google-java-format 1.12.0 --- core/pom.xml | 2 +- pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 5ebdb591b..08f2b8dc0 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -22,7 +22,7 @@ com.google.googlejavaformat google-java-format-parent - HEAD-SNAPSHOT + 1.12.0 google-java-format diff --git a/pom.xml b/pom.xml index 75a18293d..55e5346df 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ com.google.googlejavaformat google-java-format-parent pom - HEAD-SNAPSHOT + 1.12.0 core