From 97a7a9db28d5153068aeab744e72d2e05699e3b1 Mon Sep 17 00:00:00 2001 From: Ivan Galkin Date: Sun, 5 Aug 2018 03:00:17 +0200 Subject: [PATCH] implement explanation for cyclomatic complexity checks 1. implement multi-location messages for squidsensor * move CxxReportIssue and CxxReportLocation from cxx-sensors to cxx-squid in order to satisfy dependencies 2. introduce CxxCyclomaticComplexityCheck * this class is used for all scoped cyclomatic complexity checks * implement * ClassComplexityCheck * FileComplexityCheck * FunctionComplexityCheck * they respect the scopes (important for nested classes) * they provide detailed information about complexity sources * FileComplexityCheck previously had a wrong calculation: cyclomatic complexity of functions was always + 1 -> fixed now This change addresses #1494: * cyclomatic complexity checks (done) * cognitive complexity checks (todo, depends on #1535 --- .../checks/metrics/ClassComplexityCheck.java | 39 ++-- .../metrics/CxxCyclomaticComplexityCheck.java | 211 ++++++++++++++++++ .../checks/metrics/FileComplexityCheck.java | 28 ++- .../FunctionCognitiveComplexityCheck.java | 2 + .../metrics/FunctionComplexityCheck.java | 41 ++-- .../metrics/ClassComplexityCheckTest.java | 81 ++++++- .../metrics/FileComplexityCheckTest.java | 27 ++- .../metrics/FunctionComplexityCheckTest.java | 93 +++++++- .../test/resources/checks/ClassComplexity.cc | 37 +++ .../resources/checks/FunctionComplexity.cc | 25 +++ .../cxx/sensors/clangsa/CxxClangSASensor.java | 2 +- .../sensors/clangtidy/CxxClangTidySensor.java | 2 +- .../sensors/compiler/CxxCompilerSensor.java | 2 +- .../sensors/cppcheck/CppcheckParserV1.java | 2 +- .../sensors/cppcheck/CppcheckParserV2.java | 4 +- .../sensors/drmemory/CxxDrMemorySensor.java | 2 +- .../cxx/sensors/other/CxxOtherSensor.java | 2 +- .../cxx/sensors/pclint/CxxPCLintSensor.java | 2 +- .../sonar/cxx/sensors/rats/CxxRatsSensor.java | 2 +- .../sonar/cxx/sensors/squid/CxxChecks.java | 5 +- .../cxx/sensors/squid/CxxSquidSensor.java | 43 +++- .../sensors/utils/CxxIssuesReportSensor.java | 2 + .../sensors/valgrind/CxxValgrindSensor.java | 2 +- .../cxx/sensors/veraxx/CxxVeraxxSensor.java | 2 +- .../compiler/MockCxxCompilerSensor.java | 4 +- .../java/org/sonar/cxx/CxxAstScanner.java | 19 +- .../org/sonar/cxx/CxxComplexityConstants.java | 55 +++++ .../org/sonar/cxx}/utils/CxxReportIssue.java | 2 +- .../sonar/cxx}/utils/CxxReportLocation.java | 2 +- .../org/sonar/cxx/utils/package-info.java | 23 ++ .../visitors/AbstractCxxPublicApiVisitor.java | 1 - .../visitors/MultiLocatitionSquidCheck.java | 138 ++++++++++++ .../sonar/cxx}/utils/CxxReportIssueTest.java | 2 +- .../org/sonar/cxx/utils/package-info.java | 23 ++ 34 files changed, 797 insertions(+), 130 deletions(-) create mode 100644 cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/CxxCyclomaticComplexityCheck.java create mode 100644 cxx-squid/src/main/java/org/sonar/cxx/CxxComplexityConstants.java rename {cxx-sensors/src/main/java/org/sonar/cxx/sensors => cxx-squid/src/main/java/org/sonar/cxx}/utils/CxxReportIssue.java (95%) rename {cxx-sensors/src/main/java/org/sonar/cxx/sensors => cxx-squid/src/main/java/org/sonar/cxx}/utils/CxxReportLocation.java (94%) create mode 100644 cxx-squid/src/main/java/org/sonar/cxx/utils/package-info.java create mode 100644 cxx-squid/src/main/java/org/sonar/cxx/visitors/MultiLocatitionSquidCheck.java rename {cxx-sensors/src/test/java/org/sonar/cxx/sensors => cxx-squid/src/test/java/org/sonar/cxx}/utils/CxxReportIssueTest.java (98%) create mode 100644 cxx-squid/src/test/java/org/sonar/cxx/utils/package-info.java diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/ClassComplexityCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/ClassComplexityCheck.java index e6f2584a30..9f168913a5 100644 --- a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/ClassComplexityCheck.java +++ b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/ClassComplexityCheck.java @@ -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", @@ -41,7 +40,7 @@ coeff = "1min", offset = "10min", effortToFixDescription = "per complexity point over the threshold") -public class ClassComplexityCheck extends SquidCheck { +public class ClassComplexityCheck extends CxxCyclomaticComplexityCheck { private static final int DEFAULT_MAXIMUM_CLASS_COMPLEXITY_THRESHOLD = 60; @@ -49,28 +48,24 @@ public class ClassComplexityCheck extends SquidCheck { 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 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; + } } diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/CxxCyclomaticComplexityCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/CxxCyclomaticComplexityCheck.java new file mode 100644 index 0000000000..bc3132d777 --- /dev/null +++ b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/CxxCyclomaticComplexityCheck.java @@ -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 + */ +public abstract class CxxCyclomaticComplexityCheck extends MultiLocatitionSquidCheck { + + /** + * 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 getSources() { + return sources; + } + + public int getComplexity() { + return complexity; + } + + public String getStartingLine() { + return Integer.valueOf(startingLine).toString(); + } + + private List 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 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 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 (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()); + } + } + + if (getScopeType().isPresent() && astNode.is(getScopeType().get())) { + complexityScopes.addFirst(new ComplexityScope(astNode.getTokenLine())); + } + } + + @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); + } + } +} \ No newline at end of file diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FileComplexityCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FileComplexityCheck.java index f91e8e9f4c..b6c7367b20 100644 --- a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FileComplexityCheck.java +++ b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FileComplexityCheck.java @@ -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", @@ -39,7 +41,7 @@ coeff = "1min", offset = "30min", effortToFixDescription = "per complexity point above the threshold") -public class FileComplexityCheck extends AbstractFileComplexityCheck { +public class FileComplexityCheck extends CxxCyclomaticComplexityCheck { private static final int DEFAULT_MAX = 200; @@ -49,18 +51,22 @@ public class FileComplexityCheck extends AbstractFileComplexityCheck { defaultValue = "" + DEFAULT_MAX) private int max = DEFAULT_MAX; - public void setMax(int max) { - this.max = max; + @Override + protected Optional 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; + } } diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FunctionCognitiveComplexityCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FunctionCognitiveComplexityCheck.java index 00276eb4cc..45c1966ea1 100644 --- a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FunctionCognitiveComplexityCheck.java +++ b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FunctionCognitiveComplexityCheck.java @@ -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; diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FunctionComplexityCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FunctionComplexityCheck.java index fb45dc21b7..2c7e310465 100644 --- a/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FunctionComplexityCheck.java +++ b/cxx-checks/src/main/java/org/sonar/cxx/checks/metrics/FunctionComplexityCheck.java @@ -19,19 +19,18 @@ */ package org.sonar.cxx.checks.metrics; +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.squidbridge.api.SourceFunction; -import org.sonar.squidbridge.checks.ChecksHelper; -import org.sonar.squidbridge.checks.SquidCheck; -import com.sonar.sslr.api.AstNode; -import com.sonar.sslr.api.Grammar; +import org.sonar.cxx.tag.Tag; import org.sonar.squidbridge.annotations.ActivatedByDefault; import org.sonar.squidbridge.annotations.SqaleLinearWithOffsetRemediation; -import org.sonar.cxx.tag.Tag; + +import com.sonar.sslr.api.AstNodeType; +import com.sonar.sslr.api.Grammar; @Rule( key = "FunctionComplexity", @@ -43,7 +42,9 @@ coeff = "1min", offset = "10min", effortToFixDescription = "per complexity point above the threshold") -public class FunctionComplexityCheck extends SquidCheck { +public class FunctionComplexityCheck extends CxxCyclomaticComplexityCheck { + + // TODO MultiLineSquidCheck private static final int DEFAULT_MAX = 10; @@ -54,25 +55,21 @@ public class FunctionComplexityCheck extends SquidCheck { private int max = DEFAULT_MAX; @Override - public void init() { - subscribeTo(CxxGrammarImpl.functionDefinition); + protected Optional getScopeType() { + return Optional.of(CxxGrammarImpl.functionDefinition); + } + + @Override + protected String getScopeName() { + return "function"; } @Override - public void leaveNode(AstNode node) { - SourceFunction sourceFunction = (SourceFunction) getContext().peekSourceCode(); - int complexity = ChecksHelper.getRecursiveMeasureInt(sourceFunction, CxxMetric.COMPLEXITY); - if (complexity > max) { - getContext().createLineViolation(this, - "The Cyclomatic Complexity of this function is {0,number,integer} which is greater than " - + "{1,number,integer} authorized.", - node, - complexity, - max); - } + protected int getMaxComplexity() { + return max; } - public void setMax(int max) { + public void setMaxComplexity(int max) { this.max = max; } } diff --git a/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/ClassComplexityCheckTest.java b/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/ClassComplexityCheckTest.java index 796269e42a..10c103a7c9 100644 --- a/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/ClassComplexityCheckTest.java +++ b/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/ClassComplexityCheckTest.java @@ -19,29 +19,90 @@ */ package org.sonar.cxx.checks.metrics; -import org.sonar.squidbridge.checks.CheckMessagesVerifier; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.util.Set; + +import org.assertj.core.api.SoftAssertions; import org.junit.Test; -import org.sonar.squidbridge.api.SourceFile; import org.sonar.cxx.CxxAstScanner; import org.sonar.cxx.checks.CxxFileTester; import org.sonar.cxx.checks.CxxFileTesterHelper; - -import java.io.IOException; -import java.io.UnsupportedEncodingException; +import org.sonar.cxx.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportLocation; +import org.sonar.cxx.visitors.MultiLocatitionSquidCheck; +import org.sonar.squidbridge.api.SourceFile; public class ClassComplexityCheckTest { @Test - @SuppressWarnings("squid:S2699") // ... verify contains the assertion public void test() throws UnsupportedEncodingException, IOException { ClassComplexityCheck check = new ClassComplexityCheck(); - check.setMaximumClassComplexityThreshold(5); + check.setMaxComplexity(5); CxxFileTester tester = CxxFileTesterHelper.CreateCxxFileTester("src/test/resources/checks/ClassComplexity.cc", "."); SourceFile file = CxxAstScanner.scanSingleFile(tester.cxxFile, tester.sensorContext, CxxFileTesterHelper.mockCxxLanguage(), check); - CheckMessagesVerifier.verify(file.getCheckMessages()) - .next().atLine(9).withMessage("Class has a complexity of 10 which is greater than 5 authorized.") - .noMore(); + + Set issues = MultiLocatitionSquidCheck.getMultiLocationCheckMessages(file); + assertThat(issues).isNotNull(); + SoftAssertions softly = new SoftAssertions(); + softly.assertThat(issues).hasSize(3); + softly.assertThat(issues).allSatisfy(issue -> "ClassComplexity".equals(issue.getRuleId())); + + CxxReportIssue issue0 = issues.stream().filter(issue -> issue.getLocations().get(0).getLine().equals("9")) + .findFirst().orElseThrow(() -> new AssertionError("No issue at line 9")); + softly.assertThat(issue0.getLocations()).containsOnly( + new CxxReportLocation(null, "9", + "The Cyclomatic Complexity of this class is 12 which is greater than 5 authorized."), + new CxxReportLocation(null, "14", "+1: function definition"), + new CxxReportLocation(null, "16", "+1: function definition"), + new CxxReportLocation(null, "21", "+1: function definition"), + new CxxReportLocation(null, "22", "+1: function definition"), + new CxxReportLocation(null, "25", "+1: function definition"), + new CxxReportLocation(null, "26", "+1: if statement"), + new CxxReportLocation(null, "27", "+1: if statement"), + new CxxReportLocation(null, "28", "+1: conditional operator"), + new CxxReportLocation(null, "30", "+1: conditional operator"), + new CxxReportLocation(null, "33", "+1: if statement"), + new CxxReportLocation(null, "34", "+1: conditional operator"), + new CxxReportLocation(null, "36", "+1: conditional operator")); + + CxxReportIssue issue1 = issues.stream().filter(issue -> issue.getLocations().get(0).getLine().equals("42")) + .findFirst().orElseThrow(() -> new AssertionError("No issue at line 42")); + softly.assertThat(issue1.getLocations()).containsOnly( + new CxxReportLocation(null, "42", + "The Cyclomatic Complexity of this class is 10 which is greater than 5 authorized."), + new CxxReportLocation(null, "47", "+1: function definition"), + new CxxReportLocation(null, "49", "+1: function definition"), + new CxxReportLocation(null, "51", "+1: switch label"), + new CxxReportLocation(null, "53", "+1: switch label"), + new CxxReportLocation(null, "57", "+1: function definition"), + new CxxReportLocation(null, "58", "+1: for loop"), + new CxxReportLocation(null, "59", "+1: if statement"), + new CxxReportLocation(null, "59", "+1: logical operator"), + new CxxReportLocation(null, "59", "+1: logical operator"), + new CxxReportLocation(null, "65", "+1: function definition") + ); + + CxxReportIssue issue2 = issues.stream().filter(issue -> issue.getLocations().get(0).getLine().equals("45")) + .findFirst().orElseThrow(() -> new AssertionError("No issue at line 45")); + softly.assertThat(issue2.getLocations()).containsOnly( + new CxxReportLocation(null, "45", + "The Cyclomatic Complexity of this class is 9 which is greater than 5 authorized."), + new CxxReportLocation(null, "47", "+1: function definition"), + new CxxReportLocation(null, "49", "+1: function definition"), + new CxxReportLocation(null, "51", "+1: switch label"), + new CxxReportLocation(null, "53", "+1: switch label"), + new CxxReportLocation(null, "57", "+1: function definition"), + new CxxReportLocation(null, "58", "+1: for loop"), + new CxxReportLocation(null, "59", "+1: if statement"), + new CxxReportLocation(null, "59", "+1: logical operator"), + new CxxReportLocation(null, "59", "+1: logical operator")); + + softly.assertAll(); + } } diff --git a/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/FileComplexityCheckTest.java b/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/FileComplexityCheckTest.java index 2baed20913..e9fa7cba7b 100644 --- a/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/FileComplexityCheckTest.java +++ b/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/FileComplexityCheckTest.java @@ -19,28 +19,45 @@ */ package org.sonar.cxx.checks.metrics; +import static org.assertj.core.api.Assertions.assertThat; + import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.util.Set; + +import org.assertj.core.api.SoftAssertions; import org.junit.Test; import org.sonar.cxx.CxxAstScanner; import org.sonar.cxx.checks.CxxFileTester; import org.sonar.cxx.checks.CxxFileTesterHelper; +import org.sonar.cxx.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportLocation; +import org.sonar.cxx.visitors.MultiLocatitionSquidCheck; import org.sonar.squidbridge.api.SourceFile; -import org.sonar.squidbridge.checks.CheckMessagesVerifier; public class FileComplexityCheckTest { @Test - @SuppressWarnings("squid:S2699") // ... verify contains the assertion public void check() throws UnsupportedEncodingException, IOException { FileComplexityCheck check = new FileComplexityCheck(); - check.setMax(1); + check.setMaxComplexity(1); CxxFileTester tester = CxxFileTesterHelper.CreateCxxFileTester("src/test/resources/checks/functions.cc", "."); SourceFile file = CxxAstScanner.scanSingleFile(tester.cxxFile, tester.sensorContext, CxxFileTesterHelper.mockCxxLanguage(), check); - CheckMessagesVerifier.verify(file.getCheckMessages()) - .next().noMore(); + Set issues = MultiLocatitionSquidCheck.getMultiLocationCheckMessages(file); + assertThat(issues).isNotNull(); + SoftAssertions softly = new SoftAssertions(); + softly.assertThat(issues).hasSize(1); + + CxxReportIssue actualIssue = issues.iterator().next(); + softly.assertThat(actualIssue.getRuleId()).isEqualTo("FileComplexity"); + softly.assertThat(actualIssue.getLocations()).containsOnly( + new CxxReportLocation(null, "1", + "The Cyclomatic Complexity of this file is 2 which is greater than 1 authorized."), + new CxxReportLocation(null, "3", "+1: function definition"), + new CxxReportLocation(null, "5", "+1: function definition")); + softly.assertAll(); } } diff --git a/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/FunctionComplexityCheckTest.java b/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/FunctionComplexityCheckTest.java index 0bf078ef57..29c3fb33f8 100644 --- a/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/FunctionComplexityCheckTest.java +++ b/cxx-checks/src/test/java/org/sonar/cxx/checks/metrics/FunctionComplexityCheckTest.java @@ -19,31 +19,108 @@ */ package org.sonar.cxx.checks.metrics; +import static org.assertj.core.api.Assertions.assertThat; + import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.util.Set; +import org.assertj.core.api.SoftAssertions; import org.junit.Test; import org.sonar.cxx.CxxAstScanner; import org.sonar.cxx.checks.CxxFileTester; import org.sonar.cxx.checks.CxxFileTesterHelper; +import org.sonar.cxx.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportLocation; +import org.sonar.cxx.visitors.MultiLocatitionSquidCheck; import org.sonar.squidbridge.api.SourceFile; -import org.sonar.squidbridge.checks.CheckMessagesVerifier; public class FunctionComplexityCheckTest { @Test - @SuppressWarnings("squid:S2699") // ... verify contains the assertion public void check() throws UnsupportedEncodingException, IOException { FunctionComplexityCheck check = new FunctionComplexityCheck(); - check.setMax(5); + check.setMaxComplexity(5); CxxFileTester tester = CxxFileTesterHelper.CreateCxxFileTester("src/test/resources/checks/FunctionComplexity.cc", "."); SourceFile file = CxxAstScanner.scanSingleFile(tester.cxxFile, tester.sensorContext, CxxFileTesterHelper.mockCxxLanguage(), check); - CheckMessagesVerifier.verify(file.getCheckMessages()) - .next().atLine(13) - .next().atLine(33) - .next().atLine(51) - .next().atLine(72); + Set issues = MultiLocatitionSquidCheck.getMultiLocationCheckMessages(file); + assertThat(issues).isNotNull(); + SoftAssertions softly = new SoftAssertions(); + softly.assertThat(issues).hasSize(5); + softly.assertThat(issues).allSatisfy(issue -> "FunctionComplexity".equals(issue.getRuleId())); + + CxxReportIssue issue0 = issues.stream().filter(issue -> issue.getLocations().get(0).getLine().equals("13")) + .findFirst().orElseThrow(() -> new AssertionError("No issue at line 13")); + softly.assertThat(issue0.getLocations()).containsOnly( + new CxxReportLocation(null, "13", + "The Cyclomatic Complexity of this function is 7 which is greater than 5 authorized."), + new CxxReportLocation(null, "14", "+1: if statement"), + new CxxReportLocation(null, "15", "+1: if statement"), + new CxxReportLocation(null, "16", "+1: conditional operator"), + new CxxReportLocation(null, "18", "+1: conditional operator"), + new CxxReportLocation(null, "21", "+1: if statement"), + new CxxReportLocation(null, "22", "+1: conditional operator"), + new CxxReportLocation(null, "24", "+1: conditional operator")); + + CxxReportIssue issue1 = issues.stream().filter(issue -> issue.getLocations().get(0).getLine().equals("33")) + .findFirst().orElseThrow(() -> new AssertionError("No issue at line 33")); + softly.assertThat(issue1.getLocations()).containsOnly( + new CxxReportLocation(null, "33", + "The Cyclomatic Complexity of this function is 7 which is greater than 5 authorized."), + new CxxReportLocation(null, "34", "+1: if statement"), + new CxxReportLocation(null, "35", "+1: if statement"), + new CxxReportLocation(null, "36", "+1: conditional operator"), + new CxxReportLocation(null, "38", "+1: conditional operator"), + new CxxReportLocation(null, "41", "+1: if statement"), + new CxxReportLocation(null, "42", "+1: conditional operator"), + new CxxReportLocation(null, "44", "+1: conditional operator")); + + CxxReportIssue issue2 = issues.stream().filter(issue -> issue.getLocations().get(0).getLine().equals("51")) + .findFirst().orElseThrow(() -> new AssertionError("No issue at line 51")); + softly.assertThat(issue2.getLocations()).containsOnly( + new CxxReportLocation(null, "51", + "The Cyclomatic Complexity of this function is 7 which is greater than 5 authorized."), + new CxxReportLocation(null, "52", "+1: if statement"), + new CxxReportLocation(null, "53", "+1: if statement"), + new CxxReportLocation(null, "54", "+1: conditional operator"), + new CxxReportLocation(null, "56", "+1: conditional operator"), + new CxxReportLocation(null, "59", "+1: if statement"), + new CxxReportLocation(null, "60", "+1: conditional operator"), + new CxxReportLocation(null, "62", "+1: conditional operator")); + + CxxReportIssue issue3 = issues.stream().filter(issue -> issue.getLocations().get(0).getLine().equals("72")) + .findFirst().orElseThrow(() -> new AssertionError("No issue at line 72")); + softly.assertThat(issue3.getLocations()).containsOnly( + new CxxReportLocation(null, "72", + "The Cyclomatic Complexity of this function is 7 which is greater than 5 authorized."), + new CxxReportLocation(null, "73", "+1: if statement"), + new CxxReportLocation(null, "74", "+1: if statement"), + new CxxReportLocation(null, "75", "+1: conditional operator"), + new CxxReportLocation(null, "77", "+1: conditional operator"), + new CxxReportLocation(null, "80", "+1: if statement"), + new CxxReportLocation(null, "81", "+1: conditional operator"), + new CxxReportLocation(null, "83", "+1: conditional operator")); + + CxxReportIssue issue4 = issues.stream().filter(issue -> issue.getLocations().get(0).getLine().equals("89")) + .findFirst().orElseThrow(() -> new AssertionError("No issue at line 89")); + softly.assertThat(issue4.getLocations()).containsOnly( + new CxxReportLocation(null, "89", + "The Cyclomatic Complexity of this function is 13 which is greater than 5 authorized."), + new CxxReportLocation(null, "91", "+1: if statement"), + new CxxReportLocation(null, "91", "+1: logical operator"), + new CxxReportLocation(null, "91", "+1: logical operator"), + new CxxReportLocation(null, "94", "+1: catch-clause"), + new CxxReportLocation(null, "96", "+1: catch-clause"), + new CxxReportLocation(null, "98", "+1: catch-clause"), + new CxxReportLocation(null, "100", "+1: catch-clause"), + new CxxReportLocation(null, "102", "+1: catch-clause"), + new CxxReportLocation(null, "104", "+1: catch-clause"), + new CxxReportLocation(null, "106", "+1: catch-clause"), + new CxxReportLocation(null, "107", "+1: while loop"), + new CxxReportLocation(null, "108", "+1: conditional operator"), + new CxxReportLocation(null, "110", "+1: conditional operator")); + softly.assertAll(); } } diff --git a/cxx-checks/src/test/resources/checks/ClassComplexity.cc b/cxx-checks/src/test/resources/checks/ClassComplexity.cc index 65954905c0..6958f7f56e 100644 --- a/cxx-checks/src/test/resources/checks/ClassComplexity.cc +++ b/cxx-checks/src/test/resources/checks/ClassComplexity.cc @@ -8,6 +8,16 @@ class MyClass1 { class MyClass2 { public: + // nested class adds complexity to the outer one + class SimpleNestedClass { + public: + SimpleNestedClass() { + } + int Method0(int a, int b) { + return 0; + } + }; + MyClass2() {}; int Method1(int a, int b) { return 0; @@ -28,3 +38,30 @@ class MyClass2 { } } }; + +class MyClass3 { +public: + // nested class adds complexity to the outer one + class ComplexNestedClass { + public: + ComplexNestedClass() { + } + int Method0(int in) { + switch (in) { + case 0: + return 1; + default: + return 2; + } + } + int Method1() { + for (int i = 0; i < 10; ++i) { + if (i != 0 && i % 2 == 0 && i % 3 == 0) { + return i; + } + } + } + }; + void Method1() { + } +}; diff --git a/cxx-checks/src/test/resources/checks/FunctionComplexity.cc b/cxx-checks/src/test/resources/checks/FunctionComplexity.cc index 62c6a1d609..0932fb85c9 100644 --- a/cxx-checks/src/test/resources/checks/FunctionComplexity.cc +++ b/cxx-checks/src/test/resources/checks/FunctionComplexity.cc @@ -85,3 +85,28 @@ class MyTemplate { } } }; + +std::string func_with_trycatch(int i) { + try { + if ((i % 2 == 0 && i % 3 == 0) || (i % 6 == 0)) { + return std::to_string(i); + } + } catch (const std::logic_error& e) { + return "std::logic_error"; + } catch (const std::bad_cast& e) { + return std::string { "std::bad_cast" }; + } catch (const std::invalid_argument& e) { + return std::string { "std::invalid_argument" }; + } catch (const std::length_error& e) { + return std::string { "std::length_error" }; + } catch (const std::out_of_range& e) { + return std::string { "std::out_of_range" }; + } catch (const std::exception& e) { + return std::string("std::exception"); + } catch (...) { + while (i >= 0) { + return (i % 3 == 0) ? "unknown exception" : "unexpected exception"; + } + return (i % 2 == 0) ? "exotic exception" : "strange exception"; + } +} diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangsa/CxxClangSASensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangsa/CxxClangSASensor.java index d7999df57d..ad9014c26f 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangsa/CxxClangSASensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangsa/CxxClangSASensor.java @@ -30,7 +30,7 @@ import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportIssue; import com.dd.plist.NSArray; import com.dd.plist.NSDictionary; diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java index 02758869a5..853b1613b4 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/clangtidy/CxxClangTidySensor.java @@ -32,7 +32,7 @@ import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportIssue; /** * Sensor for clang-tidy diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/compiler/CxxCompilerSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/compiler/CxxCompilerSensor.java index 69ae28cd08..15acd29f54 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/compiler/CxxCompilerSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/compiler/CxxCompilerSensor.java @@ -29,7 +29,7 @@ import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportIssue; /** * compiler for C++ with advanced analysis features (e.g. for VC 2008 team edition or 2010/2012/2013/2015/2017 premium diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV1.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV1.java index 41230a5f94..612209d4c2 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV1.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV1.java @@ -26,9 +26,9 @@ 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.CxxReportIssue; import org.sonar.cxx.sensors.utils.EmptyReportException; import org.sonar.cxx.sensors.utils.StaxParser; +import org.sonar.cxx.utils.CxxReportIssue; /** * {@inheritDoc} diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java index adf7c9ef92..3189e465c2 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java @@ -30,10 +30,10 @@ 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.CxxReportIssue; -import org.sonar.cxx.sensors.utils.CxxReportLocation; import org.sonar.cxx.sensors.utils.EmptyReportException; import org.sonar.cxx.sensors.utils.StaxParser; +import org.sonar.cxx.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportLocation; /** * {@inheritDoc} diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/drmemory/CxxDrMemorySensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/drmemory/CxxDrMemorySensor.java index 3b2445db61..e8befe6e00 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/drmemory/CxxDrMemorySensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/drmemory/CxxDrMemorySensor.java @@ -31,7 +31,7 @@ import org.sonar.cxx.sensors.drmemory.DrMemoryParser.DrMemoryError; import org.sonar.cxx.sensors.drmemory.DrMemoryParser.DrMemoryError.Location; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportIssue; /** * Dr. Memory is a memory monitoring tool capable of identifying memory-related programming errors such as accesses of diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherSensor.java index 88f50aefec..e09e0fc6bb 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherSensor.java @@ -40,9 +40,9 @@ import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; import org.sonar.cxx.sensors.utils.CxxUtils; import org.sonar.cxx.sensors.utils.StaxParser; +import org.sonar.cxx.utils.CxxReportIssue; /** * Custom Rule Import, all static analysis are supported. diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/pclint/CxxPCLintSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/pclint/CxxPCLintSensor.java index 900ae6db1f..e390523dbe 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/pclint/CxxPCLintSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/pclint/CxxPCLintSensor.java @@ -35,10 +35,10 @@ import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; import org.sonar.cxx.sensors.utils.CxxUtils; import org.sonar.cxx.sensors.utils.EmptyReportException; import org.sonar.cxx.sensors.utils.StaxParser; +import org.sonar.cxx.utils.CxxReportIssue; /** * PC-lint is an equivalent to pmd but for C++ The first version of the tool was release 1985 and the tool analyzes diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/rats/CxxRatsSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/rats/CxxRatsSensor.java index 9c5f7397b6..a3744d8044 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/rats/CxxRatsSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/rats/CxxRatsSensor.java @@ -34,8 +34,8 @@ import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; import org.sonar.cxx.sensors.utils.CxxUtils; +import org.sonar.cxx.utils.CxxReportIssue; /** * {@inheritDoc} diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/squid/CxxChecks.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/squid/CxxChecks.java index c7f19f39d9..a01f49ae0c 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/squid/CxxChecks.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/squid/CxxChecks.java @@ -76,11 +76,8 @@ public List> all() { @Nullable public RuleKey ruleKey(SquidAstVisitor check) { - RuleKey ruleKey; - for (Checks> checks : checksByRepository) { - ruleKey = checks.ruleKey(check); - + RuleKey ruleKey = checks.ruleKey(check); if (ruleKey != null) { return ruleKey; } diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/squid/CxxSquidSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/squid/CxxSquidSensor.java index a0f2e11c51..f59c6bb93e 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/squid/CxxSquidSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/squid/CxxSquidSensor.java @@ -53,6 +53,9 @@ import org.sonar.cxx.sensors.visitors.CxxCpdVisitor; import org.sonar.cxx.sensors.visitors.CxxFileLinesVisitor; import org.sonar.cxx.sensors.visitors.CxxHighlighterVisitor; +import org.sonar.cxx.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportLocation; +import org.sonar.cxx.visitors.MultiLocatitionSquidCheck; import org.sonar.squidbridge.AstScanner; import org.sonar.squidbridge.SquidAstVisitor; import org.sonar.squidbridge.api.CheckMessage; @@ -278,29 +281,45 @@ private void saveMeasures(InputFile inputFile, SourceFile squidFile, SensorConte } private int saveViolations(InputFile inputFile, SourceFile squidFile, SensorContext sensorContext) { - Collection messages = squidFile.getCheckMessages(); int violationsCount = 0; - if (messages != null) { - for (CheckMessage message : messages) { + if (squidFile.hasCheckMessages()) { + for (CheckMessage message : squidFile.getCheckMessages()) { int line = 1; if (message.getLine() != null && message.getLine() > 0) { line = message.getLine(); } - NewIssue newIssue = sensorContext - .newIssue() - .forRule(RuleKey.of(this.language.getRepositoryKey(), checks.ruleKey((SquidAstVisitor) message.getCheck()).rule())); - NewIssueLocation location = newIssue.newLocation() - .on(inputFile) - .at(inputFile.selectLine(line)) - .message(message.getText(Locale.ENGLISH)); + NewIssue newIssue = sensorContext.newIssue().forRule(RuleKey.of(this.language.getRepositoryKey(), + checks.ruleKey((SquidAstVisitor) message.getCheck()).rule())); + NewIssueLocation location = newIssue.newLocation().on(inputFile).at(inputFile.selectLine(line)) + .message(message.getText(Locale.ENGLISH)); newIssue.at(location); newIssue.save(); + ++violationsCount; + } + } - // @todo - this will add a issue regardless of the save - violationsCount++; + if (MultiLocatitionSquidCheck.hasMultiLocationCheckMessages(squidFile)) { + for (CxxReportIssue issue : MultiLocatitionSquidCheck.getMultiLocationCheckMessages(squidFile)) { + final NewIssue newIssue = sensorContext.newIssue() + .forRule(RuleKey.of(language.getRepositoryKey(), issue.getRuleId())); + int locationNr = 0; + for (CxxReportLocation location : issue.getLocations()) { + final Integer line = Integer.valueOf(location.getLine()); + final NewIssueLocation newIssueLocation = newIssue.newLocation().on(inputFile).at(inputFile.selectLine(line)) + .message(location.getInfo()); + if (locationNr == 0) { + newIssue.at(newIssueLocation); + } else { + newIssue.addLocation(newIssueLocation); + } + ++locationNr; + } + newIssue.save(); + ++violationsCount; } + MultiLocatitionSquidCheck.eraseMultilineCheckMessages(squidFile); } return violationsCount; diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxIssuesReportSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxIssuesReportSensor.java index 336187ab7a..ce0716f436 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxIssuesReportSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxIssuesReportSensor.java @@ -39,6 +39,8 @@ import org.sonar.api.utils.log.Loggers; import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; +import org.sonar.cxx.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportLocation; /** * This class is used as base for all sensors which import external reports, diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/CxxValgrindSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/CxxValgrindSensor.java index 11acc89622..fd02846655 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/CxxValgrindSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/CxxValgrindSensor.java @@ -29,7 +29,7 @@ import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportIssue; /** * {@inheritDoc} diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/veraxx/CxxVeraxxSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/veraxx/CxxVeraxxSensor.java index 6f9e2ae3d5..4c349cc359 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/veraxx/CxxVeraxxSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/veraxx/CxxVeraxxSensor.java @@ -30,10 +30,10 @@ import org.sonar.cxx.CxxLanguage; import org.sonar.cxx.CxxMetricsFactory; import org.sonar.cxx.sensors.utils.CxxIssuesReportSensor; -import org.sonar.cxx.sensors.utils.CxxReportIssue; import org.sonar.cxx.sensors.utils.CxxUtils; import org.sonar.cxx.sensors.utils.EmptyReportException; import org.sonar.cxx.sensors.utils.StaxParser; +import org.sonar.cxx.utils.CxxReportIssue; /** * {@inheritDoc} diff --git a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/compiler/MockCxxCompilerSensor.java b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/compiler/MockCxxCompilerSensor.java index e3b2cf4993..16ea94312b 100644 --- a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/compiler/MockCxxCompilerSensor.java +++ b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/compiler/MockCxxCompilerSensor.java @@ -34,8 +34,8 @@ import org.sonar.api.batch.sensor.SensorDescriptor; import org.sonar.api.profiles.RulesProfile; import org.sonar.cxx.CxxLanguage; -import org.sonar.cxx.sensors.utils.CxxReportIssue; -import org.sonar.cxx.sensors.utils.CxxReportLocation; +import org.sonar.cxx.utils.CxxReportIssue; +import org.sonar.cxx.utils.CxxReportLocation; public class MockCxxCompilerSensor extends CxxCompilerSensor { diff --git a/cxx-squid/src/main/java/org/sonar/cxx/CxxAstScanner.java b/cxx-squid/src/main/java/org/sonar/cxx/CxxAstScanner.java index a26f6d381d..9b5c249a80 100644 --- a/cxx-squid/src/main/java/org/sonar/cxx/CxxAstScanner.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/CxxAstScanner.java @@ -23,9 +23,7 @@ import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.sensor.SensorContext; -import org.sonar.cxx.api.CxxKeyword; import org.sonar.cxx.api.CxxMetric; -import org.sonar.cxx.api.CxxPunctuator; import org.sonar.cxx.parser.CxxGrammarImpl; import org.sonar.cxx.parser.CxxParser; import org.sonar.cxx.visitors.CxxCharsetAwareVisitor; @@ -55,7 +53,6 @@ import org.sonar.squidbridge.metrics.LinesVisitor; import com.sonar.sslr.api.AstNode; -import com.sonar.sslr.api.AstNodeType; import com.sonar.sslr.api.GenericTokenType; import com.sonar.sslr.api.Grammar; import com.sonar.sslr.api.Token; @@ -217,23 +214,9 @@ public SourceCode createSourceCode(SourceCode parentSourceCode, AstNode astNode) .subscribeTo(CxxGrammarImpl.statement) .build()); - AstNodeType[] complexityAstNodeType = new AstNodeType[]{ - // Entry points - CxxGrammarImpl.functionDefinition, - CxxKeyword.IF, - CxxKeyword.FOR, - CxxKeyword.WHILE, - CxxKeyword.CATCH, - CxxKeyword.CASE, - CxxKeyword.DEFAULT, - CxxPunctuator.AND, - CxxPunctuator.OR, - CxxPunctuator.QUEST - }; - builder.withSquidAstVisitor(ComplexityVisitor.builder() .setMetricDef(CxxMetric.COMPLEXITY) - .subscribeTo(complexityAstNodeType) + .subscribeTo(CxxComplexityConstants.CyclomaticComplexityAstNodeTypes) .build()); builder.withSquidAstVisitor(CxxCognitiveComplexityVisitor.builder() diff --git a/cxx-squid/src/main/java/org/sonar/cxx/CxxComplexityConstants.java b/cxx-squid/src/main/java/org/sonar/cxx/CxxComplexityConstants.java new file mode 100644 index 0000000000..ccde7acd71 --- /dev/null +++ b/cxx-squid/src/main/java/org/sonar/cxx/CxxComplexityConstants.java @@ -0,0 +1,55 @@ +/* + * 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; + +import org.sonar.cxx.api.CxxKeyword; +import org.sonar.cxx.api.CxxPunctuator; +import org.sonar.cxx.parser.CxxGrammarImpl; + +import com.sonar.sslr.api.AstNodeType; + +public class CxxComplexityConstants { + + private CxxComplexityConstants() { + /* utility class */ + } + + /** + * From original SonarQube documentation:
The complexity is + * measured by the number of if, while, do, for, ?:, catch, switch, case + * statements, and operators && and || (plus one) in the body of a + * constructor, method, static initializer, or instance initializer. + *
+ * + * @see CyclomaticComplexityExclusionAstNodeTypes + */ + public static final AstNodeType[] CyclomaticComplexityAstNodeTypes = new AstNodeType[] { + CxxGrammarImpl.functionDefinition, + CxxKeyword.IF, + CxxKeyword.FOR, + CxxKeyword.WHILE, + CxxKeyword.CATCH, + CxxKeyword.CASE, + CxxKeyword.DEFAULT, + CxxPunctuator.AND, + CxxPunctuator.OR, + CxxPunctuator.QUEST }; + +} diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportIssue.java b/cxx-squid/src/main/java/org/sonar/cxx/utils/CxxReportIssue.java similarity index 95% rename from cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportIssue.java rename to cxx-squid/src/main/java/org/sonar/cxx/utils/CxxReportIssue.java index fa13fbe593..c091a240d2 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportIssue.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/utils/CxxReportIssue.java @@ -17,7 +17,7 @@ * 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.sensors.utils; +package org.sonar.cxx.utils; import java.util.ArrayList; import java.util.Collections; diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java b/cxx-squid/src/main/java/org/sonar/cxx/utils/CxxReportLocation.java similarity index 94% rename from cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java rename to cxx-squid/src/main/java/org/sonar/cxx/utils/CxxReportLocation.java index fc9eb28d40..0c30a66662 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/utils/CxxReportLocation.java @@ -17,7 +17,7 @@ * 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.sensors.utils; +package org.sonar.cxx.utils; import java.util.Objects; import javax.annotation.Nullable; diff --git a/cxx-squid/src/main/java/org/sonar/cxx/utils/package-info.java b/cxx-squid/src/main/java/org/sonar/cxx/utils/package-info.java new file mode 100644 index 0000000000..ca803e640e --- /dev/null +++ b/cxx-squid/src/main/java/org/sonar/cxx/utils/package-info.java @@ -0,0 +1,23 @@ +/* + * 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. + */ +@ParametersAreNonnullByDefault +package org.sonar.cxx.utils; + +import javax.annotation.ParametersAreNonnullByDefault; diff --git a/cxx-squid/src/main/java/org/sonar/cxx/visitors/AbstractCxxPublicApiVisitor.java b/cxx-squid/src/main/java/org/sonar/cxx/visitors/AbstractCxxPublicApiVisitor.java index 5731c2e23b..cab78b4fbf 100644 --- a/cxx-squid/src/main/java/org/sonar/cxx/visitors/AbstractCxxPublicApiVisitor.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/visitors/AbstractCxxPublicApiVisitor.java @@ -20,7 +20,6 @@ package org.sonar.cxx.visitors; import com.sonar.sslr.api.AstNode; -import com.sonar.sslr.api.AstVisitor; import com.sonar.sslr.api.GenericTokenType; import com.sonar.sslr.api.Grammar; import com.sonar.sslr.api.Token; diff --git a/cxx-squid/src/main/java/org/sonar/cxx/visitors/MultiLocatitionSquidCheck.java b/cxx-squid/src/main/java/org/sonar/cxx/visitors/MultiLocatitionSquidCheck.java new file mode 100644 index 0000000000..4b1ca902f2 --- /dev/null +++ b/cxx-squid/src/main/java/org/sonar/cxx/visitors/MultiLocatitionSquidCheck.java @@ -0,0 +1,138 @@ +/* + * 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.visitors; + +import java.util.HashSet; +import java.util.Set; + +import org.sonar.api.utils.AnnotationUtils; +import org.sonar.cxx.utils.CxxReportIssue; +import org.sonar.squidbridge.SquidAstVisitorContext; +import org.sonar.squidbridge.api.SourceFile; +import org.sonar.squidbridge.checks.SquidCheck; +import org.sonar.squidbridge.measures.CalculatedMetricFormula; +import org.sonar.squidbridge.measures.MetricDef; + +import com.sonar.sslr.api.Grammar; + +/** + * Derivation of {@link SquidCheck}, which can create issues with multiple + * locations (1 primary location, arbitrary number of secondary locations + * + * See also org.sonar.squidbridge.SquidAstVisitorContext.createLineViolation + * + * @param + */ +public class MultiLocatitionSquidCheck extends SquidCheck { + + private static enum DataKey implements MetricDef { + FILE_VIOLATIONS_WITH_MULTIPLE_LOCATIONS; + + @Override + public String getName() { + return FILE_VIOLATIONS_WITH_MULTIPLE_LOCATIONS.getName(); + } + + @Override + public boolean isCalculatedMetric() { + return false; + } + + @Override + public boolean aggregateIfThereIsAlreadyAValue() { + return false; + } + + @Override + public boolean isThereAggregationFormula() { + return false; + } + + @Override + public CalculatedMetricFormula getCalculatedMetricFormula() { + return null; + } + } + + /** + * @return the rule key of this check visitor + * @see org.sonar.check.Rule + */ + protected String getRuleKey() { + org.sonar.check.Rule ruleAnnotation = AnnotationUtils.getAnnotation(this, org.sonar.check.Rule.class); + if (ruleAnnotation != null && ruleAnnotation.key() != null) { + return ruleAnnotation.key(); + } + throw new IllegalStateException("Check must be annotated with @Rule( key = )"); + } + + private SourceFile getSourceFile() { + final SquidAstVisitorContext c = getContext(); + if (c.peekSourceCode() instanceof SourceFile) { + return (SourceFile) c.peekSourceCode(); + } else if (c.peekSourceCode().getParent(SourceFile.class) != null) { + return c.peekSourceCode().getParent(SourceFile.class); + } else { + throw new IllegalStateException("Unable to get SourceFile on source code '" + + (c.peekSourceCode() == null ? "[NULL]" : c.peekSourceCode().getKey()) + "'"); + } + } + + /** + * Add the given message to the current SourceFile object + * @see SquidAstVisitorContext.createLineViolation() for simple violations + */ + protected void createMultiLocationViolation(CxxReportIssue message) { + SourceFile sourceFile = getSourceFile(); + Set messages = getMultiLocationCheckMessages(sourceFile); + if (messages == null) { + messages = new HashSet<>(); + } + messages.add(message); + setMultiLocationViolation(sourceFile, messages); + } + + /** + * @return set of multi-location issues, raised on the given file; might be + * null + * @see SourceFile.getCheckMessages() for simple violations + */ + @SuppressWarnings("unchecked") + public static Set getMultiLocationCheckMessages(SourceFile sourceFile) { + return (Set) sourceFile.getData(DataKey.FILE_VIOLATIONS_WITH_MULTIPLE_LOCATIONS); + } + + /** + * @return true if the given file has mult-location issues + * @see SourceFile.hasCheckMessages() for simple violations + */ + public static boolean hasMultiLocationCheckMessages(SourceFile sourceFile) { + Set issues = getMultiLocationCheckMessages(sourceFile); + return issues != null && !issues.isEmpty(); + } + + private static void setMultiLocationViolation(SourceFile sourceFile, Set messages) { + sourceFile.addData(DataKey.FILE_VIOLATIONS_WITH_MULTIPLE_LOCATIONS, messages); + } + + public static void eraseMultilineCheckMessages(SourceFile sourceFile) { + setMultiLocationViolation(sourceFile, null); + } +} diff --git a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/utils/CxxReportIssueTest.java b/cxx-squid/src/test/java/org/sonar/cxx/utils/CxxReportIssueTest.java similarity index 98% rename from cxx-sensors/src/test/java/org/sonar/cxx/sensors/utils/CxxReportIssueTest.java rename to cxx-squid/src/test/java/org/sonar/cxx/utils/CxxReportIssueTest.java index 284595c5e1..9fd20cefc7 100644 --- a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/utils/CxxReportIssueTest.java +++ b/cxx-squid/src/test/java/org/sonar/cxx/utils/CxxReportIssueTest.java @@ -17,7 +17,7 @@ * 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.sensors.utils; +package org.sonar.cxx.utils; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; diff --git a/cxx-squid/src/test/java/org/sonar/cxx/utils/package-info.java b/cxx-squid/src/test/java/org/sonar/cxx/utils/package-info.java new file mode 100644 index 0000000000..ca803e640e --- /dev/null +++ b/cxx-squid/src/test/java/org/sonar/cxx/utils/package-info.java @@ -0,0 +1,23 @@ +/* + * 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. + */ +@ParametersAreNonnullByDefault +package org.sonar.cxx.utils; + +import javax.annotation.ParametersAreNonnullByDefault;