Skip to content

Commit

Permalink
Add option to CLI and IDEA plugin to format JavaDoc palantir#724
Browse files Browse the repository at this point in the history
* 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
  (palantir#724 (comment)),
  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.
  • Loading branch information
awa-xima committed Jan 21, 2024
1 parent bb4f09d commit 29dab8c
Show file tree
Hide file tree
Showing 18 changed files with 321 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -59,28 +61,36 @@ 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) {
int jdkMajorVersion = cacheKey.jdkMajorVersion;
List<Path> 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));
}

/**
Expand Down Expand Up @@ -188,16 +198,22 @@ private static final class FormatterCacheKey {
private final int jdkMajorVersion;
private final Optional<List<URI>> implementationClassPath;
private final boolean useBundledImplementation;
private final boolean formatJavadoc;
private final JavaFormatterOptions.Style style;

FormatterCacheKey(
Project project,
int jdkMajorVersion,
Optional<List<URI>> 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
Expand All @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public void runActivity(@NotNull Project project) {
PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(project);
if (settings.isUninitialized()) {
settings.setEnabled(false);
settings.setFormatJavadoc(false);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,72 +1,81 @@
<?xml version="1.0" encoding="UTF-8"?>
<form xmlns="http://www.intellij.com/uidesigner/form/" version="1" bind-to-class="com.palantir.javaformat.intellij.PalantirJavaFormatConfigurable">
<grid id="27dc6" binding="panel" layout-manager="GridLayoutManager" row-count="5" column-count="2" same-size-horizontally="false" same-size-vertically="false" hgap="-1" vgap="-1">
<grid id="27dc6" binding="panel" layout-manager="GridLayoutManager" row-count="6" column-count="2" same-size-horizontally="false" same-size-vertically="false" hgap="-1" vgap="-1">
<margin top="0" left="0" bottom="0" right="0"/>
<constraints>
<xy x="20" y="20" width="500" height="400"/>
</constraints>
<properties/>
<border type="none"/>
<children>
<component id="4a87f" class="javax.swing.JCheckBox" binding="enable" default-binding="true">
<component id="f9d77" class="javax.swing.JCheckBox" binding="formatJavadoc" default-binding="true">
<constraints>
<grid row="0" column="0" row-span="1" col-span="2" vsize-policy="0" hsize-policy="3" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
<grid row="1" column="0" row-span="1" col-span="2" vsize-policy="0" hsize-policy="3" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
</constraints>
<properties>
<text value="Enable palantir-java-format"/>
<hideActionText value="false"/>
<text value="Format JavaDoc"/>
</properties>
</component>
<vspacer id="19e83">
<constraints>
<grid row="4" column="0" row-span="1" col-span="2" vsize-policy="6" hsize-policy="1" anchor="0" fill="2" indent="0" use-parent-layout="false"/>
<grid row="5" column="0" row-span="1" col-span="2" vsize-policy="6" hsize-policy="1" anchor="0" fill="2" indent="0" use-parent-layout="false"/>
</constraints>
</vspacer>
<component id="c93e1" class="javax.swing.JLabel">
<constraints>
<grid row="1" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
<grid row="2" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
</constraints>
<properties>
<text value="Code style"/>
</properties>
</component>
<component id="31761" class="javax.swing.JComboBox" binding="styleComboBox" custom-create="true">
<constraints>
<grid row="1" column="1" row-span="1" col-span="1" vsize-policy="0" hsize-policy="2" anchor="8" fill="1" indent="1" use-parent-layout="false"/>
<grid row="2" column="1" row-span="1" col-span="1" vsize-policy="0" hsize-policy="2" anchor="8" fill="1" indent="1" use-parent-layout="false"/>
</constraints>
<properties/>
</component>
<component id="d2ce8" class="javax.swing.JLabel">
<constraints>
<grid row="3" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
<grid row="4" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
</constraints>
<properties>
<text value="Implementation version"/>
</properties>
</component>
<component id="f9300" class="javax.swing.JLabel" binding="formatterVersion">
<constraints>
<grid row="3" column="1" row-span="1" col-span="1" vsize-policy="0" hsize-policy="2" anchor="0" fill="1" indent="1" use-parent-layout="false"/>
<grid row="4" column="1" row-span="1" col-span="1" vsize-policy="0" hsize-policy="2" anchor="0" fill="1" indent="1" use-parent-layout="false"/>
</constraints>
<properties>
<text value="what version are we running with?"/>
</properties>
</component>
<component id="6e9b7" class="javax.swing.JLabel">
<constraints>
<grid row="2" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
<grid row="3" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
</constraints>
<properties>
<text value="Plugin version"/>
</properties>
</component>
<component id="ba751" class="javax.swing.JLabel" binding="pluginVersion">
<constraints>
<grid row="2" column="1" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="1" use-parent-layout="false"/>
<grid row="3" column="1" row-span="1" col-span="1" vsize-policy="0" hsize-policy="0" anchor="8" fill="0" indent="1" use-parent-layout="false"/>
</constraints>
<properties>
<text value="plugin version"/>
</properties>
</component>
<component id="4a87f" class="javax.swing.JCheckBox" binding="enable" default-binding="true">
<constraints>
<grid row="0" column="0" row-span="1" col-span="2" vsize-policy="0" hsize-policy="3" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
</constraints>
<properties>
<text value="Enable palantir-java-format"/>
</properties>
</component>
</children>
</grid>
</form>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

Expand All @@ -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));
Expand All @@ -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()));
}

Expand Down Expand Up @@ -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,
Expand All @@ -177,7 +200,7 @@ private void createUIComponents() {
panel.add(
label1,
new GridConstraints(
1,
2,
0,
1,
1,
Expand All @@ -193,7 +216,7 @@ private void createUIComponents() {
panel.add(
styleComboBox,
new GridConstraints(
1,
2,
1,
1,
1,
Expand All @@ -211,7 +234,7 @@ private void createUIComponents() {
panel.add(
label2,
new GridConstraints(
3,
4,
0,
1,
1,
Expand All @@ -229,7 +252,7 @@ private void createUIComponents() {
panel.add(
formatterVersion,
new GridConstraints(
3,
4,
1,
1,
1,
Expand All @@ -247,7 +270,7 @@ private void createUIComponents() {
panel.add(
label3,
new GridConstraints(
2,
3,
0,
1,
1,
Expand All @@ -265,7 +288,7 @@ private void createUIComponents() {
panel.add(
pluginVersion,
new GridConstraints(
2,
3,
1,
1,
1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -137,6 +145,7 @@ static class State {
private EnabledState enabled = EnabledState.UNKNOWN;
private Optional<List<URI>> implementationClassPath = Optional.empty();

private boolean formatJavadoc = false;
public JavaFormatterOptions.Style style = JavaFormatterOptions.Style.PALANTIR;

public void setImplementationClassPath(@Nullable List<String> value) {
Expand Down Expand Up @@ -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="
Expand Down
Loading

0 comments on commit 29dab8c

Please sign in to comment.