Merge pull request #637 - adds FormatterStep.NeedsFile

Author: nedtwigg

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)).

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:

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,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;
-	}
 }

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<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);
 	}
 
 	/**
@@ -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);
+		}
+	}
 }

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("");
 }

lib/src/main/java/com/diffplug/spotless/generic/LicenseHeaderStep.java

@@ -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(), () -> {

lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java

@@ -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;
@@ -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) {

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));
 					}
 				}));
 	}