Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

SONARPY-186 Rule: Cognitive Complexity of functions should not be too high#74

Merged
ivandalbosco merged 6 commits intomasterSonarSource/sonar-python:masterfrom
SONARPY-186SonarSource/sonar-python:SONARPY-186Copy head branch name to clipboard
Jan 11, 2017
Merged

SONARPY-186 Rule: Cognitive Complexity of functions should not be too high#74
ivandalbosco merged 6 commits intomasterSonarSource/sonar-python:masterfrom
SONARPY-186SonarSource/sonar-python:SONARPY-186Copy head branch name to clipboard

Conversation

@vilchik-elena
Copy link
Contributor

No description provided.

@pynicolas pynicolas changed the title SONARJS-186 Rule: Cognitive Complexity of functions should not be too high SONARPY-186 Rule: Cognitive Complexity of functions should not be too high Jan 10, 2017
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vilchik-elena com.sonar.sslr.impl.ast.AstWalker is not part of the public API of SSLR. However, I'm not sure that there's another way to have "secondary" visitors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vilchik-elena do we really need the returns?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vilchik-elena I would prefer to have a dedicated test file for the default threshold where we have one function exactly on the "max value" and one function which is the almost the same but has "max value + 1".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vilchik-elena I'm not sure it would work but another approach would seem more natural to me. Instead of nestingLevelByNode, I would have a nestingLevel field which is incremented when entering a SUITE and decremented when leaving the SUITE (but not in try: SUITE).

except SomeError1: # +1 (nesting level +1)
if condition: # +2
pass
except SomeError2: # +1 (nesting level +1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vilchik-elena How about explicitly testing multiple exceptions, as in
except (SomeError2, SomeError3): #+1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return "+1";

} else{
return String.format("+%s (incl %s for nesting)", complexity, complexity - 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vilchik-elena Can we test this secondary message? It is currently not done in the test file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivandalbosco No, it's not implemented in python, I've tested line numbers only

@ivandalbosco ivandalbosco merged commit 5c015d7 into master Jan 11, 2017
@vilchik-elena vilchik-elena deleted the SONARPY-186 branch January 11, 2017 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.