From 29dab8c5feb00fdef7fd908b3000e4168963760d Mon Sep 17 00:00:00 2001 From: Andre Wachsmuth Date: Sun, 21 Jan 2024 16:15:16 +0100 Subject: [PATCH] Add option to CLI and IDEA plugin to format JavaDoc #724 * This adds an option to the IDEA plugin (checkbox) to format JavaDoc comments. It is disabled by default. * As the IDEA plugin (sometimes) uses the bootstrapping formatter service, which in turn uses the command line tool, this also add an option `--format-javadoc` to the command line tool. * To pass the options to the FormatterService loaded via service provider, I added a method `withOptions` to the service provided interface, which returns a new (immutable) instance with the updated options. Compared with the suggestion from the issue comment (https://github.com/palantir/palantir-java-format/issues/724#issuecomment-1109804294), this (a) does not require deprecating methods and (b) the options can be set independently of where the formatter is actually used. * Since the infrastructure to pass options to the formatter now exists, we could also add more options to the code style select configuration in the IDEA plugin, but I'm not sure if that makes sense (I want to use Palantir, after all). * The issue also mentioned API compatibility. The service provider interface is compatible, most other classes and methods are internal. But if somebody more familar with the code base could take another look, that would be great. * I also added a few tests for the new option. --- .../intellij/FormatterProvider.java | 31 ++++++++++--- .../InitialConfigurationStartupActivity.java | 1 + .../PalantirJavaFormatConfigurable.form | 31 ++++++++----- .../PalantirJavaFormatConfigurable.java | 43 +++++++++++++----- .../intellij/PalantirJavaFormatSettings.java | 19 ++++++++ .../BootstrappingFormatterService.java | 45 +++++++++++++++++-- .../BootstrappingFormatterServiceTest.java | 13 ++++++ .../src/test/resources/formatJavadoc.input | 16 +++++++ .../src/test/resources/formatJavadoc.output | 23 ++++++++++ .../javaformat/java/FormatterService.java | 19 ++++++++ .../javaformat/java/JavaFormatterOptions.java | 21 +++++++++ .../javaformat/java/CommandLineOptions.java | 15 +++++++ .../java/CommandLineOptionsParser.java | 4 ++ .../javaformat/java/FormatterServiceImpl.java | 16 ++++++- .../com/palantir/javaformat/java/Main.java | 1 + .../javaformat/java/UsageException.java | 2 + .../java/CommandLineOptionsParserTest.java | 10 ++++- .../palantir/javaformat/java/MainTest.java | 45 ++++++++++++++++++- 18 files changed, 321 insertions(+), 34 deletions(-) create mode 100644 palantir-java-format-jdk-bootstrap/src/test/resources/formatJavadoc.input create mode 100644 palantir-java-format-jdk-bootstrap/src/test/resources/formatJavadoc.output diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java index 18d3e0724..b247cb2cd 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java @@ -19,6 +19,7 @@ import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Functions; import com.google.common.collect.Iterables; import com.intellij.openapi.application.ApplicationInfo; import com.intellij.openapi.project.Project; @@ -29,6 +30,7 @@ import com.intellij.openapi.util.SystemInfo; import com.palantir.javaformat.bootstrap.BootstrappingFormatterService; import com.palantir.javaformat.java.FormatterService; +import com.palantir.javaformat.java.JavaFormatterOptions; import com.palantir.logsafe.Preconditions; import java.io.IOException; import java.net.MalformedURLException; @@ -59,7 +61,9 @@ FormatterService get(Project project, PalantirJavaFormatSettings settings) { project, getSdkVersion(project), settings.getImplementationClassPath(), - settings.injectedVersionIsOutdated())); + settings.injectedVersionIsOutdated(), + settings.getStyle(), + settings.isFormatJavadoc())); } private static FormatterService createFormatter(FormatterCacheKey cacheKey) { @@ -67,20 +71,26 @@ private static FormatterService createFormatter(FormatterCacheKey cacheKey) { List implementationClasspath = getImplementationUrls(cacheKey.implementationClassPath, cacheKey.useBundledImplementation); + JavaFormatterOptions options = JavaFormatterOptions.builder() + .formatJavadoc(cacheKey.formatJavadoc) + .style(cacheKey.style != null ? cacheKey.style : JavaFormatterOptions.Style.PALANTIR) + .build(); + // When running with JDK 15+ or using newer language features, we use the bootstrapping formatter which injects // required "--add-exports" args. if (useBootstrappingFormatter( jdkMajorVersion, ApplicationInfo.getInstance().getBuild())) { Path jdkPath = getJdkPath(cacheKey.project); log.info("Using bootstrapping formatter with jdk version {} and path: {}", jdkMajorVersion, jdkPath); - return new BootstrappingFormatterService(jdkPath, jdkMajorVersion, implementationClasspath); + return new BootstrappingFormatterService(jdkPath, jdkMajorVersion, implementationClasspath, options); } // Use "in-process" formatter service log.info("Using in-process formatter for jdk version {}", jdkMajorVersion); URL[] implementationUrls = toUrlsUnchecked(implementationClasspath); ClassLoader classLoader = new URLClassLoader(implementationUrls, FormatterService.class.getClassLoader()); - return Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class, classLoader)); + return Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class, classLoader)) + .withOptions(Functions.constant(options)); } /** @@ -188,16 +198,22 @@ private static final class FormatterCacheKey { private final int jdkMajorVersion; private final Optional> implementationClassPath; private final boolean useBundledImplementation; + private final boolean formatJavadoc; + private final JavaFormatterOptions.Style style; FormatterCacheKey( Project project, int jdkMajorVersion, Optional> implementationClassPath, - boolean useBundledImplementation) { + boolean useBundledImplementation, + JavaFormatterOptions.Style style, + boolean formatJavadoc) { this.project = project; this.jdkMajorVersion = jdkMajorVersion; this.implementationClassPath = implementationClassPath; this.useBundledImplementation = useBundledImplementation; + this.style = style; + this.formatJavadoc = formatJavadoc; } @Override @@ -212,12 +228,15 @@ public boolean equals(Object o) { return jdkMajorVersion == that.jdkMajorVersion && useBundledImplementation == that.useBundledImplementation && Objects.equals(project, that.project) - && Objects.equals(implementationClassPath, that.implementationClassPath); + && Objects.equals(implementationClassPath, that.implementationClassPath) + && Objects.equals(style, that.style) + && Objects.equals(formatJavadoc, that.formatJavadoc); } @Override public int hashCode() { - return Objects.hash(project, jdkMajorVersion, implementationClassPath, useBundledImplementation); + return Objects.hash( + project, jdkMajorVersion, implementationClassPath, useBundledImplementation, style, formatJavadoc); } } } diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/InitialConfigurationStartupActivity.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/InitialConfigurationStartupActivity.java index 941d1603f..2916a67fe 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/InitialConfigurationStartupActivity.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/InitialConfigurationStartupActivity.java @@ -26,6 +26,7 @@ public void runActivity(@NotNull Project project) { PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(project); if (settings.isUninitialized()) { settings.setEnabled(false); + settings.setFormatJavadoc(false); } } } diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.form b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.form index 9f05f8bb5..22951dafc 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.form +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.form @@ -1,6 +1,6 @@
- + @@ -8,22 +8,23 @@ - + - + - + + - + - + @@ -31,13 +32,13 @@ - + - + @@ -45,7 +46,7 @@ - + @@ -53,7 +54,7 @@ - + @@ -61,12 +62,20 @@ - + + + + + + + + + diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.java index 40e289cc7..2ddff6087 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurable.java @@ -25,21 +25,23 @@ import com.intellij.uiDesigner.core.GridLayoutManager; import com.intellij.uiDesigner.core.Spacer; import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.EnabledState; -import java.awt.Insets; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + import javax.annotation.Nullable; import javax.swing.JCheckBox; import javax.swing.JComboBox; import javax.swing.JComponent; import javax.swing.JLabel; import javax.swing.JPanel; -import org.jetbrains.annotations.Nls; -import org.jetbrains.annotations.NotNull; +import java.awt.Insets; class PalantirJavaFormatConfigurable extends BaseConfigurable implements SearchableConfigurable { private final Project project; private JPanel panel; private JCheckBox enable; + private JCheckBox formatJavadoc; private JComboBox styleComboBox; private JLabel formatterVersion; private JLabel pluginVersion; @@ -82,6 +84,7 @@ public JComponent createComponent() { public void apply() throws ConfigurationException { PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(project); settings.setEnabled(enable.isSelected() ? EnabledState.ENABLED : getDisabledState()); + settings.setFormatJavadoc(formatJavadoc.isSelected()); settings.setStyle(((UiFormatterStyle) styleComboBox.getSelectedItem()).convert()); } @@ -96,6 +99,7 @@ private EnabledState getDisabledState() { public void reset() { PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(project); enable.setSelected(settings.isEnabled()); + formatJavadoc.setSelected(settings.isFormatJavadoc()); styleComboBox.setSelectedItem(UiFormatterStyle.convert(settings.getStyle())); pluginVersion.setText(settings.getImplementationVersion().orElse("unknown")); formatterVersion.setText(getFormatterVersionText(settings)); @@ -105,6 +109,7 @@ public void reset() { public boolean isModified() { PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(project); return enable.isSelected() != settings.isEnabled() + || formatJavadoc.isSelected() != settings.isFormatJavadoc() || !styleComboBox.getSelectedItem().equals(UiFormatterStyle.convert(settings.getStyle())); } @@ -155,11 +160,29 @@ private void createUIComponents() { null, 0, false)); + formatJavadoc = new JCheckBox(); + formatJavadoc.setText("Format JavaDoc"); + panel.add( + formatJavadoc, + new GridConstraints( + 1, + 0, + 1, + 2, + GridConstraints.ANCHOR_WEST, + GridConstraints.FILL_NONE, + GridConstraints.SIZEPOLICY_CAN_SHRINK | GridConstraints.SIZEPOLICY_CAN_GROW, + GridConstraints.SIZEPOLICY_FIXED, + null, + null, + null, + 0, + false)); final Spacer spacer1 = new Spacer(); panel.add( spacer1, new GridConstraints( - 4, + 5, 0, 1, 2, @@ -177,7 +200,7 @@ private void createUIComponents() { panel.add( label1, new GridConstraints( - 1, + 2, 0, 1, 1, @@ -193,7 +216,7 @@ private void createUIComponents() { panel.add( styleComboBox, new GridConstraints( - 1, + 2, 1, 1, 1, @@ -211,7 +234,7 @@ private void createUIComponents() { panel.add( label2, new GridConstraints( - 3, + 4, 0, 1, 1, @@ -229,7 +252,7 @@ private void createUIComponents() { panel.add( formatterVersion, new GridConstraints( - 3, + 4, 1, 1, 1, @@ -247,7 +270,7 @@ private void createUIComponents() { panel.add( label3, new GridConstraints( - 2, + 3, 0, 1, 1, @@ -265,7 +288,7 @@ private void createUIComponents() { panel.add( pluginVersion, new GridConstraints( - 2, + 3, 1, 1, 1, diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatSettings.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatSettings.java index 4898ab596..4bf0956e4 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatSettings.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatSettings.java @@ -68,6 +68,14 @@ void setEnabled(EnabledState enabled) { state.enabled = enabled; } + boolean isFormatJavadoc() { + return state.formatJavadoc; + } + + void setFormatJavadoc(boolean formatJavadoc) { + state.formatJavadoc = formatJavadoc; + } + boolean isUninitialized() { return state.enabled.equals(EnabledState.UNKNOWN); } @@ -137,6 +145,7 @@ static class State { private EnabledState enabled = EnabledState.UNKNOWN; private Optional> implementationClassPath = Optional.empty(); + private boolean formatJavadoc = false; public JavaFormatterOptions.Style style = JavaFormatterOptions.Style.PALANTIR; public void setImplementationClassPath(@Nullable List value) { @@ -172,11 +181,21 @@ public String getEnabled() { } } + public boolean isFormatJavadoc() { + return formatJavadoc; + } + + public void setFormatJavadoc(boolean formatJavadoc) { + this.formatJavadoc = formatJavadoc; + } + @Override public String toString() { return "PalantirJavaFormatSettings{" + "enabled=" + enabled + + ", formatJavadoc=" + + formatJavadoc + ", formatterPath=" + implementationClassPath + ", style=" diff --git a/palantir-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java b/palantir-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java index 5fffc9113..0f75db99f 100644 --- a/palantir-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java +++ b/palantir-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java @@ -25,14 +25,18 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; import com.palantir.javaformat.java.FormatterService; +import com.palantir.javaformat.java.JavaFormatterOptions; import com.palantir.javaformat.java.Replacement; +import org.immutables.value.Value; + import java.io.IOException; import java.nio.file.Path; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.Optional; +import java.util.function.Function; import java.util.stream.Collectors; -import org.immutables.value.Value; public final class BootstrappingFormatterService implements FormatterService { private static final ObjectMapper MAPPER = @@ -44,10 +48,24 @@ public final class BootstrappingFormatterService implements FormatterService { private final Integer jdkMajorVersion; private final List implementationClassPath; + private final JavaFormatterOptions options; + public BootstrappingFormatterService(Path jdkPath, Integer jdkMajorVersion, List implementationClassPath) { + this( + jdkPath, + jdkMajorVersion, + implementationClassPath, + JavaFormatterOptions.builder() + .style(JavaFormatterOptions.Style.PALANTIR) + .build()); + } + + public BootstrappingFormatterService( + Path jdkPath, Integer jdkMajorVersion, List implementationClassPath, JavaFormatterOptions options) { this.jdkPath = jdkPath; this.jdkMajorVersion = jdkMajorVersion; this.implementationClassPath = implementationClassPath; + this.options = Objects.requireNonNull(options, "Formatter options are required"); } @Override @@ -68,6 +86,13 @@ public String formatSourceReflowStringsAndFixImports(String input) { } } + @Override + public FormatterService withOptions( + Function optionsTransformer) { + JavaFormatterOptions newOptions = optionsTransformer.apply(options); + return new BootstrappingFormatterService(jdkPath, jdkMajorVersion, implementationClassPath, newOptions); + } + private ImmutableList getFormatReplacementsInternal(String input, Collection> ranges) throws IOException { FormatterCliArgs command = FormatterCliArgs.builder() @@ -75,6 +100,8 @@ private ImmutableList getFormatReplacementsInternal(String input, C .withJvmArgsForVersion(jdkMajorVersion) .implementationClasspath(implementationClassPath) .outputReplacements(true) + .formatJavadoc(options.formatJavadoc()) + .style(options.style()) .characterRanges(ranges.stream() .map(BootstrappingFormatterService::toStringRange) .collect(Collectors.toList())) @@ -93,6 +120,8 @@ private String formatSourceReflowStringsAndFixImportsInternal(String input) thro .withJvmArgsForVersion(jdkMajorVersion) .implementationClasspath(implementationClassPath) .outputReplacements(false) + .formatJavadoc(options.formatJavadoc()) + .style(options.style()) .build(); return FormatterCommandRunner.runWithStdin(command.toArgs(), input).orElse(input); } @@ -119,6 +148,10 @@ interface FormatterCliArgs { boolean outputReplacements(); + boolean formatJavadoc(); + + JavaFormatterOptions.Style style(); + default List toArgs() { ImmutableList.Builder args = ImmutableList.builder() .add(jdkPath().toAbsolutePath().toString()) @@ -136,10 +169,16 @@ default List toArgs() { if (outputReplacements()) { args.add("--output-replacements"); } + if (formatJavadoc()) { + args.add("--format-javadoc"); + } + if (style() == JavaFormatterOptions.Style.AOSP) { + args.add("--aosp"); + } else if (style() == JavaFormatterOptions.Style.PALANTIR) { + args.add("--palantir"); + } return args - // Use palantir style - .add("--palantir") // Trailing "-" enables formatting stdin -> stdout .add("-") .build(); diff --git a/palantir-java-format-jdk-bootstrap/src/test/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterServiceTest.java b/palantir-java-format-jdk-bootstrap/src/test/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterServiceTest.java index 3986adeca..9d4f86b3b 100644 --- a/palantir-java-format-jdk-bootstrap/src/test/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterServiceTest.java +++ b/palantir-java-format-jdk-bootstrap/src/test/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterServiceTest.java @@ -22,6 +22,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; +import com.palantir.javaformat.java.FormatterException; import com.palantir.javaformat.java.Replacement; import java.net.URI; import java.nio.file.Files; @@ -45,6 +46,18 @@ void can_format_file_with_replacement() { assertThat(replacements.get(0).getReplacementString()).isEqualTo(expectedOutput); } + @Test + void can_format_file_with_format_javadoc() throws FormatterException { + String input = getTestResourceContent("formatJavadoc.input"); + String expectedOutput = getTestResourceContent("formatJavadoc.output"); + + String actualOutput = getFormatter() + .withOptions(o -> o.toBuilder().formatJavadoc(true).build()) + .formatSourceReflowStringsAndFixImports(input); + + assertThat(actualOutput).isEqualTo(expectedOutput); + } + @Test void can_format_full_file() { String input = getTestResourceContent("format.input"); diff --git a/palantir-java-format-jdk-bootstrap/src/test/resources/formatJavadoc.input b/palantir-java-format-jdk-bootstrap/src/test/resources/formatJavadoc.input new file mode 100644 index 000000000..8ead172ba --- /dev/null +++ b/palantir-java-format-jdk-bootstrap/src/test/resources/formatJavadoc.input @@ -0,0 +1,16 @@ +package com.google.googlejavaformat.java.test; + +/** + * Some long doc comment bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla + *
  • item1
  • item2
  • item3
+ */ +class B { + int x; private int y; public int z; + + void f() { + while (true != false) { + if (false == true) + break; + } + } +} diff --git a/palantir-java-format-jdk-bootstrap/src/test/resources/formatJavadoc.output b/palantir-java-format-jdk-bootstrap/src/test/resources/formatJavadoc.output new file mode 100644 index 000000000..b209289d3 --- /dev/null +++ b/palantir-java-format-jdk-bootstrap/src/test/resources/formatJavadoc.output @@ -0,0 +1,23 @@ +package com.google.googlejavaformat.java.test; + +/** + * Some long doc comment bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla + * bla bla bla bla bla bla bla bla bla bla bla bla + * + *
    + *
  • item1 + *
  • item2 + *
  • item3 + *
+ */ +class B { + int x; + private int y; + public int z; + + void f() { + while (true != false) { + if (false == true) break; + } + } +} diff --git a/palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterService.java b/palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterService.java index 538f0a983..b904f5880 100644 --- a/palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterService.java +++ b/palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterService.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; import java.util.Collection; +import java.util.function.Function; /** * A stable facade for palantir-java-format. The implementation must be ServiceLoaded, to ensure its classpath remains @@ -48,4 +49,22 @@ ImmutableList getFormatReplacements(String input, Collection */ String formatSourceReflowStringsAndFixImports(String input) throws FormatterException; + + /** + * Derives a new formatter service from this service with the given options. Note + * that formatter services are immutable, this instance can still be used with the + * old settings. + *

+ * Note: The default implementation exists for backwards compatibility and simply returns + * this instance. + * @param optionsTransformer An operator that is given the current set of options and returns the new set of + * options. You can either ignore the given options and build a new set of options from scratch, or use + * {@link JavaFormatterOptions.Builder#from(JavaFormatterOptions) JavaFormatterOptions.Builder.from()} to modify + * only some options. + * @return A formatter service that formats code with the given options. + */ + default FormatterService withOptions( + Function optionsTransformer) { + return this; + } } diff --git a/palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/JavaFormatterOptions.java b/palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/JavaFormatterOptions.java index e7be557be..6ec2a3cb8 100644 --- a/palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/JavaFormatterOptions.java +++ b/palantir-java-format-spi/src/main/java/com/palantir/javaformat/java/JavaFormatterOptions.java @@ -92,6 +92,15 @@ public static Builder builder() { return new Builder(); } + /** + * Creates a new builder derived from the settings of this instance. + * @return Returns a builder for {@link JavaFormatterOptions} with the defaults + * taken from these options. + */ + public Builder toBuilder() { + return Builder.from(this); + } + /** A builder for {@link JavaFormatterOptions}. */ public static final class Builder { // default is still GOOGLE just because lots of hand-rolled tests rely on this behaviour @@ -114,5 +123,17 @@ public Builder formatJavadoc(boolean formatJavadoc) { public JavaFormatterOptions build() { return new JavaFormatterOptions(style, formatJavadoc); } + + /** + * Creates a new builder with the defaults taken from the given options. + * @param options Options to use the default for the new builder. + * @return A new builder with the given defaults. + */ + public static Builder from(JavaFormatterOptions options) { + Builder builder = new Builder(); + builder.formatJavadoc = options.formatJavadoc; + builder.style = options.style; + return builder; + } } } diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java index 81defda5d..a3059e65d 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java @@ -40,6 +40,7 @@ final class CommandLineOptions { private final boolean fixImportsOnly; private final boolean sortImports; private final boolean removeUnusedImports; + private final boolean formatJavadoc; private final boolean dryRun; private final boolean setExitIfChanged; private final Optional assumeFilename; @@ -61,6 +62,7 @@ final class CommandLineOptions { boolean fixImportsOnly, boolean sortImports, boolean removeUnusedImports, + boolean formatJavadoc, boolean dryRun, boolean setExitIfChanged, Optional assumeFilename, @@ -80,6 +82,7 @@ final class CommandLineOptions { this.fixImportsOnly = fixImportsOnly; this.sortImports = sortImports; this.removeUnusedImports = removeUnusedImports; + this.formatJavadoc = formatJavadoc; this.dryRun = dryRun; this.setExitIfChanged = setExitIfChanged; this.assumeFilename = assumeFilename; @@ -157,6 +160,11 @@ boolean removeUnusedImports() { return removeUnusedImports; } + /** Format JavaDoc comments as well */ + boolean formatJavadoc() { + return formatJavadoc; + } + /** Print the paths of the files whose contents would change if the formatter were run normally. */ boolean dryRun() { return dryRun; @@ -205,6 +213,7 @@ static final class Builder { private boolean fixImportsOnly = false; private boolean sortImports = true; private boolean removeUnusedImports = true; + private boolean formatJavadoc = false; private boolean dryRun = false; private boolean setExitIfChanged = false; private Optional assumeFilename = Optional.empty(); @@ -280,6 +289,11 @@ Builder removeUnusedImports(boolean removeUnusedImports) { return this; } + Builder formatJavadoc(boolean formatJavadoc) { + this.formatJavadoc = formatJavadoc; + return this; + } + Builder dryRun(boolean dryRun) { this.dryRun = dryRun; return this; @@ -322,6 +336,7 @@ CommandLineOptions build() { fixImportsOnly, sortImports, removeUnusedImports, + formatJavadoc, dryRun, setExitIfChanged, assumeFilename, diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java index 8e04a1c7e..f42e1e944 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java @@ -115,6 +115,10 @@ static CommandLineOptions parse(Iterable options) { case "--skip-removing-unused-imports": optionsBuilder.removeUnusedImports(false); break; + case "-format-javadoc": + case "--format-javadoc": + optionsBuilder.formatJavadoc(true); + break; case "--skip-reflowing-long-strings": optionsBuilder.reflowLongStrings(false); break; diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/FormatterServiceImpl.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/FormatterServiceImpl.java index b544e0dbb..29924195b 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/FormatterServiceImpl.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/FormatterServiceImpl.java @@ -20,16 +20,21 @@ import com.google.common.collect.Range; import com.palantir.javaformat.java.JavaFormatterOptions.Style; import java.util.Collection; +import java.util.function.Function; @AutoService(FormatterService.class) public final class FormatterServiceImpl implements FormatterService { private final Formatter formatter; + private final JavaFormatterOptions options; public FormatterServiceImpl() { - JavaFormatterOptions options = - JavaFormatterOptions.builder().style(Style.PALANTIR).build(); + this(JavaFormatterOptions.builder().style(Style.PALANTIR).build()); + } + + private FormatterServiceImpl(JavaFormatterOptions options) { formatter = Formatter.createFormatter(options); + this.options = options; } @Override @@ -42,4 +47,11 @@ public ImmutableList getFormatReplacements(String text, Collection< public String formatSourceReflowStringsAndFixImports(String input) throws FormatterException { return formatter.formatSourceAndFixImports(input); } + + @Override + public FormatterService withOptions( + Function optionsTransformer) { + JavaFormatterOptions newOptions = optionsTransformer.apply(options); + return new FormatterServiceImpl(newOptions); + } } diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/Main.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/Main.java index c4a9067cf..cd65c57f3 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/Main.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/Main.java @@ -96,6 +96,7 @@ public int format(String... args) throws UsageException { // TODO(someone): update this to always use Style.PALANTIR JavaFormatterOptions options = JavaFormatterOptions.builder() .style(parameters.aosp() ? Style.AOSP : parameters.palantirStyle() ? Style.PALANTIR : Style.GOOGLE) + .formatJavadoc(parameters.formatJavadoc()) .build(); if (parameters.stdin()) { diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/UsageException.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/UsageException.java index 4ed72b46c..064441d30 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/UsageException.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/UsageException.java @@ -50,6 +50,8 @@ final class UsageException extends Exception { " Do not remove unused imports. Imports will still be sorted.", " . --skip-reflowing-long-strings", " Do not reflow string literals that exceed the column limit.", + " . --format-javadoc", + " Also format JavaDoc comments.", " --dry-run, -n", " Prints the paths of the files whose contents would change if the formatter were run" + " normally.", " --set-exit-if-changed", diff --git a/palantir-java-format/src/test/java/com/palantir/javaformat/java/CommandLineOptionsParserTest.java b/palantir-java-format/src/test/java/com/palantir/javaformat/java/CommandLineOptionsParserTest.java index 37fc33140..8a12afd6c 100644 --- a/palantir-java-format/src/test/java/com/palantir/javaformat/java/CommandLineOptionsParserTest.java +++ b/palantir-java-format/src/test/java/com/palantir/javaformat/java/CommandLineOptionsParserTest.java @@ -15,7 +15,6 @@ package com.palantir.javaformat.java; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth8.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.fail; @@ -135,6 +134,15 @@ public void skipRemovingUnusedImports() { .isFalse(); } + @Test + public void formatJavadoc() { + assertThat(CommandLineOptionsParser.parse(Arrays.asList()).formatJavadoc()) + .isFalse(); + assertThat(CommandLineOptionsParser.parse(Arrays.asList("--format-javadoc")) + .formatJavadoc()) + .isTrue(); + } + @Test public void dryRun() { assertThat(CommandLineOptionsParser.parse(Arrays.asList("--dry-run")).dryRun()) diff --git a/palantir-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java b/palantir-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java index f97717ca3..b85bc6871 100644 --- a/palantir-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java +++ b/palantir-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java @@ -119,7 +119,7 @@ public void testMain() throws Exception { // end to end javadoc formatting test @Test - public void javadoc() throws Exception { + public void javadocFormattingDisabled() throws Exception { String[] input = { "/**", " * graph", @@ -163,6 +163,49 @@ public void javadoc() throws Exception { assertThat(out.toString()).isEqualTo(joiner.join(expected)); } + @Test + public void javadocFormattingEnabled() throws Exception { + String[] input = { + "/**", + " * graph", + " *", + " * graph", + " *", + " * @param foo lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do" + + " eiusmod tempor incididunt ut labore et dolore magna aliqua", + " */", + "class Test {", + " /**", + " * creates entropy", + " */", + " public static void main(String... args) {}", + "}", + }; + String[] expected = { + "/**", + " * graph", + " *", + " *

graph", + " *", + " * @param foo lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor", + " * incididunt ut labore et dolore magna aliqua", + " */", + "class Test {", + " /** creates entropy */", + " public static void main(String... args) {}", + "}", + "", + }; + InputStream in = new ByteArrayInputStream(joiner.join(input).getBytes(UTF_8)); + StringWriter out = new StringWriter(); + Main main = new Main( + new PrintWriter(out, true), + new PrintWriter(new BufferedWriter(new OutputStreamWriter(System.err, UTF_8)), true), + in); + assertThat(main.format("--format-javadoc", "-")).isEqualTo(0); + assertThat(out.toString()).isEqualTo(joiner.join(expected)); + } + // end to end import fixing test @Test public void imports() throws Exception {