From 0e13e32e235382233c71d0293caa84729ca36494 Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Fri, 2 Oct 2026 21:46:57 +0300 Subject: [PATCH 1/2] Use records for the bootstrap's command-line arguments FormatterCliArgs and FormatterNativeImageArgs only gathered the arguments of a formatter process and turned them into its command line; Immutables generated a builder for each. They are records now, built through their constructors, and jvmArgsForVersion does what the builder's withJvmArgsForVersion did. open-java-format-jdk-bootstrap no longer runs the Immutables processor. FormatterServicesTest, which runs both services against a JDK and the native image, passes with -PnativeImage=true. --- open-java-format-jdk-bootstrap/build.gradle | 8 -- .../BootstrappingFormatterService.java | 85 ++++++++----------- .../NativeImageFormatterService.java | 42 +++------ 3 files changed, 48 insertions(+), 87 deletions(-) diff --git a/open-java-format-jdk-bootstrap/build.gradle b/open-java-format-jdk-bootstrap/build.gradle index e19bff8d8..fc516204a 100644 --- a/open-java-format-jdk-bootstrap/build.gradle +++ b/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) diff --git a/open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java b/open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java index 0d44b82c5..e46312c6b 100644 --- a/open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java +++ b/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 getFormatReplacementsInternal(String input, Collection> 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 output = @@ -98,43 +96,50 @@ private ImmutableList 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 characterRanges(); - - boolean outputReplacements(); - - Path jdkPath(); - - List implementationClasspath(); + private static List 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 jvmArgs(); + record FormatterCliArgs( + Path jdkPath, + List jvmArgs, + List implementationClasspath, + boolean outputReplacements, + List characterRanges) { - default List toArgs() { + List toArgs() { ImmutableList.Builder args = ImmutableList.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 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; - } - } } } diff --git a/open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/NativeImageFormatterService.java b/open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/NativeImageFormatterService.java index d852fb025..bfaa5607b 100644 --- a/open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/NativeImageFormatterService.java +++ b/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 getFormatReplacements(String input, Collection> ranges) { Optional 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 characterRanges(); - - boolean outputReplacements(); - - Path nativeImagePath(); + record FormatterNativeImageArgs(Path nativeImagePath, boolean outputReplacements, List characterRanges) { - default List toArgs() { + List toArgs() { ImmutableList.Builder args = ImmutableList.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 toArgs() { .add("-") .build(); } - - static FormatterNativeImageArgs.Builder builder() { - return new FormatterNativeImageArgs.Builder(); - } - - final class Builder extends ImmutableFormatterNativeImageArgs.Builder {} } } From 90d3a135672ad9a80835a828a03053d87ad9db79 Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Fri, 2 Oct 2026 21:46:57 +0300 Subject: [PATCH 2/2] Use records for the formatter's plain value types Six of the types Immutables generated in open-java-format were plain values: State's BreakState, LevelState and TokState, Level's SplitsBreaks, OpsBuilder's OpsOutput and InputMetadata. They are records now. The call sites use their constructors instead of builders and of(...) factories, and the accessors keep their names. InputMetadata keeps @Immutable, which Error Prone now checks against the record's components instead of skipping a generated class. That needed BlankLineWanted to say it is immutable as well, and it is: its two subclasses hold an Optional and an ImmutableList. OpenOp, Break and State stay on Immutables. OpenOp and Break extend HasUniqueId, whose per-instance id orders the formatter's persistent collections, and a record cannot extend a class. State copies itself through its builder in a dozen places. The 15,747 files of the JDK 21 sources format exactly as before. --- .../com/palantir/javaformat/OpsBuilder.java | 25 ++------- .../com/palantir/javaformat/doc/Comment.java | 2 +- .../com/palantir/javaformat/doc/Level.java | 39 +++++-------- .../com/palantir/javaformat/doc/State.java | 55 ++++--------------- .../javaformat/java/InputMetadata.java | 26 +++------ .../javaformat/java/InputMetadataBuilder.java | 5 +- 6 files changed, 37 insertions(+), 115 deletions(-) diff --git a/open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java b/open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java index f61321788..d8d04b160 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java +++ b/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 ops(); - - InputMetadata inputMetadata(); - } + public record OpsOutput(ImmutableList 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) { diff --git a/open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java b/open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java index 124551df6..3750f74cf 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java +++ b/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 diff --git a/open-java-format/src/main/java/com/palantir/javaformat/doc/Level.java b/open-java-format/src/main/java/com/palantir/javaformat/doc/Level.java index 90f9df610..4f613f9ed 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/doc/Level.java +++ b/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 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 docs) { - ImmutableSplitsBreaks.Builder builder = ImmutableSplitsBreaks.builder(); + ImmutableList.Builder> splits = ImmutableList.builder(); + ImmutableList.Builder breaks = ImmutableList.builder(); ImmutableList.Builder 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> splits(); - - /** {@link Break}s between {@link Doc}s in the current {@link Level}. */ - ImmutableList 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> splits, ImmutableList breaks) {} } diff --git a/open-java-format/src/main/java/com/palantir/javaformat/doc/State.java b/open-java-format/src/main/java/com/palantir/javaformat/doc/State.java index e0fc41c75..d5b53f13b 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/doc/State.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/doc/State.java @@ -22,13 +22,8 @@ import com.palantir.javaformat.Indent; import fj.data.Set; import fj.data.TreeMap; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; import java.util.Objects; import org.immutables.value.Value; -import org.immutables.value.Value.Parameter; /** State for writing. */ // Automatically suppressed to unblock enforcement in new code @@ -95,7 +90,7 @@ public static State startingState() { } public BreakState getBreakState(Break brk) { - return breakStates().get(brk).orSome(ImmutableBreakState.of(false, -1)); + return breakStates().get(brk).orSome(new BreakState(false, -1)); } public boolean wasBreakTaken(BreakTag breakTag) { @@ -163,7 +158,7 @@ State withBreak(Break brk, boolean broken) { .lastIndent(indent()) .column(newColumn) .numLines(numLines() + 1) - .breakStates(breakStates().set(brk, ImmutableBreakState.of(true, newColumn))) + .breakStates(breakStates().set(brk, new BreakState(true, newColumn))) .build(); } else { return builder.column(column() + brk.getFlat().length()).build(); @@ -249,44 +244,14 @@ public static Builder builder() { return new Builder(); } - @Target(ElementType.TYPE) - @Retention(RetentionPolicy.SOURCE) - @Value.Style(overshadowImplementation = true) - @interface BreakStateStyle {} + record BreakState(boolean broken, int newIndent) {} - @BreakStateStyle - @Value.Immutable - @JsonSerialize(as = ImmutableBreakState.class) - interface BreakState { - @Parameter - boolean broken(); - - @Parameter - int newIndent(); - } - - @Target(ElementType.TYPE) - @Retention(RetentionPolicy.SOURCE) - @Value.Style(overshadowImplementation = true) - @interface LevelStateStyle {} - - @LevelStateStyle - @Value.Immutable - interface LevelState { - /** True if the entire {@link Level} fits on one line. */ - @Parameter - boolean oneLine(); - } - - @Target(ElementType.TYPE) - @Retention(RetentionPolicy.SOURCE) - @Value.Style(overshadowImplementation = true) - @interface TokStateStyle {} + /** + * How a {@link Level} was laid out. + * + * @param oneLine true if the entire level fits on one line + */ + record LevelState(boolean oneLine) {} - @TokStateStyle - @Value.Immutable - interface TokState { - @Parameter - String text(); - } + record TokState(String text) {} } diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadata.java b/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadata.java index 815786b81..feeb1307c 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadata.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadata.java @@ -19,28 +19,16 @@ import com.google.common.collect.ImmutableRangeSet; import com.google.errorprone.annotations.Immutable; import com.palantir.javaformat.OpsBuilder.BlankLineWanted; -import org.immutables.value.Value; -import org.immutables.value.Value.Default; /** * Records metadata about the input, namely existing blank lines that we might want to preserve, as well as what ranges * can be partially formatted. + * + * @param blankLines remembers preferences from the input about whether blank lines are wanted or not at a given token + * index + * @param partialFormatRanges marks regions that can be partially formatted, used to determine the actual ranges that + * will be formatted when ranges are requested */ -// Automatically suppressed to unblock enforcement in new code -@SuppressWarnings("ImmutablesStyle") @Immutable -@Value.Immutable -@Value.Style(overshadowImplementation = true) -public interface InputMetadata { - /** Remembers preferences from the input about whether blank lines are wanted or not at a given token index. */ - ImmutableMap blankLines(); - - /** - * Marks regions that can be partially formatted, used to determine the actual ranges that will be formatted when - * ranges are requested. - */ - @Default - default ImmutableRangeSet partialFormatRanges() { - return ImmutableRangeSet.of(); - } -} +public record InputMetadata( + ImmutableMap blankLines, ImmutableRangeSet partialFormatRanges) {} diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java b/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java index b5d281ad7..ef0669f3c 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java @@ -51,9 +51,6 @@ public void markForPartialFormat(Input.Token start, Input.Token end) { } public InputMetadata build() { - return ImmutableInputMetadata.builder() - .blankLines(ImmutableMap.copyOf(blankLines)) - .partialFormatRanges(ImmutableRangeSet.copyOf(partialFormatRanges)) - .build(); + return new InputMetadata(ImmutableMap.copyOf(blankLines), ImmutableRangeSet.copyOf(partialFormatRanges)); } }