diff --git a/CHANGES.md b/CHANGES.md index 82b6839819..2a52513606 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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)). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7af707b059..8b3ef78542 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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. @@ -73,7 +73,7 @@ public final class ReplaceStep { } FormatterFunc toFormatter() { - return raw -> raw.replace(target, replacement); + return unixStr -> unixStr.replace(target, replacement); } } } @@ -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: diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/wtp/EclipseWtpFormatterStep.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/wtp/EclipseWtpFormatterStep.java index 22006a985a..63e8673774 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/wtp/EclipseWtpFormatterStep.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/wtp/EclipseWtpFormatterStep.java @@ -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; @@ -71,13 +70,13 @@ 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; @@ -85,13 +84,4 @@ private static FormatterFuncWithFile applyWithFile(String className, EclipseBase } }; } - - 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; - } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java index ab4ca219b1..f675896db9 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java @@ -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. @@ -19,16 +19,18 @@ import java.util.Objects; /** - * A `Function` which can throw an exception. - * Also the `BiFunction` is supported, whereas the default - * implementation only requires the `Function` implementation. + * A `Function` 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, ThrowingEx.BiFunction { - @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); } /** @@ -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` 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); + } + } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java index 4f25d3c1e4..12c90b3031 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java @@ -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. @@ -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(""); } diff --git a/lib/src/main/java/com/diffplug/spotless/generic/LicenseHeaderStep.java b/lib/src/main/java/com/diffplug/spotless/generic/LicenseHeaderStep.java index cd5ccebc91..59077f797e 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/LicenseHeaderStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/LicenseHeaderStep.java @@ -89,30 +89,12 @@ public LicenseHeaderStep withYearModeLazy(Supplier 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(), () -> { diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java index b181fca796..a7f95aa7c2 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java @@ -58,7 +58,7 @@ public static FormatterStep create(Map 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; @@ -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; @@ -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) { diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java index df8de0fbe6..296ccadc11 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java @@ -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)); } })); }