Merge pull request spotbugs#55 from h3xstream/fix/minor_refactoring

Minor refactoring

pom.xml

@@ -4,7 +4,7 @@
 
   <groupId>org.sonarsource.sonar-findbugs-plugin</groupId>
   <artifactId>sonar-findbugs-plugin</artifactId>
-  <version>3.4.3</version>
+  <version>3.4.4-SNAPSHOT</version>
   <packaging>sonar-plugin</packaging>
 
   <name>SonarQube Findbugs Plugin</name>

src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java

@@ -188,7 +188,9 @@ public static List<File> scanForAdditionalClasses(File folder) throws IOExceptio
     Queue<File> dirs = new LinkedList<File>();
     dirs.add(folder);
     while (!dirs.isEmpty()) {
-      for (File f : dirs.poll().listFiles()) {
+      File dirPoll = dirs.poll();
+      if(dirPoll == null) break; //poll() result could be null if the queue is empty.
+      for (File f : dirPoll.listFiles()) {
         if (f.isDirectory()) {
           dirs.add(f);
         } else if (f.isFile()&& f.getName().endsWith(".class")) {
@@ -248,6 +250,7 @@ public void copyLibs() {
   /**
    * Invoked by PicoContainer to remove temporary files.
    */
+  @SuppressWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE")
   public void stop() {
     if (jsr305Lib != null) {
       jsr305Lib.delete();

src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java

@@ -36,6 +36,7 @@
 import org.sonar.api.profiles.RulesProfile;
 import org.sonar.plugins.findbugs.language.Jsp;
 import org.sonar.plugins.findbugs.resource.ByteCodeResourceLocator;
+import org.sonar.plugins.findbugs.resource.ClassMetadataLoadingException;
 import org.sonar.plugins.findbugs.resource.SmapParser;
 import org.sonar.plugins.findbugs.rules.FbContribRulesDefinition;
 import org.sonar.plugins.findbugs.rules.FindSecurityBugsJspRulesDefinition;
@@ -94,8 +95,6 @@ public void execute(SensorContext context) {
 
     Collection<ReportedBug> collection = executor.execute(hasActiveFbContribRules(), hasActiveFindSecBugsRules() || hasActiveFindSecBugsJspRules());
 
-    Set<String> locationReported = new HashSet<>();
-
     for (ReportedBug bugInstance : collection) {
 
       try {
@@ -141,31 +140,37 @@ public void execute(SensorContext context) {
         //More advanced mapping if the original source is not Java files
         if (classFile != null) {
           //Attempt to load SMAP debug metadata
-          SmapParser.SmapLocation location = byteCodeResourceLocator.extractSmapLocation(className, line, classFile);
-          if (location != null) {
-            if (!location.isPrimaryFile) //Avoid reporting issue in double when a source file was include inline
-              continue;
-
-            //SMAP was found
-            resource = byteCodeResourceLocator.buildInputFile(location.fileInfo.path, fs);
-            if (resource != null) {
-              insertIssue(rule, resource, location.line, longMessage);
-              continue;
-            }
-          } else {
-            //SMAP was not found or unparsable.. The orgininal source file will be guess based on the class name
-            resource = byteCodeResourceLocator.findTemplateFile(className, this.fs);
-            if (resource != null) {
-              insertIssue(rule, resource, line, longMessage);
-              continue;
+          try {
+            SmapParser.SmapLocation location = byteCodeResourceLocator.extractSmapLocation(className, line, classFile);
+            if (location != null) {
+              if (!location.isPrimaryFile) { //Avoid reporting issue in double when a source file was include inline
+                continue;
+              }
+
+              //SMAP was found
+              resource = byteCodeResourceLocator.buildInputFile(location.fileInfo.path, fs);
+              if (resource != null) {
+                insertIssue(rule, resource, location.line, longMessage);
+                continue;
+              }
+            } else {
+              //SMAP was not found or unparsable.. The orgininal source file will be guess based on the class name
+              resource = byteCodeResourceLocator.findTemplateFile(className, this.fs);
+              if (resource != null) {
+                insertIssue(rule, resource, line, longMessage);
+                continue;
+              }
             }
           }
+          catch (ClassMetadataLoadingException e) {
+            LOG.warn("Failed to load the class file metadata", e);
+          }
         }
 
-        LOG.warn("The class '" + className + "' could not be match to its original source file. It might be a dynamically generated class.");
+        LOG.warn("The class '" + className + "' could not be matched to its original source file. It might be a dynamically generated class.");
       }
       catch (Exception e) {
-        String bugInstanceDebug = String.format("[BugInstance type=%s, line=%s]", bugInstance.getType(),bugInstance.getStartLine());
+        String bugInstanceDebug = String.format("[BugInstance type=%s, class=%s, line=%s]", bugInstance.getType(), bugInstance.getClassName(), bugInstance.getStartLine());
         LOG.warn("An error occurs while processing the bug instance "+bugInstanceDebug,e);
         //Continue to the bug without aborting the report
       }

src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java

@@ -27,6 +27,7 @@
 import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
 
+import javax.annotation.Nullable;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -43,6 +44,8 @@ public class ByteCodeResourceLocator implements BatchExtension {
 
     private static final Logger LOG = LoggerFactory.getLogger(ByteCodeResourceLocator.class);
 
+    private static final String[] SOURCE_DIRECTORIES = {"src/main/java","src/main/webapp","src/main/resources", "src", "/src/java"};
+
     /**
      * Find the file system location of a given class name.<br/>
      * (ie : <code>test.SomeClass</code> ->  <code>src/main/java/test/SomeClass.java</code>)
@@ -121,8 +124,8 @@ public InputFile findTemplateFile(String className, FileSystem fs) {
     }
 
     public InputFile buildInputFile(String fileName,FileSystem fs) {
-        for(String sourceDir : Arrays.asList("src/main/java","src/main/webapp","src/main/resources","src")) {
-            System.out.println("Source file tested : "+sourceDir+"/"+fileName);
+        for(String sourceDir : SOURCE_DIRECTORIES) {
+            //System.out.println("Source file tested : "+sourceDir+"/"+fileName);
             Iterable<InputFile> files = fs.inputFiles(fs.predicates().hasRelativePath(sourceDir+"/"+fileName));
             for (InputFile f : files) {
                 return f;
@@ -141,11 +144,14 @@ public InputFile buildInputFile(String fileName,FileSystem fs) {
      * @param classFile (Optional)
      * @return JSP line number
      */
+    @Nullable
     public SmapParser.SmapLocation extractSmapLocation(String className, int originalLine, File classFile) {
         //Extract the SMAP (JSR45) from the class file (SourceDebugExtension section)
         try (InputStream in = new FileInputStream(classFile)) {
             DebugExtensionExtractor debug = new DebugExtensionExtractor();
-            return getJspLineNumberFromSmap(debug.getDebugExtFromClass(in), originalLine);
+            String smap = debug.getDebugExtFromClass(in);
+            if(smap != null)
+                return getJspLineNumberFromSmap(smap, originalLine);
         }
         catch (IOException e) {
             LOG.warn("An error occurs while opening classfile : " + classFile.getPath());

src/main/java/org/sonar/plugins/findbugs/resource/ClassMetadataLoadingException.java

@@ -0,0 +1,8 @@
+package org.sonar.plugins.findbugs.resource;
+
+public class ClassMetadataLoadingException extends RuntimeException {
+
+    public ClassMetadataLoadingException(Throwable cause) {
+        super("ASM failed to load classfile metadata", cause);
+    }
+}

src/main/java/org/sonar/plugins/findbugs/resource/DebugExtensionExtractor.java

@@ -23,6 +23,7 @@
 import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.Opcodes;
 
+import javax.annotation.Nullable;
 import java.io.IOException;
 import java.io.InputStream;
 
@@ -34,25 +35,36 @@
  */
 public class DebugExtensionExtractor {
 
+    @Nullable
     public String getDebugExtFromClass(InputStream classIn) throws IOException {
 
         AbstractClassVisitor visitor = new AbstractClassVisitor();
-        ClassReader classReader= new ClassReader(classIn);
-        classReader.accept(visitor, 0);
+        try {
+            ClassReader classReader = new ClassReader(classIn);
+            classReader.accept(visitor, 0);
 
-        return visitor.debug;
+            return visitor.debug;
+        }
+        catch (Exception e) {
+            throw new ClassMetadataLoadingException(e);
+        }
     }
 
     public String getDebugSourceFromClass(InputStream classIn) throws IOException {
 
         AbstractClassVisitor visitor = new AbstractClassVisitor();
-        ClassReader classReader= new ClassReader(classIn);
-        classReader.accept(visitor, 0);
+        try {
+            ClassReader classReader= new ClassReader(classIn);
+            classReader.accept(visitor, 0);
 
-        return visitor.source;
+            return visitor.source;
+        }
+        catch (Exception e) {
+            throw new ClassMetadataLoadingException(e);
+        }
     }
 
-    private class AbstractClassVisitor extends ClassVisitor {
+    private static class AbstractClassVisitor extends ClassVisitor {
 
         protected String source;
         protected String debug;