Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Redesign of language-specific metrics
The following problems were identified and solved:

PROBLEM 1
* CxxLanguage::MetricsCache was not filled while
  construction of CxxLanguage object but only in
  CxxSquidSensor::CxxSquidSensor()

SOLUTION
* Introduce CxxMetricsFactory.
* It has no dependencies to sensors.
* It creates language-dependent metrics in the constructor
  of CxxSquidSensor

PROBLEM 2
* During the test MetricsCache was almost always empty.
  In order to prevent warnings (?) missing metrics' lookups
  were just ignored

SOLUTON
* Introduce `protected abstract Optional<CxxMetricsFactory.Key> CxxReportSensor::getMetricKey();`
* By means of Optional<> semantic sensors show if metric is required or not
* CxxLanguage::getMetric() checks thows an exception if metric hasn't been found

PROBLEM 3
* CxxLanguage::getMetric(<required metric key>) expected the key to be typed as String
* In order to fill the MetricsCache the format {METRIC_KEY} was used
* Cache lookups were done however in the format {LANG_PROP_KEY}-{METRIC_KEY}

SOLUTION
* In order to make the lookup key type-safe and less error-prone I introduced the type
  `CxxMetricsFactory.Key`

PROBLEM 4a
* The plugin-specific metrics for public API were missing on module level
  All CoreMetrics (incls. former Public API) have automatic aggregation:
  1. ComputeEngine implements some server-side task
  2. This task iterates over the project hierarchy
     `FILE -> DIRECTORY/MODULE -> ... -> ... -> PROJECT`
     and performs a recursive sum for all metrics raised
     on children component
  3. This sum is stored as aggregated metric for the parent component
  4. The number of hierarchy levels can be arbitrary

  5. Starting with version 6.2 the public API metrics were
     deprecated and Sonar-CXX introduces its own plugin-specific
     public API metrics
  6. These custom metrics are not aggregated automatically
     anymore

PROBLEM 4b
* PR SonarOpenCommunity#1398 introduced 9 new complexity metrics
  They supported the aggregation of the file-level metrics.
  The aggregation was implemeted inside of the `AstVisitor`,
  however the aggregation was done for the parent module only.

  Ths solution worked for the simple SonarQube projects only.
  Due to hierarchy depth == 2, aggregated results were stored
  on the project level. Multi-module projects however might
  have an arbitrary depth. So there was no proper aggregation
  for such projects.

  After public API metrics were deprecated as CoreMetrics
  the automatic aggregation
  It was not enough to set them on the file level. One had to aggregate these
  measurements and set the result on the module level. Otherwise
  they were not displayed in the SonarQube's Activity UI

SOLUTION
* Unify implementation of custom metrics for Public-API and complexity
* Implement 2 Compute Engine tasks (see `MeasureComputer`), which perform
  the aggregation and computation of these metrics
* Computaton/aggregation works for both types of SonarQube project -
  the simple and the multi-module ones

PROBLEM 5
* The metric `CoreMetrics.COGNITIVE_COMPLEXITY` was measured but not persisted

MISC
* Test has been added
* Minor refactoring and comments
  • Loading branch information
ivangalkin committed Jul 19, 2018
1 parent e082c4a commit 6e08f1c
Show file tree
Hide file tree
Showing 63 changed files with 1,837 additions and 1,583 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public static CxxLanguage mockCxxLanguage() {
when(language.IsRecoveryEnabled()).thenReturn(Optional.of(Boolean.TRUE));
when(language.getFileSuffixes())
.thenReturn(new String[]{".cpp", ".hpp", ".h", ".cxx", ".c", ".cc", ".hxx", ".hh"});
when(language.getHeaderFileSuffixes()).thenReturn(new String[] { ".hpp", ".h", ".hxx", ".hh" });

return language;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
public class CppLanguage extends CxxLanguage {

public CppLanguage(Configuration config) {
super("c++", config);
super("c++", "cxx", config);
}

@Override
Expand All @@ -48,11 +48,6 @@ public String[] getHeaderFileSuffixes() {
return new String[]{"hpp", "h", "hxx"};
}

@Override
public String getPropertiesKey() {
return "cxx";
}

@Override
public List<Class> getChecks() {
return CxxCheckList.getChecks();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@
import com.dd.plist.NSString;
import com.dd.plist.PropertyListParser;
import java.io.File;
import java.util.Optional;

import javax.annotation.Nullable;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.CxxMetricsFactory;
import org.sonar.cxx.sensors.utils.CxxReportIssue;
import org.sonar.cxx.sensors.utils.CxxReportSensor;

Expand Down Expand Up @@ -123,4 +126,9 @@ protected void processReport(final SensorContext context, File report)
protected String getSensorKey() {
return KEY;
}

@Override
protected Optional<CxxMetricsFactory.Key> getMetricKey() {
return Optional.of(CxxMetricsFactory.Key.CLANG_SA_SENSOR_ISSUES_KEY);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.sonar.cxx.sensors.clangtidy;

import java.io.File;
import java.util.Optional;
import java.util.Scanner;
import java.util.regex.MatchResult;
import java.util.regex.Matcher;
Expand All @@ -29,6 +30,7 @@
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.CxxMetricsFactory;
import org.sonar.cxx.sensors.utils.CxxReportIssue;
import org.sonar.cxx.sensors.utils.CxxReportSensor;

Expand Down Expand Up @@ -108,4 +110,9 @@ protected void processReport(final SensorContext context, File report) {
protected String getSensorKey() {
return KEY;
}

@Override
protected Optional<CxxMetricsFactory.Key> getMetricKey() {
return Optional.of(CxxMetricsFactory.Key.CLANG_TIDY_SENSOR_ISSUES_KEY);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.CxxMetricsFactory;
import org.sonar.cxx.sensors.utils.CxxReportIssue;
import org.sonar.cxx.sensors.utils.CxxReportSensor;

Expand Down Expand Up @@ -136,4 +137,9 @@ private static boolean isInputValid(CompilerParser.Warning warning) {
protected String getSensorKey() {
return KEY;
}

@Override
protected Optional<CxxMetricsFactory.Key> getMetricKey() {
return Optional.of(CxxMetricsFactory.Key.COMPILER_SENSOR_ISSUES_KEY);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import javax.xml.stream.XMLStreamException;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.sensor.SensorContext;
Expand All @@ -36,6 +38,7 @@
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.CxxMetricsFactory;
import org.sonar.cxx.sensors.utils.CxxReportSensor;
import org.sonar.cxx.sensors.utils.CxxUtils;
import org.sonar.cxx.sensors.utils.EmptyReportException;
Expand Down Expand Up @@ -218,7 +221,7 @@ private void checkCoverage(NewCoverage newCoverage, CoverageMeasure measure) {
newCoverage.lineHits(measure.getLine(), measure.getHits());
newCoverage.conditions(measure.getLine(), measure.getConditions(), measure.getCoveredConditions());
if (LOG.isDebugEnabled()) {
LOG.debug("line '{}' Hits '{}' Conditions '{}:{}'",measure.getLine(), measure.getHits(),
LOG.debug("line '{}' Hits '{}' Conditions '{}:{}'",measure.getLine(), measure.getHits(),
measure.getConditions(), measure.getCoveredConditions() );
}
} catch (RuntimeException ex) {
Expand All @@ -233,4 +236,9 @@ protected String getSensorKey() {
return KEY;
}

@Override
protected Optional<CxxMetricsFactory.Key> getMetricKey() {
return Optional.empty();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@
import java.io.File;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;

import javax.xml.stream.XMLStreamException;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.CxxMetricsFactory;
import org.sonar.cxx.sensors.utils.CxxReportSensor;

/**
Expand Down Expand Up @@ -94,4 +97,9 @@ protected void processReport(final SensorContext context, File report)
protected String getSensorKey() {
return KEY;
}

@Override
protected Optional<CxxMetricsFactory.Key> getMetricKey() {
return Optional.of(CxxMetricsFactory.Key.CPPCHECK_SENSOR_ISSUES_KEY);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@

import java.io.File;
import java.nio.charset.StandardCharsets;
import java.util.Optional;

import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.CxxMetricsFactory;
import org.sonar.cxx.sensors.drmemory.DrMemoryParser.DrMemoryError;
import org.sonar.cxx.sensors.drmemory.DrMemoryParser.DrMemoryError.Location;
import org.sonar.cxx.sensors.utils.CxxReportIssue;
Expand Down Expand Up @@ -131,4 +134,9 @@ protected void processReport(final SensorContext context, File report) {
protected String getSensorKey() {
return KEY;
}

@Override
protected Optional<CxxMetricsFactory.Key> getMetricKey() {
return Optional.of(CxxMetricsFactory.Key.DRMEMORY_SENSOR_ISSUES_KEY);
}
}

This file was deleted.

Loading

0 comments on commit 6e08f1c

Please sign in to comment.