Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement explanation for cyclomatic complexity checks #1537

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,17 @@
*/
package org.sonar.cxx.checks.metrics;

import com.sonar.sslr.api.AstNode;
import com.sonar.sslr.api.Grammar;
import java.util.Optional;

import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.cxx.api.CxxMetric;
import org.sonar.cxx.parser.CxxGrammarImpl;
import org.sonar.cxx.tag.Tag;
import org.sonar.squidbridge.annotations.SqaleLinearWithOffsetRemediation;
import org.sonar.squidbridge.api.SourceClass;
import org.sonar.squidbridge.checks.ChecksHelper;
import org.sonar.squidbridge.checks.SquidCheck;

import com.sonar.sslr.api.AstNodeType;
import com.sonar.sslr.api.Grammar;

@Rule(
key = "ClassComplexity",
Expand All @@ -41,36 +40,32 @@
coeff = "1min",
offset = "10min",
effortToFixDescription = "per complexity point over the threshold")
public class ClassComplexityCheck extends SquidCheck<Grammar> {
public class ClassComplexityCheck extends CxxCyclomaticComplexityCheck<Grammar> {

private static final int DEFAULT_MAXIMUM_CLASS_COMPLEXITY_THRESHOLD = 60;

@RuleProperty(
key = "maximumClassComplexityThreshold",
description = "Max value of complexity allowed in a class",
defaultValue = "" + DEFAULT_MAXIMUM_CLASS_COMPLEXITY_THRESHOLD)
private int maximumClassComplexityThreshold = DEFAULT_MAXIMUM_CLASS_COMPLEXITY_THRESHOLD;
private int max = DEFAULT_MAXIMUM_CLASS_COMPLEXITY_THRESHOLD;

@Override
public void init() {
subscribeTo(CxxGrammarImpl.classSpecifier);
protected Optional<AstNodeType> getScopeType() {
return Optional.of(CxxGrammarImpl.classSpecifier);
}

@Override
public void leaveNode(AstNode node) {
SourceClass sourceClass = (SourceClass) getContext().peekSourceCode();
int complexity = ChecksHelper.getRecursiveMeasureInt(sourceClass, CxxMetric.COMPLEXITY);
if (complexity > maximumClassComplexityThreshold) {
getContext().createLineViolation(this,
"Class has a complexity of {0,number,integer} which is greater than {1,number,integer} authorized.",
node,
complexity,
maximumClassComplexityThreshold);
}
protected String getScopeName() {
return "class";
}

public void setMaximumClassComplexityThreshold(int threshold) {
this.maximumClassComplexityThreshold = threshold;
@Override
protected int getMaxComplexity() {
return max;
}

public void setMaxComplexity(int threshold) {
this.max = threshold;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
/*
* Sonar C++ Plugin (Community)
* Copyright (C) 2010-2018 SonarOpenCommunity
* http://github.com/SonarOpenCommunity/sonar-cxx
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.cxx.checks.metrics;

import java.util.Deque;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;

import org.sonar.cxx.CxxComplexityConstants;
import org.sonar.cxx.api.CxxKeyword;
import org.sonar.cxx.api.CxxPunctuator;
import org.sonar.cxx.parser.CxxGrammarImpl;
import org.sonar.cxx.utils.CxxReportIssue;
import org.sonar.cxx.visitors.MultiLocatitionSquidCheck;

import com.sonar.sslr.api.AstNode;
import com.sonar.sslr.api.AstNodeType;
import com.sonar.sslr.api.Grammar;

/**
* This is an enhanced version of
* org.sonar.squidbridge.metrics.ComplexityVisitor, which is used in order to
* compute the Cyclomatic Complexity.
*
* @param <G>
*/
public abstract class CxxCyclomaticComplexityCheck<G extends Grammar> extends MultiLocatitionSquidCheck<G> {

/**
* Structure, that tracks all nodes, which increase the code complexity
*/
private class ComplexitySource {
private final int line;
private final AstNodeType type;

public ComplexitySource(int line, AstNodeType nodeType) {
super();
this.line = line;
this.type = nodeType;
}

public String getLine() {
return Integer.valueOf(line).toString();
}

public String getExplanation() {
if (type == CxxGrammarImpl.functionDefinition) {
return "+1: function definition";
} else if (type == CxxKeyword.IF) {
return "+1: if statement";
} else if (type == CxxKeyword.FOR) {
return "+1: for loop";
} else if (type == CxxKeyword.WHILE) {
return "+1: while loop";
} else if (type == CxxKeyword.CATCH) {
return "+1: catch-clause";
} else if (type == CxxKeyword.CASE || type == CxxKeyword.DEFAULT) {
return "+1: switch label";
} else if (type == CxxPunctuator.AND || type == CxxPunctuator.OR) {
return "+1: logical operator";
} else if (type == CxxPunctuator.QUEST) {
return "+1: conditional operator";
}
return "+1";
}
}

/**
* Describe a code scope (function definition, class definition, entire file
* etc) in terms of complexity sources
*/
private class ComplexityScope {
public ComplexityScope(int startingLine) {
this.sources = new LinkedList<>();
this.complexity = 0;
this.startingLine = startingLine;
}

public void addComplexitySource(int line, AstNodeType nodeType) {
sources.add(new ComplexitySource(line, nodeType));
++complexity;
}

public List<ComplexitySource> getSources() {
return sources;
}

public int getComplexity() {
return complexity;
}

public String getStartingLine() {
return Integer.valueOf(startingLine).toString();
}

private List<ComplexitySource> sources;
private int complexity;
private int startingLine;
}

/**
* Stack for tracking the nested scopes (e.g. declaration of classes can be
* nested). Complexity of the inner scopes is added to the complexity of outer
* scopes.
*/
private Deque<ComplexityScope> complexityScopes;

/**
* @return the maximum allowed complexity for this scope
*/
protected abstract int getMaxComplexity();

/**
* @return valid AstNodeType if complexity is calculated for some language
* constructs only (e.g. function definition, class definition etc).
* Return Optional.empty() if the complexity is calculated for entire
* file.
*/
protected abstract Optional<AstNodeType> getScopeType();

/**
* @return the name of analyzed scope: "function", "class", "file" etc
*/
protected abstract String getScopeName();

@Override
public void init() {
subscribeTo(CxxComplexityConstants.CyclomaticComplexityAstNodeTypes);

if (getScopeType().isPresent()) {
final AstNodeType additionalNode = getScopeType().get();
if (!getAstNodeTypesToVisit().contains(additionalNode)) {
subscribeTo(additionalNode);
}
}

complexityScopes = new LinkedList<>();
}

@Override
public void visitFile(AstNode astNode) {
if (!getScopeType().isPresent()) {
complexityScopes.addFirst(new ComplexityScope(1));
}
}

@Override
public void leaveFile(AstNode astNode) {
if (!getScopeType().isPresent()) {
analyzeScopeComplexity();
}
}

@Override
public void visitNode(AstNode astNode) {
if (getScopeType().isPresent() && astNode.is(getScopeType().get())) {
complexityScopes.addFirst(new ComplexityScope(astNode.getTokenLine()));
}

if (astNode.is(CxxComplexityConstants.CyclomaticComplexityAstNodeTypes)) {
// for nested scopes (e.g. nested classes) the inner classes
// add complexity to the outer ones
for (ComplexityScope scope : complexityScopes) {
scope.addComplexitySource(astNode.getTokenLine(), astNode.getType());
}
}
}

@Override
public void leaveNode(AstNode astNode) {
if (getScopeType().isPresent() && astNode.is(getScopeType().get())) {
analyzeScopeComplexity();
}
}

private void analyzeScopeComplexity() {
ComplexityScope scope = complexityScopes.removeFirst();

final int maxComplexity = getMaxComplexity();
final int currentComplexity = scope.getComplexity();
if (scope.getComplexity() > maxComplexity) {
final StringBuilder msg = new StringBuilder();
msg.append("The Cyclomatic Complexity of this ").append(getScopeName()).append(" is ").append(currentComplexity)
.append(" which is greater than ").append(maxComplexity).append(" authorized.");

final CxxReportIssue issue = new CxxReportIssue(getRuleKey(), null, scope.getStartingLine(), msg.toString());
for (ComplexitySource source : scope.getSources()) {
issue.addLocation(null, source.getLine(), source.getExplanation());
}
createMultiLocationViolation(issue);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,17 @@
*/
package org.sonar.cxx.checks.metrics;

import com.sonar.sslr.api.Grammar;
import java.util.Optional;

import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.cxx.api.CxxMetric;
import org.sonar.cxx.tag.Tag;
import org.sonar.squidbridge.annotations.SqaleLinearWithOffsetRemediation;
import org.sonar.squidbridge.checks.AbstractFileComplexityCheck;
import org.sonar.squidbridge.measures.MetricDef;

import com.sonar.sslr.api.AstNode;
import com.sonar.sslr.api.AstNodeType;
import com.sonar.sslr.api.Grammar;

@Rule(
key = "FileComplexity",
Expand All @@ -39,7 +41,7 @@
coeff = "1min",
offset = "30min",
effortToFixDescription = "per complexity point above the threshold")
public class FileComplexityCheck extends AbstractFileComplexityCheck<Grammar> {
public class FileComplexityCheck extends CxxCyclomaticComplexityCheck<Grammar> {

private static final int DEFAULT_MAX = 200;

Expand All @@ -49,18 +51,22 @@ public class FileComplexityCheck extends AbstractFileComplexityCheck<Grammar> {
defaultValue = "" + DEFAULT_MAX)
private int max = DEFAULT_MAX;

public void setMax(int max) {
this.max = max;
@Override
protected Optional<AstNodeType> getScopeType() {
return Optional.empty();
}

@Override
public int getMaximumFileComplexity() {
return this.max;
protected String getScopeName() {
return "file";
}

@Override
public MetricDef getComplexityMetric() {
return CxxMetric.COMPLEXITY;
protected int getMaxComplexity() {
return max;
}

public void setMaxComplexity(int max) {
this.max = max;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import org.sonar.cxx.api.CxxMetric;
import org.sonar.cxx.parser.CxxGrammarImpl;
import org.sonar.cxx.tag.Tag;
import org.sonar.cxx.utils.CxxReportIssue;
import org.sonar.cxx.visitors.MultiLocatitionSquidCheck;
import org.sonar.squidbridge.annotations.ActivatedByDefault;
import org.sonar.squidbridge.annotations.SqaleLinearWithOffsetRemediation;
import org.sonar.squidbridge.api.SourceFunction;
Expand Down
Loading