Skip to content

Commit

Permalink
Merge pull request #1701 from guwirth/real_path_coverage
Browse files Browse the repository at this point in the history 
support real paths in coverage sensor for C++/CLI
  • Loading branch information
@guwirth
guwirth authored Apr 14, 2019
2 parents 8e16538 + 4267417 commit 3491006
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 102 deletions.
4 changes: 2 additions & 2 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CxxCoverageSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public void describe(SensorDescriptor descriptor) {
* {@inheritDoc}
*/
@Override
public void execute(SensorContext context) {
public void executeImpl(SensorContext context) {
Configuration conf = context.config();
String[] reportsKey = conf.getStringArray(getReportPathKey());
LOG.info("Searching coverage reports by path with basedir '{}' and search prop '{}'",
Expand Down Expand Up @@ -162,7 +162,7 @@ private void saveMeasures(SensorContext context,
for (Map.Entry<String, CoverageMeasures> entry : coverageMeasures.entrySet()) {
final String filePath = PathUtils.sanitize(entry.getKey());
if (filePath != null) {
InputFile cxxFile = context.fileSystem().inputFile(context.fileSystem().predicates().hasPath(filePath));
InputFile cxxFile = getInputFileIfInProject(context, filePath);
if (LOG.isDebugEnabled()) {
LOG.debug("save coverage measure for file: '{}' cxxFile = '{}'", filePath, cxxFile);
}
Expand Down
5 changes: 2 additions & 3 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.io.IOException;
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import javax.xml.stream.XMLStreamException;
Expand Down Expand Up @@ -79,9 +78,9 @@ public void describe(SensorDescriptor descriptor) {
* {@inheritDoc}
*/
@Override
public void execute(SensorContext context) {
public void executeImpl(SensorContext context) {
transformFiles(context.fileSystem().baseDir(), context);
super.execute(context);
super.executeImpl(context);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion cxx-sensors/src/main/java/org/sonar/cxx/sensors/tests/xunit/CxxXunitSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void describe(SensorDescriptor descriptor) {
* {@inheritDoc}
*/
@Override
public void execute(SensorContext context) {
public void executeImpl(SensorContext context) {
LOG.debug("Root module imports test metrics: Module Key = '{}'", context.module());

try {
Expand Down
113 changes: 19 additions & 94 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxIssuesReportSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@
package org.sonar.cxx.sensors.utils;

import java.io.File;
import java.io.IOException;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -51,7 +48,6 @@ public abstract class CxxIssuesReportSensor extends CxxReportSensor {

private static final Logger LOG = Loggers.get(CxxIssuesReportSensor.class);

private final Set<String> notFoundFiles = new HashSet<>();
private final Set<CxxReportIssue> uniqueIssues = new HashSet<>();
private final Map<InputFile, Integer> violationsPerFileCount = new HashMap<>();
private int violationsPerModuleCount;
Expand All @@ -66,7 +62,7 @@ protected CxxIssuesReportSensor(CxxLanguage language, String propertiesKeyPathTo
}

private static NewIssueLocation createNewIssueLocationModule(SensorContext sensorContext, NewIssue newIssue,
CxxReportLocation location) {
CxxReportLocation location) {
return newIssue.newLocation().on(sensorContext.module()).message(location.getInfo());
}

Expand All @@ -78,12 +74,11 @@ public String getRuleRepositoryKey() {
* {@inheritDoc}
*/
@Override
public void execute(SensorContext context) {
public void executeImpl(SensorContext context) {
try {
LOG.info("Searching reports by relative path with basedir '{}' and search prop '{}'",
context.fileSystem().baseDir(), getReportPathKey());
context.fileSystem().baseDir(), getReportPathKey());
List<File> reports = getReports(context.config(), context.fileSystem().baseDir(), getReportPathKey());
notFoundFiles.clear();
violationsPerFileCount.clear();
violationsPerModuleCount = 0;

Expand All @@ -98,10 +93,10 @@ public void execute(SensorContext context) {

for (Map.Entry<InputFile, Integer> entry : violationsPerFileCount.entrySet()) {
context.<Integer>newMeasure()
.forMetric(metric)
.on(entry.getKey())
.withValue(entry.getValue())
.save();
.forMetric(metric)
.on(entry.getKey())
.withValue(entry.getValue())
.save();
}

// this sensor could be executed on module without any files
Expand All @@ -110,17 +105,17 @@ public void execute(SensorContext context) {
// let AggregateMeasureComputer calculate the correct value
if (violationsPerModuleCount != 0) {
context.<Integer>newMeasure()
.forMetric(metric)
.on(context.module())
.withValue(violationsPerModuleCount)
.save();
.forMetric(metric)
.on(context.module())
.withValue(violationsPerModuleCount)
.save();
}
} catch (Exception e) {
String msg = new StringBuilder(256)
.append("Cannot feed the data into sonar, details: '")
.append(CxxUtils.getStackTrace(e))
.append("'")
.toString();
.append("Cannot feed the data into sonar, details: '")
.append(CxxUtils.getStackTrace(e))
.append("'")
.toString();
LOG.error(msg);
CxxUtils.validateRecovery(e, getLanguage());
}
Expand All @@ -138,76 +133,6 @@ public void saveUniqueViolation(SensorContext sensorContext, CxxReportIssue issu
}
}

private InputFile getInputFileTryRealPath(SensorContext sensorContext, String path) {
final Path absolutePath = sensorContext.fileSystem().baseDir().toPath().resolve(path);
Path realPath;
try {
realPath = absolutePath.toRealPath(LinkOption.NOFOLLOW_LINKS);
} catch (IOException | RuntimeException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Unable to get the real path: module '{}', baseDir '{}', path '{}', exception '{}'",
sensorContext.module().key(), sensorContext.fileSystem().baseDir(), path, e.getMessage());
}
return null;
}

// if the real path is equal to the given one - skip search; we already
// tried such path
//
// IMPORTANT: don't use Path::equals(), since it's dependent on a file-system.
// SonarQube plugin API works with string paths, so the equality of strings
// is important
final String realPathString = realPath.toString();
if (absolutePath.toString().equals(realPathString)) {
return null;
}

return sensorContext.fileSystem()
.inputFile(sensorContext.fileSystem().predicates().hasAbsolutePath(realPathString));
}

public InputFile getInputFileIfInProject(SensorContext sensorContext, String path) {
if (notFoundFiles.contains(path)) {
return null;
}

// 1. try the most generic search predicate first; usually it's the right
// one
InputFile inputFile = sensorContext.fileSystem()
.inputFile(sensorContext.fileSystem().predicates().hasPath(path));

// 2. if there was nothing found, try to normalize the path by means of
// Path::toRealPath(). This helps if some 3rd party tools obfuscate the
// paths. E.g. the MS VC compiler tends to transform file paths to the lower
// case in its logs.
//
// IMPORTANT: SQ plugin API allows creation of NewIssue only on locations,
// which belong to the module. This internal check is performed by means
// of comparison of the paths. The paths which are managed by the framework
// (the reference paths) are NOT stored in the canonical form.
// E.g. the plugin API neither resolves symbolic links nor performs
// case-insensitive path normalization (could be relevant on Windows)
//
// Normalization by means of File::getCanonicalFile() or Path::toRealPath()
// can produce paths, which don't pass the mentioned check. E.g. resolution
// of symbolic links or letter case transformation
// might lead to the paths, which don't belong to the module's base
// directory (at least not in terms of parent-child semantic). This is the
// reason why we should avoid the resolution of symbolic links and not use
// the Path::toRealPath() as the only search predicate.

if (inputFile == null) {
inputFile = getInputFileTryRealPath(sensorContext, path);
}

if (inputFile == null) {
LOG.warn("Cannot find the file '{}' in module '{}' base dir '{}', skipping violations.",
path, sensorContext.module().key(), sensorContext.fileSystem().baseDir());
notFoundFiles.add(path);
}
return inputFile;
}

/**
* @param context
* @param report
Expand All @@ -220,7 +145,7 @@ private void executeReport(SensorContext context, File report, int prevViolation
if (LOG.isDebugEnabled()) {
Metric<Integer> metric = getLanguage().getMetric(this.getMetricKey());
LOG.debug("{} processed = {}", metric.getKey(),
violationsPerModuleCount - prevViolationsCount);
violationsPerModuleCount - prevViolationsCount);
}
} catch (EmptyReportException e) {
LOG.warn("The report '{}' seems to be empty, ignoring.", report);
Expand All @@ -230,13 +155,13 @@ private void executeReport(SensorContext context, File report, int prevViolation
}

private NewIssueLocation createNewIssueLocationFile(SensorContext sensorContext, NewIssue newIssue,
CxxReportLocation location, Set<InputFile> affectedFiles) {
CxxReportLocation location, Set<InputFile> affectedFiles) {
InputFile inputFile = getInputFileIfInProject(sensorContext, location.getFile());
if (inputFile != null) {
int lines = inputFile.lines();
int lineNr = Integer.max(1, getLineAsInt(location.getLine(), lines));
NewIssueLocation newIssueLocation = newIssue.newLocation().on(inputFile).at(inputFile.selectLine(lineNr))
.message(location.getInfo());
.message(location.getInfo());
affectedFiles.add(inputFile);
return newIssueLocation;
}
Expand All @@ -257,7 +182,7 @@ private void saveViolation(SensorContext sensorContext, CxxReportIssue issue) {
for (CxxReportLocation location : issue.getLocations()) {
if (location.getFile() != null && !location.getFile().isEmpty()) {
NewIssueLocation newIssueLocation = createNewIssueLocationFile(sensorContext, newIssue, location,
affectedFiles);
affectedFiles);
if (newIssueLocation != null) {
newIssueLocations.add(newIssueLocation);
}
Expand Down
90 changes: 89 additions & 1 deletion cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,20 @@
package org.sonar.cxx.sensors.utils;

import java.io.File;
import java.io.IOException;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.apache.commons.io.FilenameUtils;
import org.apache.tools.ant.DirectoryScanner;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.sensor.Sensor;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.config.Configuration;
Expand All @@ -41,8 +47,10 @@
public abstract class CxxReportSensor implements Sensor {

private static final Logger LOG = Loggers.get(CxxReportSensor.class);

private final CxxLanguage language;
private final String propertiesKeyPathToReports;
private final Set<String> notFoundFiles = new HashSet<>();

/**
* {@inheritDoc}
Expand Down Expand Up @@ -124,7 +132,7 @@ public static List<File> getReports(Configuration settings, final File moduleBas

if (existingReportPaths.length == 0) {
LOG.warn("Property '{}': cannot find any files matching the Ant pattern(s) '{}'", reportPathKey,
String.join(", ", normalizedReportPaths));
String.join(", ", normalizedReportPaths));
return Collections.emptyList();
}

Expand Down Expand Up @@ -165,6 +173,86 @@ private static List<String> normalizeReportPaths(final File moduleBaseDir, Strin
return includes;
}

private InputFile getInputFileTryRealPath(SensorContext sensorContext, String path) {
final Path absolutePath = sensorContext.fileSystem().baseDir().toPath().resolve(path);
Path realPath;
try {
realPath = absolutePath.toRealPath(LinkOption.NOFOLLOW_LINKS);
} catch (IOException | RuntimeException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Unable to get the real path: module '{}', baseDir '{}', path '{}', exception '{}'",
sensorContext.module().key(), sensorContext.fileSystem().baseDir(), path, e.getMessage());
}
return null;
}

// if the real path is equal to the given one - skip search; we already
// tried such path
//
// IMPORTANT: don't use Path::equals(), since it's dependent on a file-system.
// SonarQube plugin API works with string paths, so the equality of strings
// is important
final String realPathString = realPath.toString();
if (absolutePath.toString().equals(realPathString)) {
return null;
}

return sensorContext.fileSystem()
.inputFile(sensorContext.fileSystem().predicates().hasAbsolutePath(realPathString));
}

public InputFile getInputFileIfInProject(SensorContext sensorContext, String path) {
if (notFoundFiles.contains(path)) {
return null;
}

// 1. try the most generic search predicate first; usually it's the right
// one
InputFile inputFile = sensorContext.fileSystem()
.inputFile(sensorContext.fileSystem().predicates().hasPath(path));

// 2. if there was nothing found, try to normalize the path by means of
// Path::toRealPath(). This helps if some 3rd party tools obfuscate the
// paths. E.g. the MS VC compiler tends to transform file paths to the lower
// case in its logs.
//
// IMPORTANT: SQ plugin API allows creation of NewIssue only on locations,
// which belong to the module. This internal check is performed by means
// of comparison of the paths. The paths which are managed by the framework
// (the reference paths) are NOT stored in the canonical form.
// E.g. the plugin API neither resolves symbolic links nor performs
// case-insensitive path normalization (could be relevant on Windows)
//
// Normalization by means of File::getCanonicalFile() or Path::toRealPath()
// can produce paths, which don't pass the mentioned check. E.g. resolution
// of symbolic links or letter case transformation
// might lead to the paths, which don't belong to the module's base
// directory (at least not in terms of parent-child semantic). This is the
// reason why we should avoid the resolution of symbolic links and not use
// the Path::toRealPath() as the only search predicate.
if (inputFile == null) {
inputFile = getInputFileTryRealPath(sensorContext, path);
}

if (inputFile == null) {
LOG.warn("Cannot find the file '{}' in module '{}' base dir '{}', skipping violations.",
path, sensorContext.module().key(), sensorContext.fileSystem().baseDir());
notFoundFiles.add(path);
}
return inputFile;
}

/**
* override always executeImpl instead of execute
*/
protected abstract void executeImpl(SensorContext context);

@Override
public void execute(SensorContext context) {
notFoundFiles.clear();
executeImpl(context);
}

public CxxLanguage getLanguage() {
return language;
}
Expand Down
2 changes: 1 addition & 1 deletion cxx-sensors/src/test/java/org/sonar/cxx/sensors/utils/CxxReportSensorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public CxxReportSensorImpl(CxxLanguage language, MapSettings settings) {
}

@Override
public void execute(SensorContext sc) {
public void executeImpl(SensorContext sc) {
}

@Override
Expand Down

0 comments on commit 3491006

Please sign in to comment.