Rework SpotlessTask to use LintState so that we can know which step is failing and properly use the FormatExceptionPolicyStrict.

Author: nedtwigg

lib/src/main/java/com/diffplug/spotless/DirtyState.java

@@ -84,7 +84,7 @@ public static DirtyState of(Formatter formatter, File file, byte[] rawBytes, Str
 		return state;
 	}
 
-	public static DirtyState of(Formatter formatter, File file, byte[] rawBytes, String raw, ValuePerStep<Throwable> exceptionPerStep) {
+	static DirtyState of(Formatter formatter, File file, byte[] rawBytes, String raw, ValuePerStep<Throwable> exceptionPerStep) {
 		// check that all characters were encodable
 		String encodingError = EncodingErrorMsg.msg(raw, rawBytes, formatter.getEncoding());
 		if (encodingError != null) {

lib/src/main/java/com/diffplug/spotless/Lint.java

@@ -27,6 +27,16 @@
  * to Spotless, then it is by definition.
  */
 public final class Lint implements Serializable {
+	/** Returns a runtime exception which, if thrown, will lint the entire file. */
+	public static ShortcutException entireFile(String code, String msg) {
+		return new ShortcutException(Lint.create(code, msg, -1));
+	}
+
+	/** Returns a runtime exception which, if thrown, will lint a specific line. */
+	public static ShortcutException atLine(int line, String code, String msg) {
+		return new ShortcutException(Lint.create(code, msg, line));
+	}
+
 	/** Any exception which implements this interface will have its lints extracted and reported cleanly to the user. */
 	public interface Has {
 		List<Lint> getLints();

lib/src/main/java/com/diffplug/spotless/LintState.java

@@ -41,6 +41,10 @@ public boolean isHasLints() {
 		return lintsPerStep != null;
 	}
 
+	public boolean isClean() {
+		return dirtyState.isClean() && !isHasLints();
+	}
+
 	public Map<FormatterStep, List<Lint>> getLints(Formatter formatter) {
 		if (lintsPerStep == null) {
 			throw new IllegalStateException("Check `isHasLints` first!");
@@ -137,7 +141,7 @@ public static LintState of(Formatter formatter, File file, byte[] rawBytes) {
 			List<Lint> lintsForStep;
 			if (exceptionForLint instanceof Lint.Has) {
 				lintsForStep = ((Lint.Has) exceptionForLint).getLints();
-			} else if (exceptionForLint != null) {
+			} else if (exceptionForLint != null && exceptionForLint != formatStepCausedNoChange()) {
 				lintsForStep = List.of(Lint.createFromThrowable(step, toLint, exceptionForLint));
 			} else {
 				lintsForStep = List.of();
@@ -149,6 +153,13 @@ public static LintState of(Formatter formatter, File file, byte[] rawBytes) {
 		return new LintState(dirty, lints.indexOfFirstValue() == -1 ? null : lints);
 	}
 
+	/** Returns the DirtyState which corresponds to {@code isClean()}. */
+	public static LintState clean() {
+		return isClean;
+	}
+
+	private static final LintState isClean = new LintState(DirtyState.clean(), null);
+
 	static Throwable formatStepCausedNoChange() {
 		return FormatterCausedNoChange.INSTANCE;
 	}

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java

@@ -339,7 +339,8 @@ private static void relativizeIfSubdir(List<String> relativePaths, File root, Fi
 		if (!destPath.startsWith(rootPath)) {
 			return null;
 		} else {
-			return destPath.substring(rootPath.length());
+			String relativized = destPath.substring(rootPath.length());
+			return relativized.startsWith("/") ? relativized.substring(1) : relativized;
 		}
 	}
 

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java

@@ -157,6 +157,14 @@ public File getOutputDirectory() {
 		return outputDirectory;
 	}
 
+	protected File lintDirectory = new File(getProject().getLayout().getBuildDirectory().getAsFile().get(),
+			"spotless-lint/" + getName());
+
+	@OutputDirectory
+	public File getLintDirectory() {
+		return lintDirectory;
+	}
+
 	private final ConfigurationCacheHackList stepsInternalRoundtrip = ConfigurationCacheHackList.forRoundtrip();
 	private final ConfigurationCacheHackList stepsInternalEquality = ConfigurationCacheHackList.forEquality();
 

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java

@@ -20,6 +20,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.util.List;
 
 import javax.annotation.Nullable;
 import javax.inject.Inject;
@@ -37,8 +38,8 @@
 
 import com.diffplug.common.annotations.VisibleForTesting;
 import com.diffplug.common.base.StringPrinter;
-import com.diffplug.spotless.DirtyState;
 import com.diffplug.spotless.Formatter;
+import com.diffplug.spotless.LintState;
 import com.diffplug.spotless.extra.GitRatchet;
 
 @CacheableTask
@@ -83,7 +84,7 @@ public void performAction(InputChanges inputs) throws Exception {
 			for (FileChange fileChange : inputs.getFileChanges(target)) {
 				File input = fileChange.getFile();
 				if (fileChange.getChangeType() == ChangeType.REMOVED) {
-					deletePreviousResult(input);
+					deletePreviousResults(input);
 				} else {
 					if (input.isFile()) {
 						processInputFile(ratchet, formatter, input);
@@ -95,24 +96,23 @@ public void performAction(InputChanges inputs) throws Exception {
 
 	@VisibleForTesting
 	void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File input) throws IOException {
-		File output = getOutputFile(input);
+		File output = getOutputFileWithBaseDir(input, outputDirectory);
+		File lint = getOutputFileWithBaseDir(input, lintDirectory);
 		getLogger().debug("Applying format to {} and writing to {}", input, output);
-		DirtyState dirtyState;
+		LintState lintState;
 		if (ratchet != null && ratchet.isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) {
-			dirtyState = DirtyState.clean();
+			lintState = LintState.clean();
 		} else {
 			try {
-				dirtyState = DirtyState.of(formatter, input);
-			} catch (IOException e) {
-				throw new IOException("Issue processing file: " + input, e);
-			} catch (RuntimeException e) {
+				lintState = LintState.of(formatter, input);
+			} catch (Throwable e) {
 				throw new IllegalArgumentException("Issue processing file: " + input, e);
 			}
 		}
-		if (dirtyState.isClean()) {
+		if (lintState.getDirtyState().isClean()) {
 			// Remove previous output if it exists
 			Files.deleteIfExists(output.toPath());
-		} else if (dirtyState.didNotConverge()) {
+		} else if (lintState.getDirtyState().didNotConverge()) {
 			getLogger().warn("Skipping '{}' because it does not converge.  Run {@code spotlessDiagnose} to understand why", input);
 		} else {
 			Path parentDir = output.toPath().getParent();
@@ -124,20 +124,32 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in
 			Files.copy(input.toPath(), output.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES);
 
 			getLogger().info(String.format("Writing clean file: %s", output));
-			dirtyState.writeCanonicalTo(output);
+			lintState.getDirtyState().writeCanonicalTo(output);
+		}
+		if (lintState.isHasLints()) {
+			var lints = lintState.getLints(formatter);
+			var first = lints.entrySet().iterator().next();
+			getExceptionPolicy().handleError(new Throwable(first.getValue().get(0).toString()), first.getKey(), FormatExtension.relativize(getProjectDir().get().getAsFile(), input));
+			//			Files.createDirectories(lint.toPath().getParent());
+			//			Files.write(lint.toPath(), lintState.asString().getBytes());
+		} else {
+			Files.deleteIfExists(lint.toPath());
 		}
 	}
 
-	private void deletePreviousResult(File input) throws IOException {
-		File output = getOutputFile(input);
-		if (output.isDirectory()) {
-			getFs().delete(d -> d.delete(output));
-		} else {
-			Files.deleteIfExists(output.toPath());
+	private void deletePreviousResults(File input) throws IOException {
+		File output = getOutputFileWithBaseDir(input, outputDirectory);
+		File lint = getOutputFileWithBaseDir(input, lintDirectory);
+		for (File file : List.of(output, lint)) {
+			if (file.isDirectory()) {
+				getFs().delete(d -> d.delete(file));
+			} else {
+				Files.deleteIfExists(file.toPath());
+			}
 		}
 	}
 
-	private File getOutputFile(File input) {
+	private File getOutputFileWithBaseDir(File input, File baseDir) {
 		File projectDir = getProjectDir().get().getAsFile();
 		String outputFileName = FormatExtension.relativize(projectDir, input);
 		if (outputFileName == null) {
@@ -147,6 +159,6 @@ private File getOutputFile(File input) {
 				printer.println("       target: " + input.getAbsolutePath());
 			}));
 		}
-		return new File(outputDirectory, outputFileName);
+		return new File(baseDir, outputFileName);
 	}
 }

plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java

@@ -23,13 +23,11 @@
 import org.assertj.core.api.Assertions;
 import org.gradle.testkit.runner.BuildResult;
 import org.gradle.testkit.runner.TaskOutcome;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import com.diffplug.common.base.CharMatcher;
 import com.diffplug.common.base.Splitter;
 import com.diffplug.spotless.LineEnding;
-import com.diffplug.spotless.tag.ForLintRefactor;
 
 /** Tests the desired behavior from https://github.com/diffplug/spotless/issues/46. */
 class ErrorShouldRethrowTest extends GradleIntegrationHarness {
@@ -45,7 +43,7 @@ private void writeBuild(String... toInsert) throws IOException {
 		lines.add("        target file('README.md')");
 		lines.add("        custom 'no swearing', {");
 		lines.add("             if (it.toLowerCase(Locale.ROOT).contains('fubar')) {");
-		lines.add("                 throw new RuntimeException('No swearing!');");
+		lines.add("                 throw com.diffplug.spotless.Lint.entireFile('swearing', 'No swearing!');");
 		lines.add("             }");
 		lines.add("        }");
 		lines.addAll(Arrays.asList(toInsert));
@@ -62,8 +60,6 @@ void passesIfNoException() throws Exception {
 	}
 
 	@Test
-	@Disabled
-	@ForLintRefactor
 	void anyExceptionShouldFail() throws Exception {
 		writeBuild(
 				"    } // format",
@@ -72,8 +68,8 @@ void anyExceptionShouldFail() throws Exception {
 		runWithFailure(
 				"> Task :spotlessMisc FAILED\n" +
 						"Step 'no swearing' found problem in 'README.md':\n" +
-						"No swearing!\n" +
-						"java.lang.RuntimeException: No swearing!");
+						"-1: (swearing) No swearing!\n" +
+						"java.lang.Throwable: -1: (swearing) No swearing!");
 	}
 
 	@Test
@@ -86,9 +82,6 @@ void unlessEnforceCheckIsFalse() throws Exception {
 		runWithSuccess("> Task :processResources NO-SOURCE");
 	}
 
-	@Disabled
-	@ForLintRefactor
-	@Test
 	void unlessExemptedByStep() throws Exception {
 		writeBuild(
 				"        ignoreErrorForStep 'no swearing'",
@@ -99,8 +92,6 @@ void unlessExemptedByStep() throws Exception {
 				"Unable to apply step 'no swearing' to 'README.md'");
 	}
 
-	@Disabled
-	@ForLintRefactor
 	@Test
 	void unlessExemptedByPath() throws Exception {
 		writeBuild(
@@ -113,8 +104,6 @@ void unlessExemptedByPath() throws Exception {
 	}
 
 	@Test
-	@Disabled
-	@ForLintRefactor
 	void failsIfNeitherStepNorFileExempted() throws Exception {
 		writeBuild(
 				"        ignoreErrorForStep 'nope'",
@@ -124,8 +113,8 @@ void failsIfNeitherStepNorFileExempted() throws Exception {
 		setFile("README.md").toContent("This code is fubar.");
 		runWithFailure("> Task :spotlessMisc FAILED\n" +
 				"Step 'no swearing' found problem in 'README.md':\n" +
-				"No swearing!\n" +
-				"java.lang.RuntimeException: No swearing!");
+				"-1: (swearing) No swearing!\n" +
+				"java.lang.Throwable: -1: (swearing) No swearing!");
 	}
 
 	private void runWithSuccess(String expectedToStartWith) throws Exception {