Format annotated fields in records (#1427)

crogoz · web-flow · commit d1cde3c11711 · 2025-10-07T14:39:38.000Z
Format annotated fields in records
diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java
@@ -157,6 +157,12 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> {
      */
     private static final int METHOD_CHAIN_COLUMN_LIMIT = 80;
 
+    /**
+     * Maximum column at which the annotated parameter of a record should start. This exists in particular to improve
+     * readability when one or multiple annotations are added to the parameters of a record.
+     */
+    private static final int ANNOTATION_RECORD_PARAMETER_COLUMN_LIMIT = 60;
+
     /** Direction for Annotations (usually VERTICAL). */
     protected enum Direction {
         VERTICAL,
@@ -2212,13 +2218,23 @@ protected List<Op> visitModifiers(
             ModifiersTree modifiersTree,
             Direction annotationsDirection,
             Optional<BreakTag> declarationAnnotationBreak) {
-        return visitModifiers(modifiersTree.getAnnotations(), annotationsDirection, declarationAnnotationBreak);
+        boolean isRecordParameter = modifiersTree instanceof JCTree.JCModifiers && isInRecord(modifiersTree);
+        return visitModifiers(
+                modifiersTree.getAnnotations(), annotationsDirection, declarationAnnotationBreak, isRecordParameter);
     }
 
     private List<Op> visitModifiers(
             List<? extends AnnotationTree> annotationTrees,
             Direction annotationsDirection,
             Optional<BreakTag> declarationAnnotationBreak) {
+        return visitModifiers(annotationTrees, annotationsDirection, declarationAnnotationBreak, false);
+    }
+
+    private List<Op> visitModifiers(
+            List<? extends AnnotationTree> annotationTrees,
+            Direction annotationsDirection,
+            Optional<BreakTag> declarationAnnotationBreak,
+            boolean isRecordParameter) {
         if (annotationTrees.isEmpty() && !nextIsModifier()) {
             return EMPTY_LIST;
         }
@@ -2241,9 +2257,19 @@ private List<Op> visitModifiers(
             lastWasAnnotation = true;
         }
         builder.close();
-        ImmutableList<Op> trailingBreak = annotationsDirection.isVertical()
-                ? forceBreakList(declarationAnnotationBreak)
-                : breakList(declarationAnnotationBreak);
+
+        // pjf specific: record params should take into consideration the columnLimit
+        ImmutableList<Op> trailingBreak = isRecordParameter
+                ? ImmutableList.of(Break.builder()
+                        .fillMode(FillMode.UNIFIED)
+                        .flat(" ")
+                        .plusIndent(ZERO)
+                        .hasColumnLimit(true)
+                        .optTag(declarationAnnotationBreak)
+                        .build())
+                : annotationsDirection.isVertical()
+                        ? forceBreakList(declarationAnnotationBreak)
+                        : breakList(declarationAnnotationBreak);
         if (annotations.isEmpty() && !nextIsModifier()) {
             return trailingBreak;
         }
@@ -2387,6 +2413,11 @@ protected void visitFormals(Optional<VariableTree> receiver, List<? extends Vari
         if (!receiver.isPresent() && parameters.isEmpty()) {
             return;
         }
+        boolean isRecordParams = false;
+        if (!parameters.isEmpty() && parameters.get(0) instanceof JCTree.JCVariableDecl) {
+            isRecordParams = isInRecord(((JCTree.JCVariableDecl) parameters.get(0)).mods);
+        }
+
         builder.open(ZERO);
         boolean first = true;
         if (receiver.isPresent()) {
@@ -2410,13 +2441,26 @@ protected void visitFormals(Optional<VariableTree> receiver, List<? extends Vari
             if (!first) {
                 builder.breakOp(" ");
             }
+            BreakTag declarationAnnotationBreak = new BreakTag();
+
+            // pfj specific: add a conditional blank line if the annotated parameter was broken (only for records)
+            if (isRecordParams && !first) {
+                builder.blankLineWanted(BlankLineWanted.conditional(declarationAnnotationBreak));
+            }
+
             visitToDeclare(
                     DeclarationKind.PARAMETER,
                     Direction.HORIZONTAL,
                     parameter,
                     /* initializer= */ Optional.empty(),
                     "=",
-                    i < parameters.size() - 1 ? Optional.of(",") : /* a= */ Optional.empty());
+                    i < parameters.size() - 1 ? Optional.of(",") : /* a= */ Optional.empty(),
+                    Optional.of(declarationAnnotationBreak));
+
+            // pfj specific: add a conditional blank line if the annotated parameter was broken (only for records)
+            if (isRecordParams && i < parameters.size() - 1) {
+                builder.blankLineWanted(BlankLineWanted.conditional(declarationAnnotationBreak));
+            }
             first = false;
         }
         builder.close();
@@ -2586,6 +2630,17 @@ private void visitToDeclare(
             Optional<ExpressionTree> initializer,
             String equals,
             Optional<String> trailing) {
+        visitToDeclare(kind, annotationsDirection, node, initializer, equals, trailing, Optional.empty());
+    }
+
+    private void visitToDeclare(
+            DeclarationKind kind,
+            Direction annotationsDirection,
+            VariableTree node,
+            Optional<ExpressionTree> initializer,
+            String equals,
+            Optional<String> trailing,
+            Optional<BreakTag> annotationBreakForRecords) {
         sync(node);
         declareOne(
                 kind,
@@ -2598,7 +2653,8 @@ private void visitToDeclare(
                 initializer,
                 trailing,
                 /* receiverExpression= */ Optional.empty(),
-                /* typeWithDims= */ Optional.empty());
+                /* typeWithDims= */ Optional.empty(),
+                annotationBreakForRecords);
     }
 
     /** Does not omit the leading '<', which should be associated with the type name. */
@@ -3402,9 +3458,39 @@ int declareOne(
             Optional<String> trailing,
             Optional<ExpressionTree> receiverExpression,
             Optional<TypeWithDims> typeWithDims) {
+        return declareOne(
+                kind,
+                annotationsDirection,
+                modifiers,
+                type,
+                name,
+                op,
+                equals,
+                initializer,
+                trailing,
+                receiverExpression,
+                typeWithDims,
+                Optional.empty());
+    }
+
+    /** Declare one variable or variable-like thing. */
+    @SuppressWarnings("TooManyArguments")
+    int declareOne(
+            DeclarationKind kind,
+            Direction annotationsDirection,
+            Optional<ModifiersTree> modifiers,
+            Tree type,
+            Name name,
+            String op,
+            String equals,
+            Optional<ExpressionTree> initializer,
+            Optional<String> trailing,
+            Optional<ExpressionTree> receiverExpression,
+            Optional<TypeWithDims> typeWithDims,
+            Optional<BreakTag> verticalAnnotationBreakForRecords) {
 
         BreakTag typeBreak = new BreakTag();
-        BreakTag verticalAnnotationBreak = new BreakTag();
+        BreakTag verticalAnnotationBreak = verticalAnnotationBreakForRecords.orElseGet(BreakTag::new);
 
         // If the node is a field declaration, try to output any declaration
         // annotations in-line. If the entire declaration doesn't fit on a single
@@ -3419,12 +3505,22 @@ int declareOne(
                 new ArrayDeque<>(typeWithDims.isPresent() ? typeWithDims.get().dims : Collections.emptyList());
         int baseDims = 0;
 
-        builder.open(
-                kind == DeclarationKind.PARAMETER
-                                && (modifiers.isPresent()
-                                        && !modifiers.get().getAnnotations().isEmpty())
-                        ? plusFour
-                        : ZERO);
+        if (kind == DeclarationKind.PARAMETER
+                && modifiers.isPresent()
+                && !modifiers.get().getAnnotations().isEmpty()) {
+            if (!isInRecord(modifiers.get())) {
+                builder.open(plusFour);
+            } else {
+                // pjf specific: we are enforcing the column limit for annotated parameters of records
+                builder.open(OpenOp.builder()
+                        .debugName("visitParam")
+                        .plusIndent(ZERO)
+                        .columnLimitBeforeLastBreak(ANNOTATION_RECORD_PARAMETER_COLUMN_LIMIT)
+                        .build());
+            }
+        } else {
+            builder.open(ZERO);
+        }
         {
             if (modifiers.isPresent()) {
                 visitAndBreakModifiers(modifiers.get(), annotationsDirection, Optional.of(verticalAnnotationBreak));
@@ -3794,6 +3890,13 @@ private Direction inlineAnnotationDirection(ModifiersTree modifiers) {
         return Direction.HORIZONTAL;
     }
 
+    /**
+     * Checks if the modifiers are parameters of a record definition.
+     */
+    private boolean isInRecord(ModifiersTree modifiers) {
+        return (((JCTree.JCModifiers) modifiers).flags & RECORD) == RECORD;
+    }
+
     /**
      * Emit a {@link Token}.
      *
diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/AnnotationFields.input b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/AnnotationFields.input
@@ -0,0 +1,110 @@
+@Schema(description = "Type of quote being requested", example = "NEW_BUSINESS", somethingElse= "new", other="my other long string")
+public record QuoteRequest(
+        @Schema(description = "US state of the product being quoted", example = "TX")
+                RegulatoryState regulatoryState,
+        @Schema(description = "Reason for a quote", example = "New Business") String amendmentReason,
+        @Schema(description = "Type of quote being requested", example = "NEW_BUSINESS", somethingElse= "new", other="my other long string") QuoteType quoteType,
+        @Schema(description = "Date the quoted changes become active", example = "2023-06-25")
+                LocalDate quoteEffectiveDate
+) {}
+
+public record QuoteRequest(
+
+        int value,
+
+        @SomeInput RegulatoryState regulatoryState,
+
+        @SomeInput
+        @NotNull
+        @Deprecated
+        @JsonValue(name = "something") @Schema(description = "US state of the product being quoted", example = "TX") RegulatoryState regulatoryState,
+        @Schema(description = "Reason for a quote", example = "New Business") String amendmentReason,
+        int x,
+        int j) {}
+
+public record Url(
+        int var1,
+        @JsonValue("value") String var2,
+        @SomeInput @JsonValue String var3,
+        int var3,
+        int var4,
+        int var5,
+        int var6) {}
+
+public record Url(
+        @NotNull
+        @JsonValue
+        @Deprecated
+        @Annotation1
+        @Annotation2
+        @Annotation3
+        @Annotation4
+        @Annotation5
+        @Annotation6
+        @Annotation7
+        @Annotation8
+        @Annotation9
+        @Annotation10
+        @Annotation11
+        @Annotation12
+        String value) {}
+
+public record Url(@NotNull String value, int number) {}
+
+public record Url(
+        int number,
+        @JsonValue("value") String value) {}
+
+public record Url(@New @JsonValue("value") String value) {}
+
+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonInclude(JsonInclude.Include.NON_NULL)
+public record Game(
+    @JsonProperty("accounting_group") @JsonPropertyDescription("The accounting group of the game.")
+        String accountingGroup,
+    @JsonProperty("accumulating")
+        @JsonPropertyDescription("Marks which games with accumulating bonuses.") Boolean accumulating,
+    @JsonProperty("bonus_buy") @JsonPropertyDescription("Games with purchasable bonuses.") Boolean bonusBuy,
+    @JsonProperty("category")
+        @JsonPropertyDescription(
+            "Game's category. Allowed values: slots, roulette, card, "
+                + "casual, lottery, poker, craps, video_poker")
+        String category,
+    @JsonProperty("hd") @JsonPropertyDescription("HD format games.") Boolean hd,
+
+    @JsonProperty("new") @JsonPropertyDescription("New format games.") Boolean assa,
+    @JsonProperty("hit_rate") @JsonPropertyDescription(
+            "Frequency of wins per 100 bets. The higher, the hit rate the lower the volatility "
+                + "rating, and vice versa. Positive value. For slots only. "
+                + "Match pattern: ^\\d{1,18}(\\.\\d{1,12})?$") String hitRate,
+    @JsonProperty("params") @JsonPropertyDescription("Game's custom parameters.")
+        Map<String, String> params) {
+
+        private void get(@JsonNode("value") String value) {}
+
+        private void get2(@JsonNode(value="value") String value) {}
+        }
+
+record ApiEndpoint(@JsonValue("name") String name, HttpMethod method, String url) {
+    ApiEndpoint {
+        name = name.toUpperCase();
+    }
+}
+
+record ApiEndpoint(@JsonProperty("bonus_buy") @JsonValue(some_other_name_that_is_here="value", name_which_is_super_long_that_goes_beyond_the_limit="name", other="other") String name, HttpMethod method, String url) {
+    ApiEndpoint {
+        name = name.toUpperCase();
+    }
+}
+
+record ApiEndpoint(@JsonValue(name="name") String name, HttpMethod method, String url) {
+    ApiEndpoint {
+        name = name.toUpperCase();
+    }
+}
+
+record ApiEndpoint(@JsonValue(name="name") String name, HttpMethod method, String url, @JsonValue(name="name") String name) {
+    ApiEndpoint {
+        name = name.toUpperCase();
+    }
+}
diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/AnnotationFields.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/AnnotationFields.output
