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

ReportIssue: Remove obsolete overloads from SonarAnalysisContextExtensions #9356

Merged
merged 1 commit into from
May 30, 2024
Merged

ReportIssue: Remove obsolete overloads from SonarAnalysisContextExtensions #9356

merged 1 commit into from
May 30, 2024

Conversation

pavel-mikula-sonarsource
Copy link
Contributor

Follow up of #9298 effort

This PR:

  • Removes two obsolete SonarAnalysisContextExtensions extensions
  • Deduplicates extensions to extend a base class, instead of duplicating extensions for SonarCompilationReportingContext and SonarSymbolReportingContext separately

@pavel-mikula-sonarsource pavel-mikula-sonarsource mentioned this pull request May 29, 2024
Merged
martin-strecker-sonarsource
martin-strecker-sonarsource approved these changes May 29, 2024
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs
{
cc.ReportIssue(diagnostic);
}
ReportUnusedPrivateMembers(cc, usageCollector, removableInternalTypes, SyntaxConstants.Internal, new());
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated but important. We report in CompilationEnd and as such we should add the CompilationEnd tag to the diagnostic desscriptor. Otherwise we do violate RS1037
If we fail to do so, some IDE scenarios are broken and issues appear and disappear in the IDE error list. See
dotnet/roslyn-analyzers#6282 for more links to more details about the issue. We do have several such violations in our code base. See also #8437

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our DescriptorFactory doesn't have a parameter to accept this, so I can't do anything in this PR.

@pavel-mikula-sonarsource pavel-mikula-sonarsource enabled auto-merge (squash) May 30, 2024 06:18
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'Sonar .NET Java Plugin'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'SonarAnalyzer for .NET'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@pavel-mikula-sonarsource pavel-mikula-sonarsource merged commit 870c35b into master May 30, 2024
26 checks passed
@pavel-mikula-sonarsource pavel-mikula-sonarsource deleted the Pavel/RemoveExtension branch May 30, 2024 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martin-strecker-sonarsource martin-strecker-sonarsource martin-strecker-sonarsource approved these changes

Assignees

@pavel-mikula-sonarsource pavel-mikula-sonarsource

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pavel-mikula-sonarsource @martin-strecker-sonarsource