diff --git a/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index 4253d721e3b4..3ffe186216c9 100644 --- a/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -404,11 +404,53 @@ bool IsIsEnabledInvocation(IInvocationOperation invocation) invocation.TargetMethod.IsOverrideOrImplementationOfInterfaceMember(_isEnabledMethod); } - static bool AreInvocationsOnSameInstance(IInvocationOperation invocation1, IInvocationOperation invocation2) + bool AreInvocationsOnSameInstance(IInvocationOperation invocation1, IInvocationOperation invocation2) { - return SymbolEqualityComparer.Default.Equals( - GetInstanceResolvingConditionalAccess(invocation1).GetReferencedMemberOrLocalOrParameter(), - GetInstanceResolvingConditionalAccess(invocation2).GetReferencedMemberOrLocalOrParameter()); + var instance1 = GetInstanceResolvingConditionalAccess(invocation1).GetReferencedMemberOrLocalOrParameter(); + var instance2 = GetInstanceResolvingConditionalAccess(invocation2).GetReferencedMemberOrLocalOrParameter(); + + if (SymbolEqualityComparer.Default.Equals(instance1, instance2)) + { + return true; + } + + if (invocation1.TargetMethod.GetAttribute(_loggerMessageAttributeType) is null) + { + return false; + } + + var method = invocation1.TargetMethod; + var loggerParameter = method.Parameters.FirstOrDefault(p => + SymbolEqualityComparer.Default.Equals(p.Type, _logMethod.ContainingType) || + p.Type.AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(i, _logMethod.ContainingType))); + + if (loggerParameter is not null) + { + var argument = invocation1.Arguments.GetArgumentForParameterAtIndex(loggerParameter.Ordinal); + if (SymbolEqualityComparer.Default.Equals(argument?.Value.GetReferencedMemberOrLocalOrParameter(), instance2)) + { + return true; + } + } + else if (!method.IsStatic) + { + if (!SymbolEqualityComparer.Default.Equals(instance2?.ContainingType, method.ContainingType)) + { + return false; + } + + // This is a heuristic, if a call to LoggerMessage is guarded by an IsEnabled check in + // the same class, it is assumed they use the same logger instance. This would lead to + // false negatives if this isn't the case. + switch (instance2) + { + case IFieldSymbol or IPropertySymbol: + case IParameterSymbol { ContainingSymbol: IMethodSymbol { MethodKind: MethodKind.Constructor } }: + return true; + } + } + + return false; } static IOperation? GetInstanceResolvingConditionalAccess(IInvocationOperation invocation) diff --git a/src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index e6a6aeba9899..5fb03737fd09 100644 --- a/src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -6162,6 +6162,39 @@ void M(ILogger logger, Exception exception) await VerifyCSharpDiagnosticAsync(source, editorConfigText: editorconfig); } + [Fact] + public async Task GuardedLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + sealed partial class Class1(ILogger logger) + { + public void M() + { + if (logger.IsEnabled(LogLevel.Information)) + { + Log(LogLevel.Information, Guid.NewGuid()); + } + + if (logger.IsEnabled(LogLevel.Information)) + { + Log(logger, LogLevel.Information, Guid.NewGuid()); + } + } + + [LoggerMessage(EventId = 0, Message = "{Id}")] + partial void Log(LogLevel logLevel, Guid id); + + [LoggerMessage(EventId = 1, Message = "{Id}")] + static partial void Log(ILogger logger, LogLevel logLevel, Guid id); + } + """; + + await VerifyCSharpDiagnosticAsync(source, CodeAnalysis.CSharp.LanguageVersion.CSharp12); + } + // Helpers private static async Task VerifyCSharpDiagnosticAsync([StringSyntax($"{LanguageNames.CSharp}-Test")] string source, CodeAnalysis.CSharp.LanguageVersion? languageVersion = null, (string, string)? editorConfigText = null)