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

FormatterStep.NeedsFile #637

Merged
merged 7 commits into from
Jul 2, 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
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
* `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))
* `GitRatchet` now lives in `lib-extra`, and is shared across `plugin-gradle` and `plugin-maven` ([#626](https://github.com/diffplug/spotless/pull/626)).
* Added ANTLR4 support ([#326](https://github.com/diffplug/spotless/issues/326)).
* `FormatterFunc.NeedsFile` for implementing file-based formatters more cleanly than we have so far ([#637](https://github.com/diffplug/spotless/issues/637)).
### Changed
* **BREAKING** `FileSignature` can no longer sign folders, only files. Signatures are now based only on filename (not path), size, and a content hash. It throws an error if a signature is attempted on a folder or on multiple files with different paths but the same filename - it never breaks silently. This change does not break any of Spotless' internal logic, so it is unlikely to affect any of Spotless' consumers either. ([#571](https://github.com/diffplug/spotless/pull/571))
* This change allows the maven plugin to cache classloaders across subprojects when loading config resources from the classpath (fixes [#559](https://github.com/diffplug/spotless/issues/559)).
Expand Down
8 changes: 6 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ For the folders below in monospace text, they are published on maven central at
The easiest way to create a FormatterStep is `FormatterStep createNeverUpToDate(String name, FormatterFunc function)`, which you can use like this:

```java
FormatterStep identityStep = FormatterStep.createNeverUpToDate("identity", unix -> unix)
FormatterStep identityStep = FormatterStep.createNeverUpToDate("identity", unixStr -> unixStr)
```

This creates a step which will fail up-to-date checks (it is equal only to itself), and will use the function you passed in to do the formatting pass.
Expand Down Expand Up @@ -73,7 +73,7 @@ public final class ReplaceStep {
}

FormatterFunc toFormatter() {
return raw -> raw.replace(target, replacement);
return unixStr -> unixStr.replace(target, replacement);
}
}
}
Expand All @@ -100,6 +100,10 @@ Here's a checklist for creating a new step for Spotless:
- [ ] Test class has test methods to verify behavior.
- [ ] Test class has a test method `equality()` which tests equality using `StepEqualityTester` (see existing methods for examples).

### Accessing the underlying File

In order for Spotless' model to work, each step needs to look only at the `String` input, otherwise they cannot compose. However, there are some cases where the source `File` is useful, such as to look at the file extension. In this case, you can pass a `FormatterFunc.NeedsFile` instead of a `FormatterFunc`. This should only be used in [rare circumstances](https://github.com/diffplug/spotless/pull/637), be careful that you don't accidentally depend on the bytes inside of the `File`!

## How to enable the _ext projects

The `_ext` projects are disabled per default, since:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package com.diffplug.spotless.extra.wtp;

import java.io.File;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Properties;
Expand Down Expand Up @@ -71,27 +70,18 @@ private static FormatterFunc applyWithoutFile(String className, EclipseBasedStep
};
}

private static FormatterFuncWithFile applyWithFile(String className, EclipseBasedStepBuilder.State state) throws Exception {
private static FormatterFunc.NeedsFile applyWithFile(String className, EclipseBasedStepBuilder.State state) throws Exception {
Class<?> formatterClazz = state.loadClass(FORMATTER_PACKAGE + className);
Object formatter = formatterClazz.getConstructor(Properties.class).newInstance(state.getPreferences());
Method method = formatterClazz.getMethod(FORMATTER_METHOD, String.class, String.class);
return (input, source) -> {
return (unixString, file) -> {
try {
return (String) method.invoke(formatter, input, source.getAbsolutePath());
return (String) method.invoke(formatter, unixString, file.getAbsolutePath());
} catch (InvocationTargetException exceptionWrapper) {
Throwable throwable = exceptionWrapper.getTargetException();
Exception exception = (throwable instanceof Exception) ? (Exception) throwable : null;
throw (null == exception) ? exceptionWrapper : exception;
}
};
}

private static interface FormatterFuncWithFile extends FormatterFunc {
@Override
default String apply(String input) throws Exception {
throw new UnsupportedOperationException("Formatter requires file path of source.");
}

public String apply(String input, File source) throws Exception;
}
}
52 changes: 41 additions & 11 deletions lib/src/main/java/com/diffplug/spotless/FormatterFunc.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 @@ -19,16 +19,18 @@
import java.util.Objects;

/**
* A `Function<String, String>` which can throw an exception.
* Also the `BiFunction<String, File, String>` is supported, whereas the default
* implementation only requires the `Function<String, String>` implementation.
* A `Function<String, String>` which can throw an exception. Technically, there
* is also a `File` argument which gets passed around as well, but that is invisible
* to formatters. If you need the File, see {@link NeedsFile}.
*/
@FunctionalInterface
public interface FormatterFunc
extends ThrowingEx.Function<String, String>, ThrowingEx.BiFunction<String, File, String> {

@Override
default String apply(String input, File source) throws Exception {
return apply(input);
String apply(String input) throws Exception;

default String apply(String unix, File file) throws Exception {
return apply(unix);
}

/**
Expand All @@ -50,16 +52,44 @@ public void close() {
}

@Override
public String apply(String input, File source) throws Exception {
return function.apply(Objects.requireNonNull(input), Objects.requireNonNull(source));
public String apply(String unix, File file) throws Exception {
return function.apply(Objects.requireNonNull(unix), Objects.requireNonNull(file));
}

@Override
public String apply(String input) throws Exception {
return function.apply(Objects.requireNonNull(input));
public String apply(String unix) throws Exception {
return function.apply(Objects.requireNonNull(unix));
}
};
}
}

/**
* Ideally, formatters don't need the underlying file. But in case they do, they should only use it's path,
* and should never read the content inside the file, because that breaks the `Function<String, String>` composition
* that Spotless is based on. For the rare case that you need access to the file, use this method
* or {@link NeedsFile} to create a {@link FormatterFunc} which needs the File.
*/
static FormatterFunc needsFile(NeedsFile needsFile) {
return needsFile;
}

/** @see FormatterFunc#needsFile(NeedsFile) */
@FunctionalInterface
interface NeedsFile extends FormatterFunc {
String applyWithFile(String unix, File file) throws Exception;

@Override
default String apply(String unix, File file) throws Exception {
if (file == FormatterStepImpl.SENTINEL) {
throw new IllegalArgumentException("This step requires the underlying file. If this is a test, use StepHarnessWithFile");
}
return applyWithFile(unix, file);
}

@Override
default String apply(String unix) throws Exception {
return apply(unix, FormatterStepImpl.SENTINEL);
}
}
}
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 Down Expand Up @@ -110,4 +110,7 @@ protected String format(Integer state, String rawUnix, File file) throws Excepti
return formatter.apply(rawUnix, file);
}
}

/** A dummy SENTINEL file. */
static final File SENTINEL = new File("");
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,30 +89,12 @@ public LicenseHeaderStep withYearModeLazy(Supplier<YearMode> yearMode) {
return new LicenseHeaderStep(headerLazy, delimiter, yearSeparator, yearMode);
}

private static class SetFromGitFormatterFunc implements FormatterFunc {
private final Runtime runtime;

private SetFromGitFormatterFunc(Runtime runtime) {
this.runtime = runtime;
}

@Override
public String apply(String input, File source) throws Exception {
return runtime.setLicenseHeaderYearsFromGitHistory(input, source);
}

@Override
public String apply(String input) throws Exception {
throw new UnsupportedOperationException();
}
}

public FormatterStep build() {
if (yearMode.get() == YearMode.SET_FROM_GIT) {
return FormatterStep.createNeverUpToDateLazy(LicenseHeaderStep.name(), () -> {
boolean updateYear = false; // doesn't matter
Runtime step = new Runtime(headerLazy.get(), delimiter, yearSeparator, updateYear);
return new SetFromGitFormatterFunc(step);
Runtime runtime = new Runtime(headerLazy.get(), delimiter, yearSeparator, updateYear);
return FormatterFunc.needsFile(runtime::setLicenseHeaderYearsFromGitHistory);
});
} else {
return FormatterStep.createLazy(LicenseHeaderStep.name(), () -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static FormatterStep create(Map<String, String> devDependencies, Provisio
State::createFormatterFunc);
}

public static class State extends NpmFormatterStepStateBase implements Serializable {
private static class State extends NpmFormatterStepStateBase implements Serializable {

private static final long serialVersionUID = -539537027004745812L;
private final PrettierConfig prettierConfig;
Expand Down Expand Up @@ -100,7 +100,7 @@ private void endServer(PrettierRestService restService, ServerProcessInfo restSe

}

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

Expand All @@ -110,16 +110,9 @@ public PrettierFilePathPassingFormatterFunc(String prettierConfigOptions, Pretti
}

@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);
public String applyWithFile(String unix, File file) throws Exception {
final String prettierConfigOptionsWithFilepath = assertFilepathInConfigOptions(file);
return restService.format(unix, prettierConfigOptionsWithFilepath);
}

private String assertFilepathInConfigOptions(File file) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ public static StepHarnessWithFile forStep(FormatterStep step) {
},
new FormatterFunc() {
@Override
public String apply(String input) throws Exception {
return apply(input, new File(""));
public String apply(String unix) throws Exception {
return apply(unix, new File(""));
}

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