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

Feature/pass filename to prettier #620

Merged
merged 12 commits into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ This document is intended for Spotless developers.
We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).

## [Unreleased]
### Added
* `prettier` will now autodetect the parser (and formatter) to use based on the filename, unless you override this using `config` or `configFile` with the option `parser` or `filepath`. ([#620](https://github.com/diffplug/spotless/pull/620))

## [1.34.1] - 2020-06-17
### Changed
Expand Down
3 changes: 2 additions & 1 deletion lib/src/main/java/com/diffplug/spotless/npm/NpmProcess.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ void install() {
}

Process start() {
return npm("start");
// adding --scripts-prepend-node-path=true due to https://github.com/diffplug/spotless/issues/619#issuecomment-648018679
return npm("start", "--scripts-prepend-node-path=true");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a reaction to the discussion in #619 - make sure we always use the node binary associated with the npm binary we use.

}

private void npmAwait(String... args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public FormatterFunc createFormatterFunc() {
ServerProcessInfo prettierRestServer = npmRunServer();
PrettierRestService restService = new PrettierRestService(prettierRestServer.getBaseUrl());
String prettierConfigOptions = restService.resolveConfig(this.prettierConfig.getPrettierConfigPath(), this.prettierConfig.getOptions());
return Closeable.of(() -> endServer(restService, prettierRestServer), input -> restService.format(input, prettierConfigOptions));
return Closeable.of(() -> endServer(restService, prettierRestServer), new PrettierFilePathPassingFormatterFunc(prettierConfigOptions, restService));
} catch (Exception e) {
throw ThrowingEx.asRuntime(e);
}
Expand All @@ -104,4 +104,43 @@ private void endServer(PrettierRestService restService, ServerProcessInfo restSe
}

}

public static class PrettierFilePathPassingFormatterFunc implements FormatterFunc {
private final String prettierConfigOptions;
private final PrettierRestService restService;

public PrettierFilePathPassingFormatterFunc(String prettierConfigOptions, PrettierRestService restService) {
this.prettierConfigOptions = requireNonNull(prettierConfigOptions);
this.restService = requireNonNull(restService);
}

@Override
public String apply(String input) throws Exception {
return apply(input, new File(""));
}

@Override
public String apply(String input, File source) throws Exception {
requireNonNull(input, "input must not be null");
requireNonNull(source, "source must not be null");
final String prettierConfigOptionsWithFilepath = assertFilepathInConfigOptions(source);
return restService.format(input, prettierConfigOptionsWithFilepath);
}

private String assertFilepathInConfigOptions(File file) {
// if it is already in the options, we do nothing
if (prettierConfigOptions.contains("\"filepath\"") || prettierConfigOptions.contains("\"parser\"")) {
return prettierConfigOptions;
}
// if the file has no name, we cannot use it
if (file.getName().trim().length() == 0) {
return prettierConfigOptions;
}
// if it is not there, we add it at the beginning of the Options
final int startOfConfigOption = prettierConfigOptions.indexOf('{');
final boolean hasAnyConfigOption = prettierConfigOptions.indexOf(':', startOfConfigOption + 1) != -1;
final String filePathOption = "\"filepath\":\"" + file.getName() + "\"";
return "{" + filePathOption + (hasAnyConfigOption ? "," : "") + prettierConfigOptions.substring(startOfConfigOption + 1);
}
}
}
2 changes: 2 additions & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
**5.x preview:** If you are using Gradle 5.4+, you can preview thew upcoming `com.diffplug.spotless` plugin by adding `-PspotlessModern`, see [CHANGES-5.x-PREVIEW.md](CHANGES-5.x-PREVIEW.md) for details.

## [Unreleased]
### Added
* `prettier` will now autodetect the parser (and formatter) to use based on the filename, unless you override this using `config()` or `configFile()` with the option `parser` or `filepath`. ([#620](https://github.com/diffplug/spotless/pull/620))

## [4.4.0] - 2020-06-19
### Added
Expand Down
12 changes: 12 additions & 0 deletions plugin-gradle/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,18 @@ spotless {
}
```

Or you might even let prettier detect the file type and choose the parser on its own such as:

```gradle
spotless {
format 'webResources', {
target 'src/*/webapp/**', 'app/**'
prettier()
}
}
```


<a name="prettier-plugins"></a>
### Using plugins for prettier

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,25 @@ public void useFileConfig() throws IOException {
assertFile("test.ts").sameAsResource("npm/prettier/config/typescript.configfile.clean");
}

@Test
public void chooseParserBasedOnFilename() throws IOException {
setFile("build.gradle").toLines(
"buildscript { repositories { mavenCentral() } }",
"plugins {",
" id 'com.diffplug.gradle.spotless'",
"}",
"spotless {",
" format 'webResources', {",
" target 'dirty.*'",
" prettier()",
" }",
"}");
setFile("dirty.json").toResource("npm/prettier/filename/dirty.json");
final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build();
Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL");
assertFile("dirty.json").sameAsResource("npm/prettier/filename/clean.json");
}

@Test
public void useJavaCommunityPlugin() throws IOException {
setFile("build.gradle").toLines(
Expand Down
2 changes: 2 additions & 0 deletions plugin-maven/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).

## [Unreleased]
### Added
* `prettier` will now autodetect the parser (and formatter) to use based on the filename, unless you override this using `config` or `configFile` with the option `parser` or `filepath`. ([#620](https://github.com/diffplug/spotless/pull/620))

## [1.31.3] - 2020-06-17
### Changed
Expand Down
17 changes: 17 additions & 0 deletions plugin-maven/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,23 @@ Supported config file variants are documented on [prettier.io](https://prettier.

To apply prettier to more kinds of files, just add more formats.


Or you might even let prettier detect the file type and choose the parser on its own such as:

```xml
<configuration>
<formats>
<format>
<includes>
<include>src/*/webapp/**</include>
<include>app/**</include>
</includes>
<prettier/>
</format>
</formats>
</configuration>
```

<a name="prettier-plugins"></a>
### Using plugins for prettier

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,16 @@ public void custom_plugin() throws Exception {
mavenRunner().withArguments("spotless:apply").runNoError();
assertFile("php-example.php").sameAsResource("npm/prettier/plugins/php.clean");
}

@Test
public void autodetect_parser_based_on_filename() throws Exception {
writePomWithFormatSteps(
"<includes><include>dirty.json</include></includes>",
"<prettier/>");

setFile("dirty.json").toResource("npm/prettier/filename/dirty.json");
mavenRunner().withArguments("spotless:apply").runNoError();
assertFile("dirty.json").sameAsResource("npm/prettier/filename/clean.json");
}

}
7 changes: 3 additions & 4 deletions testlib/src/main/java/com/diffplug/spotless/StepHarness.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016 DiffPlug
* Copyright 2016-2020 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,12 +23,11 @@
import org.assertj.core.api.Assertions;
import org.junit.Assert;

/** An api for adding test cases. */
/** An api for testing a `FormatterStep` that doesn't depend on the File path. DO NOT ADD FILE SUPPORT TO THIS, use {@link StepHarnessWithFile} if you need that. */
public class StepHarness implements AutoCloseable {
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
private final FormatterFunc formatter;

/** Creates a StepHarness around the given FormatterFunc. */
public StepHarness(FormatterFunc formatter) {
private StepHarness(FormatterFunc formatter) {
this.formatter = Objects.requireNonNull(formatter);
}

Expand Down
111 changes: 111 additions & 0 deletions testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* Copyright 2016-2020 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.diffplug.spotless;

import java.io.File;
import java.util.Objects;
import java.util.function.Consumer;

import org.assertj.core.api.AbstractThrowableAssert;
import org.assertj.core.api.Assertions;
import org.junit.Assert;

/** An api for testing a `FormatterStep` that depends on the File path. */
public class StepHarnessWithFile implements AutoCloseable {
private final FormatterFunc formatter;

private StepHarnessWithFile(FormatterFunc formatter) {
this.formatter = Objects.requireNonNull(formatter);
}

/** Creates a harness for testing steps which do depend on the file. */
public static StepHarnessWithFile forStep(FormatterStep step) {
// We don't care if an individual FormatterStep is misbehaving on line-endings, because
// Formatter fixes that. No reason to care in tests either. It's likely to pop up when
// running tests on Windows from time-to-time
return new StepHarnessWithFile(FormatterFunc.Closeable.of(
() -> {
if (step instanceof FormatterStepImpl.Standard) {
((FormatterStepImpl.Standard<?>) step).cleanupFormatterFunc();
}
},
new FormatterFunc() {
@Override
public String apply(String input) throws Exception {
return apply(input, new File(""));
}

@Override
public String apply(String input, File source) throws Exception {
return LineEnding.toUnix(step.format(input, source));
}
}));
}

/** Creates a harness for testing a formatter whose steps do depend on the file. */
public static StepHarnessWithFile forFormatter(Formatter formatter) {
return new StepHarnessWithFile(FormatterFunc.Closeable.of(
formatter::close,
input -> formatter.compute(input, new File(""))));
}

/** Asserts that the given element is transformed as expected, and that the result is idempotent. */
public StepHarnessWithFile test(File file, String before, String after) throws Exception {
String actual = formatter.apply(before, file);
Assert.assertEquals("Step application failed", after, actual);
return testUnaffected(file, after);
}

/** Asserts that the given element is idempotent w.r.t the step under test. */
public StepHarnessWithFile testUnaffected(File file, String idempotentElement) throws Exception {
String actual = formatter.apply(idempotentElement, file);
Assert.assertEquals("Step is not idempotent", idempotentElement, actual);
return this;
}

/** Asserts that the given elements in the resources directory are transformed as expected. */
public StepHarnessWithFile testResource(File file, String resourceBefore, String resourceAfter) throws Exception {
String before = ResourceHarness.getTestResource(resourceBefore);
String after = ResourceHarness.getTestResource(resourceAfter);
return test(file, before, after);
}

/** Asserts that the given elements in the resources directory are transformed as expected. */
public StepHarnessWithFile testResourceUnaffected(File file, String resourceIdempotent) throws Exception {
String idempotentElement = ResourceHarness.getTestResource(resourceIdempotent);
return testUnaffected(file, idempotentElement);
}

/** Asserts that the given elements in the resources directory are transformed as expected. */
public StepHarnessWithFile testException(File file, String resourceBefore, Consumer<AbstractThrowableAssert<?, ? extends Throwable>> exceptionAssertion) throws Exception {
String before = ResourceHarness.getTestResource(resourceBefore);
try {
formatter.apply(before, file);
Assert.fail();
} catch (Throwable t) {
AbstractThrowableAssert<?, ? extends Throwable> abstractAssert = Assertions.assertThat(t);
exceptionAssertion.accept(abstractAssert);
}
return this;
}

@Override
public void close() {
if (formatter instanceof FormatterFunc.Closeable) {
((FormatterFunc.Closeable) formatter).close();
}
}
}
11 changes: 11 additions & 0 deletions testlib/src/main/resources/npm/prettier/filename/clean.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"hello": "world",
"hello int": 123,
"hello array": ["1", "2", "3", "4"],
"hello object": {
"property1": "111111",
"property2": "2222222",
"property3": "333333333",
"property4": "44444444"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ "hello": "world", "hello int": 123, "hello array":[ "1", "2", "3", "4"], "hello object":{ "property1":"111111","property2":"2222222","property3":"333333333","property4":"44444444"} }
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.io.File;
import java.util.Arrays;
import java.util.Collections;

import org.junit.Test;
import org.junit.experimental.categories.Category;
Expand Down Expand Up @@ -69,7 +70,7 @@ public void formattingUsingConfigFile() throws Exception {
public static class SpecificPrettierFormatterStepTests extends NpmFormatterStepCommonTests {

@Test
public void parserInferenceIsWorking() throws Exception {
public void parserInferenceBasedOnExplicitFilepathIsWorking() throws Exception {
String filedir = "npm/prettier/filetypes/json/";

final String dirtyFile = filedir + "json.dirty";
Expand All @@ -87,6 +88,25 @@ public void parserInferenceIsWorking() throws Exception {
}
}

@Test
public void parserInferenceBasedOnFilenameIsWorking() throws Exception {
String filedir = "npm/prettier/filename/";

final String dirtyFile = filedir + "dirty.json";
final String cleanFile = filedir + "clean.json";

final FormatterStep formatterStep = PrettierFormatterStep.create(
PrettierFormatterStep.defaultDevDependencies(),
TestProvisioner.mavenCentral(),
buildDir(),
npmExecutable(),
new PrettierConfig(null, Collections.emptyMap()));

try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(formatterStep)) {
stepHarness.testResource(new File("test.json"), dirtyFile, cleanFile);
}
}

@Test
public void verifyPrettierErrorMessageIsRelayed() throws Exception {
FormatterStep formatterStep = PrettierFormatterStep.create(
Expand Down