Skip to content

Commit

Permalink
Merge pull request spotbugs#45 from h3xstream/fix/outerclass_bug
Browse files Browse the repository at this point in the history 
Fix a bug that fail to map bug instance to outer class spotbugs#40
  • Loading branch information
@h3xstream
h3xstream authored Aug 16, 2016
2 parents 50374cc + d3ebb71 commit 9f646a0
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 45 deletions.
10 changes: 9 additions & 1 deletion src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
34 changes: 16 additions & 18 deletions src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -60,20 +57,19 @@ public InputFile findJavaClassFile(String className, FileSystem fs) {
className = className.substring(0, indexDollarSign); //Remove innerClass from the class name
}

Iterable<InputFile> 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());
Expand Down Expand Up @@ -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<InputFile> 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<InputFile> files = fs.inputFiles(fs.predicates().hasRelativePath(sourceDir+"/"+fileName));
for (InputFile f : files) {
return f;
}
Expand Down Expand Up @@ -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.
}

Expand All @@ -182,4 +179,5 @@ private SmapParser.SmapLocation getJspLineNumberFromSmap(String smap, Integer or
SmapParser parser = new SmapParser(smap);
return parser.getSmapLocation(originalLine);
}

}
54 changes: 28 additions & 26 deletions src/test/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,57 +10,63 @@

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<InputFile>());
}


@Test
public void findJavaClassFile_normalClassName() {
//No Input file need to be return for the test
when(fs.inputFiles(Matchers.<FilePredicate>any())).thenReturn(new ArrayList<InputFile>());

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.<FilePredicate>any())).thenReturn(new ArrayList<InputFile>());

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
public void findTemplateFile_weblogicFileName() {

ByteCodeResourceLocator locator = new ByteCodeResourceLocator();

//No Input file need to be return for the test
when(fs.inputFiles(Matchers.<FilePredicate>any())).thenReturn(new ArrayList<InputFile>());
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
Expand All @@ -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.<FilePredicate>any())).thenReturn(new ArrayList<InputFile>());

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);
}

}
Expand Down

0 comments on commit 9f646a0

Please sign in to comment.