Compact canonical constructor support

src/org/jetbrains/java/decompiler/main/ClassWriter.java

@@ -1160,104 +1160,10 @@ public boolean writeMethod(ClassNode node, StructMethod mt, int methodIndex, Tex
         }
 
         buffer.appendMethod(toValidJavaIdentifier(name), true, cl.qualifiedName, mt.getName(), md);
-        buffer.append('(');
 
-        List<VarVersionPair> mask = methodWrapper.synthParameters;
-
-        int lastVisibleParameterIndex = -1;
-        for (int i = 0; i < md.params.length; i++) {
-          if (mask == null || mask.get(i) == null) {
-            lastVisibleParameterIndex = i;
-          }
-        }
-        if (lastVisibleParameterIndex != -1) {
-          buffer.pushNewlineGroup(indent, 1);
-          buffer.appendPossibleNewline();
-        }
-
-        List<StructMethodParametersAttribute.Entry> methodParameters = null;
-        if (DecompilerContext.getOption(IFernflowerPreferences.USE_METHOD_PARAMETERS)) {
-          StructMethodParametersAttribute attr = mt.getAttribute(StructGeneralAttribute.ATTRIBUTE_METHOD_PARAMETERS);
-          if (attr != null) {
-            methodParameters = attr.getEntries();
-          }
-        }
-
-        int index = isEnum && init ? 3 : thisVar ? 1 : 0;
-        int start = isEnum && init ? 2 : 0;
-        boolean hasDescriptor = descriptor != null;
-        //mask should now have the Outer.this in it... so this *shouldn't* be nessasary.
-        //if (init && !isEnum && ((node.access & CodeConstants.ACC_STATIC) == 0) && node.type == ClassNode.CLASS_MEMBER)
-        //    index++;
-
-        buffer.pushNewlineGroup(indent, 0);
-        for (int i = start; i < md.params.length; i++) {
-          boolean real = mask == null || mask.get(i) == null;
-          VarType parameterType = real && hasDescriptor && paramCount < descriptor.parameterTypes.size() ? descriptor.parameterTypes.get(paramCount) : md.params[i];
-          if (real) {
-            if (paramCount > 0) {
-              buffer.append(",");
-              buffer.appendPossibleNewline(" ");
-            }
-
-            appendParameterAnnotations(buffer, mt, paramCount);
-
-            if (methodParameters != null && i < methodParameters.size()) {
-              appendModifiers(buffer, methodParameters.get(i).myAccessFlags, CodeConstants.ACC_FINAL, isInterface, 0);
-            }
-            else if (methodWrapper.varproc.getVarFinal(new VarVersionPair(index, 0)) == VarTypeProcessor.FinalType.EXPLICIT_FINAL) {
-              buffer.append("final ");
-            }
-
-            String typeName;
-            boolean isVarArg = i == lastVisibleParameterIndex && mt.hasModifier(CodeConstants.ACC_VARARGS) && parameterType.arrayDim > 0;
-            if (isVarArg) {
-                parameterType = parameterType.decreaseArrayDim();
-            }
-            typeName = ExprProcessor.getCastTypeName(parameterType);
-
-            if (ExprProcessor.UNDEFINED_TYPE_STRING.equals(typeName) &&
-                DecompilerContext.getOption(IFernflowerPreferences.UNDEFINED_PARAM_TYPE_OBJECT)) {
-              typeName = ExprProcessor.getCastTypeName(VarType.VARTYPE_OBJECT);
-            }
-            buffer.appendCastTypeName(typeName, parameterType);
-            if (isVarArg) {
-              buffer.append("...");
-            }
-
-            buffer.append(' ');
-
-            String parameterName;
-            String clashingName = methodWrapper.varproc.getClashingName(new VarVersionPair(index, 0));
-            if (clashingName != null) {
-              parameterName = clashingName;
-            } else if (methodParameters != null && i < methodParameters.size() && methodParameters.get(i).myName != null) {
-              parameterName = methodParameters.get(i).myName;
-            } else {
-              parameterName = methodWrapper.varproc.getVarName(new VarVersionPair(index, 0));
-            }
-
-            String newParameterName = methodWrapper.methodStruct.getVariableNamer().renameParameter(flags, parameterType, parameterName, index);
-            if ((flags & (CodeConstants.ACC_ABSTRACT | CodeConstants.ACC_NATIVE)) != 0 && Objects.equals(newParameterName, parameterName)) {
-              newParameterName = DecompilerContext.getStructContext().renameAbstractParameter(methodWrapper.methodStruct.getClassQualifiedName(), mt.getName(), mt.getDescriptor(), index - (((flags & CodeConstants.ACC_STATIC) == 0) ? 1 : 0), parameterName);
-            }
-            parameterName = newParameterName;
-
-            buffer.appendVariable(parameterName == null ? "param" + index : parameterName, // null iff decompiled with errors
-              true, true, cl.qualifiedName, mt.getName(), md, index, parameterName);
-
-            paramCount++;
-          }
-
-          index += parameterType.stackSize;
-        }
-        buffer.popNewlineGroup();
-
-        if (lastVisibleParameterIndex != -1) {
-          buffer.appendPossibleNewline("", true);
-          buffer.popNewlineGroup();
+        if (!methodWrapper.isCompactRecordConstructor) {
+          paramCount = writeMethodParameterHeader(mt, buffer, indent, methodWrapper, md, isEnum, init, thisVar, descriptor, paramCount, isInterface, flags, cl);
         }
-        buffer.append(')');
 
         StructExceptionsAttribute attr = mt.getAttribute(StructGeneralAttribute.ATTRIBUTE_EXCEPTIONS);
         if ((descriptor != null && !descriptor.exceptionTypes.isEmpty()) || attr != null) {
@@ -1305,7 +1211,7 @@ else if (methodWrapper.varproc.getVarFinal(new VarVersionPair(index, 0)) == VarT
             } else {
               TextBuffer code = root.toJava(indent + 1);
               code.addBytecodeMapping(root.getDummyExit().bytecode);
-              hideMethod = code.length() == 0 && (clInit || dInit || hideConstructor(node, init, throwsExceptions, paramCount, flags));
+              hideMethod = code.length() == 0 && (clInit || dInit || hideConstructor(node, init, throwsExceptions, paramCount, flags, mt));
               buffer.append(code, cl.qualifiedName, InterpreterUtil.makeUniqueKey(mt.getName(), mt.getDescriptor()));
             }
           }
@@ -1336,6 +1242,108 @@ else if (methodWrapper.varproc.getVarFinal(new VarVersionPair(index, 0)) == VarT
     return !hideMethod;
   }
 
+  private static int writeMethodParameterHeader(StructMethod mt, TextBuffer buffer, int indent, MethodWrapper methodWrapper, MethodDescriptor md, boolean isEnum, boolean init, boolean thisVar, GenericMethodDescriptor descriptor, int paramCount, boolean isInterface, int flags, StructClass cl) {
+    buffer.append('(');
+
+    List<VarVersionPair> mask = methodWrapper.synthParameters;
+
+    int lastVisibleParameterIndex = -1;
+    for (int i = 0; i < md.params.length; i++) {
+      if (mask == null || mask.get(i) == null) {
+        lastVisibleParameterIndex = i;
+      }
+    }
+    if (lastVisibleParameterIndex != -1) {
+      buffer.pushNewlineGroup(indent, 1);
+      buffer.appendPossibleNewline();
+    }
+
+    List<StructMethodParametersAttribute.Entry> methodParameters = null;
+    if (DecompilerContext.getOption(IFernflowerPreferences.USE_METHOD_PARAMETERS)) {
+      StructMethodParametersAttribute attr = mt.getAttribute(StructGeneralAttribute.ATTRIBUTE_METHOD_PARAMETERS);
+      if (attr != null) {
+        methodParameters = attr.getEntries();
+      }
+    }
+
+    int index = isEnum && init ? 3 : thisVar ? 1 : 0;
+    int start = isEnum && init ? 2 : 0;
+    boolean hasDescriptor = descriptor != null;
+    //mask should now have the Outer.this in it... so this *shouldn't* be nessasary.
+    //if (init && !isEnum && ((node.access & CodeConstants.ACC_STATIC) == 0) && node.type == ClassNode.CLASS_MEMBER)
+    //    index++;
+
+    buffer.pushNewlineGroup(indent, 0);
+    for (int i = start; i < md.params.length; i++) {
+      boolean real = mask == null || mask.get(i) == null;
+      VarType parameterType = real && hasDescriptor && paramCount < descriptor.parameterTypes.size() ? descriptor.parameterTypes.get(paramCount) : md.params[i];
+      if (real) {
+        if (paramCount > 0) {
+          buffer.append(",");
+          buffer.appendPossibleNewline(" ");
+        }
+
+        appendParameterAnnotations(buffer, mt, paramCount);
+
+        if (methodParameters != null && i < methodParameters.size()) {
+          appendModifiers(buffer, methodParameters.get(i).myAccessFlags, CodeConstants.ACC_FINAL, isInterface, 0);
+        }
+        else if (methodWrapper.varproc.getVarFinal(new VarVersionPair(index, 0)) == VarTypeProcessor.FinalType.EXPLICIT_FINAL) {
+          buffer.append("final ");
+        }
+
+        String typeName;
+        boolean isVarArg = i == lastVisibleParameterIndex && mt.hasModifier(CodeConstants.ACC_VARARGS) && parameterType.arrayDim > 0;
+        if (isVarArg) {
+            parameterType = parameterType.decreaseArrayDim();
+        }
+        typeName = ExprProcessor.getCastTypeName(parameterType);
+
+        if (ExprProcessor.UNDEFINED_TYPE_STRING.equals(typeName) &&
+            DecompilerContext.getOption(IFernflowerPreferences.UNDEFINED_PARAM_TYPE_OBJECT)) {
+          typeName = ExprProcessor.getCastTypeName(VarType.VARTYPE_OBJECT);
+        }
+        buffer.appendCastTypeName(typeName, parameterType);
+        if (isVarArg) {
+          buffer.append("...");
+        }
+
+        buffer.append(' ');
+
+        String parameterName;
+        String clashingName = methodWrapper.varproc.getClashingName(new VarVersionPair(index, 0));
+        if (clashingName != null) {
+          parameterName = clashingName;
+        } else if (methodParameters != null && i < methodParameters.size() && methodParameters.get(i).myName != null) {
+          parameterName = methodParameters.get(i).myName;
+        } else {
+          parameterName = methodWrapper.varproc.getVarName(new VarVersionPair(index, 0));
+        }
+
+        String newParameterName = methodWrapper.methodStruct.getVariableNamer().renameParameter(flags, parameterType, parameterName, index);
+        if ((flags & (CodeConstants.ACC_ABSTRACT | CodeConstants.ACC_NATIVE)) != 0 && Objects.equals(newParameterName, parameterName)) {
+          newParameterName = DecompilerContext.getStructContext().renameAbstractParameter(methodWrapper.methodStruct.getClassQualifiedName(), mt.getName(), mt.getDescriptor(), index - (((flags & CodeConstants.ACC_STATIC) == 0) ? 1 : 0), parameterName);
+        }
+        parameterName = newParameterName;
+
+        buffer.appendVariable(parameterName == null ? "param" + index : parameterName, // null iff decompiled with errors
+          true, true, cl.qualifiedName, mt.getName(), md, index, parameterName);
+
+        paramCount++;
+      }
+
+      index += parameterType.stackSize;
+    }
+    buffer.popNewlineGroup();
+
+    if (lastVisibleParameterIndex != -1) {
+      buffer.appendPossibleNewline("", true);
+      buffer.popNewlineGroup();
+    }
+    buffer.append(')');
+    return paramCount;
+  }
+
   private static void dumpError(TextBuffer buffer, MethodWrapper wrapper, int indent) {
     List<String> lines = new ArrayList<>();
     lines.add("$VF: Couldn't be decompiled");
@@ -1522,7 +1530,7 @@ private static void appendConstant(StringBuilder sb, PooledConstant constant) {
     }
   }
 
-  private static boolean hideConstructor(ClassNode node, boolean init, boolean throwsExceptions, int paramCount, int methodAccessFlags) {
+  private static boolean hideConstructor(ClassNode node, boolean init, boolean throwsExceptions, int paramCount, int methodAccessFlags, StructMethod structMethod) {
     if (!init || throwsExceptions || paramCount > 0 || !DecompilerContext.getOption(IFernflowerPreferences.HIDE_DEFAULT_CONSTRUCTOR)) {
       return false;
     }
@@ -1532,17 +1540,27 @@ private static boolean hideConstructor(ClassNode node, boolean init, boolean thr
 
     int classAccessFlags = node.type == ClassNode.Type.ROOT ? cl.getAccessFlags() : node.access;
     boolean isEnum = cl.hasModifier(CodeConstants.ACC_ENUM) && DecompilerContext.getOption(IFernflowerPreferences.DECOMPILE_ENUM);
+    boolean isRecord = cl.getRecordComponents() != null;
 
     // default constructor requires same accessibility flags. Exception: enum constructor which is always private
-    if(!isEnum && ((classAccessFlags & ACCESSIBILITY_FLAGS) != (methodAccessFlags & ACCESSIBILITY_FLAGS))) {
+    // Another exception: record classes can sometimes be generated with a private constructor
+    if(!isEnum && !isRecord && ((classAccessFlags & ACCESSIBILITY_FLAGS) != (methodAccessFlags & ACCESSIBILITY_FLAGS))) {
       return false;
     }
 
-    int count = 0;
-    for (StructMethod mt : cl.getMethods()) {
-      if (CodeConstants.INIT_NAME.equals(mt.getName())) {
-        if (++count > 1) {
-          return false;
+    // Constructor with an annotation, we dont want to hide this.
+    if (isRecord && RecordHelper.hasAnnotations(structMethod)) {
+      return false;
+    }
+
+    // We should not run this check in records
+    if (!isRecord) {
+      int count = 0;
+      for (StructMethod mt : cl.getMethods()) {
+        if (CodeConstants.INIT_NAME.equals(mt.getName())) {
+          if (++count > 1) {
+            return false;
+          }
         }
       }
     }

src/org/jetbrains/java/decompiler/main/RecordHelper.java

@@ -212,7 +212,7 @@ private static void recordComponentToJava(TextBuffer buffer, StructClass cl, Str
     buffer.appendField(cd.getName(), true, cl.qualifiedName, cd.getName(), cd.getDescriptor());
   }
 
-  private static boolean hasAnnotations(StructMethod mt) {
+  public static boolean hasAnnotations(StructMethod mt) {
     return mt.getAttribute(StructGeneralAttribute.ATTRIBUTE_RUNTIME_INVISIBLE_ANNOTATIONS) != null ||
       mt.getAttribute(StructGeneralAttribute.ATTRIBUTE_RUNTIME_VISIBLE_ANNOTATIONS) != null;
   }
@@ -239,5 +239,36 @@ public static void fixupCanonicalConstructor(MethodWrapper mw, StructClass cl) {
 
       varidx += md.params[i].stackSize;
     }
+
+    // Prune all field assignments from the canonical constructor
+    if (isCompactCanonicalConstructor(mw)) {
+      mw.getOrBuildGraph().iterateExprents(exprent -> {
+        if (exprent instanceof AssignmentExprent assignmentExprent && assignmentExprent.getLeft() instanceof FieldExprent) {
+          return 2;
+        }
+
+        return 0;
+      });
+      mw.isCompactRecordConstructor = true;
+    }
   }
+
+  // Ideally this is iterated backwards.
+  // However, what we do is check that the last exprents are field invocations to local variables.
+  // (And that the name of the lvt matches the field)
+  private static boolean isCompactCanonicalConstructor(MethodWrapper mw) {
+    DirectGraph graph = mw.getOrBuildGraph();
+    boolean[] valid = new boolean[1];
+    graph.iterateExprents(exprent -> {
+      if (exprent instanceof AssignmentExprent assignmentExprent && assignmentExprent.getLeft() instanceof FieldExprent fieldExprent && assignmentExprent.getRight() instanceof VarExprent varExprent) {
+        valid[0] = varExprent.getLVT() != null && fieldExprent.getName().equals(varExprent.getName());
+      } else {
+        valid[0] = false;
+      }
+
+      return 0;
+    });
+    return valid[0];
+  }
+
 }

src/org/jetbrains/java/decompiler/main/rels/MethodWrapper.java

@@ -26,6 +26,7 @@ public class MethodWrapper {
   public Throwable decompileError;
   public Set<String> commentLines = null;
   public boolean addErrorComment = false;
+  public boolean isCompactRecordConstructor = false;
 
   public MethodWrapper(RootStatement root, VarProcessor varproc, StructMethod methodStruct, StructClass classStruct, CounterContainer counter) {
     this.root = root;

src/org/jetbrains/java/decompiler/struct/StructClass.java

@@ -213,7 +213,7 @@ public ConstantPool getPool() {
     if (recordAttr == null) {
       // If our class extends j.l.Record but also has no components, it's probably malformed.
       // Force processing as a record anyway, in the hopes that we can come to a better result.
-      if (this.superClass.getString().equals("java/lang/Record")) {
+      if (this.superClass != null && this.superClass.getString().equals("java/lang/Record")) {
         return new ArrayList<>();
       }
 

testData/results/pkg/TestRecordEmptyConstructor.dec

@@ -1,9 +1,8 @@
 package pkg;
 
 public record TestRecordEmptyConstructor(int val) {
-   public TestRecordEmptyConstructor(int val) {
+   public TestRecordEmptyConstructor {
       System.out.println(val);// 5
-      this.val = val;// 4
    }// 6
 }
 
@@ -16,16 +15,12 @@ class 'pkg/TestRecordEmptyConstructor' {
       8      4
       9      4
       a      4
-      b      5
-      c      5
-      d      5
-      e      5
-      f      5
-      10      6
+      10      5
    }
 }
 
 Lines mapping:
-4 <-> 6
 5 <-> 5
-6 <-> 7
+6 <-> 6
+Not mapped:
+4

testData/results/pkg/TestRecordGenericVararg.dec

@@ -2,25 +2,13 @@ package pkg;
 
 public record TestRecordGenericVararg<T>(T first, T... other) {
    @SafeVarargs
-   public TestRecordGenericVararg(T first, T... other) {
-      this.first = first;// 5
-      this.other = other;
-   }
+   public TestRecordGenericVararg {
+   }// 5
 }
 
 class 'pkg/TestRecordGenericVararg' {
    method '<init> (Ljava/lang/Object;[Ljava/lang/Object;)V' {
-      4      5
-      5      5
-      6      5
-      7      5
-      8      5
-      9      6
-      a      6
-      b      6
-      c      6
-      d      6
-      e      7
+      e      5
    }
 }