Revert 58 to fix spotbugs/spotbugs!819

.travis.yml

@@ -8,7 +8,6 @@ jdk:
 env:
   matrix:
     - GRADLE_VERSION=default
-    - GRADLE_VERSION=5.0
     - GRADLE_VERSION=5.1
   global:
     - SONAR_HOST_URL="https://sonarcloud.io"

CHANGELOG.md

@@ -8,8 +8,14 @@ Currently the versioning policy of this project follows [Semantic Versioning](ht
 
 ### Fixed
 
+* "Trying to add already registered factory" problem reported as [a spotbugs issue](https://github.com/spotbugs/spotbugs/issues/819)
 * Result caching works across checkouts. Previously it was using absolute paths and therefore didn't work. ([#96](https://github.com/spotbugs/spotbugs-gradle-plugin/pull/96))
 
+### Removed
+
+* Usage of worker API introduced at [#57](https://github.com/spotbugs/spotbugs-gradle-plugin/issues/57)
+* Drop support for Gradle 4, that causes [SLF4J related problem](https://github.com/gradle/gradle/issues/2657)
+
 ### Changed
 
 * Use SpotBugs 3.1.11 by default

build.gradle

@@ -14,6 +14,7 @@ apply from: "$rootDir/gradle/deploy.gradle"
 version = '1.6.10-SNAPSHOT'
 group = "com.github.spotbugs"
 def spotBugsVersion = '3.1.11'
+def slf4jVersion = '1.8.0-beta2'
 sourceCompatibility = 1.8
 targetCompatibility = 1.8
 
@@ -55,6 +56,7 @@ task processVersionFile(type: WriteProperties) {
   outputFile file('src/main/resources/spotbugs-gradle-plugin.properties')
 
   property 'spotbugs-version', spotBugsVersion
+  property 'slf4j-version', slf4jVersion
 }
 tasks.processResources.dependsOn processVersionFile
 

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.3-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-5.0-bin.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

src/main/java/com/github/spotbugs/SpotBugsPlugin.java

@@ -5,6 +5,8 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.Callable;
 import java.util.stream.StreamSupport;
@@ -40,7 +42,7 @@ public class SpotBugsPlugin extends AbstractCodeQualityPlugin<SpotBugsTask> {
      * Supported Gradle version described at <a href="http://spotbugs.readthedocs.io/en/latest/gradle.html">official
      * manual site</a>.
      */
-    private static final GradleVersion SUPPORTED_VERSION = GradleVersion.version("4.0");
+    private static final GradleVersion SUPPORTED_VERSION = GradleVersion.version("5.0");
 
     private SpotBugsExtension extension;
 
@@ -122,11 +124,19 @@ protected CodeQualityExtension createExtension() {
     }
 
     String loadToolVersion() {
+        return loadVersion("spotbugs-version");
+    }
+
+    String loadSlf4jVersion() {
+        return loadVersion("slf4j-version");
+    }
+
+    private String loadVersion(String name) {
         URL url = SpotBugsPlugin.class.getClassLoader().getResource("spotbugs-gradle-plugin.properties");
         try (InputStream input = url.openStream()) {
             Properties prop = new Properties();
             prop.load(input);
-            return prop.getProperty("spotbugs-version");
+            return prop.getProperty(name);
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
@@ -146,10 +156,37 @@ protected void configureConfiguration(Configuration configuration) {
         // https://github.com/spotbugs/spotbugs-gradle-plugin/issues/22
     }
 
+    /**
+     * Overriding this method, to include SLF4J into {@code spotbugsClasspath}. SLF4J is necessary in worker process.
+     */
+    @Override
+    protected void createConfigurations() {
+        Configuration configuration = project.getConfigurations().create(getConfigurationName());
+        configuration.setVisible(false);
+        configuration.setTransitive(true);
+        configuration.setDescription("The " + getToolName() + " libraries to be used for this project.");
+        // Don't need these things, they're provided by the runtime
+        configuration.exclude(excludeProperties("ant", "ant"));
+        configuration.exclude(excludeProperties("org.apache.ant", "ant"));
+        configuration.exclude(excludeProperties("org.apache.ant", "ant-launcher"));
+        configuration.exclude(excludeProperties("org.slf4j", "jcl-over-slf4j"));
+        configuration.exclude(excludeProperties("org.slf4j", "log4j-over-slf4j"));
+        configuration.exclude(excludeProperties("commons-logging", "commons-logging"));
+        configuration.exclude(excludeProperties("log4j", "log4j"));
+        configureConfiguration(configuration);
+    }
+
+    private Map<String, String> excludeProperties(String group, String module) {
+        Map<String, String> map = new HashMap<>();
+        map.put("group", group);
+        map.put("module", module);
+        return map;
+    }
     private void configureDefaultDependencies(Configuration configuration) {
-        configuration.defaultDependencies((DependencySet dependencies) ->
-            dependencies.add(project.getDependencies().create("com.github.spotbugs:spotbugs:" + extension.getToolVersion()))
-        );
+        configuration.defaultDependencies((DependencySet dependencies) -> {
+            dependencies.add(project.getDependencies().create("org.slf4j:slf4j-simple:" + loadSlf4jVersion()));
+            dependencies.add(project.getDependencies().create("com.github.spotbugs:spotbugs:" + extension.getToolVersion()));
+        });
     }
 
     private void configureTaskConventionMapping(Configuration configuration, SpotBugsTask task) {

src/main/java/com/github/spotbugs/SpotBugsTask.java

@@ -1,6 +1,7 @@
 package com.github.spotbugs;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -9,15 +10,15 @@
 
 import javax.inject.Inject;
 
-import com.github.spotbugs.internal.spotbugs.SpotBugsRunner;
-import groovy.lang.Closure;
 import org.gradle.api.Action;
+import org.gradle.api.GradleException;
 import org.gradle.api.Incubating;
 import org.gradle.api.JavaVersion;
-import org.gradle.api.Task;
 import org.gradle.api.file.FileCollection;
 import org.gradle.api.file.FileTree;
+import org.gradle.api.logging.LogLevel;
 import org.gradle.api.reporting.Reporting;
+import org.gradle.api.reporting.SingleFileReport;
 import org.gradle.api.resources.TextResource;
 import org.gradle.api.tasks.CacheableTask;
 import org.gradle.api.tasks.Classpath;
@@ -33,19 +34,20 @@
 import org.gradle.api.tasks.SourceTask;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.api.tasks.VerificationTask;
+import org.gradle.internal.logging.ConsoleRenderer;
+import org.gradle.internal.reflect.Instantiator;
+import org.gradle.process.internal.worker.WorkerProcessFactory;
 import org.gradle.util.ConfigureUtil;
-import org.gradle.util.DeprecationLogger;
-import org.gradle.util.GradleVersion;
-import org.gradle.workers.ForkMode;
-import org.gradle.workers.IsolationMode;
-import org.gradle.workers.WorkerExecutor;
 
 import com.github.spotbugs.internal.SpotBugsReportsImpl;
 import com.github.spotbugs.internal.SpotBugsReportsInternal;
 import com.github.spotbugs.internal.spotbugs.SpotBugsClasspathValidator;
+import com.github.spotbugs.internal.spotbugs.SpotBugsResult;
 import com.github.spotbugs.internal.spotbugs.SpotBugsSpec;
 import com.github.spotbugs.internal.spotbugs.SpotBugsSpecBuilder;
+import com.github.spotbugs.internal.spotbugs.SpotBugsWorkerManager;
 
+import groovy.lang.Closure;
 
 /**
  * Analyzes code with <a href="https://spotbugs.github.io/">SpotBugs</a>. See the
@@ -91,28 +93,18 @@ public class SpotBugsTask extends SourceTask implements VerificationTask, Report
     @Nested
     private final SpotBugsReportsInternal reports;
 
-    @Inject
-    public SpotBugsTask(WorkerExecutor workerExecutor) {
-        super();
-        this.workerExecutor = workerExecutor;
-        reports = createReports(this);
+    public SpotBugsTask() {
+        reports = getInstantiator().newInstance(SpotBugsReportsImpl.class, this);
     }
 
-    private SpotBugsReportsInternal createReports(Task task) {
-        return DeprecationLogger.whileDisabled(() -> {
-            if (GradleVersion.current().compareTo(GRADLE_42()) < 0) {
-                return new SpotBugsReportsImpl(task);
-            } else {
-                //ObjectFactory#newInstance was introduced in Gradle 4.2
-                return getProject().getObjects().newInstance(SpotBugsReportsImpl.class, task);
-            }
-        });
+    @Inject
+    public Instantiator getInstantiator() {
+        throw new UnsupportedOperationException();
     }
 
-    private final WorkerExecutor workerExecutor;
-
-    private GradleVersion GRADLE_42() {
-        return GradleVersion.version("4.2");
+    @Inject
+    public WorkerProcessFactory getWorkerProcessBuilderFactory() {
+        throw new UnsupportedOperationException();
     }
 
     /**
@@ -237,28 +229,17 @@ public void setExcludeBugsFilter(File filter) {
     }
 
     @TaskAction
-    public void run() {
+    public void run() throws IOException, InterruptedException {
         new SpotBugsClasspathValidator(JavaVersion.current()).validateClasspath(
                 getSpotbugsClasspath().getFiles().stream().map(File::getName).collect(Collectors.toSet()));
         SpotBugsSpec spec = generateSpec();
+        SpotBugsWorkerManager manager = new SpotBugsWorkerManager();
 
-        //workaround for https://github.com/spotbugs/spotbugs-gradle-plugin/issues/61
-        if(reports.getEnabledReports().isEmpty()) {
-            getProject().getLogger().lifecycle("WARNING: No SpotBugs report(s) were configured; aborting execution of {}", getPath());
-            return;
-        }
+        getLogging().captureStandardOutput(LogLevel.DEBUG);
+        getLogging().captureStandardError(LogLevel.DEBUG);
 
-        workerExecutor.submit(SpotBugsRunner.class, config -> {
-            config.params(spec, getIgnoreFailures(), reports.getFirstEnabled().getDestination());
-            config.setClasspath(getSpotbugsClasspath());
-            config.setForkMode(ForkMode.ALWAYS);
-            config.forkOptions( options -> {
-                options.setDebug(spec.isDebugEnabled());
-                options.setJvmArgs(spec.getJvmArgs());
-                options.setMaxHeapSize(spec.getMaxHeapSize());
-            });
-            config.setIsolationMode(IsolationMode.PROCESS);
-        });
+        SpotBugsResult result = manager.runWorker(getProject().getProjectDir(), getWorkerProcessBuilderFactory(), getSpotbugsClasspath(), spec);
+        evaluateResult(result);
     }
 
     SpotBugsSpec generateSpec() {
@@ -283,6 +264,33 @@ SpotBugsSpec generateSpec() {
         return specBuilder.build();
     }
 
+    void evaluateResult(SpotBugsResult result) {
+        if (result.getException() != null) {
+            throw new GradleException("SpotBugs encountered an error. Run with --debug to get more information.", result.getException());
+        }
+
+        if (result.getErrorCount() > 0) {
+            throw new GradleException("SpotBugs encountered an error. Run with --debug to get more information.");
+        }
+
+        if (result.getBugCount() > 0) {
+            String message = "SpotBugs rule violations were found.";
+            SingleFileReport report = reports.getFirstEnabled();
+            if (report != null) {
+                String reportUrl = new ConsoleRenderer().asClickableFileUrl(report.getDestination());
+                message += " See the report at: " + reportUrl;
+            }
+
+            if (getIgnoreFailures()) {
+                getLogger().warn(message);
+            } else {
+                throw new GradleException(message);
+            }
+
+        }
+
+    }
+
     public SpotBugsTask extraArgs(Iterable<String> arguments) {
         for (String argument : arguments) {
             extraArgs.add(argument);

src/main/java/com/github/spotbugs/internal/SpotBugsReportsImpl.java

@@ -1,6 +1,5 @@
 package com.github.spotbugs.internal;
 
-import javax.inject.Inject;
 import org.gradle.api.Task;
 import org.gradle.api.reporting.SingleFileReport;
 import org.gradle.api.reporting.internal.CustomizableHtmlReportImpl;
@@ -12,7 +11,6 @@
 
 public class SpotBugsReportsImpl extends TaskReportContainer<SingleFileReport> implements SpotBugsReportsInternal {
 
-  @Inject
   public SpotBugsReportsImpl(Task task) {
       super(SingleFileReport.class, task);
 

src/main/java/com/github/spotbugs/internal/spotbugs/SpotBugsExecuter.java

@@ -0,0 +1,37 @@
+package com.github.spotbugs.internal.spotbugs;
+
+import java.io.IOException;
+import java.util.List;
+
+import edu.umd.cs.findbugs.FindBugs;
+import edu.umd.cs.findbugs.FindBugs2;
+import edu.umd.cs.findbugs.IFindBugsEngine;
+import edu.umd.cs.findbugs.TextUICommandLine;
+
+public class SpotBugsExecuter implements SpotBugsWorker {
+  @Override
+  public SpotBugsResult runSpotbugs(SpotBugsSpec spec) throws IOException, InterruptedException {
+      final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+      try {
+          final List<String> args = spec.getArguments();
+          String[] strArray = args.toArray(new String[0]);
+
+          Thread.currentThread().setContextClassLoader(FindBugs2.class.getClassLoader());
+          FindBugs2 findBugs2 = new FindBugs2();
+          TextUICommandLine commandLine = new TextUICommandLine();
+          FindBugs.processCommandLine(commandLine, strArray, findBugs2);
+          findBugs2.execute();
+
+          return createSpotbugsResult(findBugs2);
+      } finally {
+          Thread.currentThread().setContextClassLoader(contextClassLoader);
+      }
+  }
+
+  SpotBugsResult createSpotbugsResult(IFindBugsEngine findBugs) {
+          int bugCount = findBugs.getBugCount();
+          int missingClassCount = findBugs.getMissingClassCount();
+          int errorCount = findBugs.getErrorCount();
+          return new SpotBugsResult(bugCount, missingClassCount, errorCount);
+      }
+}