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.

Sign up for GitHub

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

Jump to bottom

New Rule S3398: Private static methods called only by inner class #6781

Merged
merged 19 commits into from Feb 28, 2023
Merged

New Rule S3398: Private static methods called only by inner class #6781

merged 19 commits into from Feb 28, 2023

Conversation

zsolt-kolbay-sonarsource
Copy link
Contributor

Fixes #6706
RSPEC PR

andrei-epure-sonarsource
andrei-epure-sonarsource reviewed Feb 23, 2023
View reviewed changes
Copy link
Contributor

@andrei-epure-sonarsource andrei-epure-sonarsource left a comment

Choose a reason for hiding this comment

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

The tests look pretty comprehensive, great work on them.

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs Outdated Show resolved Hide resolved
analyzers/rspec/cs/S3398_c#.html Show resolved Hide resolved
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource marked this pull request as ready for review February 28, 2023 10:30
andrei-epure-sonarsource
andrei-epure-sonarsource approved these changes Feb 28, 2023
View reviewed changes
Copy link
Contributor

@andrei-epure-sonarsource andrei-epure-sonarsource left a comment

Choose a reason for hiding this comment

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

LGTM!

we can live w/ the code smells

analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
}
else
{
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a code smell about the break, but I'm not sure that removing it will lead to more readable code

Also, there's still a small thing typeHierarchyFromTopToBottom.All doing the check also on firstPath itself, but overall it's clear and more readable probably then using some loops for this

@andrei-epure-sonarsource andrei-epure-sonarsource force-pushed the Zsolt/S3398-private-methods-called-only-by-inner-class branch from 361b512 to 7a57f2b Compare February 28, 2023 12:53
@andrei-epure-sonarsource andrei-epure-sonarsource force-pushed the Zsolt/S3398-private-methods-called-only-by-inner-class branch from 7a57f2b to e5a2d45 Compare February 28, 2023 12:55
@sonarcloud
Copy link

sonarcloud bot commented Feb 28, 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 Feb 28, 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 2 Code Smells

98.8% 98.8% Coverage
0.0% 0.0% Duplication

@andrei-epure-sonarsource andrei-epure-sonarsource merged commit b89dd7a into master Feb 28, 2023
@andrei-epure-sonarsource andrei-epure-sonarsource deleted the Zsolt/S3398-private-methods-called-only-by-inner-class branch February 28, 2023 14:25
@martin-strecker-sonarsource
Copy link
Contributor

Peach validation: There is a AD0001 in the the lucene project

CSC : error AD0001: Analyzer 'SonarAnalyzer.Rules.CSharp.PrivateStaticMethodUsedOnlyByNestedClass' threw an exception of type 'System.InsufficientExecutionStackException' with message 'Insufficient stack to continue executing the program safely. This can happen from having too many functions on the call stack or function on the stack using too much stack space.'. [C:\Project\src\Lucene.Net.Analysis.Common\Lucene.Net.Analysis.Common.csproj]

The log can be found here
https://cirrus-ci.com/task/4607194994311168

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrei-epure-sonarsource andrei-epure-sonarsource andrei-epure-sonarsource approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New Rule S3398: "private" methods called only by inner classes should be moved to those classes
3 participants
@zsolt-kolbay-sonarsource @martin-strecker-sonarsource @andrei-epure-sonarsource