Fix formatting of records without an explicit constructor

Fixes #460 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=308658958
google · Apr 27, 2020 · ca529a2 · ca529a2
1 parent 7ba82f7
commit ca529a2
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 14 deletions.
diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java
@@ -1332,9 +1332,12 @@ public Void visitAnnotatedType(AnnotatedTypeTree node, Void unused) {
     return null;
   }
 
-  // TODO(cushon): Use Flags.COMPACT_RECORD_CONSTRUCTOR once if/when we drop support for Java 11
+  // TODO(cushon): Use Flags if/when we drop support for Java 11
+
   protected static final long COMPACT_RECORD_CONSTRUCTOR = 1L << 51;
 
+  protected static final long RECORD = 1L << 61;
+
   @Override
   public Void visitMethod(MethodTree node, Void unused) {
     sync(node);

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
@@ -15,9 +15,10 @@
 package com.google.googlejavaformat.java.java14;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.common.collect.MoreCollectors.onlyElement;
+import static com.google.common.collect.MoreCollectors.toOptional;
 
 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.java.JavaInputAstVisitor;
@@ -26,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.MethodTree;
 import com.sun.source.tree.SwitchExpressionTree;
 import com.sun.source.tree.Tree;
 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;
 import java.util.Optional;
@@ -112,20 +113,17 @@ public void visitRecordDeclaration(ClassTree node) {
       if (!node.getTypeParameters().isEmpty()) {
         typeParametersRest(node.getTypeParameters(), hasSuperInterfaceTypes ? plusFour : ZERO);
       }
-      MethodTree constructor =
-          node.getMembers().stream()
-              .filter(JCMethodDecl.class::isInstance)
-              .map(JCMethodDecl.class::cast)
-              .filter(
-                  m -> (m.mods.flags & COMPACT_RECORD_CONSTRUCTOR) == COMPACT_RECORD_CONSTRUCTOR)
-              .collect(onlyElement());
+      ImmutableList<JCVariableDecl> parameters =
+          compactRecordConstructor(node)
+              .map(m -> ImmutableList.copyOf(m.getParameters()))
+              .orElseGet(() -> recordVariables(node));
       token("(");
-      if (!constructor.getParameters().isEmpty() || constructor.getReceiverParameter() != null) {
+      if (!parameters.isEmpty()) {
         // Break before args.
         builder.breakToFill("");
       }
-      visitFormals(
-          Optional.ofNullable(constructor.getReceiverParameter()), constructor.getParameters());
+      // record headers can't declare receiver parameters
+      visitFormals(/* receiver= */ Optional.empty(), parameters);
       token(")");
       if (hasSuperInterfaceTypes) {
         builder.breakToFill(" ");
@@ -157,6 +155,22 @@ 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)
+        .map(JCVariableDecl.class::cast)
+        .filter(m -> (m.mods.flags & RECORD) == RECORD)
+        .collect(toImmutableList());
+  }
+
   @Override
   public Void visitInstanceOf(InstanceOfTree node, Void unused) {
     sync(node);

diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/java14.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/java14.input
@@ -19,6 +19,14 @@ class Java14 {
     }
   }
 
+  record Foo(int id) {}
+
+  record Rcv(int id) {
+    public Rcv(Rcv this, int id) {
+      this.id = id;
+    }
+  }
+
   void g() {
     var block = """
     hello
@@ -35,4 +43,4 @@ class Java14 {
         case WEDNESDAY              -> 9;
     };
   }
-}
+}
diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/java14.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/java14.output
@@ -19,6 +19,14 @@ class Java14 {
     }
   }
 
+  record Foo(int id) {}
+
+  record Rcv(int id) {
+    public Rcv(Rcv this, int id) {
+      this.id = id;
+    }
+  }
+
   void g() {
     var block = """
     hello