remove the helpers CxxUtils::normalizePathFull() completely...

Path absolutization is never a good idea. Especially in the coverage sensor, which caches the file paths. See SonarOpenCommunity#1465
ivangalkin · Oct 14, 2018 · e2cf094 · e2cf094
1 parent f8f599a
commit e2cf094
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 67 deletions.
diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/BullseyeParser.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/BullseyeParser.java
@@ -28,7 +28,6 @@
 import javax.xml.stream.XMLStreamException;
 import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
-import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.utils.PathUtils;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
@@ -45,14 +44,14 @@ public class BullseyeParser extends CxxCoverageParser {
   private static int totalcoveredconditions;
 
   public BullseyeParser() {
-    // no operation but necessary for list of coverage parsers 
+    // no operation but necessary for list of coverage parsers
   }
 
   /**
    * {@inheritDoc}
    */
   @Override
-  public void processReport(final SensorContext context, File report, final Map<String, CoverageMeasures> coverageData)
+  public void processReport(File report, final Map<String, CoverageMeasures> coverageData)
     throws XMLStreamException {
     LOG.debug("Parsing 'Bullseye' format");
     StaxParser topLevelparser = new StaxParser(new StaxParser.XmlStreamHandler() {

diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CoberturaParser.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CoberturaParser.java
@@ -20,17 +20,17 @@
 package org.sonar.cxx.sensors.coverage;
 
 import java.io.File;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Map;
+import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import javax.xml.stream.XMLStreamException;
 import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
-import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
-import org.sonar.cxx.sensors.utils.CxxUtils;
 import org.sonar.cxx.sensors.utils.StaxParser;
 
 /**
@@ -39,7 +39,7 @@
 public class CoberturaParser extends CxxCoverageParser {
 
   private static final Logger LOG = Loggers.get(CoberturaParser.class);
-  private String baseDir;
+  private Optional<Path> baseDir;
   private static final Pattern conditionsPattern = Pattern.compile("\\((.*?)\\)");
 
   public CoberturaParser() {
@@ -50,10 +50,10 @@ public CoberturaParser() {
    * {@inheritDoc}
    */
   @Override
-  public void processReport(final SensorContext context, File report, final Map<String, CoverageMeasures> coverageData)
+  public void processReport(File report, final Map<String, CoverageMeasures> coverageData)
     throws XMLStreamException {
     LOG.debug("Parsing 'Cobertura' format");
-    baseDir = context.fileSystem().baseDir().getAbsolutePath();
+    baseDir = Optional.empty();
 
     StaxParser sourceParser = new StaxParser((SMHierarchicCursor rootCursor) -> {
       rootCursor.advance();
@@ -63,7 +63,7 @@ public void processReport(final SensorContext context, File report, final Map<St
 
     StaxParser packageParser = new StaxParser((SMHierarchicCursor rootCursor) -> {
       rootCursor.advance();
-      collectPackageMeasures(context, rootCursor.descendantElementCursor("package"), coverageData);
+      collectPackageMeasures(rootCursor.descendantElementCursor("package"), coverageData);
     });
     packageParser.parse(report);
   }
@@ -72,33 +72,37 @@ private void readBaseDir(SMInputCursor source) throws XMLStreamException {
     while (source.getNext() != null) {
       String sourceValue = source.getElemStringValue().trim();
       if (!sourceValue.isEmpty()) {
-        baseDir = Paths.get(baseDir).resolve(sourceValue).normalize().toString();
+        baseDir = Optional.of(Paths.get(sourceValue));
         break;
       }
     }
   }
 
-  private static void collectPackageMeasures(SensorContext sensorContext, SMInputCursor pack,
-    Map<String, CoverageMeasures> coverageData)
-    throws XMLStreamException {
+  private void collectPackageMeasures(SMInputCursor pack, Map<String, CoverageMeasures> coverageData)
+      throws XMLStreamException {
     while (pack.getNext() != null) {
-      collectFileMeasures(sensorContext, pack.descendantElementCursor("class"), coverageData);
+      collectFileMeasures(pack.descendantElementCursor("class"), coverageData);
     }
   }
 
-  private static void collectFileMeasures(SensorContext sensorContext, SMInputCursor clazz,
-    Map<String, CoverageMeasures> coverageData)
-    throws XMLStreamException {
+  private String buildPath(String filename) {
+    Path result = Paths.get(filename);
+    if (baseDir.isPresent()) {
+      result = baseDir.get().resolve(result);
+    }
+    return result.normalize().toString();
+  }
+
+  private void collectFileMeasures(SMInputCursor clazz, Map<String, CoverageMeasures> coverageData)
+      throws XMLStreamException {
     while (clazz.getNext() != null) {
-      String normalPath = CxxUtils.normalizePathFull(sensorContext, clazz.getAttrValue("filename"));
-      if (normalPath != null) {
-        CoverageMeasures builder = coverageData.get(normalPath);
-        if (builder == null) {
-          builder = CoverageMeasures.create();
-          coverageData.put(normalPath, builder);
-        }
-        collectFileData(clazz, builder);
+      String normalPath = buildPath(clazz.getAttrValue("filename"));
+      CoverageMeasures builder = coverageData.get(normalPath);
+      if (builder == null) {
+        builder = CoverageMeasures.create();
+        coverageData.put(normalPath, builder);
       }
+      collectFileData(clazz, builder);
     }
   }
 

diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CoverageParser.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CoverageParser.java
@@ -21,11 +21,12 @@
 
 import java.io.File;
 import java.util.Map;
+
 import javax.xml.stream.XMLStreamException;
-import org.sonar.api.batch.sensor.SensorContext;
 
 /**
- * The interface a coverage report parser has to implement in order to be used by CxxCoverageSensor
+ * The interface a coverage report parser has to implement in order to be used
+ * by CxxCoverageSensor
  */
 public interface CoverageParser {
 
@@ -39,11 +40,14 @@ public interface CoverageParser {
    * @param coverageData
    *          A Map mapping source file names to coverage measures. Has to be
    *          used to store the results into. Source file names might be
-   *          relative. In such case they will be resolved agains the base
-   *          directory of SonarQube module/project.
+   *          relative. In such case they will be resolved against the base
+   *          directory of SonarQube module/project.<br>
+   *
+   *          <b>ATTENTION!</b> This map is shared between modules in
+   *          multi-module projects. Don't try to resolve paths against some
+   *          specific module!
    * @throws XMLStreamException
    *           javax.xml.stream.XMLStreamException
    */
-  void processReport(final SensorContext context, File report, Map<String, CoverageMeasures> coverageData)
-    throws XMLStreamException;
+  void processReport(File report, Map<String, CoverageMeasures> coverageData) throws XMLStreamException;
 }
diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CxxCoverageSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CxxCoverageSensor.java
@@ -100,20 +100,20 @@ public void execute(SensorContext context) {
       }
 
       List<File> reports = getReports(context.config(), context.fileSystem().baseDir(), getReportPathKey());
-      coverageMeasures = processReports(context, reports, this.cache.unitCoverageCache());
+      coverageMeasures = processReports(reports, this.cache.unitCoverageCache());
       saveMeasures(context, coverageMeasures);
     }
   }
 
-  private Map<String, CoverageMeasures> processReports(final SensorContext context, List<File> reports,
-    Map<String, Map<String, CoverageMeasures>> cacheCov) {
+  private Map<String, CoverageMeasures> processReports(List<File> reports,
+      Map<String, Map<String, CoverageMeasures>> cacheCov) {
     Map<String, CoverageMeasures> measuresTotal = new HashMap<>();
 
     for (File report : reports) {
       if (!cacheCov.containsKey(report.getAbsolutePath())) {
         for (CoverageParser parser : parsers) {
           try {
-            parseCoverageReport(parser, context, report, measuresTotal);
+            parseCoverageReport(parser, report, measuresTotal);
             if (LOG.isDebugEnabled()) {
               LOG.debug("cached measures for '{}' : current cache content data = '{}'", report.getAbsolutePath(),
                 cacheCov.size());
@@ -145,11 +145,11 @@ private Map<String, CoverageMeasures> processReports(final SensorContext context
    * @param measuresTotal
    * @return true if report was parsed and results are available otherwise false
    */
-  private static void parseCoverageReport(CoverageParser parser, final SensorContext context, File report,
-    Map<String, CoverageMeasures> measuresTotal) {
+  private static void parseCoverageReport(CoverageParser parser, File report,
+      Map<String, CoverageMeasures> measuresTotal) {
     Map<String, CoverageMeasures> measuresForReport = new HashMap<>();
     try {
-      parser.processReport(context, report, measuresForReport);
+      parser.processReport(report, measuresForReport);
     } catch (XMLStreamException e) {
       throw new EmptyReportException("Coverage report" + report + "cannot be parsed by" + parser, e);
     }

diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/TestwellCtcTxtParser.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/TestwellCtcTxtParser.java
@@ -32,10 +32,10 @@
 
 import java.math.BigDecimal;
 
-import org.sonar.api.batch.sensor.SensorContext;
+import org.apache.commons.io.FilenameUtils;
+
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
-import org.sonar.cxx.sensors.utils.CxxUtils;
 
 import static org.sonar.cxx.sensors.coverage.TestwellCtcTxtResult.FILE_HEADER;
 import static org.sonar.cxx.sensors.coverage.TestwellCtcTxtResult.FILE_RESULT;
@@ -50,31 +50,31 @@
 public class TestwellCtcTxtParser extends CxxCoverageParser {
 
   private static final Logger LOG = Loggers.get(TestwellCtcTxtParser.class);
-  
+
   private Scanner scanner;
-  
+
   private static final int FROM_START = 0;
   private static final int CONDS_FALSE = 1;
   private static final int CONDS_TRUE = 2;
   private static final int LINE_NR_GROUP = 3;
 
-  
+
 
   public TestwellCtcTxtParser() {
-    // no operation but necessary for list of coverage parsers 
+    // no operation but necessary for list of coverage parsers
   }
 
   /**
    * {@inheritDoc}
    */
   @Override
-  public void processReport(final SensorContext context, File report, final Map<String, CoverageMeasures> coverageData) {
+  public void processReport(File report, final Map<String, CoverageMeasures> coverageData) {
     LOG.debug("Parsing 'Testwell CTC++' textual format");
 
     try (Scanner s = new Scanner(report).useDelimiter(SECTION_SEP)) {
       scanner = s;
       Matcher headerMatcher = FILE_HEADER.matcher(scanner.next());
-      while (parseUnit(coverageData, headerMatcher, context)) {
+      while (parseUnit(coverageData, headerMatcher)) {
         headerMatcher.reset(scanner.next());
       }
     } catch (FileNotFoundException e) {
@@ -84,37 +84,44 @@ public void processReport(final SensorContext context, File report, final Map<St
     }
   }
 
-  private boolean parseUnit(final Map<String, CoverageMeasures> coverageData, Matcher headerMatcher, SensorContext sensorContext) {
+  private boolean parseUnit(final Map<String, CoverageMeasures> coverageData, Matcher headerMatcher) {
     LOG.debug(headerMatcher.toString());
 
     if (headerMatcher.find(FROM_START)) {
-      parseFileUnit(coverageData, headerMatcher, sensorContext);
+      parseFileUnit(coverageData, headerMatcher);
     } else {
       return false;
     }
     return true;
   }
 
-  private void parseFileUnit(final Map<String, CoverageMeasures> coverageData, Matcher headerMatcher, SensorContext sensorContext) {
+  private void parseFileUnit(final Map<String, CoverageMeasures> coverageData, Matcher headerMatcher) {
     LOG.debug("Parsing file section...");
 
-    String filePath = CxxUtils.normalizePathFull(sensorContext, headerMatcher.group(1));
-    addLines(filePath, coverageData);
+    String normalFilename;
+    String filename = headerMatcher.group(1);
+    if (new File(filename).isAbsolute()) {
+      normalFilename = FilenameUtils.normalize(filename);
+    } else {
+      normalFilename = FilenameUtils.normalize("./" + filename);
+    }
+    File file = new File(normalFilename);
+    addLines(file, coverageData);
   }
 
-  private void addLines(String filePath, final Map<String, CoverageMeasures> coverageData) {
+  private void addLines(File file, final Map<String, CoverageMeasures> coverageData) {
     LOG.debug("Parsing function sections...");
 
     CoverageMeasures coverageMeasures = CoverageMeasures.create();
     for (String nextLine = scanner.next(); !FILE_RESULT.matcher(nextLine).find(); nextLine = scanner.next()) {
       parseLineSection(coverageMeasures, nextLine);
     }
-    coverageData.put(filePath, coverageMeasures);
+    coverageData.put(file.getPath(), coverageMeasures);
   }
 
   private void parseLineSection(CoverageMeasures coverageMeasures, String nextLine) {
     LOG.debug("Found line section...");
-    
+
     Matcher lineMatcher = LINE_RESULT.matcher(nextLine);
     if (lineMatcher.find(FROM_START)) {
       addEachLine(coverageMeasures, lineMatcher);

diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/VisualStudioParser.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/VisualStudioParser.java
@@ -24,7 +24,6 @@
 import javax.xml.stream.XMLStreamException;
 import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
-import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.cxx.sensors.utils.StaxParser;
@@ -37,14 +36,14 @@ public class VisualStudioParser extends CxxCoverageParser {
   private static final Logger LOG = Loggers.get(VisualStudioParser.class);
 
   public VisualStudioParser() {
-    // no operation but necessary for list of coverage parsers 
+    // no operation but necessary for list of coverage parsers
   }
 
   /**
    * {@inheritDoc}
    */
   @Override
-  public void processReport(final SensorContext context, File report, final Map<String, CoverageMeasures> coverageData)
+  public void processReport(File report, final Map<String, CoverageMeasures> coverageData)
     throws XMLStreamException {
     LOG.debug("Parsing 'Visual Studio' format");
     StaxParser parser = new StaxParser(new StaxParser.XmlStreamHandler() {

diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxUtils.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxUtils.java
@@ -31,7 +31,6 @@
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
 import org.apache.commons.lang.exception.ExceptionUtils;
-import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.cxx.CxxLanguage;
@@ -49,15 +48,6 @@ private CxxUtils() {
     // only static methods
   }
 
-  /**
-   * @return absolute, resolved and normalized <code>path</code> against the
-   *         base directory of the given SonarQube module, represented by
-   *         <code>sensorContext</code>
-   */
-  public static String normalizePathFull(SensorContext sensorContext, String path) {
-    return sensorContext.fileSystem().baseDir().getAbsoluteFile().toPath().resolve(path.trim()).normalize().toString();
-  }
-
   /**
    * transformFile
    *