diff options
Diffstat (limited to 'core/src/main/java/com/google/googlejavaformat/java')
19 files changed, 691 insertions, 251 deletions
diff --git a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java index 2023826..f7c3dec 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java @@ -54,7 +54,7 @@ final class CommandLineOptionsParser { int idx = option.indexOf('='); if (idx >= 0) { flag = option.substring(0, idx); - value = option.substring(idx + 1, option.length()); + value = option.substring(idx + 1); } else { flag = option; value = null; diff --git a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java index 3e97395..aac829d 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java @@ -14,8 +14,6 @@ package com.google.googlejavaformat.java; -import static com.google.common.base.StandardSystemProperty.JAVA_CLASS_VERSION; -import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableList; @@ -42,7 +40,6 @@ import com.sun.tools.javac.util.Log; import com.sun.tools.javac.util.Options; import java.io.IOError; import java.io.IOException; -import java.lang.reflect.Method; import java.net.URI; import java.util.ArrayList; import java.util.Collection; @@ -154,7 +151,7 @@ public final class Formatter { OpsBuilder builder = new OpsBuilder(javaInput, javaOutput); // Output the compilation unit. JavaInputAstVisitor visitor; - if (getMajor() >= 14) { + if (Runtime.version().feature() >= 14) { try { visitor = Class.forName("com.google.googlejavaformat.java.java14.Java14InputAstVisitor") @@ -176,23 +173,6 @@ public final class Formatter { javaOutput.flush(); } - // Runtime.Version was added in JDK 9, so use reflection to access it to preserve source - // compatibility with Java 8. - private static int getMajor() { - try { - Method versionMethod = Runtime.class.getMethod("version"); - Object version = versionMethod.invoke(null); - return (int) version.getClass().getMethod("major").invoke(version); - } catch (Exception e) { - // continue below - } - int version = (int) Double.parseDouble(JAVA_CLASS_VERSION.value()); - if (49 <= version && version <= 52) { - return version - (49 - 5); - } - throw new IllegalStateException("Unknown Java version: " + JAVA_SPECIFICATION_VERSION.value()); - } - static boolean errorDiagnostic(Diagnostic<?> input) { if (input.getKind() != Diagnostic.Kind.ERROR) { return false; diff --git a/core/src/main/java/com/google/googlejavaformat/java/FormatterException.java b/core/src/main/java/com/google/googlejavaformat/java/FormatterException.java index 3ccb44a..808916c 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/FormatterException.java +++ b/core/src/main/java/com/google/googlejavaformat/java/FormatterException.java @@ -26,7 +26,7 @@ import javax.tools.JavaFileObject; /** Checked exception class for formatter errors. */ public final class FormatterException extends Exception { - private ImmutableList<FormatterDiagnostic> diagnostics; + private final ImmutableList<FormatterDiagnostic> diagnostics; public FormatterException(String message) { this(FormatterDiagnostic.create(message)); @@ -47,7 +47,8 @@ public final class FormatterException extends Exception { public static FormatterException fromJavacDiagnostics( Iterable<Diagnostic<? extends JavaFileObject>> diagnostics) { - return new FormatterException(Iterables.transform(diagnostics, d -> toFormatterDiagnostic(d))); + return new FormatterException( + Iterables.transform(diagnostics, FormatterException::toFormatterDiagnostic)); } private static FormatterDiagnostic toFormatterDiagnostic(Diagnostic<?> input) { 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 0000000..7bcad4c --- /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/ImportOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java index a82715e..dcbaea1 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java @@ -346,6 +346,10 @@ public class ImportOrderer { i++; } } + while (tokenAt(i).equals(";")) { + // Extra semicolons are not allowed by the JLS but are accepted by javac. + i++; + } imports.add(new Import(importedName, trailing.toString(), isStatic)); // Remember the position just after the import we just saw, before skipping blank lines. // If the next thing after the blank lines is not another import then we don't want to 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 4d3d30d..fbb6fe7 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 class JavaFormatterOptions { return style.indentationMultiplier(); } - boolean formatJavadoc() { + public boolean formatJavadoc() { return formatJavadoc; } @@ -91,7 +91,7 @@ public class JavaFormatterOptions { return this; } - Builder formatJavadoc(boolean formatJavadoc) { + public Builder formatJavadoc(boolean formatJavadoc) { this.formatJavadoc = formatJavadoc; return this; } diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java index 999c8fb..165bdeb 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java @@ -311,7 +311,7 @@ public final class JavaInput extends Input { for (Tok tok : toks) { builder.put(tok.getPosition(), tok.getColumn()); } - return builder.build(); + return builder.buildOrThrow(); } /** @@ -557,33 +557,28 @@ public final class JavaInput extends Input { } /** - * Convert from an offset and length flag pair to a token range. + * Convert from a character range to a token range. * - * @param offset the {@code 0}-based offset in characters - * @param length the length in characters + * @param characterRange the {@code 0}-based {@link Range} of characters * @return the {@code 0}-based {@link Range} of tokens - * @throws FormatterException if offset + length is outside the file + * @throws FormatterException if the upper endpoint of the range is outside the file */ - Range<Integer> characterRangeToTokenRange(int offset, int length) throws FormatterException { - int requiredLength = offset + length; - if (requiredLength > text.length()) { + Range<Integer> characterRangeToTokenRange(Range<Integer> characterRange) + throws FormatterException { + if (characterRange.upperEndpoint() > text.length()) { throw new FormatterException( String.format( "error: invalid length %d, offset + length (%d) is outside the file", - length, requiredLength)); - } - if (length < 0) { - return EMPTY_RANGE; - } - if (length == 0) { - // 0 stands for "format the line under the cursor" - length = 1; - } + characterRange.upperEndpoint() - characterRange.lowerEndpoint(), + characterRange.upperEndpoint())); + } + // empty range stands for "format the line under the cursor" + Range<Integer> nonEmptyRange = + characterRange.isEmpty() + ? Range.closedOpen(characterRange.lowerEndpoint(), characterRange.lowerEndpoint() + 1) + : characterRange; ImmutableCollection<Token> enclosed = - getPositionTokenMap() - .subRangeMap(Range.closedOpen(offset, offset + length)) - .asMapOfRanges() - .values(); + getPositionTokenMap().subRangeMap(nonEmptyRange).asMapOfRanges().values(); if (enclosed.isEmpty()) { return EMPTY_RANGE; } @@ -594,7 +589,7 @@ public final class JavaInput extends Input { /** * Get the number of toks. * - * @return the number of toks, including the EOF tok + * @return the number of toks, excluding the EOF tok */ @Override public int getkN() { @@ -604,7 +599,7 @@ public final class JavaInput extends Input { /** * Get the Token by index. * - * @param k the token index + * @param k the Tok index */ @Override public Token getToken(int k) { @@ -664,12 +659,9 @@ public final class JavaInput extends Input { public RangeSet<Integer> characterRangesToTokenRanges(Collection<Range<Integer>> characterRanges) throws FormatterException { RangeSet<Integer> tokenRangeSet = TreeRangeSet.create(); - for (Range<Integer> characterRange0 : characterRanges) { - Range<Integer> characterRange = characterRange0.canonical(DiscreteDomain.integers()); + for (Range<Integer> characterRange : characterRanges) { tokenRangeSet.add( - characterRangeToTokenRange( - characterRange.lowerEndpoint(), - characterRange.upperEndpoint() - characterRange.lowerEndpoint())); + characterRangeToTokenRange(characterRange.canonical(DiscreteDomain.integers()))); } return tokenRangeSet; } 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 6ce0f66..daed250 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.UNION_TYPE; 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,8 +147,9 @@ import com.sun.tools.javac.tree.TreeScanner; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; +import java.util.Comparator; import java.util.Deque; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -168,7 +177,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } /** Whether to break or not. */ - enum BreakOrNot { + protected enum BreakOrNot { YES, NO; @@ -178,7 +187,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } /** Whether to collapse empty blocks. */ - enum CollapseEmptyOrNot { + protected enum CollapseEmptyOrNot { YES, NO; @@ -192,7 +201,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } /** Whether to allow leading blank lines in blocks. */ - enum AllowLeadingBlankLine { + protected enum AllowLeadingBlankLine { YES, NO; @@ -202,7 +211,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } /** Whether to allow trailing blank lines in blocks. */ - enum AllowTrailingBlankLine { + protected enum AllowTrailingBlankLine { YES, NO; @@ -269,6 +278,21 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } } + // TODO(cushon): generalize this + private static final ImmutableMultimap<String, String> TYPE_ANNOTATIONS = typeAnnotations(); + + private static ImmutableSetMultimap<String, String> typeAnnotations() { + ImmutableSetMultimap.Builder<String, String> 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; @@ -278,6 +302,8 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { protected final Indent.Const plusTwo; protected final Indent.Const plusFour; + private final Set<Name> typeAnnotationSimpleNames = new HashSet<>(); + private static final ImmutableList<Op> breakList(Optional<BreakTag> breakTag) { return ImmutableList.of(Doc.Break.make(Doc.FillMode.UNIFIED, " ", ZERO, breakTag)); } @@ -293,8 +319,6 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { return ImmutableList.of(Doc.Break.make(FillMode.FORCED, "", Indent.Const.ZERO, breakTag)); } - private static final ImmutableList<Op> 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. @@ -377,11 +401,14 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { first = false; dropEmptyDeclarations(); } + handleModule(first, node); // set a partial format marker at EOF to make sure we can format the entire file markForPartialFormat(); return null; } + protected void handleModule(boolean first, CompilationUnitTree node) {} + /** Skips over extra semi-colons at the top-level, or in a class member declaration lists. */ protected void dropEmptyDeclarations() { if (builder.peekToken().equals(Optional.of(";"))) { @@ -415,10 +442,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { 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"); @@ -677,9 +701,10 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { builder.space(); addTypeArguments(node.getTypeArguments(), plusFour); if (node.getClassBody() != null) { - builder.addAll( + List<AnnotationTree> 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); @@ -800,10 +825,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { 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(" "); @@ -967,7 +989,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } } - private TypeWithDims variableFragmentDims(boolean first, int leadingDims, Tree type) { + private static TypeWithDims variableFragmentDims(boolean first, int leadingDims, Tree type) { if (type == null) { return null; } @@ -1112,6 +1134,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { @Override public Void visitImport(ImportTree node, Void unused) { + checkForTypeAnnotation(node); sync(node); token("import"); builder.space(); @@ -1126,6 +1149,21 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { return null; } + private void checkForTypeAnnotation(ImportTree node) { + Name simpleName = getSimpleName(node); + Collection<String> 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); @@ -1358,9 +1396,12 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } } } - builder.addAll( + List<AnnotationTree> typeAnnotations = visitModifiers( - annotations, Direction.VERTICAL, /* declarationAnnotationBreak= */ Optional.empty())); + node.getModifiers(), + annotations, + Direction.VERTICAL, + /* declarationAnnotationBreak= */ Optional.empty()); Tree baseReturnType = null; Deque<List<? extends AnnotationTree>> dims = null; @@ -1369,6 +1410,9 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { DimensionHelpers.extractDims(node.getReturnType(), SortedDims.YES); baseReturnType = extractedDims.node; dims = new ArrayDeque<>(extractedDims.dims); + } else { + verticalAnnotations(typeAnnotations); + typeAnnotations = ImmutableList.of(); } builder.open(plusFour); @@ -1377,7 +1421,14 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { 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()) { @@ -1600,11 +1651,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { public Void visitLiteral(LiteralTree node, Void unused) { sync(node); String sourceForNode = getSourceForNode(node, getCurrentPath()); - // A negative numeric literal -n is usually represented as unary minus on n, - // but that doesn't work for integer or long MIN_VALUE. The parser works - // around that by representing it directly as a signed literal (with no - // unary minus), but the lexer still expects two tokens. - if (sourceForNode.startsWith("-")) { + if (isUnaryMinusLiteral(sourceForNode)) { token("-"); sourceForNode = sourceForNode.substring(1).trim(); } @@ -1612,6 +1659,14 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { return null; } + // A negative numeric literal -n is usually represented as unary minus on n, + // but that doesn't work for integer or long MIN_VALUE. The parser works + // around that by representing it directly as a signed literal (with no + // unary minus), but the lexer still expects two tokens. + private static boolean isUnaryMinusLiteral(String literalTreeSource) { + return literalTreeSource.startsWith("-"); + } + private void visitPackage( ExpressionTree packageName, List<? extends AnnotationTree> packageAnnotations) { if (!packageAnnotations.isEmpty()) { @@ -1697,10 +1752,10 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { default: return false; } - if (!(node.getExpression() instanceof UnaryTree)) { + JCTree.Tag tag = unaryTag(node.getExpression()); + if (tag == null) { return false; } - JCTree.Tag tag = ((JCTree) node.getExpression()).getTag(); if (tag.isPostUnaryOp()) { return false; } @@ -1710,6 +1765,17 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { return true; } + private JCTree.Tag unaryTag(ExpressionTree expression) { + if (expression instanceof UnaryTree) { + return ((JCTree) expression).getTag(); + } + if (expression instanceof LiteralTree + && isUnaryMinusLiteral(getSourceForNode(expression, getCurrentPath()))) { + return JCTree.Tag.MINUS; + } + return null; + } + @Override public Void visitPrimitiveType(PrimitiveTypeTree node, Void unused) { sync(node); @@ -1939,14 +2005,11 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { public void visitClassDeclaration(ClassTree node) { sync(node); - List<Op> breaks = - visitModifiers( - node.getModifiers(), - Direction.VERTICAL, - /* declarationAnnotationBreak= */ Optional.empty()); + typeDeclarationModifiers(node.getModifiers()); + List<? extends Tree> permitsTypes = getPermitsClause(node); boolean hasSuperclassType = node.getExtendsClause() != null; boolean hasSuperInterfaceTypes = !node.getImplementsClause().isEmpty(); - builder.addAll(breaks); + boolean hasPermitsTypes = !permitsTypes.isEmpty(); token(node.getKind() == Tree.Kind.INTERFACE ? "interface" : "class"); builder.space(); visit(node.getSimpleName()); @@ -1958,7 +2021,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { if (!node.getTypeParameters().isEmpty()) { typeParametersRest( node.getTypeParameters(), - hasSuperclassType || hasSuperInterfaceTypes ? plusFour : ZERO); + hasSuperclassType || hasSuperInterfaceTypes || hasPermitsTypes ? plusFour : ZERO); } if (hasSuperclassType) { builder.breakToFill(" "); @@ -1966,22 +2029,10 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { builder.space(); scan(node.getExtendsClause(), null); } - if (hasSuperInterfaceTypes) { - builder.breakToFill(" "); - builder.open(node.getImplementsClause().size() > 1 ? plusFour : ZERO); - token(node.getKind() == Tree.Kind.INTERFACE ? "extends" : "implements"); - builder.space(); - boolean first = true; - for (Tree superInterfaceType : node.getImplementsClause()) { - if (!first) { - token(","); - builder.breakOp(" "); - } - scan(superInterfaceType, null); - first = false; - } - builder.close(); - } + classDeclarationTypeList( + node.getKind() == Tree.Kind.INTERFACE ? "extends" : "implements", + node.getImplementsClause()); + classDeclarationTypeList("permits", permitsTypes); } builder.close(); if (node.getMembers() == null) { @@ -2062,7 +2113,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { // Helper methods. /** Helper method for annotations. */ - void visitAnnotations( + protected void visitAnnotations( List<? extends AnnotationTree> annotations, BreakOrNot breakBefore, BreakOrNot breakAfter) { if (!annotations.isEmpty()) { if (breakBefore.isYes()) { @@ -2082,8 +2133,16 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } } + void verticalAnnotations(List<AnnotationTree> annotations) { + for (AnnotationTree annotation : annotations) { + builder.forcedBreak(); + scan(annotation, null); + builder.forcedBreak(); + } + } + /** Helper method for blocks. */ - private void visitBlock( + protected void visitBlock( BlockTree node, CollapseEmptyOrNot collapseEmptyOrNot, AllowLeadingBlankLine allowLeadingBlankLine, @@ -2169,12 +2228,21 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } } + protected void typeDeclarationModifiers(ModifiersTree modifiers) { + List<AnnotationTree> 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<BreakTag> declarationAnnotationBreak) { - builder.addAll(visitModifiers(modifiers, annotationDirection, declarationAnnotationBreak)); + List<AnnotationTree> typeAnnotations = + visitModifiers(modifiers, annotationDirection, declarationAnnotationBreak); + visitAnnotations(typeAnnotations, BreakOrNot.NO, BreakOrNot.YES); } @Override @@ -2183,36 +2251,50 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } /** Output combined modifiers and annotations and returns the trailing break. */ - protected List<Op> visitModifiers( + @CheckReturnValue + protected ImmutableList<AnnotationTree> visitModifiers( ModifiersTree modifiersTree, Direction annotationsDirection, Optional<BreakTag> declarationAnnotationBreak) { return visitModifiers( - modifiersTree.getAnnotations(), annotationsDirection, declarationAnnotationBreak); + modifiersTree, + modifiersTree.getAnnotations(), + annotationsDirection, + declarationAnnotationBreak); } - protected List<Op> visitModifiers( + @CheckReturnValue + protected ImmutableList<AnnotationTree> visitModifiers( + ModifiersTree modifiersTree, List<? extends AnnotationTree> annotationTrees, Direction annotationsDirection, Optional<BreakTag> declarationAnnotationBreak) { - if (annotationTrees.isEmpty() && !nextIsModifier()) { - return EMPTY_LIST; + DeclarationModifiersAndTypeAnnotations splitModifiers = + splitModifiers(modifiersTree, annotationTrees); + return visitModifiers(splitModifiers, annotationsDirection, declarationAnnotationBreak); + } + + @CheckReturnValue + private ImmutableList<AnnotationTree> visitModifiers( + DeclarationModifiersAndTypeAnnotations splitModifiers, + Direction annotationsDirection, + Optional<BreakTag> declarationAnnotationBreak) { + if (splitModifiers.declarationModifiers().isEmpty()) { + return splitModifiers.typeAnnotations(); } - Deque<AnnotationTree> annotations = new ArrayDeque<>(annotationTrees); + Deque<AnnotationOrModifier> 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); first = false; lastWasAnnotation = true; } @@ -2221,8 +2303,9 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { 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); @@ -2230,24 +2313,171 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { 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); 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<AnnotationOrModifier> { + 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<AnnotationOrModifier> 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. + * + * <p>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<AnnotationOrModifier> declarationModifiers(); + + abstract ImmutableList<AnnotationTree> typeAnnotations(); + + static DeclarationModifiersAndTypeAnnotations create( + ImmutableList<AnnotationOrModifier> declarationModifiers, + ImmutableList<AnnotationTree> 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<? extends AnnotationTree> annotations) { + if (annotations.isEmpty() && !isModifier(builder.peekToken().get())) { + return DeclarationModifiersAndTypeAnnotations.empty(); + } + RangeSet<Integer> annotationRanges = TreeRangeSet.create(); + for (AnnotationTree annotationTree : annotations) { + annotationRanges.add( + Range.closedOpen( + getStartPosition(annotationTree), getEndPosition(annotationTree, getCurrentPath()))); + } + ImmutableList<Input.Tok> 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<AnnotationOrModifier> 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<AnnotationTree> 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()); } - boolean nextIsModifier() { - switch (builder.peekToken().get()) { + private void formatAnnotationOrModifier(Deque<AnnotationOrModifier> modifiers) { + AnnotationOrModifier modifier = modifiers.removeFirst(); + switch (modifier.getKind()) { + case MODIFIER: + token(modifier.modifier().getText()); + if (modifier.modifier().getText().equals("non")) { + token(modifiers.removeFirst().modifier().getText()); + token(modifiers.removeFirst().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()); + } + + private static boolean isModifier(String token) { + switch (token) { case "public": case "protected": case "private": @@ -2260,6 +2490,9 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { case "native": case "strictfp": case "default": + case "sealed": + case "non": + case "-": return true; default: return false; @@ -2366,7 +2599,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { builder.open(ZERO); boolean first = true; if (receiver.isPresent()) { - // TODO(jdd): Use builders. + // TODO(user): Use builders. declareOne( DeclarationKind.PARAMETER, Direction.HORIZONTAL, @@ -2868,7 +3101,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } /** Returns the simple names of expressions in a "." chain. */ - private List<String> simpleNames(Deque<ExpressionTree> stack) { + private static ImmutableList<String> simpleNames(Deque<ExpressionTree> stack) { ImmutableList.Builder<String> simpleNames = ImmutableList.builder(); OUTER: for (ExpressionTree expression : stack) { @@ -2906,7 +3139,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { if (!methodInvocation.getTypeArguments().isEmpty()) { builder.open(plusFour); addTypeArguments(methodInvocation.getTypeArguments(), ZERO); - // TODO(jdd): Should indent the name -4. + // TODO(user): Should indent the name -4. builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO, tyargTag); builder.close(); } @@ -2925,14 +3158,14 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { * 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; } @@ -2973,7 +3206,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { * Returns all array indices for the given expression, e.g. given {@code foo[0][0]} returns the * expressions for {@code [0][0]}. */ - private Deque<ExpressionTree> getArrayIndices(ExpressionTree expression) { + private static Deque<ExpressionTree> getArrayIndices(ExpressionTree expression) { Deque<ExpressionTree> indices = new ArrayDeque<>(); while (expression instanceof ArrayAccessTree) { ArrayAccessTree array = (ArrayAccessTree) expression; @@ -3270,20 +3503,25 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } Deque<List<? extends AnnotationTree>> dims = - new ArrayDeque<>( - typeWithDims.isPresent() ? typeWithDims.get().dims : Collections.emptyList()); + 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<AnnotationTree> annotations = + visitModifiers( + declarationAndTypeModifiers, + annotationsDirection, + Optional.of(verticalAnnotationBreak)); boolean isVar = builder.peekToken().get().equals("var") && (!name.contentEquals("var") || builder.peekToken(1).get().equals("var")); @@ -3294,6 +3532,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { { 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(); @@ -3533,6 +3772,31 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { } } + /** Gets the permits clause for the given node. This is only available in Java 15 and later. */ + protected List<? extends Tree> getPermitsClause(ClassTree node) { + return ImmutableList.of(); + } + + private void classDeclarationTypeList(String token, List<? extends Tree> types) { + if (types.isEmpty()) { + return; + } + builder.breakToFill(" "); + builder.open(types.size() > 1 ? plusFour : ZERO); + token(token); + builder.space(); + boolean first = true; + for (Tree type : types) { + if (!first) { + token(","); + builder.breakOp(" "); + } + scan(type, null); + first = false; + } + builder.close(); + } + /** * The parser expands multi-variable declarations into separate single-variable declarations. All * of the fragments in the original declaration have the same start position, so we use that as a @@ -3540,7 +3804,8 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { * * <p>e.g. {@code int x, y;} is parsed as {@code int x; int y;}. */ - private List<VariableTree> variableFragments(PeekingIterator<? extends Tree> it, Tree first) { + private static List<VariableTree> variableFragments( + PeekingIterator<? extends Tree> it, Tree first) { List<VariableTree> fragments = new ArrayList<>(); if (first.getKind() == VARIABLE) { int start = getStartPosition(first); @@ -3590,7 +3855,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { * @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()) { @@ -3607,7 +3872,7 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> { * 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/JavaOutput.java b/core/src/main/java/com/google/googlejavaformat/java/JavaOutput.java index c059318..c43a91a 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaOutput.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaOutput.java @@ -14,6 +14,7 @@ package com.google.googlejavaformat.java; +import static java.lang.Math.min; import static java.util.Comparator.comparing; import com.google.common.base.CharMatcher; @@ -89,7 +90,7 @@ public final class JavaOutput extends Output { partialFormatRanges.add(Range.closed(lo, hi)); } - // TODO(jdd): Add invariant. + // TODO(user): Add invariant. @Override public void append(String text, Range<Integer> range) { if (!range.isEmpty()) { @@ -261,8 +262,7 @@ public final class JavaOutput extends Output { } } - int replaceTo = - Math.min(endTok.getPosition() + endTok.length(), javaInput.getText().length()); + int replaceTo = min(endTok.getPosition() + endTok.length(), javaInput.getText().length()); // If the formatted ranged ended in the trailing trivia of the last token before EOF, // format all the way up to EOF to deal with trailing whitespace correctly. if (endTok.getIndex() == javaInput.getkN() - 1) { @@ -304,7 +304,7 @@ public final class JavaOutput extends Output { } else { if (newline == -1) { // If there wasn't a trailing newline in the input, indent the next line. - replacement.append(after.substring(0, idx)); + replacement.append(after, 0, idx); } break; } @@ -352,7 +352,7 @@ public final class JavaOutput extends Output { public static int startPosition(Token token) { int min = token.getTok().getPosition(); for (Input.Tok tok : token.getToksBefore()) { - min = Math.min(min, tok.getPosition()); + min = min(min, tok.getPosition()); } return min; } diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java b/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java index a8c9efd..ba7e3b7 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java @@ -128,10 +128,28 @@ class JavacTokens { @Override protected Comment processComment(int pos, int endPos, CommentStyle style) { - char[] buf = reader.getRawCharacters(pos, endPos); + char[] buf = getRawCharactersReflectively(pos, endPos); return new CommentWithTextAndPosition( pos, endPos, new AccessibleReader(fac, buf, buf.length), style); } + + private char[] getRawCharactersReflectively(int beginIndex, int endIndex) { + Object instance; + try { + instance = JavaTokenizer.class.getDeclaredField("reader").get(this); + } catch (ReflectiveOperationException e) { + instance = this; + } + try { + return (char[]) + instance + .getClass() + .getMethod("getRawCharacters", int.class, int.class) + .invoke(instance, beginIndex, endIndex); + } catch (ReflectiveOperationException e) { + throw new LinkageError(e.getMessage(), e); + } + } } /** A {@link Comment} that saves its text and start position. */ 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 9231bda..953ca58 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Main.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Main.java @@ -14,6 +14,7 @@ package com.google.googlejavaformat.java; +import static java.lang.Math.min; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.io.ByteStreams; @@ -40,7 +41,7 @@ public final class Main { private static final int MAX_THREADS = 20; private static final String STDIN_FILENAME = "<stdin>"; - static final String versionString() { + static String versionString() { return "google-java-format: Version " + GoogleJavaFormatVersion.version(); } @@ -62,20 +63,27 @@ public final class Main { * @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); } /** @@ -109,7 +117,7 @@ public final class Main { } private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions options) { - int numThreads = Math.min(MAX_THREADS, parameters.files().size()); + int numThreads = min(MAX_THREADS, parameters.files().size()); ExecutorService executorService = Executors.newFixedThreadPool(numThreads); Map<Path, String> inputs = new LinkedHashMap<>(); @@ -146,7 +154,7 @@ public final class Main { } catch (ExecutionException e) { if (e.getCause() instanceof FormatterException) { for (FormatterDiagnostic diagnostic : ((FormatterException) e.getCause()).diagnostics()) { - errWriter.println(path + ":" + diagnostic.toString()); + errWriter.println(path + ":" + diagnostic); } } else { errWriter.println(path + ": error: " + e.getCause().getMessage()); @@ -205,7 +213,7 @@ public final class Main { } } catch (FormatterException e) { for (FormatterDiagnostic diagnostic : e.diagnostics()) { - errWriter.println(stdinFilename + ":" + diagnostic.toString()); + errWriter.println(stdinFilename + ":" + diagnostic); } ok = false; // TODO(cpovirk): Catch other types of exception (as we do in the formatFiles case). diff --git a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java index f7f610b..e14b290 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java @@ -36,44 +36,6 @@ import javax.lang.model.element.Modifier; /** Fixes sequences of modifiers to be in JLS order. */ final class ModifierOrderer { - /** - * Returns the {@link javax.lang.model.element.Modifier} for the given token kind, or {@code - * null}. - */ - private static Modifier getModifier(TokenKind kind) { - if (kind == null) { - return null; - } - switch (kind) { - case PUBLIC: - return Modifier.PUBLIC; - case PROTECTED: - return Modifier.PROTECTED; - case PRIVATE: - return Modifier.PRIVATE; - case ABSTRACT: - return Modifier.ABSTRACT; - case STATIC: - return Modifier.STATIC; - case DEFAULT: - return Modifier.DEFAULT; - case FINAL: - return Modifier.FINAL; - case TRANSIENT: - return Modifier.TRANSIENT; - case VOLATILE: - return Modifier.VOLATILE; - case SYNCHRONIZED: - return Modifier.SYNCHRONIZED; - case NATIVE: - return Modifier.NATIVE; - case STRICTFP: - return Modifier.STRICTFP; - default: - return null; - } - } - /** Reorders all modifiers in the given text to be in JLS order. */ static JavaInput reorderModifiers(String text) throws FormatterException { return reorderModifiers( @@ -130,7 +92,7 @@ final class ModifierOrderer { if (i > 0) { addTrivia(replacement, modifierTokens.get(i).getToksBefore()); } - replacement.append(mods.get(i).toString()); + replacement.append(mods.get(i)); if (i < (modifierTokens.size() - 1)) { addTrivia(replacement, modifierTokens.get(i).getToksAfter()); } @@ -152,7 +114,44 @@ final class ModifierOrderer { * is not a modifier. */ private static Modifier asModifier(Token token) { - return getModifier(((JavaInput.Tok) token.getTok()).kind()); + TokenKind kind = ((JavaInput.Tok) token.getTok()).kind(); + if (kind != null) { + switch (kind) { + case PUBLIC: + return Modifier.PUBLIC; + case PROTECTED: + return Modifier.PROTECTED; + case PRIVATE: + return Modifier.PRIVATE; + case ABSTRACT: + return Modifier.ABSTRACT; + case STATIC: + return Modifier.STATIC; + case DEFAULT: + return Modifier.DEFAULT; + case FINAL: + return Modifier.FINAL; + case TRANSIENT: + return Modifier.TRANSIENT; + case VOLATILE: + return Modifier.VOLATILE; + case SYNCHRONIZED: + return Modifier.SYNCHRONIZED; + case NATIVE: + return Modifier.NATIVE; + case STRICTFP: + return Modifier.STRICTFP; + default: // fall out + } + } + switch (token.getTok().getText()) { + case "non-sealed": + return Modifier.valueOf("NON_SEALED"); + case "sealed": + return Modifier.valueOf("SEALED"); + default: + return null; + } } /** Applies replacements to the given string. */ diff --git a/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java b/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java index d939480..20e55e9 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java +++ b/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java @@ -16,6 +16,7 @@ package com.google.googlejavaformat.java; +import static java.lang.Math.max; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.CharMatcher; @@ -31,6 +32,7 @@ import com.google.common.collect.TreeRangeSet; import com.google.googlejavaformat.Newlines; import com.sun.source.doctree.DocCommentTree; import com.sun.source.doctree.ReferenceTree; +import com.sun.source.tree.CaseTree; import com.sun.source.tree.IdentifierTree; import com.sun.source.tree.ImportTree; import com.sun.source.tree.Tree; @@ -54,8 +56,10 @@ import com.sun.tools.javac.util.Log; import com.sun.tools.javac.util.Options; import java.io.IOError; import java.io.IOException; +import java.lang.reflect.Method; import java.net.URI; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Set; import javax.tools.Diagnostic; @@ -114,6 +118,31 @@ public class RemoveUnusedImports { return null; } + // TODO(cushon): remove this override when pattern matching in switch is no longer a preview + // feature, and TreePathScanner visits CaseTree#getLabels instead of CaseTree#getExpressions + @SuppressWarnings("unchecked") // reflection + @Override + public Void visitCase(CaseTree tree, Void unused) { + if (CASE_TREE_GET_LABELS != null) { + try { + scan((List<? extends Tree>) CASE_TREE_GET_LABELS.invoke(tree), null); + } catch (ReflectiveOperationException e) { + throw new LinkageError(e.getMessage(), e); + } + } + return super.visitCase(tree, null); + } + + private static final Method CASE_TREE_GET_LABELS = caseTreeGetLabels(); + + private static Method caseTreeGetLabels() { + try { + return CaseTree.class.getMethod("getLabels"); + } catch (NoSuchMethodException e) { + return null; + } + } + @Override public Void scan(Tree tree, Void unused) { if (tree == null) { @@ -145,7 +174,9 @@ public class RemoveUnusedImports { public Void visitReference(ReferenceTree referenceTree, Void unused) { DCReference reference = (DCReference) referenceTree; long basePos = - reference.getSourcePosition((DCTree.DCDocComment) getCurrentPath().getDocComment()); + reference + .pos((DCTree.DCDocComment) getCurrentPath().getDocComment()) + .getStartPosition(); // the position of trees inside the reference node aren't stored, but the qualifier's // start position is the beginning of the reference node if (reference.qualifierExpression != null) { @@ -247,7 +278,7 @@ public class RemoveUnusedImports { } // delete the import int endPosition = importTree.getEndPosition(unit.endPositions); - endPosition = Math.max(CharMatcher.isNot(' ').indexIn(contents, endPosition), endPosition); + endPosition = max(CharMatcher.isNot(' ').indexIn(contents, endPosition), endPosition); String sep = Newlines.guessLineSeparator(contents); if (endPosition + sep.length() < contents.length() && contents.subSequence(endPosition, endPosition + sep.length()).toString().equals(sep)) { diff --git a/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java b/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java index e41bb66..c0f16e9 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java +++ b/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java @@ -239,9 +239,10 @@ public final class StringWrapper { * @param separator the line separator * @param columnLimit the number of columns to wrap at * @param startColumn the column position of the beginning of the original text - * @param trailing extra space to leave after the last line - * @param components the text to reflow - * @param first0 true if the text includes the beginning of its enclosing concat chain, i.e. a + * @param trailing extra space to leave after the last line, to accommodate a ; or ) + * @param components the text to reflow. This is a list of “words” of a single literal. Its first + * and last quotes have been stripped + * @param first0 true if the text includes the beginning of its enclosing concat chain */ private static String reflow( String separator, @@ -251,7 +252,7 @@ public final class StringWrapper { ImmutableList<String> components, boolean first0) { // We have space between the start column and the limit to output the first line. - // Reserve two spaces for the quotes. + // Reserve two spaces for the start and end quotes. int width = columnLimit - startColumn - 2; Deque<String> input = new ArrayDeque<>(components); List<String> lines = new ArrayList<>(); @@ -259,10 +260,13 @@ public final class StringWrapper { while (!input.isEmpty()) { int length = 0; List<String> line = new ArrayList<>(); - if (input.stream().mapToInt(x -> x.length()).sum() <= width) { + // If we know this is going to be the last line, then remove a bit of width to account for the + // trailing characters. + if (input.stream().mapToInt(String::length).sum() <= width) { + // This isn’t quite optimal, but arguably good enough. See b/179561701 width -= trailing; } - while (!input.isEmpty() && (length <= 4 || (length + input.peekFirst().length()) < width)) { + while (!input.isEmpty() && (length <= 4 || (length + input.peekFirst().length()) <= width)) { String text = input.removeFirst(); line.add(text); length += text.length(); diff --git a/core/src/main/java/com/google/googlejavaformat/java/TypeNameClassifier.java b/core/src/main/java/com/google/googlejavaformat/java/TypeNameClassifier.java index 4e871a6..21fae5f 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/TypeNameClassifier.java +++ b/core/src/main/java/com/google/googlejavaformat/java/TypeNameClassifier.java @@ -164,7 +164,7 @@ public final class TypeNameClassifier { hasLowercase |= Character.isLowerCase(c); } if (firstUppercase) { - return hasLowercase ? UPPER_CAMEL : UPPERCASE; + return (hasLowercase || name.length() == 1) ? UPPER_CAMEL : UPPERCASE; } else { return hasUppercase ? LOWER_CAMEL : LOWERCASE; } diff --git a/core/src/main/java/com/google/googlejavaformat/java/UsageException.java b/core/src/main/java/com/google/googlejavaformat/java/UsageException.java index 82c0843..a10f2d0 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/UsageException.java +++ b/core/src/main/java/com/google/googlejavaformat/java/UsageException.java @@ -46,9 +46,9 @@ final class UsageException extends Exception { " Do not fix the import order. Unused imports will still be removed.", " --skip-removing-unused-imports", " Do not remove unused imports. Imports will still be sorted.", - " . --skip-reflowing-long-strings", + " --skip-reflowing-long-strings", " Do not reflow string literals that exceed the column limit.", - " . --skip-javadoc-formatting", + " --skip-javadoc-formatting", " Do not reformat javadoc.", " --dry-run, -n", " Prints the paths of the files whose contents would change if the formatter were run" 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 78cfd66..890687f 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 @@ -15,48 +15,113 @@ package com.google.googlejavaformat.java.java14; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.MoreCollectors.toOptional; +import static com.google.common.collect.Iterables.getOnlyElement; 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; import com.sun.source.tree.ClassTree; -import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.CompilationUnitTree; import com.sun.source.tree.InstanceOfTree; +import com.sun.source.tree.ModifiersTree; +import com.sun.source.tree.ModuleTree; import com.sun.source.tree.SwitchExpressionTree; import com.sun.source.tree.Tree; +import com.sun.source.tree.VariableTree; import com.sun.source.tree.YieldTree; import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.tree.JCTree; -import com.sun.tools.javac.tree.JCTree.JCMethodDecl; import com.sun.tools.javac.tree.JCTree.JCVariableDecl; import com.sun.tools.javac.tree.TreeInfo; +import java.lang.reflect.Method; import java.util.List; import java.util.Optional; +import javax.lang.model.element.Name; /** * Extends {@link JavaInputAstVisitor} with support for AST nodes that were added or modified for * Java 14. */ public class Java14InputAstVisitor extends JavaInputAstVisitor { + private static final Method COMPILATION_UNIT_TREE_GET_MODULE = + maybeGetMethod(CompilationUnitTree.class, "getModule"); + private static final Method CLASS_TREE_GET_PERMITS_CLAUSE = + maybeGetMethod(ClassTree.class, "getPermitsClause"); + private static final Method BINDING_PATTERN_TREE_GET_VARIABLE = + maybeGetMethod(BindingPatternTree.class, "getVariable"); + private static final Method BINDING_PATTERN_TREE_GET_TYPE = + maybeGetMethod(BindingPatternTree.class, "getType"); + private static final Method BINDING_PATTERN_TREE_GET_BINDING = + maybeGetMethod(BindingPatternTree.class, "getBinding"); + private static final Method CASE_TREE_GET_LABELS = maybeGetMethod(CaseTree.class, "getLabels"); public Java14InputAstVisitor(OpsBuilder builder, int indentMultiplier) { super(builder, indentMultiplier); } @Override + protected void handleModule(boolean first, CompilationUnitTree node) { + if (COMPILATION_UNIT_TREE_GET_MODULE == null) { + // Java < 17, see https://bugs.openjdk.java.net/browse/JDK-8255464 + return; + } + ModuleTree module = (ModuleTree) invoke(COMPILATION_UNIT_TREE_GET_MODULE, node); + if (module != null) { + if (!first) { + builder.blankLineWanted(BlankLineWanted.YES); + } + markForPartialFormat(); + visitModule(module, null); + builder.forcedBreak(); + } + } + + @Override + protected List<? extends Tree> getPermitsClause(ClassTree node) { + if (CLASS_TREE_GET_PERMITS_CLAUSE != null) { + return (List<? extends Tree>) invoke(CLASS_TREE_GET_PERMITS_CLAUSE, node); + } else { + // Java < 15 + return super.getPermitsClause(node); + } + } + + @Override public Void visitBindingPattern(BindingPatternTree node, Void unused) { sync(node); - scan(node.getType(), null); - builder.breakOp(" "); - visit(node.getBinding()); + if (BINDING_PATTERN_TREE_GET_VARIABLE != null) { + VariableTree variableTree = (VariableTree) invoke(BINDING_PATTERN_TREE_GET_VARIABLE, node); + visitBindingPattern( + variableTree.getModifiers(), variableTree.getType(), variableTree.getName()); + } else if (BINDING_PATTERN_TREE_GET_TYPE != null && BINDING_PATTERN_TREE_GET_BINDING != null) { + Tree type = (Tree) invoke(BINDING_PATTERN_TREE_GET_TYPE, node); + Name name = (Name) invoke(BINDING_PATTERN_TREE_GET_BINDING, node); + visitBindingPattern(/* modifiers= */ null, type, name); + } else { + throw new LinkageError( + "BindingPatternTree must have either getVariable() or both getType() and getBinding()," + + " but does not"); + } return null; } + private void visitBindingPattern(ModifiersTree modifiers, Tree type, Name name) { + if (modifiers != null) { + List<AnnotationTree> annotations = + visitModifiers(modifiers, Direction.HORIZONTAL, Optional.empty()); + visitAnnotations(annotations, BreakOrNot.NO, BreakOrNot.YES); + } + scan(type, null); + builder.breakOp(" "); + visit(name); + } + @Override public Void visitYield(YieldTree node, Void aVoid) { sync(node); @@ -98,14 +163,9 @@ public class Java14InputAstVisitor extends JavaInputAstVisitor { public void visitRecordDeclaration(ClassTree node) { sync(node); - List<Op> 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()); @@ -117,10 +177,7 @@ public class Java14InputAstVisitor extends JavaInputAstVisitor { if (!node.getTypeParameters().isEmpty()) { typeParametersRest(node.getTypeParameters(), hasSuperInterfaceTypes ? plusFour : ZERO); } - ImmutableList<JCVariableDecl> parameters = - compactRecordConstructor(node) - .map(m -> ImmutableList.copyOf(m.getParameters())) - .orElseGet(() -> recordVariables(node)); + ImmutableList<JCVariableDecl> parameters = recordVariables(node); token("("); if (!parameters.isEmpty()) { // Break before args. @@ -159,14 +216,6 @@ public class Java14InputAstVisitor extends JavaInputAstVisitor { dropEmptyDeclarations(); } - private static Optional<JCMethodDecl> compactRecordConstructor(ClassTree node) { - return node.getMembers().stream() - .filter(JCMethodDecl.class::isInstance) - .map(JCMethodDecl.class::cast) - .filter(m -> (m.mods.flags & COMPACT_RECORD_CONSTRUCTOR) == COMPACT_RECORD_CONSTRUCTOR) - .collect(toOptional()); - } - private static ImmutableList<JCVariableDecl> recordVariables(ClassTree node) { return node.getMembers().stream() .filter(JCVariableDecl.class::isInstance) @@ -199,20 +248,33 @@ public class Java14InputAstVisitor extends JavaInputAstVisitor { sync(node); markForPartialFormat(); builder.forcedBreak(); - if (node.getExpressions().isEmpty()) { + List<? extends Tree> labels; + boolean isDefault; + if (CASE_TREE_GET_LABELS != null) { + labels = (List<? extends Tree>) invoke(CASE_TREE_GET_LABELS, node); + isDefault = + labels.size() == 1 + && getOnlyElement(labels).getKind().name().equals("DEFAULT_CASE_LABEL"); + } else { + labels = node.getExpressions(); + isDefault = labels.isEmpty(); + } + if (isDefault) { token("default", plusTwo); } else { token("case", plusTwo); + builder.open(labels.size() > 1 ? plusFour : ZERO); builder.space(); boolean first = true; - for (ExpressionTree expression : node.getExpressions()) { + for (Tree expression : labels) { if (!first) { token(","); - builder.space(); + builder.breakOp(" "); } scan(expression, null); first = false; } + builder.close(); } switch (node.getCaseKind()) { case STATEMENT: @@ -226,7 +288,16 @@ public class Java14InputAstVisitor extends JavaInputAstVisitor { token("-"); token(">"); builder.space(); - scan(node.getBody(), null); + if (node.getBody().getKind() == Tree.Kind.BLOCK) { + // Explicit call with {@link CollapseEmptyOrNot.YES} to handle empty case blocks. + visitBlock( + (BlockTree) node.getBody(), + CollapseEmptyOrNot.YES, + AllowLeadingBlankLine.NO, + AllowTrailingBlankLine.NO); + } else { + scan(node.getBody(), null); + } builder.guessToken(";"); break; default: @@ -234,4 +305,20 @@ public class Java14InputAstVisitor extends JavaInputAstVisitor { } return null; } + + private static Method maybeGetMethod(Class<?> c, String name) { + try { + return c.getMethod(name); + } catch (ReflectiveOperationException e) { + return null; + } + } + + private static Object invoke(Method m, Object target) { + try { + return m.invoke(target); + } catch (ReflectiveOperationException e) { + throw new LinkageError(e.getMessage(), e); + } + } } diff --git a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java index 5addc67..03938a6 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocFormatter.java @@ -166,15 +166,30 @@ public final class JavadocFormatter { * fits on one line. */ private static String makeSingleLineIfPossible(int blockIndent, String input) { - int oneLinerContentLength = MAX_LINE_LENGTH - "/** */".length() - blockIndent; Matcher matcher = ONE_CONTENT_LINE_PATTERN.matcher(input); - if (matcher.matches() && matcher.group(1).isEmpty()) { - return "/** */"; - } else if (matcher.matches() && matcher.group(1).length() <= oneLinerContentLength) { - return "/** " + matcher.group(1) + " */"; + if (matcher.matches()) { + String line = matcher.group(1); + if (line.isEmpty()) { + return "/** */"; + } else if (oneLineJavadoc(line, blockIndent)) { + return "/** " + line + " */"; + } } return input; } + private static boolean oneLineJavadoc(String line, int blockIndent) { + int oneLinerContentLength = MAX_LINE_LENGTH - "/** */".length() - blockIndent; + if (line.length() > oneLinerContentLength) { + return false; + } + // If the javadoc contains only a tag, use multiple lines to encourage writing a summary + // fragment, unless it's /* @hide */. + if (line.startsWith("@") && !line.equals("@hide")) { + return false; + } + return true; + } + private JavadocFormatter() {} } 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 c2431c4..0361415 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 @@ -15,6 +15,7 @@ package com.google.googlejavaformat.java.javadoc; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.Comparators.max; import static com.google.common.collect.Sets.immutableEnumSet; import static com.google.googlejavaformat.java.javadoc.JavadocWriter.AutoIndent.AUTO_INDENT; import static com.google.googlejavaformat.java.javadoc.JavadocWriter.AutoIndent.NO_AUTO_INDENT; @@ -26,9 +27,7 @@ import static com.google.googlejavaformat.java.javadoc.Token.Type.HEADER_OPEN_TA 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.common.collect.Ordering; import com.google.googlejavaformat.java.javadoc.Token.Type; /** @@ -270,8 +269,7 @@ final class JavadocWriter { } private void requestWhitespace(RequestedWhitespace requestedWhitespace) { - this.requestedWhitespace = - Ordering.natural().max(requestedWhitespace, this.requestedWhitespace); + this.requestedWhitespace = max(requestedWhitespace, this.requestedWhitespace); } /** @@ -396,7 +394,7 @@ final class JavadocWriter { // 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)); } /** |