fail check task when there are failures in formatting (#58)

Co-authored-by: Guido Schreuder <[email protected]>
Co-authored-by: Martin Laporte <[email protected]>

src/main/java/com/coveo/AbstractFMT.java

@@ -16,7 +16,6 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-
 import org.apache.maven.plugin.AbstractMojo;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
@@ -73,6 +72,7 @@ public abstract class AbstractFMT extends AbstractMojo {
    *
    * @throws org.apache.maven.plugin.MojoExecutionException if any.
    */
+  @Override
   public void execute() throws MojoExecutionException, MojoFailureException {
     if (skip) {
       getLog().info("Skipping format check");
@@ -85,7 +85,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
     if (skipSortingImports) {
       getLog().info("Skipping sorting imports");
     }
-    List<File> directoriesToFormat = new ArrayList<File>();
+    List<File> directoriesToFormat = new ArrayList<>();
     if (sourceDirectory.exists()) {
       directoriesToFormat.add(sourceDirectory);
     } else {
@@ -145,14 +145,22 @@ public void formatSourceFilesInDirectory(File directory, Formatter formatter)
     try (Stream<Path> paths = Files.walk(Paths.get(directory.getPath()))) {
       FileFilter fileNameFilter = getFileNameFilter();
       FileFilter pathFilter = getPathFilter();
-      paths
-          .collect(Collectors.toList())
-          .parallelStream()
-          .filter(Files::isRegularFile)
-          .map(Path::toFile)
-          .filter(fileNameFilter::accept)
-          .filter(pathFilter::accept)
-          .forEach(file -> formatSourceFile(file, formatter));
+      long failures =
+          paths
+              .collect(Collectors.toList())
+              .parallelStream()
+              .filter(p -> p.toFile().exists())
+              .map(Path::toFile)
+              .filter(fileNameFilter::accept)
+              .filter(pathFilter::accept)
+              .map(file -> formatSourceFile(file, formatter))
+              .filter(r -> !r)
+              .count();
+
+      if (failures > 0) {
+        throw new MojoFailureException(
+            "There where errors when formatting files. Error count: " + failures);
+      }
     } catch (IOException exception) {
       throw new MojoFailureException(exception.getMessage());
     }
@@ -190,10 +198,10 @@ private FileFilter getPathFilter() {
     return pathname -> pathname.isDirectory() || pathname.getPath().matches(filesPathPattern);
   }
 
-  private void formatSourceFile(File file, Formatter formatter) {
+  private boolean formatSourceFile(File file, Formatter formatter) {
     if (file.isDirectory()) {
       getLog().info("File '" + file + "' is a directory. Skipping.");
-      return;
+      return true;
     }
 
     if (verbose) {
@@ -217,8 +225,10 @@ private void formatSourceFile(File file, Formatter formatter) {
         logNumberOfFilesProcessed();
       }
     } catch (FormatterException | IOException e) {
-      getLog().warn("Failed to format file '" + file + "'.", e);
+      getLog().error("Failed to format file '" + file + "'.", e);
+      return false;
     }
+    return true;
   }
 
   private void handleMissingDirectory(String directoryDisplayName, File directory)

src/test/java/com/coveo/FMTTest.java

@@ -1,10 +1,9 @@
 package com.coveo;
 
-import static com.google.common.truth.Truth.*;
+import static com.google.common.truth.Truth.assertThat;
 
 import java.io.File;
 import java.util.List;
-
 import org.apache.commons.io.IOUtils;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugin.testing.MojoRule;
@@ -163,6 +162,12 @@ public void checkSucceedsWhenNotFormattedButIgnored() throws Exception {
     check.execute();
   }
 
+  @Test(expected = MojoFailureException.class)
+  public void checkFailsWhenFormattingFails() throws Exception {
+    Check check = (Check) mojoRule.lookupConfiguredMojo(loadPom("failed_formatting"), CHECK);
+    check.execute();
+  }
+
   private File loadPom(String folderName) {
     return new File("src/test/resources/", folderName);
   }

src/test/resources/failed_formatting/pom.xml

@@ -0,0 +1,38 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>org.apache.maven.plugin.my.unit</groupId>
+    <artifactId>project-to-test</artifactId>
+    <version>1.0.0</version>
+    <packaging>jar</packaging>
+    <name>Test MyMojo</name>
+
+    <dependencies>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>3.8.1</version>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>com.coveo</groupId>
+                <artifactId>fmt-maven-plugin</artifactId>
+                <version>1.0.0</version>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>format</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+
+</project>

src/test/resources/failed_formatting/src/main/java/HelloWorld1.java

@@ -0,0 +1,25 @@
+package notestsource.src.main.java;
+
+import com.google.common.base.Preconditions;
+import java.util.List;
+// force non-contiguous imports
+import javax.annotations.Nullable;
+import static com.google.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
+
+public class HelloWorld1 {
+
+  <T> void check(@Nullable List<T> x) {
+    Preconditions.checkNodeNull(x);
+  }
+
+  void f() {
+    List<String> xs = null;
+    assertThat(xs).isNull();
+    try {
+      check(xs);
+      fail();
+    } catch (NullPointerException e) {
+    }
+  }
+}