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

ReportIssue: Remove obsolete overloads from SonarAnalysisContextExtensions #9356

Merged
merged 1 commit into from
May 30, 2024
Merged

ReportIssue: Remove obsolete overloads from SonarAnalysisContextExtensions #9356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 8 additions & 26 deletions analyzers/src/SonarAnalyzer.CSharp/Extensions/SonarAnalysisContextExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,37 +46,19 @@ public static void RegisterSemanticModelAction(this SonarAnalysisContext context
public static void RegisterCodeBlockStartAction(this SonarAnalysisContext context, Action<SonarCodeBlockStartAnalysisContext<SyntaxKind>> action) =>
context.RegisterCodeBlockStartAction(CSharpGeneratedCodeRecognizer.Instance, action);

[Obsolete("Use another overload of ReportIssue, without calling Diagnostic.Create")]
public static void ReportIssue(this SonarCompilationReportingContext context, Diagnostic diagnostic) =>
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, diagnostic);

public static void ReportIssue(this SonarCompilationReportingContext context,
DiagnosticDescriptor rule,
Location primaryLocation,
IEnumerable<SecondaryLocation> secondaryLocations,
params string[] messageArgs) =>
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, rule, primaryLocation, secondaryLocations, messageArgs);

public static void ReportIssue(this SonarCompilationReportingContext context, DiagnosticDescriptor rule, Location location, params string[] messageArgs) =>
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, rule, location, messageArgs);

[Obsolete("Use another overload of ReportIssue, without calling Diagnostic.Create")]
public static void ReportIssue(this SonarSymbolReportingContext context, Diagnostic diagnostic) =>
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, diagnostic);

public static void ReportIssue(this SonarSymbolReportingContext context, DiagnosticDescriptor rule, SyntaxNode locationSyntax, params string[] messageArgs) =>
public static void ReportIssue<TContext>(this SonarCompilationReportingContextBase<TContext> context, DiagnosticDescriptor rule, SyntaxNode locationSyntax, params string[] messageArgs) =>
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, rule, locationSyntax, messageArgs);

public static void ReportIssue(this SonarSymbolReportingContext context, DiagnosticDescriptor rule, SyntaxToken locationToken, params string[] messageArgs) =>
public static void ReportIssue<TContext>(this SonarCompilationReportingContextBase<TContext> context, DiagnosticDescriptor rule, SyntaxToken locationToken, params string[] messageArgs) =>
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, rule, locationToken, messageArgs);

public static void ReportIssue(this SonarSymbolReportingContext context, DiagnosticDescriptor rule, Location location, params string[] messageArgs) =>
public static void ReportIssue<TContext>(this SonarCompilationReportingContextBase<TContext> context, DiagnosticDescriptor rule, Location location, params string[] messageArgs) =>
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, rule, location, messageArgs);

public static void ReportIssue(this SonarSymbolReportingContext context,
DiagnosticDescriptor rule,
Location primaryLocation,
IEnumerable<SecondaryLocation> secondaryLocations,
params string[] messageArgs) =>
public static void ReportIssue<TContext>(this SonarCompilationReportingContextBase<TContext> context,
DiagnosticDescriptor rule,
Location primaryLocation,
IEnumerable<SecondaryLocation> secondaryLocations,
params string[] messageArgs) =>
context.ReportIssue(CSharpGeneratedCodeRecognizer.Instance, rule, primaryLocation, secondaryLocations, messageArgs);
}
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/UseAspNetModelBinding.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private void RegisterForSymbols(SonarCompilationStartAnalysisContext compilation
{
foreach (var candidate in candidates.Where(x => !(hasActionFiltersOverrides && x.OriginatesFromParameter)))
{
symbolEnd.ReportIssue(Diagnostic.Create(Rule, candidate.Location, candidate.Message));
symbolEnd.ReportIssue(Rule, candidate.Location, candidate.Message);
}
});
}, SymbolKind.NamedType);
Expand Down
2 changes: 1 addition & 1 deletion ...yzers/src/SonarAnalyzer.CSharp/Rules/MarkAssemblyWithNeutralResourcesLanguageAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
{
if (hasResx && !HasNeutralResourcesLanguageAttribute(cc.Compilation.Assembly))
{
cc.ReportIssue(Rule, null, cc.Compilation.AssemblyName);
cc.ReportIssue(Rule, (Location)null, cc.Compilation.AssemblyName);
}
});
});
Expand Down
9 changes: 4 additions & 5 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/UninvokedEventDeclaration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,10 @@ private void RaiseOnUninvokedEventDeclaration(SonarSymbolReportingContext contex
var usedSymbols = GetInvokedEventSymbols(removableDeclarationCollector)
.Concat(GetPossiblyCopiedSymbols(removableDeclarationCollector))
.ToHashSet();

removableEventFields
.Where(x => !usedSymbols.Contains(x.Symbol))
.ToList()
.ForEach(x => context.ReportIssue(Diagnostic.Create(Rule, GetLocation(x.Node), x.Symbol.Name)));
foreach (var field in removableEventFields.Where(x => !usedSymbols.Contains(x.Symbol)))
{
context.ReportIssue(Rule, GetLocation(field.Node), field.Symbol.Name);
}

Location GetLocation(SyntaxNode node) =>
node is VariableDeclaratorSyntax variableDeclarator
Expand Down
92 changes: 41 additions & 51 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
{
usageCollector.SafeVisit(syntaxTree.GetRoot());
}
foreach (var diagnostic in DiagnosticsForUnusedPrivateMembers(usageCollector, removableInternalTypes, SyntaxConstants.Internal, new()))
{
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.

});
});

Expand All @@ -87,14 +84,8 @@ private static void NamedSymbolAction(SonarSymbolReportingContext context, HashS
&& new CSharpSymbolUsageCollector(context.Compilation, AssociatedSymbols(privateSymbols)) is var usageCollector
&& VisitDeclaringReferences(namedType, usageCollector, context, includeGeneratedFile: true))
{
foreach (var diagnostic in DiagnosticsForUnusedPrivateMembers(usageCollector, privateSymbols, SyntaxConstants.Private, fieldLikeSymbols))
{
context.ReportIssue(diagnostic);
}
foreach (var diagnostic in DiagnosticsForUsedButUnreadFields(usageCollector, privateSymbols))
{
context.ReportIssue(diagnostic);
}
ReportUnusedPrivateMembers(context, usageCollector, privateSymbols, SyntaxConstants.Private, fieldLikeSymbols);
ReportUsedButUnreadFields(context, usageCollector, privateSymbols);
}
}

Expand Down Expand Up @@ -140,28 +131,30 @@ static IEnumerable<BaseTypeDeclarationSyntax> PrivateNestedMembersFromNonGenerat
.SelectMany(x => x.GetSyntax().ChildNodes().OfType<BaseTypeDeclarationSyntax>());
}

private static IEnumerable<Diagnostic> DiagnosticsForUnusedPrivateMembers(CSharpSymbolUsageCollector usageCollector,
ISet<ISymbol> removableSymbols,
string accessibility,
BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
private static void ReportUnusedPrivateMembers<TContext>(SonarCompilationReportingContextBase<TContext> context,
CSharpSymbolUsageCollector usageCollector,
ISet<ISymbol> removableSymbols,
string accessibility,
BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
{
var unusedSymbols = GetUnusedSymbols(usageCollector, removableSymbols);

var propertiesWithUnusedAccessor = removableSymbols
.Intersect(usageCollector.UsedSymbols)
.OfType<IPropertySymbol>()
.Where(usageCollector.PropertyAccess.ContainsKey)
.Where(x => !IsMentionedInDebuggerDisplay(x, usageCollector))
.Select(x => GetDiagnosticsForProperty(x, usageCollector.PropertyAccess))
.WhereNotNull();

return GetDiagnosticsForMembers(unusedSymbols, accessibility, fieldLikeSymbols).Concat(propertiesWithUnusedAccessor);
.Where(x => !IsMentionedInDebuggerDisplay(x, usageCollector));
foreach (var property in propertiesWithUnusedAccessor)
{
ReportProperty(context, property, usageCollector.PropertyAccess);
}
ReportDiagnosticsForMembers(context, unusedSymbols, accessibility, fieldLikeSymbols);
}

private static bool IsMentionedInDebuggerDisplay(ISymbol symbol, CSharpSymbolUsageCollector usageCollector) =>
usageCollector.DebuggerDisplayValues.Any(x => x.Contains(symbol.Name));

private static IEnumerable<Diagnostic> DiagnosticsForUsedButUnreadFields(CSharpSymbolUsageCollector usageCollector, IEnumerable<ISymbol> removableSymbols)
private static void ReportUsedButUnreadFields(SonarSymbolReportingContext context, CSharpSymbolUsageCollector usageCollector, IEnumerable<ISymbol> removableSymbols)
{
var unusedSymbols = GetUnusedSymbols(usageCollector, removableSymbols);

Expand All @@ -171,7 +164,10 @@ private static IEnumerable<Diagnostic> DiagnosticsForUsedButUnreadFields(CSharpS
.Where(x => !unusedSymbols.Contains(x.Symbol) && !IsMentionedInDebuggerDisplay(x.Symbol, usageCollector))
.Where(x => x.Declaration is not null && !x.Readings.Any());

return GetDiagnosticsForUnreadFields(usedButUnreadFields);
foreach (var usage in usedButUnreadFields)
{
context.ReportIssue(RuleS4487, usage.Declaration.GetLocation(), GetFieldAccessibilityForMessage(usage.Symbol), usage.Symbol.Name);
}
}

private static HashSet<ISymbol> GetUnusedSymbols(CSharpSymbolUsageCollector usageCollector, IEnumerable<ISymbol> removableSymbols) =>
Expand All @@ -180,9 +176,6 @@ private static HashSet<ISymbol> GetUnusedSymbols(CSharpSymbolUsageCollector usag
.Where(x => !IsMentionedInDebuggerDisplay(x, usageCollector) && !IsAccessorUsed(x, usageCollector))
.ToHashSet();

private static IEnumerable<Diagnostic> GetDiagnosticsForUnreadFields(IEnumerable<SymbolUsage> unreadFields) =>
unreadFields.Select(x => Diagnostic.Create(RuleS4487, x.Declaration.GetLocation(), GetFieldAccessibilityForMessage(x.Symbol), x.Symbol.Name));

private static bool IsAccessorUsed(ISymbol symbol, CSharpSymbolUsageCollector usageCollector) =>
symbol is IMethodSymbol { } accessor
&& accessor.AssociatedSymbol is IPropertySymbol { } property
Expand All @@ -193,11 +186,11 @@ symbol is IMethodSymbol { } accessor
private static string GetFieldAccessibilityForMessage(ISymbol symbol) =>
symbol.DeclaredAccessibility == Accessibility.Private ? SyntaxConstants.Private : "private class";

private static IEnumerable<Diagnostic> GetDiagnosticsForMembers(ICollection<ISymbol> unusedSymbols,
string accessibility,
BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
private static void ReportDiagnosticsForMembers<TContext>(SonarCompilationReportingContextBase<TContext> context,
ICollection<ISymbol> unusedSymbols,
string accessibility,
BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
{
var diagnostics = new List<Diagnostic>();
var alreadyReportedFieldLikeSymbols = new HashSet<ISymbol>();
var unusedSymbolSyntaxPairs = unusedSymbols.SelectMany(x => x.DeclaringSyntaxReferences.Select(syntax => new NodeAndSymbol(syntax.GetSyntax(), x)));

Expand All @@ -221,11 +214,16 @@ private static IEnumerable<Diagnostic> GetDiagnosticsForMembers(ICollection<ISym
}
}

diagnostics.Add(CreateS1144Diagnostic(syntaxForLocation, unused.Symbol));
if (unused.Symbol.IsConstructor() && !syntaxForLocation.GetModifiers().Any(SyntaxKind.PrivateKeyword))
{
context.ReportIssue(RuleS1144ForPublicCtor, syntaxForLocation, accessibility, unused.Symbol.ContainingType.Name);
}
else
{
context.ReportIssue(RuleS1144, GetIdentifierLocation(syntaxForLocation), accessibility, unused.Symbol.GetClassification(), GetMemberName(unused.Symbol));
}
}

return diagnostics;

static IEnumerable<VariableDeclaratorSyntax> GetSiblingDeclarators(SyntaxNode variableDeclarator) =>
variableDeclarator.Parent.Parent switch
{
Expand All @@ -234,38 +232,30 @@ static IEnumerable<VariableDeclaratorSyntax> GetSiblingDeclarators(SyntaxNode va
_ => Enumerable.Empty<VariableDeclaratorSyntax>(),
};

Diagnostic CreateS1144Diagnostic(SyntaxNode syntaxNode, ISymbol symbol) =>
symbol.IsConstructor() && !syntaxNode.GetModifiers().Any(SyntaxKind.PrivateKeyword)
? Diagnostic.Create(RuleS1144ForPublicCtor, syntaxNode.GetLocation(), accessibility, symbol.ContainingType.Name)
: Diagnostic.Create(RuleS1144, GetIdentifierLocation(syntaxNode), accessibility, symbol.GetClassification(), GetMemberName(symbol));

static Location GetIdentifierLocation(SyntaxNode syntaxNode) =>
syntaxNode.GetIdentifier() is { } identifier
? identifier.GetLocation()
: syntaxNode.GetLocation();
}

private static Diagnostic GetDiagnosticsForProperty(IPropertySymbol property, IReadOnlyDictionary<IPropertySymbol, AccessorAccess> propertyAccessorAccess)
private static void ReportProperty<TContext>(SonarCompilationReportingContextBase<TContext> context,
IPropertySymbol property,
IReadOnlyDictionary<IPropertySymbol, AccessorAccess> propertyAccessorAccess)
{
var access = propertyAccessorAccess[property];
if (access == AccessorAccess.Get
&& property.SetMethod is { }
&& property.SetMethod is not null
&& GetAccessorSyntax(property.SetMethod) is { } setter)
{
return Diagnostic.Create(RuleS1144, setter.Keyword.GetLocation(), SyntaxConstants.Private, "set accessor in property", property.Name);
context.ReportIssue(RuleS1144, setter.Keyword.GetLocation(), SyntaxConstants.Private, "set accessor in property", property.Name);
}
else if (access == AccessorAccess.Set
&& property.GetMethod is { }
&& GetAccessorSyntax(property.GetMethod) is { } getter
&& getter.HasBodyOrExpressionBody())
&& property.GetMethod is not null
&& GetAccessorSyntax(property.GetMethod) is { } getter
&& getter.HasBodyOrExpressionBody())
{
return Diagnostic.Create(RuleS1144, getter.Keyword.GetLocation(), SyntaxConstants.Private, "get accessor in property", property.Name);
context.ReportIssue(RuleS1144, getter.Keyword.GetLocation(), SyntaxConstants.Private, "get accessor in property", property.Name);
}
else
{
return null;
}

static AccessorDeclarationSyntax GetAccessorSyntax(ISymbol symbol) =>
symbol?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() as AccessorDeclarationSyntax;
}
Expand Down Expand Up @@ -534,7 +524,7 @@ private static bool IsRemovable(ISymbol symbol) =>
private static bool HasAttributes(ISymbol symbol)
{
IEnumerable<AttributeData> attributes = symbol.GetAttributes();
if (symbol is IMethodSymbol { MethodKind: MethodKind.PropertyGet or MethodKind.PropertySet} method
if (symbol is IMethodSymbol { MethodKind: MethodKind.PropertyGet or MethodKind.PropertySet } method
&& method.AssociatedSymbol is { } property)
{
attributes = attributes.Union(property.GetAttributes());
Expand Down
9 changes: 3 additions & 6 deletions analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SonarAnalysisContextExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,12 @@ public static void RegisterSemanticModelAction(this SonarParametrizedAnalysisCon
public static void RegisterCodeBlockStartAction(this SonarAnalysisContext context, Action<SonarCodeBlockStartAnalysisContext<SyntaxKind>> action) =>
context.RegisterCodeBlockStartAction(VisualBasicGeneratedCodeRecognizer.Instance, action);

public static void ReportIssue(this SonarCompilationReportingContext context, DiagnosticDescriptor rule, Location location, params string[] messageArgs) =>
context.ReportIssue(VisualBasicGeneratedCodeRecognizer.Instance, rule, location, messageArgs);

public static void ReportIssue(this SonarSymbolReportingContext context, DiagnosticDescriptor rule, SyntaxNode locationSyntax, params string[] messageArgs) =>
public static void ReportIssue<TContext>(this SonarCompilationReportingContextBase<TContext> context, DiagnosticDescriptor rule, SyntaxNode locationSyntax, params string[] messageArgs) =>
context.ReportIssue(VisualBasicGeneratedCodeRecognizer.Instance, rule, locationSyntax, messageArgs);

public static void ReportIssue(this SonarSymbolReportingContext context, DiagnosticDescriptor rule, SyntaxToken locationToken, params string[] messageArgs) =>
public static void ReportIssue<TContext>(this SonarCompilationReportingContextBase<TContext> context, DiagnosticDescriptor rule, SyntaxToken locationToken, params string[] messageArgs) =>
context.ReportIssue(VisualBasicGeneratedCodeRecognizer.Instance, rule, locationToken, messageArgs);

public static void ReportIssue(this SonarSymbolReportingContext context, DiagnosticDescriptor rule, Location location, params string[] messageArgs) =>
public static void ReportIssue<TContext>(this SonarCompilationReportingContextBase<TContext> context, DiagnosticDescriptor rule, Location location, params string[] messageArgs) =>
context.ReportIssue(VisualBasicGeneratedCodeRecognizer.Instance, rule, location, messageArgs);
}
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.VisualBasic/Rules/OptionExplicitOn.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected override void Initialize(SonarAnalysisContext context)
{
if (!c.Compilation.VB().Options.OptionExplicit)
{
c.ReportIssue(Rule, null, string.Format(AssemblyMessageFormat, c.Compilation.AssemblyName));
c.ReportIssue(Rule, (Location)null, string.Format(AssemblyMessageFormat, c.Compilation.AssemblyName));
}
}));
}
Expand Down
Loading