From d3ebb71237a51b1da74d6923de0e1d3f7c9b962e Mon Sep 17 00:00:00 2001 From: Philippe Arteau Date: Tue, 16 Aug 2016 18:07:28 -0400 Subject: [PATCH] Fix a bug that fail to map bug instance to outer class #40 --- .../plugins/findbugs/FindbugsSensor.java | 10 +++- .../resource/ByteCodeResourceLocator.java | 34 ++++++------ .../resource/ByteCodeResourceLocatorTest.java | 54 ++++++++++--------- 3 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java index fbe4029b..14ba241b 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java @@ -127,10 +127,18 @@ public void execute(SensorContext context) { continue; } - //Class file (compiled bytecode with potential SMAP metadata) + //Locate the original class file File classFile = findOriginalClassForBug(bugInstance.getClassName()); + //If the class was an outer class, the source file will not be analog to the class name. + //The original source file is available in the class file metadata. + resource = byteCodeResourceLocator.findJavaOuterClassFile(className, classFile, this.fs); + if (resource != null) { + insertIssue(rule, resource, line, longMessage); + continue; + } + //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); diff --git a/src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java b/src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java index e89ffb01..22952d88 100644 --- a/src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java +++ b/src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java @@ -19,17 +19,14 @@ */ package org.sonar.plugins.findbugs.resource; +import org.apache.commons.io.FilenameUtils; import org.apache.commons.io.IOUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.BatchExtension; import org.sonar.api.batch.fs.FileSystem; import org.sonar.api.batch.fs.InputFile; -import org.sonar.api.resources.Project; -import org.sonar.api.resources.Resource; -import org.sonar.plugins.java.api.JavaResourceLocator; -import java.awt.event.KeyEvent; import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -60,20 +57,19 @@ public InputFile findJavaClassFile(String className, FileSystem fs) { className = className.substring(0, indexDollarSign); //Remove innerClass from the class name } - Iterable files = fs.inputFiles(fs.predicates().hasRelativePath("src/main/java/"+className.replaceAll("\\.","/")+".java")); - - for(InputFile f: files) { - return f; - } - return null; + return buildInputFile(className.replaceAll("\\.","/")+".java", fs); } - public InputFile findSourceFile(File classFile, FileSystem fs) { + public InputFile findJavaOuterClassFile(String className, File classFile, FileSystem fs) { try (InputStream in = new FileInputStream(classFile)) { DebugExtensionExtractor debug = new DebugExtensionExtractor(); - String sourceFile = debug.getDebugExtFromClass(in); + String source = debug.getDebugSourceFromClass(in); - return buildInputFile(sourceFile, fs); + if(source == null) return null; + String newClassName = FilenameUtils.getBaseName(source); + String packagePrefix = className.lastIndexOf(".") != -1 ? FilenameUtils.getBaseName(className) + "." : ""; + String fullClassName = packagePrefix + newClassName; + return findJavaClassFile(fullClassName, fs); } catch (IOException e) { LOG.warn("An error occurs while opening classfile : " + classFile.getPath()); @@ -125,8 +121,9 @@ public InputFile findTemplateFile(String className, FileSystem fs) { } public InputFile buildInputFile(String fileName,FileSystem fs) { - for(String sourceDir : Arrays.asList("src/main/webapp/","src/main/resources","src/main/java","src")) { - Iterable files = fs.inputFiles(fs.predicates().hasRelativePath(sourceDir+fileName)); + for(String sourceDir : Arrays.asList("src/main/java","src/main/webapp","src/main/resources","src")) { + System.out.println("Source file tested : "+sourceDir+"/"+fileName); + Iterable files = fs.inputFiles(fs.predicates().hasRelativePath(sourceDir+"/"+fileName)); for (InputFile f : files) { return f; } @@ -163,11 +160,11 @@ public SmapParser.SmapLocation extractSmapLocation(String className, int origina } catch (IOException e) { LOG.debug("Unable to open smap file : " + smapFile.getAbsolutePath()); - throw new RuntimeException(e); } } - - LOG.debug("No smap mapping found."); + else { + LOG.debug("No smap mapping found."); + } return null; //No smap file is present. } @@ -182,4 +179,5 @@ private SmapParser.SmapLocation getJspLineNumberFromSmap(String smap, Integer or SmapParser parser = new SmapParser(smap); return parser.getSmapLocation(originalLine); } + } diff --git a/src/test/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocatorTest.java b/src/test/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocatorTest.java index 1aa740d9..ea5bd9a8 100644 --- a/src/test/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocatorTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocatorTest.java @@ -10,44 +10,53 @@ import java.util.ArrayList; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class ByteCodeResourceLocatorTest { - FileSystem fs; - FilePredicates predicates; + + //File system that return mock input files +// FileSystem fs; +// FilePredicates predicates; + + //File system that return no Input files + FileSystem fsEmpty; + FilePredicates predicatesEmpty; @Before public void setUp() { - fs = mock(FileSystem.class); - predicates = mock(FilePredicates.class); - when(fs.predicates()).thenReturn(predicates); + //Not used for the moment +// fs = mock(FileSystem.class); +// predicates = mock(FilePredicates.class); +// when(fs.predicates()).thenReturn(predicates); + + fsEmpty = mock(FileSystem.class); + predicatesEmpty = mock(FilePredicates.class); + when(fsEmpty.predicates()).thenReturn(predicatesEmpty); + when(fsEmpty.inputFiles(any(FilePredicate.class))).thenReturn(new ArrayList()); } @Test public void findJavaClassFile_normalClassName() { - //No Input file need to be return for the test - when(fs.inputFiles(Matchers.any())).thenReturn(new ArrayList()); ByteCodeResourceLocator locator = new ByteCodeResourceLocator(); - locator.findJavaClassFile("com.helloworld.ThisIsATest",fs); + locator.findJavaClassFile("com.helloworld.ThisIsATest", fsEmpty); - verify(predicates,times(1)).hasRelativePath("src/main/java/com/helloworld/ThisIsATest.java"); + verify(predicatesEmpty,times(1)).hasRelativePath("src/main/java/com/helloworld/ThisIsATest.java"); } @Test public void findJavaClassFile_withInnerClass() { - //No Input file need to be return for the test - when(fs.inputFiles(Matchers.any())).thenReturn(new ArrayList()); ByteCodeResourceLocator locator = new ByteCodeResourceLocator(); - locator.findJavaClassFile("com.helloworld.ThisIsATest$InnerClass",fs); + locator.findJavaClassFile("com.helloworld.ThisIsATest$InnerClass",fsEmpty); - verify(predicates,times(1)).hasRelativePath("src/main/java/com/helloworld/ThisIsATest.java"); + verify(predicatesEmpty,times(1)).hasRelativePath("src/main/java/com/helloworld/ThisIsATest.java"); } @Test @@ -55,12 +64,9 @@ public void findTemplateFile_weblogicFileName() { ByteCodeResourceLocator locator = new ByteCodeResourceLocator(); - //No Input file need to be return for the test - when(fs.inputFiles(Matchers.any())).thenReturn(new ArrayList()); + locator.findTemplateFile("jsp_servlet._folder1._folder2.__helloworld", fsEmpty); - locator.findTemplateFile("jsp_servlet._folder1._folder2.__helloworld", fs); - - verify(predicates,times(1)).hasRelativePath("src/main/webapp//folder1/folder2/helloworld.jsp"); + verify(predicatesEmpty,times(1)).hasRelativePath("src/main/webapp//folder1/folder2/helloworld.jsp"); } @Test @@ -72,17 +78,13 @@ public void findTemplateFile_jasperFileName() { for(String jspPage : pages) { String name = "org.apache.jsp." + JspUtils.makeJavaPackage(jspPage); - System.out.println(name); + System.out.println("Compiled class name: "+name); ByteCodeResourceLocator locator = new ByteCodeResourceLocator(); + locator.findTemplateFile(name, fsEmpty); - //No Input file need to be return for the test - when(fs.inputFiles(Matchers.any())).thenReturn(new ArrayList()); - - locator.findTemplateFile(name, fs); - - System.out.println("Expecting: "+prefixSource + jspPage); - verify(predicates,times(1)).hasRelativePath(prefixSource + jspPage); + System.out.println("Expecting: "+ prefixSource + jspPage); + verify(predicatesEmpty,times(1)).hasRelativePath(prefixSource + jspPage); } }