Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor refactoring #55

Merged
merged 4 commits into from
Sep 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down Expand Up @@ -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();
Expand Down
47 changes: 26 additions & 21 deletions src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>)
Expand Down Expand Up @@ -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;
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
Loading