Merge remote-tracking branch 'origin/master' into fw/wrap-case-arguments

.github/workflows/ci.yml

@@ -14,7 +14,13 @@
 
 name: CI
 
-on: [ push, pull_request ]
+on:
+  push:
+    branches:
+    - master
+  pull_request:
+    branches:
+    - master
 
 jobs:
   test:
@@ -23,23 +29,27 @@ jobs:
       fail-fast: false
       matrix:
         os: [ ubuntu-latest ]
-        java: [ 15, 11 ]
+        java: [ 16, 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: 15
+            java: 16
             experimental: false
           - os: windows-latest
-            java: 15
+            java: 16
             experimental: false
           - os: ubuntu-latest
-            java: 16-ea
+            java: 17-ea
             experimental: true
     runs-on: ${{ matrix.os }}
     continue-on-error: ${{ matrix.experimental }}
     steps:
+      - name: Cancel previous
+        uses: styfle/cancel-workflow-action@0.8.0
+        with:
+          access_token: ${{ github.token }}
       - name: 'Check out repository'
         uses: actions/checkout@v2
       - name: 'Cache local Maven repository'

README.md

@@ -13,7 +13,7 @@
 and run it with:
 
 ```
-java -jar /path/to/google-java-format-1.9-all-deps.jar <options> [files...]
+java -jar /path/to/google-java-format-1.10.0-all-deps.jar <options> [files...]
 ```
 
 The formatter can act on whole files, on limited lines (`--lines`), on specific
@@ -27,6 +27,21 @@ To reformat changed lines in a specific patch, use
 formatting. This is a deliberate design decision to unify our code formatting on
 a single format.*
 
+#### JDK 16
+
+The following flags are required when running on JDK 16, due to
+[JEP 396: Strongly Encapsulate JDK Internals by Default](https://openjdk.java.net/jeps/396):
+
+```
+java \
+  --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+  --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+  --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 <options> [files...]
+```
+
 ### IntelliJ, Android Studio, and other JetBrains IDEs
 
 A
@@ -98,15 +113,15 @@ configuration.
 <dependency>
   <groupId>com.google.googlejavaformat</groupId>
   <artifactId>google-java-format</artifactId>
-  <version>1.9</version>
+  <version>1.10.0</version>
 </dependency>
 ```
 
 #### Gradle
 
 ```groovy
 dependencies {
-  compile 'com.google.googlejavaformat:google-java-format:1.9'
+  compile 'com.google.googlejavaformat:google-java-format:1.10.0'
 }
 ```
 

core/pom.xml

@@ -22,7 +22,7 @@
   <parent>
     <groupId>com.google.googlejavaformat</groupId>
     <artifactId>google-java-format-parent</artifactId>
-    <version>1.10-SNAPSHOT</version>
+  <version>HEAD-SNAPSHOT</version>
   </parent>
 
   <artifactId>google-java-format</artifactId>

core/src/main/java/com/google/googlejavaformat/Input.java

@@ -114,14 +114,14 @@ public interface Token {
   /**
    * Get the number of toks.
    *
-   * @return the number of toks, including the EOF tok
+   * @return the number of toks, excluding the EOF tok
    */
   public abstract int getkN();
 
   /**
    * Get the Token by index.
    *
-   * @param k the token index
+   * @param k the Tok index
    */
   public abstract Token getToken(int k);
 

core/src/main/java/com/google/googlejavaformat/java/JavaInput.java

@@ -594,7 +594,7 @@ Range<Integer> characterRangeToTokenRange(int offset, int length) throws Formatt
   /**
    * 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 +604,7 @@ public int getkN() {
   /**
    * Get the Token by index.
    *
-   * @param k the token index
+   * @param k the Tok index
    */
   @Override
   public Token getToken(int k) {

core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java

@@ -1599,18 +1599,22 @@ public Void visitMemberSelect(MemberSelectTree node, Void unused) {
   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();
     }
     token(sourceForNode);
     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()) {
@@ -1696,10 +1700,10 @@ private boolean ambiguousUnaryOperator(UnaryTree node, String operatorName) {
       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;
     }
@@ -1709,6 +1713,17 @@ private boolean ambiguousUnaryOperator(UnaryTree node, String operatorName) {
     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);

core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java

@@ -239,9 +239,10 @@ static int hasEscapedNewlineAt(String input, int idx) {
    * @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,18 +252,21 @@ private static String reflow(
       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<>();
     boolean first = first0;
     while (!input.isEmpty()) {
       int length = 0;
       List<String> line = new ArrayList<>();
+      // 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();

core/src/main/java/com/google/googlejavaformat/java/java14/Java14InputAstVisitor.java

@@ -15,7 +15,6 @@
 package com.google.googlejavaformat.java.java14;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.common.collect.MoreCollectors.toOptional;
 
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
@@ -28,13 +27,13 @@
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.InstanceOfTree;
+import com.sun.source.tree.ModifiersTree;
 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.util.List;
@@ -57,12 +56,13 @@ public Void visitBindingPattern(BindingPatternTree node, Void unused) {
     try {
       VariableTree variableTree =
           (VariableTree) BindingPatternTree.class.getMethod("getVariable").invoke(node);
-      visitBindingPattern(variableTree.getType(), variableTree.getName());
+      visitBindingPattern(
+          variableTree.getModifiers(), variableTree.getType(), variableTree.getName());
     } catch (ReflectiveOperationException e1) {
       try {
         Tree type = (Tree) BindingPatternTree.class.getMethod("getType").invoke(node);
-        Name name = (Name) BindingPatternTree.class.getMethod("getName").invoke(node);
-        visitBindingPattern(type, name);
+        Name name = (Name) BindingPatternTree.class.getMethod("getBinding").invoke(node);
+        visitBindingPattern(/* modifiers= */ null, type, name);
       } catch (ReflectiveOperationException e2) {
         e2.addSuppressed(e1);
         throw new LinkageError(e2.getMessage(), e2);
@@ -71,7 +71,10 @@ public Void visitBindingPattern(BindingPatternTree node, Void unused) {
     return null;
   }
 
-  private void visitBindingPattern(Tree type, Name name) {
+  private void visitBindingPattern(ModifiersTree modifiers, Tree type, Name name) {
+    if (modifiers != null) {
+      builder.addAll(visitModifiers(modifiers, Direction.HORIZONTAL, Optional.empty()));
+    }
     scan(type, null);
     builder.breakOp(" ");
     visit(name);
@@ -137,10 +140,7 @@ public void visitRecordDeclaration(ClassTree node) {
       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.
@@ -179,14 +179,6 @@ public void visitRecordDeclaration(ClassTree node) {
     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)

core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java

@@ -48,7 +48,9 @@
 public class FormatterIntegrationTest {
 
   private static final ImmutableSet<String> JAVA14_TESTS =
-      ImmutableSet.of("I477", "Records", "RSLs", "Var", "ExpressionSwitch");
+      ImmutableSet.of("I477", "Records", "RSLs", "Var", "ExpressionSwitch", "I574", "I594");
+
+  private static final ImmutableSet<String> JAVA16_TESTS = ImmutableSet.of("I588");
 
   @Parameters(name = "{index}: {0}")
   public static Iterable<Object[]> data() throws IOException {
@@ -76,7 +78,7 @@ public static Iterable<Object[]> data() throws IOException {
           case "output":
             outputs.put(baseName, contents);
             break;
-          default:
+          default: // fall out
         }
       }
     }
@@ -90,6 +92,9 @@ public static Iterable<Object[]> data() throws IOException {
       if (JAVA14_TESTS.contains(fileName) && getMajor() < 14) {
         continue;
       }
+      if (JAVA16_TESTS.contains(fileName) && getMajor() < 16) {
+        continue;
+      }
       testInputs.add(new Object[] {fileName, input, expectedOutput});
     }
     return testInputs;
@@ -125,7 +130,9 @@ public FormatterIntegrationTest(String name, String input, String expected) {
   @Test
   public void format() {
     try {
-      String output = new Formatter().formatSource(input);
+      Formatter formatter = new Formatter();
+      String output = formatter.formatSource(input);
+      output = StringWrapper.wrap(output, formatter);
       assertEquals("bad output for " + name, expected, output);
     } catch (FormatterException e) {
       fail(String.format("Formatter crashed on %s: %s", name, e.getMessage()));