Skip to content

Commit

Permalink
Improve handling of duplicate classes and methods.
Browse files Browse the repository at this point in the history
  • Loading branch information
@uhafner
uhafner committed Nov 14, 2023
1 parent 5f78905 commit 713bca8
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 28 deletions.
2 changes: 1 addition & 1 deletion doc/dependency-graph.puml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ skinparam rectangle {
BackgroundColor<<runtime>> lightBlue
BackgroundColor<<provided>> lightGray
}
rectangle "coverage-model\n\n0.29.0" as edu_hm_hafner_coverage_model_jar
rectangle "coverage-model\n\n0.30.0-SNAPSHOT" as edu_hm_hafner_coverage_model_jar
rectangle "spotbugs-annotations\n\n4.7.3" as com_github_spotbugs_spotbugs_annotations_jar
rectangle "error_prone_annotations\n\n2.23.0" as com_google_errorprone_error_prone_annotations_jar
rectangle "streamex\n\n0.8.2" as one_util_streamex_jar
Expand Down
63 changes: 41 additions & 22 deletions src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import edu.hm.hafner.coverage.CoverageParser;
import edu.hm.hafner.coverage.CyclomaticComplexity;
import edu.hm.hafner.coverage.FileNode;
import edu.hm.hafner.coverage.MethodNode;
import edu.hm.hafner.coverage.Metric;
import edu.hm.hafner.coverage.ModuleNode;
import edu.hm.hafner.coverage.Node;
Expand Down Expand Up @@ -98,13 +99,18 @@ protected ModuleNode parseReport(final Reader reader, final FilteredLog log) {
readSource(eventReader, root);
}
else if (PACKAGE.equals(tagName)) {
readPackage(eventReader, root, startElement, log);
readPackage(eventReader, root, readName(startElement), log);
isEmpty = false;
}
}
}
if (isEmpty) {
throw new NoSuchElementException("No coverage information found in the specified file.");
if (ignoreErrors()) {

Check warning on line 108 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 108 is only partially covered, one branch is missing
log.logError("No coverage information found in the specified file.");

Check warning on line 109 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 109 is not covered by tests
}
else {
throw new NoSuchElementException("No coverage information found in the specified file.");
}
}
return root;
}
Expand All @@ -114,20 +120,20 @@ else if (PACKAGE.equals(tagName)) {
}

Check warning on line 120 in src/main/java/edu/hm/hafner/coverage/parser/CoberturaParser.java

View check run for this annotation

ci.jenkins.io / PMD

CyclomaticComplexity 

NORMAL:
The method 'parseReport(Reader, FilteredLog)' has a cyclomatic complexity of 10.
Raw output 
The complexity of methods directly affects maintenance costs and readability. Concentrating too much decisional logic in a single method makes its behaviour hard to read and change. Cyclomatic complexity assesses the complexity of a method by counting the number of decision points in a method, plus one for the method entry. Decision points are places where the control flow jumps to another place in the program. As such, they include all control flow statements, such as `if`, `while`, `for`, and `case`. For more details on the calculation, see the documentation of the [Cyclo metric](pmd_java_metrics_index.html#cyclomatic-complexity-cyclo). Generally, numbers ranging from 1-4 denote low complexity, 5-7 denote moderate complexity, 8-10 denote high complexity, and 11+ is very high complexity. By default, this rule reports methods with a complexity >= 10. Additionally, classes with many methods of moderate complexity get reported as well once the total of their methods' complexities reaches 80, even if none of the methods was directly reported. Reported methods should be broken down into several smaller methods. Reported classes should probably be broken down into subcomponents. <pre> <code> class Foo { void baseCyclo() { // Cyclo = 1 highCyclo(); } void highCyclo() { // Cyclo = 10: reported! int x = 0, y = 2; boolean a = false, b = true; if (a &amp;&amp; (y == 1 ? b : true)) { // +3 if (y == x) { // +1 while (true) { // +1 if (x++ &lt; 20) { // +1 break; // +1 } } } else if (y == t &amp;&amp; !d) { // +2 x = a ? y : x; // +1 } else { x = 2; } } } } </code> </pre> <a href="https://pmd.github.io/pmd-6.55.0/pmd_rules_java_design.html#cyclomaticcomplexity"> See PMD documentation. </a>

private void readPackage(final XMLEventReader reader, final ModuleNode root,
final StartElement currentStartElement, final FilteredLog log) throws XMLStreamException {
var packageNode = root.findOrCreatePackageNode(getValueOf(currentStartElement, NAME));
final String packageName, final FilteredLog log) throws XMLStreamException {
var packageNode = root.findOrCreatePackageNode(packageName);

while (reader.hasNext()) {
XMLEvent event = reader.nextEvent();

if (event.isStartElement()) {
var nextElement = event.asStartElement();
if (CLASS.equals(nextElement.getName())) {
var fileName = getValueOf(nextElement, FILE_NAME);
var element = event.asStartElement();
if (CLASS.equals(element.getName())) {
var fileName = getValueOf(element, FILE_NAME);
var relativePath = PATH_UTIL.getRelativePath(fileName);
var fileNode = packageNode.findOrCreateFileNode(getFileName(fileName),
getTreeStringBuilder().intern(relativePath));
readClassOrMethod(reader, fileNode, fileNode, nextElement, log);
readClassOrMethod(reader, fileNode, fileNode, element, log);
}
}
else if (event.isEndElement()) {
Expand Down Expand Up @@ -176,7 +182,7 @@ private void readClassOrMethod(final XMLEventReader reader,
}
lineCoverage = lineCoverage.add(currentLineCoverage);

if (CLASS.equals(element.getName())) { // Counters are stored at file level
if (CLASS.equals(element.getName())) { // Counters are stored at file level only
int lineNumber = getIntegerValueOf(nextElement, NUMBER);
fileNode.addCounters(lineNumber, coverage.getCovered(), coverage.getMissed());
}
Expand Down Expand Up @@ -204,24 +210,37 @@ private Coverage computeLineCoverage(final int coverage) {
}

private Node createNode(final Node parentNode, final StartElement element, final FilteredLog log) {
var name = getValueOf(element, NAME);
if (StringUtils.isBlank(name)) { // each node must have a unique name
name = createId();
}
var name = readName(element);
if (CLASS.equals(element.getName())) {
if (parentNode.hasChild(name) && ignoreErrors()) {
log.logError("Found a duplicate class '%s' in '%s'", name, parentNode.getName());
name = name + "-" + createId();
}
return ((FileNode)parentNode).createClassNode(name);
return createClassNode(parentNode, log, name);
}
return createMethodNode(parentNode, element, log, name);
}

private MethodNode createMethodNode(final Node parentNode, final StartElement element, final FilteredLog log,
final String name) {
String className = name;
var signature = getValueOf(element, SIGNATURE);
var classNode = (ClassNode) parentNode;
if (classNode.findMethod(name, signature).isPresent() && ignoreErrors()) {
log.logError("Found a duplicate method '%s' with signature '%s' in '%s'", name, signature, parentNode.getName());
name = name + "-" + createId();
if (classNode.findMethod(className, signature).isPresent() && ignoreErrors()) {
log.logError("Found a duplicate method '%s' with signature '%s' in '%s'",
className, signature, parentNode.getName());
className = name + "-" + createId();
}
return classNode.createMethodNode(className, signature);
}

private ClassNode createClassNode(final Node parentNode, final FilteredLog log, final String name) {
String className = name;
if (parentNode.hasChild(className) && ignoreErrors()) {
log.logError("Found a duplicate class '%s' in '%s'", className, parentNode.getName());
className = name + "-" + createId();
}
return classNode.createMethodNode(name, signature);
return ((FileNode) parentNode).createClassNode(className);
}

private String readName(final StartElement element) {
return StringUtils.defaultIfBlank(getValueOf(element, NAME), createId());
}

private String createId() {
Expand Down
7 changes: 2 additions & 5 deletions src/test/java/edu/hm/hafner/coverage/parser/CoberturaParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ void shouldConvertCoberturaBigToTree() {
Node root = readExampleReport();

assertThat(root.getAll(MODULE)).hasSize(1);
assertThat(root.getAll(PACKAGE)).hasSize(1);
assertThat(root.getAll(PACKAGE)).hasSize(5);
assertThat(root.getAll(FILE)).hasSize(4);
assertThat(root.getAll(CLASS)).hasSize(5);
assertThat(root.getAll(METHOD)).hasSize(10);
Expand Down Expand Up @@ -389,7 +389,7 @@ void shouldConvertCoberturaBigToTree() {

assertThat(root.aggregateValues()).containsExactly(
builder.setMetric(MODULE).setCovered(1).setMissed(0).build(),
builder.setMetric(PACKAGE).setCovered(1).setMissed(0).build(),
builder.setMetric(PACKAGE).setCovered(4).setMissed(1).build(),
builder.setMetric(FILE).setCovered(4).setMissed(0).build(),
builder.setMetric(CLASS).setCovered(5).setMissed(0).build(),
builder.setMetric(METHOD).setCovered(7).setMissed(3).build(),
Expand All @@ -400,9 +400,6 @@ void shouldConvertCoberturaBigToTree() {
new FractionValue(COMPLEXITY_DENSITY, 22, 63 + 19),
new LinesOfCode(63 + 19));

assertThat(root.getChildren()).extracting(Node::getName)
.containsExactly("-");

verifyCoverageMetrics(root);

List<Node> nodes = root.getAll(FILE);
Expand Down

0 comments on commit 713bca8

Please sign in to comment.