Merge pull request #233 from sparsick/226-spotbug-fix

#226 - review and fix spotbug findings
reficio · Feb 26, 2021 · f5f539a · f5f539a
2 parents 7379b2f + 4cb592d
commit f5f539a
Show file tree

Hide file tree

Showing 12 changed files with 88 additions and 46 deletions.
diff --git a/pom.xml b/pom.xml
@@ -236,6 +236,19 @@
             <version>${tycho.version}</version>
         </dependency>
 
+        <dependency>
+            <groupId>org.codehaus.groovy</groupId>
+            <artifactId>groovy</artifactId>
+            <version>${groovy.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>com.github.spotbugs</groupId>
+            <artifactId>spotbugs-annotations</artifactId>
+            <version>4.2.1</version>
+            <scope>provided</scope>
+        </dependency>
+
+
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
@@ -248,11 +261,6 @@
             <version>3.0.0</version>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.codehaus.groovy</groupId>
-            <artifactId>groovy</artifactId>
-            <version>${groovy.version}</version>
-        </dependency>
     </dependencies>
 
     <build>

diff --git a/src/main/groovy/org/reficio/p2/resolver/eclipse/impl/DefaultEclipseResolver.groovy b/src/main/groovy/org/reficio/p2/resolver/eclipse/impl/DefaultEclipseResolver.groovy
@@ -18,12 +18,14 @@
  */
 package org.reficio.p2.resolver.eclipse.impl
 
-import org.apache.commons.io.FilenameUtils
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
+
 import org.reficio.p2.logger.Logger
 import org.reficio.p2.resolver.eclipse.EclipseResolutionRequest
 import org.reficio.p2.resolver.eclipse.EclipseResolutionResponse
 import org.reficio.p2.resolver.eclipse.EclipseResolver
 
+@SuppressFBWarnings("SE_NO_SERIALVERSIONID")
 class DefaultEclipseResolver implements EclipseResolver {
 
     final File target

diff --git a/src/main/groovy/org/reficio/p2/resolver/eclipse/impl/FileBinaryCategory.groovy b/src/main/groovy/org/reficio/p2/resolver/eclipse/impl/FileBinaryCategory.groovy
@@ -18,6 +18,9 @@
  */
 package org.reficio.p2.resolver.eclipse.impl
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+@SuppressFBWarnings("SE_NO_SERIALVERSIONID")
 class FileBinaryCategory {
     def static leftShift(File file, URL url) {
         url.withInputStream { is ->

diff --git a/src/main/groovy/org/reficio/p2/resolver/maven/impl/AetherResolver.groovy b/src/main/groovy/org/reficio/p2/resolver/maven/impl/AetherResolver.groovy
@@ -18,6 +18,8 @@
  */
 package org.reficio.p2.resolver.maven.impl
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
+
 import org.reficio.p2.logger.Logger
 import org.reficio.p2.resolver.maven.impl.facade.AetherFacade
 import org.reficio.p2.resolver.maven.Artifact
@@ -32,6 +34,7 @@ import org.reficio.p2.resolver.maven.ResolvedArtifact
  *         http://www.reficio.org
  * @since 1.0.0
  */
+@SuppressFBWarnings("SE_NO_SERIALVERSIONID")
 class AetherResolver implements ArtifactResolver {
 
     static final String DEFAULT_SCOPE = "compile"

diff --git a/src/main/groovy/org/reficio/p2/resolver/maven/impl/facade/AetherEclipseFacade.groovy b/src/main/groovy/org/reficio/p2/resolver/maven/impl/facade/AetherEclipseFacade.groovy
@@ -18,6 +18,8 @@
  */
 package org.reficio.p2.resolver.maven.impl.facade
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
+
 import org.eclipse.aether.artifact.Artifact as AetherArtifact
 import org.eclipse.aether.artifact.DefaultArtifact
 import org.eclipse.aether.collection.CollectRequest
@@ -37,6 +39,7 @@ import org.reficio.p2.resolver.maven.Artifact
  *         http://www.reficio.org
  * @since 1.1.0
  */
+@SuppressFBWarnings("SE_NO_SERIALVERSIONID")
 class AetherEclipseFacade implements AetherFacade {
 
     @Override

diff --git a/src/main/groovy/org/reficio/p2/resolver/maven/impl/facade/AetherFacade.groovy b/src/main/groovy/org/reficio/p2/resolver/maven/impl/facade/AetherFacade.groovy
@@ -18,6 +18,7 @@
  */
 package org.reficio.p2.resolver.maven.impl.facade
 
+
 import org.reficio.p2.resolver.maven.Artifact
 
 /**

diff --git a/src/main/groovy/org/reficio/p2/resolver/maven/impl/facade/AetherSonatypeFacade.groovy b/src/main/groovy/org/reficio/p2/resolver/maven/impl/facade/AetherSonatypeFacade.groovy
@@ -18,6 +18,8 @@
  */
 package org.reficio.p2.resolver.maven.impl.facade
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
+
 import org.reficio.p2.resolver.maven.Artifact
 import org.sonatype.aether.artifact.Artifact as AetherArtifact
 import org.sonatype.aether.collection.CollectRequest
@@ -37,6 +39,7 @@ import org.sonatype.aether.util.graph.PreorderNodeListGenerator
  *         http://www.reficio.org
  * @since 1.1.0
  */
+@SuppressFBWarnings("SE_NO_SERIALVERSIONID")
 class AetherSonatypeFacade implements AetherFacade {
 
     @Override

diff --git a/src/main/java/org/reficio/p2/FeatureBuilder.java b/src/main/java/org/reficio/p2/FeatureBuilder.java
@@ -26,6 +26,7 @@
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.reficio.p2.bundler.ArtifactBundlerInstructions;
 import org.reficio.p2.bundler.P2ArtifactMap;
 import org.reficio.p2.logger.Logger;
@@ -56,6 +57,7 @@ public FeatureBuilder(P2FeatureDefinition p2FeatureDefintion,
 	//cache this so that the same timestamp is used
 	private String featureTimeStamp;
 
+	@SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE")
 	public void generate(File destinationFolder) {
 		try {
 			File featureContent = new File(destinationFolder, this.getFeatureFullName());

diff --git a/src/main/java/org/reficio/p2/P2Mojo.java b/src/main/java/org/reficio/p2/P2Mojo.java
@@ -20,6 +20,7 @@
 
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.maven.execution.MavenSession;
@@ -55,12 +56,18 @@
 import org.reficio.p2.utils.Utils;
 
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Set;
 
 import static java.util.Objects.requireNonNull;
@@ -307,9 +314,9 @@ private Set<Artifact> processRootArtifacts(P2ArtifactMap<ResolvedArtifact> proce
                     	ArtifactBundlerInstructions abi = bundleArtifact(p2Artifact, resolvedArtifact);
                     	bundlerInstructions.put(p2Artifact,abi);
                     } else {
-                        String message = String.format("p2-maven-plugin misconfiguration" +
-                                "\n\n\tJar [%s] is configured as an artifact multiple times. " +
-                                "\n\tRemove the duplicate artifact definitions.\n", resolvedArtifact.getArtifact());
+                        String message = String.format(Locale.ENGLISH, "p2-maven-plugin misconfiguration" +
+                                "%n%n\tJar [%s] is configured as an artifact multiple times. " +
+                                "%n\tRemove the duplicate artifact definitions.%n", resolvedArtifact.getArtifact());
                         throw new RuntimeException(message);
                     }
                 }
@@ -334,13 +341,13 @@ private void processTransitiveArtifacts(P2ArtifactMap<ResolvedArtifact> resolved
                             bundlerInstructions.put(p2Artifact,abi);
                         } catch (final RuntimeException ex) {
                             if (skipInvalidArtifacts) {
-                                log.warn(String.format("Skip artifact=[%s]: %s", p2Artifact.getId(), ex.getMessage()));
+                                log.warn(String.format(Locale.ENGLISH,"Skip artifact=[%s]: %s", p2Artifact.getId(), ex.getMessage()));
                             } else {
                                 throw ex;
                             }
                         }
                     } else {
-                        log.debug(String.format("Not bundling transitive dependency since it has already been bundled [%s]", resolvedArtifact.getArtifact()));
+                        log.debug(String.format(Locale.ENGLISH,"Not bundling transitive dependency since it has already been bundled [%s]", resolvedArtifact.getArtifact()));
                     }
                 }
             }
@@ -456,8 +463,7 @@ private void createFeature(P2FeatureDefinition p2featureDefinition) {
 				}
 			} else {
 				//given a feature file, so build using tycho
-				File basedir = p2featureDefinition.getFeatureFile().getParentFile();
-				TychoFeatureBuilder builder = new TychoFeatureBuilder(
+                TychoFeatureBuilder builder = new TychoFeatureBuilder(
 						p2featureDefinition.getFeatureFile(),
 						this.featuresDestinationFolder.getAbsolutePath(),
 						"test.feature",  // these are only dummy values.
@@ -545,14 +551,18 @@ private void executeCategoryPublisher() throws AbstractMojoExecutionException, I
         publisher.execute();
     }
 
+    @SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE")
     private void prepareCategoryLocationFile() throws IOException {
         if (categoryFileURL == null || categoryFileURL.trim().isEmpty()) {
-            InputStream is = getClass().getResourceAsStream(DEFAULT_CATEGORY_CLASSPATH_LOCATION + DEFAULT_CATEGORY_FILE);
-            File destinationFolder = new File(destinationDirectory);
-            destinationFolder.mkdirs();
-            File categoryDefinitionFile = new File(destinationFolder, DEFAULT_CATEGORY_FILE);
-            FileWriter writer = new FileWriter(categoryDefinitionFile);
-            IOUtils.copy(is, writer, "UTF-8");
+            File categoryDefinitionFile;
+            Writer writer;
+            try (InputStream is = getClass().getResourceAsStream(DEFAULT_CATEGORY_CLASSPATH_LOCATION + DEFAULT_CATEGORY_FILE)) {
+                File destinationFolder = new File(destinationDirectory);
+                destinationFolder.mkdirs();
+                categoryDefinitionFile = new File(destinationFolder, DEFAULT_CATEGORY_FILE);
+                writer = new OutputStreamWriter(new FileOutputStream(categoryDefinitionFile), StandardCharsets.UTF_8);
+                IOUtils.copy(is, writer, StandardCharsets.UTF_8);
+            }
             IOUtils.closeQuietly(writer);
             categoryFileURL = categoryDefinitionFile.getAbsolutePath();
         }

diff --git a/src/main/java/org/reficio/p2/P2Validator.java b/src/main/java/org/reficio/p2/P2Validator.java
@@ -22,6 +22,8 @@
 import org.reficio.p2.resolver.maven.ResolvedArtifact;
 import org.reficio.p2.utils.BundleUtils;
 
+import java.util.Locale;
+
 /**
  * @author Tom Bujok (tom.bujok@gmail.com)<br>
  *         Reficio (TM) - Reestablish your software!<br>
@@ -37,7 +39,7 @@ public static void validateBundleRequest(P2Artifact p2Artifact, ResolvedArtifact
 
     private static void validateGeneralConfig(P2Artifact p2Artifact) {
         if (p2Artifact.shouldIncludeTransitive() && !p2Artifact.getCombinedInstructions().isEmpty()) {
-            String message = String.format("BND instructions are NOT applied to the transitive dependencies of %s",
+            String message = String.format(Locale.ENGLISH,"BND instructions are NOT applied to the transitive dependencies of %s",
                     p2Artifact.getId());
             Logger.getLog().warn(message);
         }
@@ -47,7 +49,7 @@ private static void validateGeneralConfig(P2Artifact p2Artifact) {
                 if (!p2Artifact.getInstructions().containsKey(propertyName))
                     continue;
 
-                String message = String.format("BND instruction <%s> from <instructions> " +
+                String message = String.format(Locale.ENGLISH,"BND instruction <%s> from <instructions> " +
                         "is overridden in <instructionsProperties>", propertyName);
                 Logger.getLog().warn(message);
             }
@@ -59,20 +61,20 @@ public static void validateArtifactConfig(P2Artifact p2Artifact, ResolvedArtifac
         if (resolvedArtifact.isRoot() && bundle) {
             // artifact is a bundle and somebody specified instructions without override
             if (!p2Artifact.shouldOverrideManifest() && !p2Artifact.getCombinedInstructions().isEmpty()) {
-                String message = String.format("p2-maven-plugin misconfiguration" +
-                        "\n\n\tJar [%s] is already a bundle. " +
-                        "\n\tBND instructions are specified, but the <override> flag is set to false." +
-                        "\n\tEither remove the instructions or set the <override> flag to true." +
-                        "\n\tWATCH OUT! Setting <override> to true will re-bundle the artifact!\n", resolvedArtifact.getArtifact().toString());
+                String message = String.format(Locale.ENGLISH, "p2-maven-plugin misconfiguration" +
+                        "%n%n\tJar [%s] is already a bundle. " +
+                        "%n\tBND instructions are specified, but the <override> flag is set to false." +
+                        "%n\tEither remove the instructions or set the <override> flag to true." +
+                        "%n\tWATCH OUT! Setting <override> to true will re-bundle the artifact!%n", resolvedArtifact.getArtifact().toString());
                 throw new RuntimeException(message);
             }
             // artifact is a bundle and somebody specified singleton flag without override
             if (!p2Artifact.shouldOverrideManifest() && p2Artifact.isSingleton()) {
-                String message = String.format("p2-maven-plugin misconfiguration" +
-                        "\n\n\tJar [%s] is already a bundle. " +
-                        "\n\tsingleton is set to true, but the <override> flag is set to false." +
-                        "\n\tEither remove the singleton flag or set the <override> flag to true." +
-                        "\n\tWATCH OUT! Setting <override> to true will re-bundle the artifact!\n", resolvedArtifact.getArtifact().toString());
+                String message = String.format(Locale.ENGLISH,"p2-maven-plugin misconfiguration" +
+                        "%n%n\tJar [%s] is already a bundle. " +
+                        "%n\tsingleton is set to true, but the <override> flag is set to false." +
+                        "%n\tEither remove the singleton flag or set the <override> flag to true." +
+                        "%n\tWATCH OUT! Setting <override> to true will re-bundle the artifact!%n", resolvedArtifact.getArtifact().toString());
                 throw new RuntimeException(message);
             }
         }

diff --git a/src/main/java/org/reficio/p2/publisher/CategoryPublisher.java b/src/main/java/org/reficio/p2/publisher/CategoryPublisher.java
@@ -18,6 +18,7 @@
  */
 package org.reficio.p2.publisher;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.maven.plugin.AbstractMojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
 import org.codehaus.plexus.util.FileUtils;
@@ -45,6 +46,7 @@ public class CategoryPublisher {
     private final String categoryFileLocation;
     private final String metadataRepositoryLocation;
 
+    @SuppressFBWarnings("EI_EXPOSE_REP2")
     public CategoryPublisher(P2ApplicationLauncher launcher, int forkedProcessTimeoutInSeconds, String[] additionalArgs,
                              String categoryFileLocation, String metadataRepositoryLocation) {
         this.launcher = launcher;

diff --git a/src/main/java/org/reficio/p2/utils/JarUtils.java b/src/main/java/org/reficio/p2/utils/JarUtils.java
@@ -23,6 +23,7 @@
 import aQute.bnd.osgi.FileResource;
 import aQute.bnd.osgi.Jar;
 import aQute.bnd.osgi.Resource;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.maven.plugin.logging.Log;
@@ -78,6 +79,7 @@ public static void adjustSnapshotOutputVersion(File inputFile, File outputFile,
      * @param timestamp - timestamp
      *
      */
+    @SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE")
     public static void adjustFeatureXml(File inputFile, File outputFile, File pluginDir, Log log, String timestamp) {
         Jar jar = null;
         File newXml = null;
@@ -178,22 +180,23 @@ public static void removeSignature(File jar) {
 
             ZipOutputStream zipOutputStream = new ZipOutputStream(new FileOutputStream(unsignedJar));
             try {
-                ZipFile zip = new ZipFile(jar);
-                for (Enumeration list = zip.entries(); list.hasMoreElements(); ) {
-                    ZipEntry entry = (ZipEntry) list.nextElement();
-                    String name = entry.getName();
-                    if (entry.isDirectory()) {
-                        continue;
-                    } else if (name.endsWith(".RSA") || name.endsWith(".DSA") || name.endsWith(".SF")) {
-                        continue;
-                    }
+                try (ZipFile zip = new ZipFile(jar)) {
+                    for (Enumeration list = zip.entries(); list.hasMoreElements(); ) {
+                        ZipEntry entry = (ZipEntry) list.nextElement();
+                        String name = entry.getName();
+                        if (entry.isDirectory()) {
+                            continue;
+                        } else if (name.endsWith(".RSA") || name.endsWith(".DSA") || name.endsWith(".SF")) {
+                            continue;
+                        }
 
-                    InputStream zipInputStream = zip.getInputStream(entry);
-                    zipOutputStream.putNextEntry(entry);
-                    try {
-                        IOUtils.copy(zipInputStream, zipOutputStream);
-                    } finally {
-                        zipInputStream.close();
+                        InputStream zipInputStream = zip.getInputStream(entry);
+                        zipOutputStream.putNextEntry(entry);
+                        try {
+                            IOUtils.copy(zipInputStream, zipOutputStream);
+                        } finally {
+                            zipInputStream.close();
+                        }
                     }
                 }
                 IOUtils.closeQuietly(zipOutputStream);
-Original file line number
+Diff line change
@@ Expand Up / @@ -18,6 +18,7 @@ @@
      */
     package org.reficio.p2.resolver.maven.impl.facade
     import org.reficio.p2.resolver.maven.Artifact
     /**
@@ Expand Down @@