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

Cognitive complexity metric does not understand newer C++ syntax #2672

Closed
Patschkowski opened this issue Apr 26, 2024 · 9 comments · Fixed by #2675
Closed

Cognitive complexity metric does not understand newer C++ syntax #2672

Patschkowski opened this issue Apr 26, 2024 · 9 comments · Fixed by #2675
Assignees
Labels
Milestone

Comments

@Patschkowski
Copy link

Describe the bug
Code complexity metric is counting r-value references as logical operators and "=default" as switch labels.

Expected behavior
Code complexity metric does not count r-value references as logical operators and "=default" as switch labels.

Screenshots
image
image

Desktop (please complete the following information):

  • OS: Windows
  • SonarQube version: 10.4.1 (build 88267)
  • cxx plugin version: 2.2 (build 626)
  • sonar-scanner version: 5.0.1.3006
@guwirth guwirth added the bug label Apr 27, 2024
@guwirth guwirth added this to the 2.1.2 milestone Apr 27, 2024
@guwirth
Copy link
Collaborator

guwirth commented Apr 27, 2024

Hello @Patschkowski, thanks for the feedback …

@guwirth
Copy link
Collaborator

guwirth commented Apr 27, 2024

Hi @Patschkowski,

most likely the problem is somewhere here: https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/cxx-squid/src/main/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitor.java

Can you confirm, that it’s a problem with cognitive complexity?

Regards,

@Patschkowski
Copy link
Author

Hi @guwirth , I'd say it is Cyclomatic Complexity

image

@guwirth
Copy link
Collaborator

guwirth commented Apr 27, 2024

Hello @Patschkowski,

there are metrics (Measues) and rule violations (Issues) in SonarQube. The first screenshot you sent seems to show the cognitive complexity metric. The second one is a complexity rule violation. These are two different things created in a different way. Also the visitor verifying the AST is for cognitive and cyclomatic complexity a different one. The reason why I’m asking is to narrow down the problem.

Regards,

@Patschkowski
Copy link
Author

Hi @guwirth

appreciate your explanation. I think it is the Cyclomatic Complexity though:

image

@guwirth
Copy link
Collaborator

guwirth commented Apr 28, 2024

Hi @Patschkowski,

thx for clarification! Can you copy me the source code of this one template host of the screenshot above into this ticket please. Makes testing easier…

Regards,

@Patschkowski
Copy link
Author

#include <memory>

template<typename T>
class host :
  public std::enable_shared_from_this<host<T>> {
public:
  template<typename... U>
  explicit host(U&&... args);

  host() = delete;

  host(const host&) = delete;

  host(host&&)      = default;

  host& operator=(const host&) = delete;

  host& operator=(host&&)      = default;
};

@guwirth
Copy link
Collaborator

guwirth commented May 3, 2024

Think problem is that CxxCognitiveComplexityVisitor.java is using functionDefinition as root node. Maybe functionBody with child compoundStatement would be better?

grafik

guwirth added a commit to guwirth/sonar-cxx that referenced this issue May 3, 2024
@guwirth guwirth changed the title Code complexity metric does not understand newer C++ syntax Cognitive complexity metric does not understand newer C++ syntax May 3, 2024
guwirth added a commit to guwirth/sonar-cxx that referenced this issue May 4, 2024
- fix problem with r-value reference
- close SonarOpenCommunity#2672
@guwirth guwirth self-assigned this May 4, 2024
@guwirth
Copy link
Collaborator

guwirth commented May 4, 2024

@SonarOpenCommunity SonarOpenCommunity locked as resolved and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

2 participants