From f16c3db32ea0a12be76b37cf000096c166d04711 Mon Sep 17 00:00:00 2001 From: guwirth Date: Fri, 3 May 2024 16:47:34 +0200 Subject: [PATCH] fix cognitive complexity - fix problem with r-value reference - close #2672 --- .../CxxCognitiveComplexityVisitor.java | 46 ++++++++++++------- .../CxxCognitiveComplexityVisitorTest.java | 10 ++++ .../src/test/resources/visitors/inline.cc | 43 +++++++++++++++++ .../src/test/resources/visitors/template.cc | 11 +++++ 4 files changed, 93 insertions(+), 17 deletions(-) create mode 100644 cxx-squid/src/test/resources/visitors/inline.cc create mode 100644 cxx-squid/src/test/resources/visitors/template.cc diff --git a/cxx-squid/src/main/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitor.java b/cxx-squid/src/main/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitor.java index c425b746d7..d1796fecfe 100644 --- a/cxx-squid/src/main/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitor.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitor.java @@ -28,10 +28,10 @@ import java.util.HashSet; import java.util.LinkedList; import java.util.Set; -import org.sonar.cxx.parser.CxxKeyword; import org.sonar.cxx.api.CxxMetric; -import org.sonar.cxx.parser.CxxPunctuator; import org.sonar.cxx.parser.CxxGrammarImpl; +import org.sonar.cxx.parser.CxxKeyword; +import org.sonar.cxx.parser.CxxPunctuator; import org.sonar.cxx.squidbridge.api.SourceCode; public class CxxCognitiveComplexityVisitor extends MultiLocatitionSquidCheck { @@ -74,7 +74,8 @@ public class CxxCognitiveComplexityVisitor extends MultiLocat private static final Set SUBSCRIPTION_NODES = new HashSet<>(); static { - SUBSCRIPTION_NODES.add(CxxGrammarImpl.functionDefinition); + SUBSCRIPTION_NODES.add(CxxGrammarImpl.functionBody); // root node for Cognitive Complexity + SUBSCRIPTION_NODES.addAll(Arrays.asList(DESCENDANT_TYPES)); SUBSCRIPTION_NODES.addAll(Arrays.asList(INCREMENT_TYPES)); SUBSCRIPTION_NODES.addAll(Arrays.asList(NESTING_LEVEL_TYPES)); @@ -102,11 +103,16 @@ public void visitNode(AstNode node) { return; } - if (node.is(CxxGrammarImpl.functionDefinition)) { + if (node.is(CxxGrammarImpl.functionBody) && node.hasDirectChildren(CxxGrammarImpl.compoundStatement)) { complexityScopes.addFirst(new CxxComplexityScope(node.getTokenLine())); + return; } - if (complexityScopes.isEmpty() || isElseIf(node)) { + if (complexityScopes.isEmpty()) { + return; + } + + if (isElseIf(node)) { return; } @@ -125,21 +131,27 @@ public void visitNode(AstNode node) { @Override public void leaveNode(AstNode node) { - if (node.getToken().isGeneratedCode()) { - return; - } + if (!complexityScopes.isEmpty()) { + if (node.getToken().isGeneratedCode()) { + return; + } - if (node.is(CxxGrammarImpl.functionDefinition)) { - analyzeComplexity(complexityScopes.removeFirst()); - } + if (node.is(CxxGrammarImpl.functionBody)) { + analyzeComplexity(complexityScopes.removeFirst()); + } - if (complexityScopes.isEmpty() || isElseIf(node)) { - return; - } + if (complexityScopes.isEmpty()) { + return; + } - if (node.is(NESTING_LEVEL_TYPES)) { - for (var scope : complexityScopes) { - scope.decreaseNesting(); + if (isElseIf(node)) { + return; + } + + if (node.is(NESTING_LEVEL_TYPES)) { + for (var scope : complexityScopes) { + scope.decreaseNesting(); + } } } } diff --git a/cxx-squid/src/test/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitorTest.java b/cxx-squid/src/test/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitorTest.java index 292477111e..6bf46ca0a9 100644 --- a/cxx-squid/src/test/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitorTest.java +++ b/cxx-squid/src/test/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitorTest.java @@ -150,6 +150,16 @@ void to_regexp() throws UnsupportedEncodingException, IOException { assertThat(testFile("src/test/resources/visitors/to_regexp.cc")).isEqualTo(20); } + @Test + void template() throws UnsupportedEncodingException, IOException { + assertThat(testFile("src/test/resources/visitors/template.cc")).isEqualTo(0); + } + + @Test + void inline() throws UnsupportedEncodingException, IOException { + assertThat(testFile("src/test/resources/visitors/inline.cc")).isEqualTo(5); + } + private int testFile(String fileName) throws UnsupportedEncodingException, IOException { var tester = CxxFileTesterHelper.create(fileName, ".", ""); SourceFile sourceFile = CxxAstScanner.scanSingleInputFile(tester.asInputFile()); diff --git a/cxx-squid/src/test/resources/visitors/inline.cc b/cxx-squid/src/test/resources/visitors/inline.cc new file mode 100644 index 0000000000..13de9ddcb5 --- /dev/null +++ b/cxx-squid/src/test/resources/visitors/inline.cc @@ -0,0 +1,43 @@ +struct DeleteSample { // = 0 + DeleteSample(const DeleteSample&) = delete; + DeleteSample& operator=(const DeleteSample&) = delete; + DeleteSample(DeleteSample&&) = delete; + DeleteSample& operator=(DeleteSample&&) = delete; + ~DeleteSample() = default; +}; + +struct DefaultSample { // = 0 + DefaultSample(const DefaultSample&) = default; + DefaultSample& operator=(const DefaultSample&) = default; + DefaultSample(DefaultSample&&) = default; + DefaultSample& operator=(DefaultSample&&) = default; + ~DefaultSample() = default; +}; + +struct Sample { // = 5 + Sample(const Sample&) { + if (true) { + std::cout << "+1" << std::endl; + } + } + Sample& operator=(const Sample&) { + if (true) { + std::cout << "+1" << std::endl; + } + } + Sample(Sample&&) { + if (true) { + std::cout << "+1" << std::endl; + } + } + Sample& operator=(Sample&&) { + if (true) { + std::cout << "+1" << std::endl; + } + } + ~Sample() { + if (true) { + std::cout << "+1" << std::endl; + } + } +}; diff --git a/cxx-squid/src/test/resources/visitors/template.cc b/cxx-squid/src/test/resources/visitors/template.cc new file mode 100644 index 0000000000..dec5d7e686 --- /dev/null +++ b/cxx-squid/src/test/resources/visitors/template.cc @@ -0,0 +1,11 @@ +template +class host : public std::enable_shared_from_this> { +public: + template + explicit host(U&&... args); // should ignore && + host() = delete; + host(const host&) = delete; + host(host&&) = default; // should ignore && and default + host& operator=(const host&) = delete; + host& operator=(host&&) = default; // should ignore && and default +};