Skip to content

Commit

Permalink
Rule S6668: Handle the rule exceptions (#8848)
Browse files Browse the repository at this point in the history
  • Loading branch information
costin-zaharia-sonarsource authored Mar 1, 2024
1 parent afbce35 commit b05eb80
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,41 +34,47 @@ public sealed class LoggingArgumentsShouldBePassedCorrectly : SonarDiagnosticAna
ImmutableArray.Create(KnownType.System_Exception, KnownType.Microsoft_Extensions_Logging_LogLevel, KnownType.Microsoft_Extensions_Logging_EventId);
private static readonly ImmutableArray<KnownType> CastleCoreInvalidTypes = ImmutableArray.Create(KnownType.System_Exception);
private static readonly ImmutableArray<KnownType> NLogAndSerilogInvalidTypes = ImmutableArray.Create(KnownType.System_Exception, KnownType.Serilog_Events_LogEventLevel, KnownType.NLog_LogLevel);
private static readonly HashSet<string> LoggingMethodNames = MicrosoftExtensionsLogging
.Concat(NLogLoggingMethods)
.Concat(Serilog)
.Concat(CastleCoreOrCommonCore)
.ToHashSet();

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(c =>
{
var invocation = (InvocationExpressionSyntax)c.Node;
if (!HasLoggingMethodName(invocation)
if (!LoggingMethodNames.Contains(invocation.GetName())
|| c.SemanticModel.GetSymbolInfo(invocation).Symbol is not IMethodSymbol invocationSymbol)
{
return;
}
if (invocationSymbol.HasContainingType(KnownType.Microsoft_Extensions_Logging_LoggerExtensions, false))
{
CheckInvalidParams(invocation, invocationSymbol, c, MicrosoftLoggingExtensionsInvalidTypes);
CheckInvalidParams(invocation, invocationSymbol, c, Filter(invocationSymbol, MicrosoftLoggingExtensionsInvalidTypes));
}
else if (invocationSymbol.HasContainingType(KnownType.Castle_Core_Logging_ILogger, true))
{
CheckInvalidParams(invocation, invocationSymbol, c, CastleCoreInvalidTypes);
CheckInvalidParams(invocation, invocationSymbol, c, Filter(invocationSymbol, CastleCoreInvalidTypes));
}
else if (invocationSymbol.HasContainingType(KnownType.Serilog_ILogger, true)
|| invocationSymbol.HasContainingType(KnownType.Serilog_Log, false)
|| invocationSymbol.HasContainingType(KnownType.NLog_ILoggerBase, true)
|| invocationSymbol.HasContainingType(KnownType.NLog_ILoggerExtensions, false))
{
CheckInvalidParams(invocation, invocationSymbol, c, NLogAndSerilogInvalidTypes);
CheckInvalidTypeParams(invocation, invocationSymbol, c, NLogAndSerilogInvalidTypes);
var knownTypes = Filter(invocationSymbol, NLogAndSerilogInvalidTypes);
CheckInvalidParams(invocation, invocationSymbol, c, knownTypes);
CheckInvalidTypeParams(invocation, invocationSymbol, c, knownTypes);
}
},
SyntaxKind.InvocationExpression);

private static void CheckInvalidParams(InvocationExpressionSyntax invocation, IMethodSymbol invocationSymbol, SonarSyntaxNodeReportingContext c, ImmutableArray<KnownType> knownTypes)
{
var paramsParameter = invocationSymbol.Parameters.FirstOrDefault(x => x.IsParams);
if (paramsParameter is null)
if (paramsParameter is null || knownTypes.IsEmpty)
{
return;
}
Expand All @@ -88,7 +94,7 @@ private static void CheckInvalidParams(InvocationExpressionSyntax invocation, IM

private static void CheckInvalidTypeParams(InvocationExpressionSyntax invocation, IMethodSymbol methodSymbol, SonarSyntaxNodeReportingContext c, ImmutableArray<KnownType> knownTypes)
{
if (!IsNLogIgnoredOverload(methodSymbol) && methodSymbol.TypeArguments.Any(x => x.DerivesFromAny(knownTypes)))
if (!knownTypes.IsEmpty && !IsNLogIgnoredOverload(methodSymbol) && methodSymbol.TypeArguments.Any(x => x.DerivesFromAny(knownTypes)))
{
var typeParameterNames = methodSymbol.TypeParameters.Select(x => x.MetadataName).ToArray();
var positions = methodSymbol.ConstructedFrom.Parameters.Where(x => typeParameterNames.Contains(x.Type.MetadataName)).Select(x => methodSymbol.ConstructedFrom.Parameters.IndexOf(x));
Expand All @@ -97,12 +103,6 @@ private static void CheckInvalidTypeParams(InvocationExpressionSyntax invocation
}
}

private static bool HasLoggingMethodName(InvocationExpressionSyntax invocation) =>
MicrosoftExtensionsLogging.Contains(invocation.GetName())
|| NLogLoggingMethods.Contains(invocation.GetName())
|| Serilog.Contains(invocation.GetName())
|| CastleCoreOrCommonCore.Contains(invocation.GetName());

private static bool IsNLogIgnoredOverload(IMethodSymbol methodSymbol) =>
// These overloads are ignored since they will try to convert the T value to an exception.
MatchesParams(methodSymbol, KnownType.System_Exception)
Expand All @@ -123,4 +123,11 @@ private static IEnumerable<ArgumentSyntax> InvalidArguments(InvocationExpression

private static bool IsInvalidArgument(ArgumentSyntax argumentSyntax, SemanticModel model, ImmutableArray<KnownType> knownTypes) =>
model.GetTypeInfo(argumentSyntax.Expression).Type?.DerivesFromAny(knownTypes) is true;

// This method filters out the types that the method accepts strongly:
// logger.Debug(exception, "template", exception)
// ^^^^^^^^^ valid
// ^^^^^^^^^ do not raise
private static ImmutableArray<KnownType> Filter(IMethodSymbol methodSymbol, ImmutableArray<KnownType> knownTypes) =>
knownTypes.Where(knownType => !methodSymbol.ConstructedFrom.Parameters.Any(x => x.Type.DerivesFrom(knownType))).ToImmutableArray();
}
Loading

0 comments on commit b05eb80

Please sign in to comment.