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

.razor: complexity/cognitive complexity return 0 #7978

Conversation

cristian-ambrosini-sonarsource
Copy link
Contributor

Part of #7936

Comment on lines 30 to 33
if (GeneratedCodeRecognizer.IsRazor(node.SyntaxTree))
{
return new(0, ImmutableArray<SecondaryLocation>.Empty);
}

Choose a reason for hiding this comment

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

This will disable also the cognitive complexity rule, not only the metric. This rule is not exclude by: #7879. Could you please check with @csaba-sagi-sonarsource if it makes sense to keep the rule active or not for razor files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personal opinion: If we don't have a reliable count of cognitive complexity for .razor files to be shown in the metrics why would we want a rule that might be raised based on the same flawed logic?

Choose a reason for hiding this comment

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

The rule is computing the complexity at the function level. For this kind of limited scope, the precision is better. Please check with @csaba-sagi-sonarsource as he has investigated this already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've checked with @csaba-sagi-sonarsource and he thinks that the count for the complexity of methods fully declared inside .razor files should be good enough. I changed the implementation to target the metrics only.

Comment on lines 41 to 44
if (GeneratedCodeRecognizer.IsRazor(syntaxNode.SyntaxTree))
{
return new(ImmutableArray<SecondaryLocation>.Empty);
}

Choose a reason for hiding this comment

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

Same for cyclomatic complexity.

@sonarcloud
Copy link

sonarcloud bot commented Sep 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Sep 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@cristian-ambrosini-sonarsource
Copy link
Contributor Author

Closing this issue since the computed global cognitive complexity and cyclomatic complexity are correct for .razor files and consider both HTML and C# code. The only question remaining would be if it makes sense to keep S1541 (cyclomatic complexity) and S3776 (cognitive complexity) active. At the moment they are both NOT raising for HTML code (everything that is inside the BuildRenderTree generated method in the generated files will be ignored), but the same HTML code escaped inside @code or @function blocks would be analyzed and an issue will be raised.

@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource deleted the cristian/razor-cognitive-complexity branch September 12, 2023 15:28
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.

.razor: update the remaining metrics from the MetricsAnalyzer
3 participants