Merge pull request #98 from openjavaformat/records-instead-of-immutables

Use records instead of Immutables for plain value types

Author: abashev

open-java-format-jdk-bootstrap/build.gradle

@@ -8,10 +8,7 @@ configurations {
 }
 
 dependencies {
-    annotationProcessor libs.immutables.value
-
     api project(':open-java-format-spi')
-    compileOnly variantOf(libs.immutables.value) { classifier('annotations') }
     implementation libs.jackson.databind
 
     testImplementation project(':open-java-format')
@@ -26,11 +23,6 @@ dependencies {
     }
 }
 
-// Immutables' processor keeps Gradle's incremental compilation working only when asked to.
-tasks.named('compileJava', JavaCompile) {
-    options.compilerArgs.add('-Aimmutables.gradle.incremental')
-}
-
 tasks.named('test', Test.class) {
     inputs.files(configurations.named('formatterNativeImage'))
     environment.put('NATIVE_IMAGE_CLASSPATH', configurations.formatterNativeImage.asPath)

open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java

@@ -33,7 +33,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
-import org.immutables.value.Value;
 
 public final class BootstrappingFormatterService implements FormatterService {
     private static final ObjectMapper MAPPER =
@@ -80,13 +79,12 @@ public String fixImports(String input) throws FormatterException {
 
     private ImmutableList<Replacement> getFormatReplacementsInternal(String input, Collection<Range<Integer>> ranges)
             throws IOException {
-        FormatterCliArgs command = FormatterCliArgs.builder()
-                .jdkPath(jdkPath)
-                .withJvmArgsForVersion(jdkMajorVersion)
-                .implementationClasspath(implementationClassPath)
-                .outputReplacements(true)
-                .characterRanges(ranges.stream().map(RangeUtils::toStringRange).collect(Collectors.toList()))
-                .build();
+        FormatterCliArgs command = new FormatterCliArgs(
+                jdkPath,
+                jvmArgsForVersion(jdkMajorVersion),
+                implementationClassPath,
+                /* outputReplacements= */ true,
+                ranges.stream().map(RangeUtils::toStringRange).collect(Collectors.toList()));
 
         @SuppressWarnings("for-rollout:NullAway")
         Optional<String> output =
@@ -98,43 +96,50 @@ private ImmutableList<Replacement> getFormatReplacementsInternal(String input, C
     }
 
     private String runFormatterCommand(String input) throws IOException {
-        FormatterCliArgs command = FormatterCliArgs.builder()
-                .jdkPath(jdkPath)
-                .withJvmArgsForVersion(jdkMajorVersion)
-                .implementationClasspath(implementationClassPath)
-                .outputReplacements(false)
-                .build();
+        FormatterCliArgs command = new FormatterCliArgs(
+                jdkPath,
+                jvmArgsForVersion(jdkMajorVersion),
+                implementationClassPath,
+                /* outputReplacements= */ false,
+                /* characterRanges= */ List.of());
         return FormatterCommandRunner.runWithStdin(command.toArgs(), input, Optional.ofNullable(jdkPath.getParent()))
                 .orElse(input);
     }
 
-    @Value.Immutable
-    interface FormatterCliArgs {
-        List<String> characterRanges();
-
-        boolean outputReplacements();
-
-        Path jdkPath();
-
-        List<Path> implementationClasspath();
+    private static List<String> jvmArgsForVersion(int majorJvmVersion) {
+        if (majorJvmVersion >= 16) {
+            return List.of(
+                    "--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");
+        }
+        return List.of();
+    }
 
-        List<String> jvmArgs();
+    record FormatterCliArgs(
+            Path jdkPath,
+            List<String> jvmArgs,
+            List<Path> implementationClasspath,
+            boolean outputReplacements,
+            List<String> characterRanges) {
 
-        default List<String> toArgs() {
+        List<String> toArgs() {
             ImmutableList.Builder<String> args = ImmutableList.<String>builder()
-                    .add(jdkPath().toAbsolutePath().toString())
-                    .addAll(jvmArgs())
+                    .add(jdkPath.toAbsolutePath().toString())
+                    .addAll(jvmArgs)
                     .add(
                             "-cp",
-                            implementationClasspath().stream()
+                            implementationClasspath.stream()
                                     .map(path -> path.toAbsolutePath().toString())
                                     .collect(Collectors.joining(System.getProperty("path.separator"))))
                     .add(FORMATTER_MAIN_CLASS);
 
-            if (!characterRanges().isEmpty()) {
-                args.add("--character-ranges", Joiner.on(',').join(characterRanges()));
+            if (!characterRanges.isEmpty()) {
+                args.add("--character-ranges", Joiner.on(',').join(characterRanges));
             }
-            if (outputReplacements()) {
+            if (outputReplacements) {
                 args.add("--output-replacements");
             }
 
@@ -143,23 +148,5 @@ default List<String> toArgs() {
                     .add("-")
                     .build();
         }
-
-        static Builder builder() {
-            return new Builder();
-        }
-
-        final class Builder extends ImmutableFormatterCliArgs.Builder {
-            Builder withJvmArgsForVersion(Integer majorJvmVersion) {
-                if (majorJvmVersion >= 16) {
-                    addJvmArgs(
-                            "--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");
-                }
-                return this;
-            }
-        }
     }
 }

open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/NativeImageFormatterService.java

@@ -32,7 +32,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
-import org.immutables.value.Value;
 
 public class NativeImageFormatterService implements FormatterService {
     private static final ObjectMapper MAPPER =
@@ -47,12 +46,10 @@ public NativeImageFormatterService(Path nativeImagePath) {
     public ImmutableList<Replacement> getFormatReplacements(String input, Collection<Range<Integer>> ranges) {
         Optional<String> output = Optional.empty();
         try {
-            FormatterNativeImageArgs command = FormatterNativeImageArgs.builder()
-                    .nativeImagePath(nativeImagePath)
-                    .outputReplacements(true)
-                    .characterRanges(
-                            ranges.stream().map(RangeUtils::toStringRange).collect(Collectors.toList()))
-                    .build();
+            FormatterNativeImageArgs command = new FormatterNativeImageArgs(
+                    nativeImagePath,
+                    /* outputReplacements= */ true,
+                    ranges.stream().map(RangeUtils::toStringRange).collect(Collectors.toList()));
 
             output = FormatterCommandRunner.runWithStdin(
                     command.toArgs(), input, Optional.ofNullable(nativeImagePath.getParent()));
@@ -85,32 +82,23 @@ public String fixImports(String input) {
     }
 
     private String runFormatterCommand(String input) throws IOException {
-        FormatterNativeImageArgs command = FormatterNativeImageArgs.builder()
-                .nativeImagePath(nativeImagePath)
-                .outputReplacements(false)
-                .build();
+        FormatterNativeImageArgs command = new FormatterNativeImageArgs(
+                nativeImagePath, /* outputReplacements= */ false, /* characterRanges= */ List.of());
         return FormatterCommandRunner.runWithStdin(
                         command.toArgs(), input, Optional.ofNullable(nativeImagePath.getParent()))
                 .orElse(input);
     }
 
-    @Value.Immutable
-    interface FormatterNativeImageArgs {
-
-        List<String> characterRanges();
-
-        boolean outputReplacements();
-
-        Path nativeImagePath();
+    record FormatterNativeImageArgs(Path nativeImagePath, boolean outputReplacements, List<String> characterRanges) {
 
-        default List<String> toArgs() {
+        List<String> toArgs() {
             ImmutableList.Builder<String> args = ImmutableList.<String>builder()
-                    .add(nativeImagePath().toAbsolutePath().toString());
+                    .add(nativeImagePath.toAbsolutePath().toString());
 
-            if (!characterRanges().isEmpty()) {
-                args.add("--character-ranges", Joiner.on(',').join(characterRanges()));
+            if (!characterRanges.isEmpty()) {
+                args.add("--character-ranges", Joiner.on(',').join(characterRanges));
             }
-            if (outputReplacements()) {
+            if (outputReplacements) {
                 args.add("--output-replacements");
             }
 
@@ -119,11 +107,5 @@ default List<String> toArgs() {
                     .add("-")
                     .build();
         }
-
-        static FormatterNativeImageArgs.Builder builder() {
-            return new FormatterNativeImageArgs.Builder();
-        }
-
-        final class Builder extends ImmutableFormatterNativeImageArgs.Builder {}
     }
 }

open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java

@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
+import com.google.errorprone.annotations.Immutable;
 import com.palantir.javaformat.Indent.Const;
 import com.palantir.javaformat.Input.Tok;
 import com.palantir.javaformat.doc.Break;
@@ -35,14 +36,9 @@
 import com.palantir.javaformat.java.FormatterDiagnostic;
 import com.palantir.javaformat.java.InputMetadata;
 import com.palantir.javaformat.java.InputMetadataBuilder;
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
-import org.immutables.value.Value;
 
 /** An {@code OpsBuilder} creates a list of {@link Op}s, which is turned into a {@link Doc} by {@link DocBuilder}. */
 public final class OpsBuilder {
@@ -86,6 +82,7 @@ public Integer actualStartColumn(int position) {
     }
 
     /** A request to add or remove a blank line in the output. */
+    @Immutable
     public abstract static class BlankLineWanted {
 
         /** Always emit a blank line. */
@@ -501,18 +498,7 @@ private static int getI(Input.Token token) {
 
     private static final NonBreakingSpace SPACE = NonBreakingSpace.make();
 
-    @Target(ElementType.TYPE)
-    @Retention(RetentionPolicy.SOURCE)
-    @Value.Style(overshadowImplementation = true)
-    @interface OpsOutputStyle {}
-
-    @OpsOutputStyle
-    @Value.Immutable
-    public interface OpsOutput {
-        ImmutableList<Op> ops();
-
-        InputMetadata inputMetadata();
-    }
+    public record OpsOutput(ImmutableList<Op> ops, InputMetadata inputMetadata) {}
 
     /** Build a list of {@link Op}s from the {@code OpsBuilder}. */
     public OpsOutput build() {
@@ -669,10 +655,7 @@ public OpsOutput build() {
                 afterForcedBreak = isForcedBreak(op);
             }
         }
-        return ImmutableOpsOutput.builder()
-                .ops(newOps.build())
-                .inputMetadata(inputMetadataBuilder.build())
-                .build();
+        return new OpsOutput(newOps.build(), inputMetadataBuilder.build());
     }
 
     private static boolean isNonNlsComment(Input.Tok tokAfter) {

open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java

@@ -96,7 +96,7 @@ public State computeBreaks(
         int column = lastLineStart == 0 ? state.column() + lastLineLength : lastLineLength;
         return state.withColumn(column)
                 .addNewLines(Iterators.size(Newlines.lineOffsetIterator(text)))
-                .withTokState(this, ImmutableTokState.of(text));
+                .withTokState(this, new State.TokState(text));
     }
 
     @Override

open-java-format/src/main/java/com/palantir/javaformat/doc/Level.java

@@ -36,10 +36,6 @@
 import com.palantir.javaformat.doc.Obs.ExplorationNode;
 import com.palantir.javaformat.doc.Obs.LevelNode;
 import com.palantir.javaformat.doc.StartsWithBreakVisitor.Result;
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -49,7 +45,6 @@
 import java.util.stream.Collector;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import org.immutables.value.Value;
 
 /** A {@code Level} inside a {@link Doc}. */
 public final class Level extends Doc {
@@ -127,7 +122,7 @@ protected Range<Integer> computeRange() {
     @Override
     public State computeBreaks(CommentsHelper commentsHelper, int maxWidth, State state, Obs.ExplorationNode observer) {
         return tryToFitOnOneLine(maxWidth, state, docs)
-                .map(newWidth -> state.withColumn(newWidth).withLevelState(this, ImmutableLevelState.of(true)))
+                .map(newWidth -> state.withColumn(newWidth).withLevelState(this, new State.LevelState(true)))
                 .orElseGet(() -> {
                     Obs.LevelNode childLevel = observer.newChildNode(this, state);
                     State newState =
@@ -631,19 +626,20 @@ private State tryToLayOutLevelOnOneLine(
     }
 
     private static SplitsBreaks splitByBreaks(List<Doc> docs) {
-        ImmutableSplitsBreaks.Builder builder = ImmutableSplitsBreaks.builder();
+        ImmutableList.Builder<ImmutableList<Doc>> splits = ImmutableList.builder();
+        ImmutableList.Builder<Break> breaks = ImmutableList.builder();
         ImmutableList.Builder<Doc> currentSplit = ImmutableList.builder();
         for (Doc doc : docs) {
             if (doc instanceof Break b) {
-                builder.addSplits(currentSplit.build());
+                splits.add(currentSplit.build());
                 currentSplit = ImmutableList.builder();
-                builder.addBreaks(b);
+                breaks.add(b);
             } else {
                 currentSplit.add(doc);
             }
         }
-        builder.addSplits(currentSplit.build());
-        return builder.build();
+        splits.add(currentSplit.build());
+        return new SplitsBreaks(splits.build(), breaks.build());
     }
 
     /** Compute breaks for a {@link Level} that spans multiple lines. */
@@ -837,18 +833,11 @@ public String toString() {
                 .toString();
     }
 
-    @Target(ElementType.TYPE)
-    @Retention(RetentionPolicy.SOURCE)
-    @Value.Style(overshadowImplementation = true)
-    @interface SplitsBreaksStyle {}
-
-    @SplitsBreaksStyle
-    @Value.Immutable
-    interface SplitsBreaks {
-        /** Groups of {@link Doc}s that are children of the current {@link Level}, separated by {@link Break}s. */
-        ImmutableList<ImmutableList<Doc>> splits();
-
-        /** {@link Break}s between {@link Doc}s in the current {@link Level}. */
-        ImmutableList<Break> breaks();
-    }
+    /**
+     * The children of the current {@link Level}, cut at its {@link Break}s.
+     *
+     * @param splits groups of {@link Doc}s that are children of the current level, separated by breaks
+     * @param breaks the breaks between those groups
+     */
+    record SplitsBreaks(ImmutableList<ImmutableList<Doc>> splits, ImmutableList<Break> breaks) {}
 }