Skip to content

Commit

Permalink
enhance report file name handling and issue counter
Browse files Browse the repository at this point in the history 
see CXX issue #102
  • Loading branch information
@Bertk
Bertk committed May 2, 2014
1 parent f53b1d8 commit 2139e1d
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 114 deletions.
74 changes: 18 additions & 56 deletions sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/compiler/CxxCompilerSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,20 @@
*/
package org.sonar.plugins.cxx.compiler;

import java.io.File;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;

import org.apache.commons.lang.StringUtils;
import org.sonar.api.batch.SensorContext;
import org.sonar.api.config.Settings;
import org.sonar.api.profiles.RulesProfile;
import org.sonar.api.resources.Project;
import org.sonar.api.rules.RuleFinder;
import org.sonar.api.scan.filesystem.ModuleFileSystem;
import org.sonar.plugins.cxx.utils.CxxReportSensor;
import org.sonar.plugins.cxx.utils.CxxUtils;
import org.sonar.api.scan.filesystem.ModuleFileSystem;

import java.io.File;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;

/**
* compiler for C++ with advanced analysis features (e.g. for VC 2008 team edition or 2010/2012/2013 premium edition)
Expand Down Expand Up @@ -75,8 +73,9 @@ private void addCompilerParser(CompilerParser parser) {
private CompilerParser getCompilerParser() {
String parserKey = getStringProperty(PARSER_KEY_DEF, DEFAULT_PARSER_DEF);
CompilerParser parser = parsers.get(parserKey);
if (parser == null)
parser = parsers.get(DEFAULT_PARSER_DEF);
if (parser == null) {

This comment has been minimized.

Sign in to view

Copy link
@wenns

wenns May 8, 2014

May sound picky, but I dont like mixing style changes with logic changes. I dont reject style chnanges per se, but please do them in a separate commit. This comment applies to half of the changes in this commit.

This comment has been minimized.

Sign in to view

Copy link
@Bertk

Bertk May 10, 2014

Author Owner

Maybe it is a good idea to define the style within the Java profile itself and add it to git.
I use "squid:RightCurlyBraceSameLineAsNextBlockCheck" or "squid:LeftCurlyBraceEndLineCheck" because I see more lines on the screen but this only a personal setting.

This comment has been minimized.

Sign in to view

Copy link
@wenns

wenns May 10, 2014

My comment is not about style (reference style is, by the way, still the ones mentioned on this page:
http://docs.codehaus.org/display/SONAR/Hosting+on+the+Forge)
but about mixing logic changes with style ones in the same commit. This complicates maintaining a codebase.
parser = parsers.get(DEFAULT_PARSER_DEF);
}
return parser;
}

Expand Down Expand Up @@ -107,31 +106,30 @@ protected String defaultReportPath() {
* @return Value of the property if set and not empty, else default value.
*/
public String getParserStringProperty(String name, String def) {
String s = getStringProperty(name, "");
if (StringUtils.isEmpty(s))
return def;
return s;
String s = getStringProperty(name, "");
if (StringUtils.isEmpty(s)) {
return def;
}
return s;
}

@Override
protected void processReport(final Project project, final SensorContext context, File report)
throws javax.xml.stream.XMLStreamException
{
throws javax.xml.stream.XMLStreamException {
int countViolations = 0;
final CompilerParser parser = getCompilerParser();
final String reportCharset = getParserStringProperty(REPORT_CHARSET_DEF, parser.defaultCharset());
final String reportRegEx = getParserStringProperty(REPORT_REGEX_DEF, parser.defaultRegexp());
final List<CompilerParser.Warning> warnings = new LinkedList<CompilerParser.Warning>();

// Iterate through the lines of the input file
CxxUtils.LOG.info("Scanner '" + parser.key() + "' initialized with report '{}'" + ", CharSet= '" + reportCharset + "'", report);
CxxUtils.LOG.info("Scanner '" + parser.key() + "' initialized with report '" + report + "', CharSet= '" + reportCharset + "'" );

This comment has been minimized.

Sign in to view

Copy link
@wenns

wenns May 8, 2014

curious: why do you prefer this notation over the previous one? Are there any advantanges Im not aware of?

This comment has been minimized.

Sign in to view

Copy link
@Bertk

Bertk May 10, 2014

Author Owner

I use a following log settings for tests (sonar-cxx-plugin\src\test\resources\logback-test.xml) and compilation fails with the other notation. By heart I did not investigate the issue.

<?xml version="1.0" encoding="UTF-8" ?>
<configuration>

  <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
    <layout>
      <pattern>
        %d{HH:mm:ss.SSS} %-5level %logger{36} - %msg%n
      </pattern>
    </layout>
  </appender>

  <root>
    <level value="DEBUG"/>
    <appender-ref ref="STDOUT" />
  </root>

</configuration>
try {
parser.ParseReport(report, reportCharset, reportRegEx, warnings);
for(CompilerParser.Warning w : warnings) {
// get filename from file system - e.g. VC writes case insensitive file name to html
String filename = getCaseSensitiveFileName(w.filename, fs.sourceDirs());
if (isInputValid(filename, w.line, w.id, w.msg)) {
if (saveUniqueViolation(project, context, parser.rulesRepositoryKey(), filename, w.line, w.id, w.msg)) {
if (isInputValid(w.filename, w.line, w.id, w.msg)) {
if (saveUniqueViolation(project, context, parser.rulesRepositoryKey(), w.filename, w.line, w.id, w.msg)) {
countViolations++;
}
} else {
Expand All @@ -150,40 +148,4 @@ private boolean isInputValid(String file, String line, String id, String msg) {
return !StringUtils.isEmpty(file) && !StringUtils.isEmpty(line)
&& !StringUtils.isEmpty(id) && !StringUtils.isEmpty(msg);
}

/**
* Supports full path and relative path in report.xml file.
*/
private String getCaseSensitiveFileName(String file, List<java.io.File> sourceDirs) {
// check whether the report file uses absolute path
File targetfile = new java.io.File(file);
if (targetfile.exists()) {
file = getRealFileName(targetfile);
} else {
Iterator<java.io.File> iterator = sourceDirs.iterator();
while (iterator.hasNext()) {
targetfile = new java.io.File(iterator.next().getPath() + java.io.File.separatorChar + file);
if (targetfile.exists()) {
file = getRealFileName(targetfile);
break;
}
}
}
return file;
}

/**
* Find the case sensitive file name - tools might use different naming schema
* e.g. VC HTML or build log report uses case insensitive file name (lower case on windows)
*/
private String getRealFileName( File filename){
try {
return filename.getCanonicalFile().getAbsolutePath();
} catch (java.io.IOException e) {
CxxUtils.LOG.error("SaveViolation GetRealFileName failed '{}'", e.toString());
}
return filename.getName();
}


}
10 changes: 6 additions & 4 deletions sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/cppcheck/CppcheckParserV1.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,22 +61,24 @@ public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException {
} catch (com.ctc.wstx.exc.WstxEOFException eofExc) {
throw new EmptyReportException();
}

int countIssues = 0;
try {
SMInputCursor errorCursor = rootCursor.childElementCursor("error"); // error
SMInputCursor errorCursor = rootCursor.childElementCursor("error");
while (errorCursor.getNext() != null) {
String file = errorCursor.getAttrValue("file");
String line = errorCursor.getAttrValue("line");
String id = errorCursor.getAttrValue("id");
//String severity = errorCursor.getAttrValue("severity");
String msg = errorCursor.getAttrValue("msg");

if (isInputValid(file, line, id, msg)) {
sensor.saveUniqueViolation(project, context, CxxCppCheckRuleRepository.KEY, file, line, id, msg);
if (sensor.saveUniqueViolation(project, context, CxxCppCheckRuleRepository.KEY, file, line, id, msg)) {
countIssues++;
}
} else {
CxxUtils.LOG.warn("Skipping invalid violation: '{}'", msg);
}
}
CxxUtils.LOG.info("CppCheck issues processed = " + countIssues);
} catch (RuntimeException e) {
parsed = false;
throw new XMLStreamException();
Expand Down
17 changes: 9 additions & 8 deletions sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/cppcheck/CppcheckParserV2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,39 +57,40 @@ public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException {
parsed = true;

try {
rootCursor.advance(); // results
rootCursor.advance();
} catch (com.ctc.wstx.exc.WstxEOFException eofExc) {
throw new EmptyReportException();
}

try {
String version = rootCursor.getAttrValue("version");
if (version.equals("2")) {
SMInputCursor errorsCursor = rootCursor.childElementCursor("errors"); // errors
int countIssues = 0;
SMInputCursor errorsCursor = rootCursor.childElementCursor("errors");
if (errorsCursor.getNext() != null) {
SMInputCursor errorCursor = errorsCursor.childElementCursor("error"); // error
SMInputCursor errorCursor = errorsCursor.childElementCursor("error");
while (errorCursor.getNext() != null) {
String id = errorCursor.getAttrValue("id");
//String severity = errorCursor.getAttrValue("severity");
String msg = errorCursor.getAttrValue("msg");
//String verbose = errorCursor.getAttrValue("verbose");
//String inconclusive = errorCursor.getAttrValue("inconclusive");
String file = null;
String line = null;

SMInputCursor locationCursor = errorCursor.childElementCursor("location"); // location
SMInputCursor locationCursor = errorCursor.childElementCursor("location");
if (locationCursor.getNext() != null) {
file = locationCursor.getAttrValue("file");
line = locationCursor.getAttrValue("line");
}

if (isInputValid(file, line, id, msg)) {
sensor.saveUniqueViolation(project, context, CxxCppCheckRuleRepository.KEY, file, line, id, msg);
if (sensor.saveUniqueViolation(project, context, CxxCppCheckRuleRepository.KEY, file, line, id, msg)) {
countIssues++;
}
} else {
CxxUtils.LOG.warn("Skipping invalid violation: '{}'", msg);
}
}
}
CxxUtils.LOG.info("CppCheck issues processed = " + countIssues);
}
} catch (RuntimeException e) {
parsed = false;
Expand Down
30 changes: 13 additions & 17 deletions sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/pclint/CxxPCLintSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
*/
package org.sonar.plugins.cxx.pclint;

import java.io.File;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.xml.stream.XMLStreamException;

import org.apache.commons.lang.StringUtils;
import org.codehaus.staxmate.in.SMHierarchicCursor;
import org.codehaus.staxmate.in.SMInputCursor;
Expand All @@ -27,18 +33,11 @@
import org.sonar.api.profiles.RulesProfile;
import org.sonar.api.resources.Project;
import org.sonar.api.rules.RuleFinder;
import org.sonar.api.scan.filesystem.ModuleFileSystem;
import org.sonar.api.utils.StaxParser;
import org.sonar.plugins.cxx.utils.CxxReportSensor;
import org.sonar.plugins.cxx.utils.CxxUtils;
import org.sonar.plugins.cxx.utils.EmptyReportException;
import org.sonar.api.scan.filesystem.ModuleFileSystem;

import javax.xml.stream.XMLStreamException;

import java.io.File;
import java.util.HashSet;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* PC-lint is an equivalent to pmd but for C++
Expand Down Expand Up @@ -82,22 +81,21 @@ protected String defaultReportPath() {

@Override
protected void processReport(final Project project, final SensorContext context, File report)
throws javax.xml.stream.XMLStreamException
{
throws javax.xml.stream.XMLStreamException {
StaxParser parser = new StaxParser(new StaxParser.XmlStreamHandler() {
/**
* {@inheritDoc}
*/

public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException {
try{
rootCursor.advance(); // results
rootCursor.advance();
}
catch(com.ctc.wstx.exc.WstxEOFException eofExc){
throw new EmptyReportException();
}

SMInputCursor errorCursor = rootCursor.childElementCursor("issue"); // error
SMInputCursor errorCursor = rootCursor.childElementCursor("issue");
int countViolations = 0;
try {
while (errorCursor.getNext() != null){
Expand All @@ -117,13 +115,11 @@ public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException {
}
} else {
CxxUtils.LOG.warn("PC-lint warning ignored: {}", msg);

String debugText = "File: " + file + ", Line: " + line +
", ID: " + id + ", msg: " + msg;
CxxUtils.LOG.debug(debugText);
CxxUtils.LOG.debug("File: " + file + ", Line: " + line + ", ID: "
+ id + ", msg: " + msg);
}
}
CxxUtils.LOG.info("PC-Lint messages processed = " + countViolations);
CxxUtils.LOG.info("PC-lint issues processed = " + countViolations);
} catch (com.ctc.wstx.exc.WstxUnexpectedCharException e) {
CxxUtils.LOG.error("Ignore XML error from PC-lint '{}'", e.toString());
}
Expand Down
15 changes: 8 additions & 7 deletions sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/rats/CxxRatsSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,11 @@ protected String defaultReportPath() {

@Override
protected void processReport(Project project, SensorContext context, File report)
throws org.jdom.JDOMException, java.io.IOException
{
try
{
throws org.jdom.JDOMException, java.io.IOException {
try {
SAXBuilder builder = new SAXBuilder(false);
Element root = builder.build(report).getRootElement();

int countIssues = 0;
@SuppressWarnings("unchecked")
List<Element> vulnerabilities = root.getChildren("vulnerability");
for (Element vulnerability : vulnerabilities) {
Expand All @@ -94,11 +92,14 @@ protected void processReport(Project project, SensorContext context, File report
List<Element> lines = file.getChildren("line");
for (Element lineElem : lines) {
String line = lineElem.getTextTrim();
saveUniqueViolation(project, context, CxxRatsRuleRepository.KEY,
fileName, line, type, message);
if (saveUniqueViolation(project, context, CxxRatsRuleRepository.KEY,
fileName, line, type, message)) {
countIssues++;
}
}
}
}
CxxUtils.LOG.info("RATS issues processed = " + countIssues);
} catch (org.jdom.input.JDOMParseException e) {
// when RATS fails the XML file might be incomplete
CxxUtils.LOG.error("Ignore incomplete XML output from RATS '{}'", e.toString());
Expand Down
33 changes: 21 additions & 12 deletions sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void analyse(Project project, SensorContext context) {
List<File> reports = getReports(conf, fs.baseDir().getPath(),
reportPathKey(), defaultReportPath());
for (File report : reports) {
CxxUtils.LOG.info("Processing report '{}'", report);
CxxUtils.LOG.info("Processing report '" + report + "'");
try{
processReport(project, context, report);
}
Expand Down Expand Up @@ -106,10 +106,11 @@ public String toString() {
}

public String getStringProperty(String name, String def) {
String value = conf.getString(name);
if (value == null)
value = def;
return value;
String value = conf.getString(name);
if (value == null) {
value = def;
}
return value;
}

protected List<File> getReports(Settings conf,
Expand Down Expand Up @@ -158,7 +159,7 @@ public boolean saveUniqueViolation(Project project, SensorContext context, Strin
* Project or file-level violations can be saved by passing null for the according parameters
* ('file' = 'line' = null for project level, 'line' = null for file-level)
*/
public boolean saveViolation(Project project, SensorContext context, String ruleRepoKey,
public boolean saveViolation(Project module, SensorContext context, String ruleRepoKey,
String file, String line, String ruleId, String msg) {
boolean added = false;
RuleQuery ruleQuery = RuleQuery.create()
Expand All @@ -171,11 +172,19 @@ public boolean saveViolation(Project project, SensorContext context, String rule
if ((file != null) && (file.length() > 0)){
String normalPath = CxxUtils.normalizePath(file);
if(normalPath != null){
org.sonar.api.resources.File resource =
org.sonar.api.resources.File.fromIOFile(new File(normalPath), project);
if (context.getResource(resource) != null) {
org.sonar.api.resources.File sonarFile =

This comment has been minimized.

Sign in to view

Copy link
@wenns

wenns May 8, 2014

This is all not needed anymore. Its all done in normalizePath. All casing issues should be solved, this is tested by several persons. Did you encounter cases where it doesnt work?
org.sonar.api.resources.File.fromIOFile(new File(normalPath), module);
if (sonarFile == null) {
// support SQ<4.2
sonarFile = org.sonar.api.resources.File.fromIOFile(new File(normalPath), module.getFileSystem().getTestDirs());
}
if (sonarFile == null){
normalPath = CxxUtils.getCaseSensitiveFileName(file, fs);
sonarFile = org.sonar.api.resources.File.fromIOFile(new File(normalPath), module);
}
if (context.getResource(sonarFile ) != null) {
// file level violation
violation = Violation.create(rule, resource);
violation = Violation.create(rule, sonarFile);

// considering the line information for file level violations only
if (line != null){
Expand All @@ -184,7 +193,7 @@ public boolean saveViolation(Project project, SensorContext context, String rule
linenr = linenr == 0 ? 1 : linenr;
violation.setLineId(linenr);
} catch(java.lang.NumberFormatException nfe){
CxxUtils.LOG.warn("Skipping invalid line number: {}", line);
CxxUtils.LOG.warn("Skipping invalid line number: " + line);
}
}
} else {
Expand All @@ -196,7 +205,7 @@ public boolean saveViolation(Project project, SensorContext context, String rule
}
} else {
// project level violation
violation = Violation.create(rule, project);
violation = Violation.create(rule, module);
}

if (violation != null){
Expand Down

0 comments on commit 2139e1d

Please sign in to comment.