Merge pull request #160 from openjavaformat/test/formatter-cases

Run formatter test cases from files under test-cases/

Author: abashev

.gitattributes

@@ -0,0 +1,2 @@
+# A formatter test case is compared byte for byte, line breaks included, so git must not convert them.
+*.case -text

open-java-format-benchmarks/build.gradle

@@ -32,7 +32,7 @@ tasks.named('jmh', JMHTask.class) {
     environment.put('JARS_CLASSPATH', configurations.named('formatterNativeJars').map { conf -> conf.collect { it.absolutePath }.join(":") })
 }
 
-// src/test/resources/PalantirDeeplyNestedCalls.java is a symlink to a formatter test input, which has to
+// src/test/resources/PalantirDeeplyNestedCalls.java is a copy of a formatter test input, which has to
 // keep its unformatted shape: leave src/test/resources out of any formatting step added to this build.
 
 jmhCompileGeneratedClasses {

open-java-format-benchmarks/src/test/resources/PalantirDeeplyNestedCalls.java

@@ -0,0 +1,93 @@
+class PalantirDeeplyNestedCalls {
+    private static MagicConfigV1 testSheetV1 = MagicConfigV1.builder()
+            .foooos(ImmutableList.of(testFieldOneV1))
+            .baaars(ImmutableList.of(MagicConfigV1.builder()
+                    .foooos(ImmutableList.of(testFieldTwoV1, testFieldThreeV1))
+                    .baaars(ImmutableList.of(MagicConfigV1.builder()
+                            .baaars(ImmutableList.of(MagicConfigV1.builder()
+                                    .baaars(ImmutableList.of(MagicConfigV1.builder()
+                                            .foooos(ImmutableList.of(testFieldFourV1))
+                                            .build()))
+                                    .build()))
+                            .build()))
+                    .build()))
+            .build();
+
+    void testMethod() {
+        assertThat(ConfigServiceV1.getAllItems(item1, item2, item3, item4))
+                .containsExactlyInAnyOrderElementsOf(Set.of(
+                        new ItemWithCustomLocator(
+                                ItemConfigV1.DEFAULT_CONFIG.toCustomString(),
+                                ItemConfigV1.createCustomConfig(ItemConfigV1Locator.of(TEST_ITEM_A))),
+                        new ItemWithCustomLocator(
+                                ItemConfigV1.builder()
+                                        .id(getId(TEST_ITEM_A))
+                                        .name(ItemName.of(TEST_API_NAME.get()))
+                                        .apiName(TEST_API_NAME)
+                                        .spec(ItemConfigV1Spec.native_(NativeItemConfigV1Spec.model(TestModel.of())))
+                                        .category(TestCatalog.TEST_CATEGORY_ALPHA)
+                                        .creator(TestCatalog.TEST_CREATOR_BETA)
+                                        .status(ItemLifecycleStatus.EXPERIMENTAL)
+                                        .statusV2(ItemLifecycleStatusV2.experimental(LifecycleStatusExperimental.of()))
+                                        .inputSpec(ItemInputSpec.unknownInputType(UnknownInputSpec.of()))
+                                        .creationDate(ItemMetadataInfo.builder().build())
+                                        .performanceAttributes(
+                                                ItemAttributes.builder().build())
+                                        .categories(TestCatalog.TEST_CATEGORY_ALPHA)
+                                        .build(),
+                                CustomItemLocator.hosted(HostedItemLocator.of(TEST_ITEM_B))),
+                        new ItemWithCustomLocator(
+                                ItemConfigV1.builder()
+                                        .id(getId(TestModelCatalog.MODEL_XYZ_123.apiName()))
+                                        .name(ItemName.of(TestModelCatalog.MODEL_XYZ_123
+                                                .apiName()
+                                                .get()))
+                                        .apiName(TestModelCatalog.MODEL_XYZ_123.apiName())
+                                        .spec(ItemConfigV1Spec.native_(
+                                                NativeItemConfigV1Spec.hubModel(HubModelSpec.of())))
+                                        .category(TestCatalog.TEST_CATEGORY_ALPHA)
+                                        .creator(TestCatalog.TEST_CREATOR_BETA)
+                                        .status(ItemLifecycleStatus.EXPERIMENTAL)
+                                        .statusV2(ItemLifecycleStatusV2.experimental(LifecycleStatusExperimental.of()))
+                                        .inputSpec(ItemInputSpec.unknownInputType(UnknownInputSpec.of()))
+                                        .creationDate(ItemMetadataInfo.builder().build())
+                                        .performanceAttributes(
+                                                ItemAttributes.builder().build())
+                                        .categories(TestCatalog.TEST_CATEGORY_ALPHA)
+                                        .build(),
+                                CustomItemLocator.hosted(HostedItemLocator.of(TEST_ITEM_C))),
+                        // Discovered items
+                        new ItemWithCustomLocator(
+                                TestModelCatalog.MODEL_ABC_456.toItemApi(),
+                                CustomItemLocator.discovered(DiscoveredItemLocator.of())),
+                        new ItemWithCustomLocator(
+                                ItemConfigV1.builder()
+                                        .id(getId(TEST_API_NAME))
+                                        .name(ItemName.of(TEST_API_NAME.get()))
+                                        .apiName(TEST_API_NAME)
+                                        .spec(ItemConfigV1Spec.native_(
+                                                NativeItemConfigV1Spec.hubModel(HubModelSpec.of())))
+                                        .category(TestCatalog.TEST_CATEGORY_ALPHA)
+                                        .creator(TestCatalog.TEST_CREATOR_BETA)
+                                        .status(ItemLifecycleStatus.EXPERIMENTAL)
+                                        .statusV2(ItemLifecycleStatusV2.experimental(LifecycleStatusExperimental.of()))
+                                        .inputSpec(ItemInputSpec.unknownInputType(UnknownInputSpec.of()))
+                                        .creationDate(ItemMetadataInfo.builder().build())
+                                        .performanceAttributes(
+                                                ItemAttributes.builder().build())
+                                        .categories(TestCatalog.TEST_CATEGORY_ALPHA)
+                                        .build(),
+                                CustomItemLocator.discovered(DiscoveredItemLocator.of()))));
+    }
+
+    public Map<FooBarRid, BazQuxMetrics> otherMethod(
+            FooAuthHeader authHeader, Set<FooBarRid> thingRids, WidgetTimeRange timeRange) {
+        return myCustomService
+                .get()
+                .fetchData(authHeader, FooMyCustomClass.of(thingRids, timeRange))
+                .entrySet()
+                .stream()
+                .collect(Collectors.toMap(
+                        Map.Entry::getKey, entry -> entry.getValue().getMetrics()));
+    }
+}

open-java-format-benchmarks/src/test/resources/PalantirDeeplyNestedCalls.java

@@ -82,10 +82,8 @@ void tearDown() throws Exception {
 
     @Test
     void defaultFormatSettings() throws Exception {
-        String input = Files.readString(
-                Paths.get("../open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/A.input"));
-        String output = Files.readString(
-                Path.of("../open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/A.output"));
+        String input = Files.readString(Paths.get("src/test/resources/A.input"));
+        String output = Files.readString(Path.of("src/test/resources/A.output"));
         PsiFile file = createPsiFile("com/foo/FormatTest.java", input);
         ReformatCodeProcessor processor = new ReformatCodeProcessor(file, false);
         WriteCommandAction.runWriteCommandAction(file.getProject(), () -> {

open-java-format-idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingServiceTest.java

@@ -566,8 +566,8 @@ case BreakOnlyIfInnerLevelsThenFitOnOneLine(boolean keepIndentWhenInlined) -> {
                             // levels that are meant to look good even if partially inlined, e.g. method
                             // chains, which will catch things like builders, but not other kinds of levels like
                             // constant expressions.
-                            // See the palantir-expression-lambdas.input test for an example of what this is
-                            // trying to avoid.
+                            // See the palantir-expression-lambdas cases in test-cases/testdata for an example of
+                            // what this is trying to avoid.
 
                             // For this, need to actually check the last inner level of `lastLevel` (2 levels down).
                             if (innerLevel.docs.isEmpty() || !(getLast(innerLevel.docs) instanceof Level)) {

…alantir/javaformat/java/testdata/A.input …t-idea-plugin/src/test/resources/A.inputopen-java-format/src/test/resources/com/palantir/javaformat/java/testdata/A.input renamed to open-java-format-idea-plugin/src/test/resources/A.input open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/A.input renamed to open-java-format-idea-plugin/src/test/resources/A.input

@@ -108,6 +108,11 @@ public static Formatter createFormatter(JavaFormatterOptions options) {
         return new Formatter(options, false);
     }
 
+    /** The options this formatter formats with. */
+    JavaFormatterOptions options() {
+        return options;
+    }
+
     /**
      * Construct a {@code Formatter} given a Java compilation unit. Parses the code; builds a {@link JavaInput} and the
      * corresponding {@link JavaOutput}.

…lantir/javaformat/java/testdata/A.output …-idea-plugin/src/test/resources/A.outputopen-java-format/src/test/resources/com/palantir/javaformat/java/testdata/A.output renamed to open-java-format-idea-plugin/src/test/resources/A.output open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/A.output renamed to open-java-format-idea-plugin/src/test/resources/A.output

@@ -82,15 +82,18 @@ private static String wrapOnce(final int columnLimit, String input, Formatter fo
             return input;
         }
 
-        TreeRangeMap<Integer, String> replacements = getReflowReplacements(columnLimit, input);
+        // The pieces continue the way the formatter continues a chain of +: four columns times the multiplier of its
+        // style, eight in OJF.
+        var continuationIndent = 4 * formatter.options().indentationMultiplier();
+        TreeRangeMap<Integer, String> replacements = getReflowReplacements(columnLimit, continuationIndent, input);
         String firstPass =
                 formatter.formatSource(input, replacements.asMapOfRanges().keySet());
 
         if (!firstPass.equals(input)) {
             // If formatting the replacement ranges resulted in a change, recalculate the replacements on
             // the updated input.
             input = firstPass;
-            replacements = getReflowReplacements(columnLimit, input);
+            replacements = getReflowReplacements(columnLimit, continuationIndent, input);
         }
 
         String result = applyReplacements(input, replacements);
@@ -99,7 +102,7 @@ private static String wrapOnce(final int columnLimit, String input, Formatter fo
         String secondPass = formatter.formatSource(result, rangesAfterAppliedReplacements(replacements));
 
         if (!secondPass.equals(result)) {
-            replacements = getReflowReplacements(columnLimit, secondPass);
+            replacements = getReflowReplacements(columnLimit, continuationIndent, secondPass);
             result = applyReplacements(secondPass, replacements);
         }
 
@@ -141,21 +144,23 @@ private static ImmutableSet<Range<Integer>> rangesAfterAppliedReplacements(
         return outputRanges.build();
     }
 
-    public static TreeRangeMap<Integer, String> getReflowReplacements(int columnLimit, final String input)
-            throws FormatterException {
-        return new Reflower(columnLimit, input).getReflowReplacements();
+    public static TreeRangeMap<Integer, String> getReflowReplacements(
+            int columnLimit, int continuationIndent, final String input) throws FormatterException {
+        return new Reflower(columnLimit, continuationIndent, input).getReflowReplacements();
     }
 
     private static class Reflower {
 
         private final String input;
         private final int columnLimit;
+        private final int continuationIndent;
         private final String separator;
         private final JCTree.JCCompilationUnit unit;
         private final Position.LineMap lineMap;
 
-        Reflower(int columnLimit, String input) throws FormatterException {
+        Reflower(int columnLimit, int continuationIndent, String input) throws FormatterException {
             this.columnLimit = columnLimit;
+            this.continuationIndent = continuationIndent;
             this.input = input;
             this.separator = Newlines.guessLineSeparator(input);
             this.unit = parse(input, /* allowStringFolding= */ false);
@@ -418,7 +423,16 @@ private void wrapLongStrings(
 
                 // Get the original source text of the string literals, excluding `"` and `+`.
                 ImmutableList<String> components = stringComponents(input, unit, flat);
-                var plain = reflow(columnLimit, trailing, components, first.get(), startColumn, fistLineCol, "", "");
+                var plain = reflow(
+                        columnLimit,
+                        continuationIndent,
+                        trailing,
+                        components,
+                        first.get(),
+                        startColumn,
+                        fistLineCol,
+                        "",
+                        "");
                 chains.add(new Chain(
                         flat,
                         first.get(),
@@ -443,6 +457,7 @@ private void wrapLongStrings(
                 var components = stringComponents(input, unit, chain.flat());
                 var pieces = reflow(
                         columnLimit,
+                        continuationIndent,
                         markers.tail(2).length(),
                         components,
                         chain.firstInChain(),
@@ -460,7 +475,7 @@ private String join(Chain chain, List<String> pieces, String firstLineMarkers, S
             return render(
                     separator,
                     pieces,
-                    chain.firstInChain() ? chain.firstLineColumn() + 4 : chain.startColumn() - 2,
+                    chain.firstInChain() ? chain.firstLineColumn() + continuationIndent : chain.startColumn() - 2,
                     firstLineMarkers,
                     otherLineMarkers);
         }
@@ -680,6 +695,8 @@ private static boolean isEscaped(String input, int idx) {
      * {@link #render}.
      *
      * @param columnLimit the number of columns to wrap at
+     * @param continuationIndent how far the lines after the first are indented from the start of the first, before
+     * their {@code + }, when the text begins its chain
      * @param trailing extra space to leave after the last line
      * @param components the text to reflow
      * @param first0 true if the text includes the beginning of its enclosing concat chain, i.e. a
@@ -692,6 +709,7 @@ private static boolean isEscaped(String input, int idx) {
      */
     private static List<String> reflow(
             int columnLimit,
+            int continuationIndent,
             int trailing,
             ImmutableList<String> components,
             boolean first0,
@@ -727,8 +745,8 @@ private static List<String> reflow(
             // add the split line to the output, and process whatever's left
             lines.add(String.join("", line));
             if (first) {
-                // Subsequent lines have a four-space continuation indent and a `+ `.
-                width -= 6;
+                // Subsequent lines have a continuation indent and a `+ `.
+                width -= continuationIndent + 2;
                 // Also, switch to firstLineStartColumn in order to account for the fact that continuations
                 // should get indented from the beginning of the first line.
                 // This is to handle cases like: