Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New feature flag: com.palantir.baseline-format.palantir-java-format=true #936

Merged
merged 13 commits into from
Oct 15, 2019
Merged
7 changes: 3 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,17 +319,16 @@ spotless {
importOrder ''
trimTrailingWhitespace
indentWithSpaces 4
// only enabled if you run `./gradlew format -Pcom.palantir.baseline-format.eclipse`
eclipse().configFile "$rootDir/.baseline/eclipse.xml"
}
}
```

We chose the Eclipse formatter because it can be run from the command line and from both IDEs (IntelliJ using the [Eclipse Code Formatter](https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter) plugin).
**Add `com.palantir.baseline-format.eclipse=true`** to your gradle.properties to format entire files with the Eclipse formatter. The Eclipse formatter can be run from IntelliJ using the [Eclipse Code Formatter](https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter) plugin.

To iterate on the eclipse.xml formatter config, you can import it into an instance of Eclipse, edit it through the preferences UI and then export it, or you can manually tune individual values by referring to the master list of [DefaultCodeFormatterConstants](https://github.com/eclipse/eclipse.jdt.core/blob/6a8cee1126829229d648db4ae0e5a6b70a5d4f13/org.eclipse.jdt.core/formatter/org/eclipse/jdt/core/formatter/DefaultCodeFormatterConstants.java) and [DefaultCodeFormatterOptions](https://github.com/eclipse/eclipse.jdt.core/blob/6a8cee1126829229d648db4ae0e5a6b70a5d4f13/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/DefaultCodeFormatterOptions.java#L41-L95). Running `./gradlew :gradle-baseline-java:test -Drecreate=true` should update all the checked-in snapshot test cases.

**Add `com.palantir.baseline-format.palantir-java-format=true`** to your gradle.properties to run our experimental fork of google-java-format. The Palantir Java Formatter can be run from IntelliJ using the [palantir-java-format](https://plugins.jetbrains.com/plugin/13180-palantir-java-format) plugin.

## com.palantir.baseline-reproducibility

This plugin is a shorthand for the following snippet, which opts-in to reproducible behaviour for all Gradle's Jar, Tar and Zip tasks. (Surprisingly, these tasks are not reproducible by default).
Expand Down
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-936.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: feature
feature:
description: Users can opt-in to format their files using our fork of google-java-format
(palantir-java-format)
links:
- https://github.com/palantir/gradle-baseline/pull/936
2 changes: 2 additions & 0 deletions gradle-baseline-java/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ dependencies {
compile 'net.ltgt.gradle:gradle-errorprone-plugin'
compile 'org.apache.maven.shared:maven-dependency-analyzer'
compile 'org.github.ngbinh.scalastyle:gradle-scalastyle-plugin_2.11'
implementation 'com.palantir.javaformat:palantir-java-format'
implementation 'com.palantir.javaformat:gradle-palantir-java-format'

testCompile gradleTestKit()
testCompile 'com.github.stefanbirkner:system-rules'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ public void apply(Project project) {

rootProject.getPluginManager().apply(BaselineConfig.class);
rootProject.getPluginManager().apply(BaselineCircleCi.class);
if (BaselineFormat.palantirJavaFormatterEnabled(project)) {
// This plugin currently sets up IntelliJ but could set up eclipse too in the future, so it doesn't
// belong in BaselineIdea.
rootProject.getPluginManager().apply("com.palantir.java-format");
}
rootProject.allprojects(proj -> {
proj.getPluginManager().apply(BaselineCheckstyle.class);
proj.getPluginManager().apply(BaselineScalastyle.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,20 @@
package com.palantir.baseline.plugins;

import com.diffplug.gradle.spotless.SpotlessExtension;
import com.diffplug.spotless.FormatterFunc;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Range;
import com.palantir.javaformat.java.FormatterService;
import com.palantir.javaformat.java.JavaFormatterOptions;
import com.palantir.javaformat.java.JavaFormatterOptions.Style;
import com.palantir.javaformat.java.JavaOutput;
import com.palantir.javaformat.java.Replacement;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ServiceLoader;
import org.gradle.api.GradleException;
import org.gradle.api.Project;
import org.gradle.api.Task;
import org.gradle.api.file.ConfigurableFileCollection;
Expand All @@ -32,6 +43,7 @@ class BaselineFormat extends AbstractBaselinePlugin {

// TODO(dfox): remove this feature flag when we've refined the eclipse.xml sufficiently
private static final String ECLIPSE_FORMATTING = "com.palantir.baseline-format.eclipse";
private static final String PJF_PROPERTY = "com.palantir.baseline-format.palantir-java-format";
private static final String GENERATED_MARKER = File.separator + "generated";

@Override
Expand All @@ -58,10 +70,20 @@ public void apply(Project project) {
// use empty string to specify one group for all non-static imports
java.importOrder("");

if (eclipseFormattingEnabled(project) && palantirJavaFormatterEnabled(project)) {
throw new GradleException(
"Can't use both eclipse and palantir-java-format at the same time, please delete one of "
+ ECLIPSE_FORMATTING + " or " + PJF_PROPERTY + " from your gradle.properties");
}

if (eclipseFormattingEnabled(project)) {
java.eclipse().configFile(project.file(eclipseXml.toString()));
}

if (palantirJavaFormatterEnabled(project)) {
java.customLazy("palantir-java-format", PalantirJavaFormatterFunc::new);
}

java.trimTrailingWhitespace();
});

Expand Down Expand Up @@ -95,7 +117,25 @@ static boolean eclipseFormattingEnabled(Project project) {
return project.hasProperty(ECLIPSE_FORMATTING);
}

static boolean palantirJavaFormatterEnabled(Project project) {
return project.hasProperty(PJF_PROPERTY);
}

static Path eclipseConfigFile(Project project) {
return project.getRootDir().toPath().resolve(".baseline/spotless/eclipse.xml");
}

private static class PalantirJavaFormatterFunc implements FormatterFunc {
private static final JavaFormatterOptions OPTIONS =
JavaFormatterOptions.builder().style(Style.PALANTIR).build();
private final FormatterService formatterService =
Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class));

@Override
public String apply(String input) throws Exception {
ImmutableList<Replacement> replacements = formatterService.getFormatReplacements(
OPTIONS, input, ImmutableList.of(Range.closedOpen(0, input.length())));
return JavaOutput.applyReplacements(input, replacements);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,30 @@ class BaselineFormatIntegrationTest extends AbstractPluginTest {
assertThatFilesAreTheSame(testedDir, expectedDir)
}

def 'palantir java format works'() {
def inputDir = new File("src/test/resources/com/palantir/baseline/formatter-in")
def expectedDir = new File("src/test/resources/com/palantir/baseline/palantirjavaformat-expected")

def testedDir = new File(projectDir, "src/main/java")
FileUtils.copyDirectory(inputDir, testedDir)

buildFile << """
plugins {
id 'java'
id 'com.palantir.baseline-format'
}
""".stripIndent()
file('gradle.properties') << "com.palantir.baseline-format.palantir-java-format=true\n"

when:
BuildResult result = with(':format').build()

then:
result.task(":format").outcome == TaskOutcome.SUCCESS
result.task(":spotlessApply").outcome == TaskOutcome.SUCCESS
assertThatFilesAreTheSame(testedDir, expectedDir)
}

def 'eclipse formatter googlejavaformat test cases'() {
def excludedFiles = [
"B19996259.java", // this causes an OOM
Expand Down Expand Up @@ -147,7 +171,7 @@ class BaselineFormatIntegrationTest extends AbstractPluginTest {
def path = file.toPath()
Path relativized = outputDir.toPath().relativize(path)
Path expectedFile = expectedDir.toPath().resolve(relativized)
if (Boolean.valueOf(System.getProperty("recreate", "false"))) {
if (Boolean.getBoolean("recreate")) {
Files.createDirectories(expectedFile.getParent())
Files.deleteIfExists(expectedFile)
Files.copy(path, expectedFile)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package test;

public class Test {
/**
* Docstring that looks like a list:
*
* <p>1. hey 2. there
*
* <p>with blank line.
*/
Void test() {
/*
Normal comment
with blank line.
*/
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package test;

public class Test {
void test() {
int x = 1;
System.out.println("Hello");
Optional.of("hello").orElseGet(() -> {
return "Hello World";
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package test;

public class Test {
void test() {
int x = 1;
System.out.println("Hello");
Optional.of("hello").orElseGet(() -> {
return "Hello World";
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package test;

class Multicatch {
void test() {
try {
// whatever
} catch (ClassNotFoundException
| ClassCastException
| NoSuchMethodException
| IllegalAccessException
| InvocationTargetException e) {
// ignore
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package test;

class MultilineStringConstant {

// NON-NLS comments are required for i18n, it's important they are kept with their strings.
private static final String MULTIPLE_LINE_NON_NLS = "field_0,"
+ //$NON-NLS-1$
"field_1,"
+ //$NON-NLS-1$
"field_2,"
+ //$NON-NLS-1$
"field_3,"
+ //$NON-NLS-1$
"field_4"; //$NON-NLS-1$

private static final String MULTIPLE_LINE_NO_COMMENT =
"field_0," + "field_1," + "field_2," + "field_3," + "field_4";
}
2 changes: 2 additions & 0 deletions versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ com.google.errorprone:error_prone_test_helpers = 2.3.3
com.google.guava:guava = 27.1-jre
com.netflix.nebula:nebula-dependency-recommender = 9.0.0
com.palantir.configurationresolver:gradle-configuration-resolver-plugin = 0.4.0
com.palantir.javaformat:* = 0.2.0
com.palantir.safe-logging:* = 1.11.0
org.apache.maven.shared:maven-dependency-analyzer = 1.11.1
org.github.ngbinh.scalastyle:gradle-scalastyle-plugin_2.11 = 1.0.1
Expand All @@ -30,3 +31,4 @@ com.palantir.tokens:auth-tokens = 3.6.1
# dependency-upgrader:OFF
# Updating to 0.8 would raise our minimum compatible version to 5.2
net.ltgt.gradle:gradle-errorprone-plugin = 0.7.1