From ec39a35e873d1911f827492aade046c28915845a Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sun, 14 Apr 2024 18:36:29 +0200 Subject: [PATCH 01/13] Add CA1873: Avoid potentially expensive logging This analyzer detects calls to 'ILogger.Log', extension methods in 'Microsoft.Extensions.Logging.LoggerExtensions' and methods decorated with '[LoggerMessage]'. It then checks if they evaluate expensive arguments without checking if logging is enabled with 'ILogger.IsEnabled'. --- .../Core/AnalyzerReleases.Unshipped.md | 1 + .../MicrosoftNetCoreAnalyzersResources.resx | 9 + ...voidPotentiallyExpensiveCallWhenLogging.cs | 333 ++ .../MicrosoftNetCoreAnalyzersResources.cs.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.de.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.es.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.fr.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.it.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.ja.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.ko.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.pl.xlf | 15 + ...crosoftNetCoreAnalyzersResources.pt-BR.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.ru.xlf | 15 + .../MicrosoftNetCoreAnalyzersResources.tr.xlf | 15 + ...osoftNetCoreAnalyzersResources.zh-Hans.xlf | 15 + ...osoftNetCoreAnalyzersResources.zh-Hant.xlf | 15 + .../Microsoft.CodeAnalysis.NetAnalyzers.md | 12 + .../Microsoft.CodeAnalysis.NetAnalyzers.sarif | 20 + src/NetAnalyzers/RulesMissingDocumentation.md | 3 +- ...otentiallyExpensiveCallWhenLoggingTests.cs | 3532 +++++++++++++++++ .../DiagnosticCategoryAndIdRanges.txt | 2 +- src/Utilities/Compiler/WellKnownTypeNames.cs | 1 + 22 files changed, 4105 insertions(+), 3 deletions(-) create mode 100644 src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs create mode 100644 src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs diff --git a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md index 6e16bd95c7..07d2e22fc1 100644 --- a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md +++ b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md @@ -8,6 +8,7 @@ CA1514 | Maintainability | Info | AvoidLengthCheckWhenSlicingToEndAnalyzer, [Doc CA1515 | Maintainability | Disabled | MakeTypesInternal, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1515) CA1871 | Performance | Info | DoNotPassNonNullableValueToArgumentNullExceptionThrowIfNull, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1871) CA1872 | Performance | Info | PreferConvertToHexStringOverBitConverterAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1872) +CA1873 | Performance | Info | AvoidPotentiallyExpensiveCallWhenLoggingAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1873) CA2022 | Reliability | Warning | AvoidUnreliableStreamReadAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2022) CA2262 | Usage | Info | ProvideHttpClientHandlerMaxResponseHeaderLengthValueCorrectly, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2262) CA2263 | Usage | Info | PreferGenericOverloadsAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2263) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx index 3c1daafb7a..face2b18a6 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx @@ -2123,6 +2123,15 @@ Widening and user defined conversions are not supported with generic types. Use char overload + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + Avoid potentially expensive logging + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs new file mode 100644 index 0000000000..f4278374a6 --- /dev/null +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -0,0 +1,333 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using Analyzer.Utilities; +using Analyzer.Utilities.Extensions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.NetCore.Analyzers.Performance +{ + using static MicrosoftNetCoreAnalyzersResources; + + /// + /// CA1873: + /// + [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] + public sealed class AvoidPotentiallyExpensiveCallWhenLoggingAnalyzer : DiagnosticAnalyzer + { + private const string RuleId = "CA1873"; + + private const string Level = nameof(Level); + private const string LogLevel = nameof(LogLevel); + + private const string Log = nameof(Log); + private const string IsEnabled = nameof(IsEnabled); + private const string LogTrace = nameof(LogTrace); + private const string LogDebug = nameof(LogDebug); + private const string LogInformation = nameof(LogInformation); + private const string LogWarning = nameof(LogWarning); + private const string LogError = nameof(LogError); + private const string LogCritical = nameof(LogCritical); + + private const int LogLevelTrace = 0; + private const int LogLevelDebug = 1; + private const int LogLevelInformation = 2; + private const int LogLevelWarning = 3; + private const int LogLevelError = 4; + private const int LogLevelCritical = 5; + private const int LogLevelPassedAsParameter = int.MinValue; + + private static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create( + RuleId, + CreateLocalizableResourceString(nameof(AvoidPotentiallyExpensiveCallWhenLoggingTitle)), + CreateLocalizableResourceString(nameof(AvoidPotentiallyExpensiveCallWhenLoggingMessage)), + DiagnosticCategory.Performance, + RuleLevel.IdeSuggestion, + CreateLocalizableResourceString(nameof(AvoidPotentiallyExpensiveCallWhenLoggingDescription)), + isPortedFxCopRule: false, + isDataflowRule: false); + + public sealed override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); + + public sealed override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private void OnCompilationStart(CompilationStartAnalysisContext context) + { + if (!RequiredSymbols.TryGetSymbols(context.Compilation, out var symbols)) + { + return; + } + + context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation); + + void AnalyzeInvocation(OperationAnalysisContext context) + { + var invocation = (IInvocationOperation)context.Operation; + + // Check if invocation is a logging invocation and capture the log level (either as IOperation or as int, depending if it is dynamic or not). + // Use these to check if the logging invocation is guarded by 'ILogger.IsEnabled' and bail out if it is. + if (!symbols.TryGetLogLevel(invocation, out var logLevelArgumentOperation, out var logLevel) || + symbols.IsGuardedByIsEnabled(currentOperation: invocation, logInvocation: invocation, logLevel, logLevelArgumentOperation)) + { + return; + } + + var arguments = invocation.Arguments.Skip(invocation.IsExtensionMethodAndHasNoInstance() ? 1 : 0); + + // At this stage we have a logging invocation that is not guarded. + // Check each argument if it is potentially expensive. + foreach (var argument in arguments) + { + // Check the argument value after conversions to prevent noise (e.g. implicit conversions from null or from int to EventId). + if (IsPotentiallyExpensive(argument.Value.WalkDownConversion())) + { + // Filter out implicit operations in the case of params arguments. + // If we would report the diagnostic on the implicit argument operation, it would flag the whole invocation. + var explicitDescendants = argument.Value.GetTopmostExplicitDescendants(); + + if (!explicitDescendants.IsEmpty) + { + context.ReportDiagnostic(explicitDescendants[0].CreateDiagnostic(Rule)); + } + } + } + } + } + + private static bool IsPotentiallyExpensive(IOperation? operation) + { + if (operation is null + // Implicit params array creation is treated as not expensive. This would otherwise cause a lot of noise. + or IArrayCreationOperation { IsImplicit: true, Initializer.ElementValues.IsEmpty: true } + or IInstanceReferenceOperation + or IConditionalAccessInstanceOperation + or ILiteralOperation + or ILocalReferenceOperation + or IParameterReferenceOperation) + { + return false; + } + + if (operation is IPropertyReferenceOperation { Arguments.IsEmpty: false } indexerReference) + { + return IsPotentiallyExpensive(indexerReference.Instance) || + indexerReference.Arguments.Any(a => IsPotentiallyExpensive(a.Value)); + } + + if (operation is IArrayElementReferenceOperation arrayElementReference) + { + return IsPotentiallyExpensive(arrayElementReference.ArrayReference) || + arrayElementReference.Indices.Any(IsPotentiallyExpensive); + } + + if (operation is IConditionalAccessOperation conditionalAccess) + { + return IsPotentiallyExpensive(conditionalAccess.WhenNotNull); + } + + if (operation is IMemberReferenceOperation memberReference) + { + return IsPotentiallyExpensive(memberReference.Instance); + } + + return true; + } + + internal sealed class RequiredSymbols + { + private RequiredSymbols( + IMethodSymbol logMethod, + IMethodSymbol isEnabledMethod, + ImmutableDictionary logExtensionsMethodsAndLevel, + INamedTypeSymbol? loggerMessageAttributeType) + { + _logMethod = logMethod; + _isEnabledMethod = isEnabledMethod; + _logExtensionsMethodsAndLevel = logExtensionsMethodsAndLevel; + _loggerMessageAttributeType = loggerMessageAttributeType; + } + + public static bool TryGetSymbols(Compilation compilation, [NotNullWhen(true)] out RequiredSymbols? symbols) + { + symbols = default; + + var iLoggerType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftExtensionsLoggingILogger); + + if (iLoggerType is null) + { + return false; + } + + var logMethod = iLoggerType.GetMembers(Log) + .OfType() + .FirstOrDefault(); + + var isEnabledMethod = iLoggerType.GetMembers(IsEnabled) + .OfType() + .FirstOrDefault(); + + if (logMethod is null || isEnabledMethod is null) + { + return false; + } + + var loggerExtensionsType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftExtensionsLoggingLoggerExtensions); + var logExtensionsMethodsBuilder = ImmutableDictionary.CreateBuilder(SymbolEqualityComparer.Default); + AddRangeIfNotNull(logExtensionsMethodsBuilder, loggerExtensionsType?.GetMembers(LogTrace).OfType(), LogLevelTrace); + AddRangeIfNotNull(logExtensionsMethodsBuilder, loggerExtensionsType?.GetMembers(LogDebug).OfType(), LogLevelDebug); + AddRangeIfNotNull(logExtensionsMethodsBuilder, loggerExtensionsType?.GetMembers(LogInformation).OfType(), LogLevelInformation); + AddRangeIfNotNull(logExtensionsMethodsBuilder, loggerExtensionsType?.GetMembers(LogWarning).OfType(), LogLevelWarning); + AddRangeIfNotNull(logExtensionsMethodsBuilder, loggerExtensionsType?.GetMembers(LogError).OfType(), LogLevelError); + AddRangeIfNotNull(logExtensionsMethodsBuilder, loggerExtensionsType?.GetMembers(LogCritical).OfType(), LogLevelCritical); + AddRangeIfNotNull(logExtensionsMethodsBuilder, loggerExtensionsType?.GetMembers(Log).OfType(), LogLevelPassedAsParameter); + + var loggerMessageAttributeType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftExtensionsLoggingLoggerMessageAttribute); + + symbols = new RequiredSymbols(logMethod, isEnabledMethod, logExtensionsMethodsBuilder.ToImmutable(), loggerMessageAttributeType); + + return true; + + void AddRangeIfNotNull(ImmutableDictionary.Builder builder, IEnumerable? range, int value) + { + if (range is not null) + { + builder.AddRange(range.Select(s => new KeyValuePair(s, value))); + } + } + } + + public bool TryGetLogLevel(IInvocationOperation invocation, out IArgumentOperation? logLevelArgumentOperation, out int logLevel) + { + logLevelArgumentOperation = default; + logLevel = LogLevelPassedAsParameter; + + var method = invocation.TargetMethod.ReducedFrom ?? invocation.TargetMethod; + + // ILogger.Log + if (SymbolEqualityComparer.Default.Equals(method.ConstructedFrom, _logMethod) || + method.ConstructedFrom.IsOverrideOrImplementationOfInterfaceMember(_logMethod)) + { + logLevelArgumentOperation = invocation.Arguments.GetArgumentForParameterAtIndex(0); + + return true; + } + + // LoggerExtensions.Log and named variants (e.g. LoggerExtensions.LogInformation) + if (_logExtensionsMethodsAndLevel.TryGetValue(method, out logLevel)) + { + // LoggerExtensions.Log + if (logLevel == LogLevelPassedAsParameter) + { + logLevelArgumentOperation = invocation.Arguments.GetArgumentForParameterAtIndex(invocation.IsExtensionMethodAndHasNoInstance() ? 1 : 0); + } + + return true; + } + + var loggerMessageAttribute = method.GetAttribute(_loggerMessageAttributeType); + + if (loggerMessageAttribute is null) + { + return false; + } + + // Try to get the log level from the attribute arguments. + logLevel = loggerMessageAttribute.NamedArguments + .FirstOrDefault(p => p.Key.Equals(Level, StringComparison.Ordinal)) + .Value.Value as int? + ?? LogLevelPassedAsParameter; + + if (logLevel == LogLevelPassedAsParameter) + { + logLevelArgumentOperation = invocation.Arguments + .FirstOrDefault(a => a.Value.Type?.Name.Equals(LogLevel, StringComparison.Ordinal) ?? false); + + if (logLevelArgumentOperation is null) + { + return false; + } + } + + return true; + } + + public bool IsGuardedByIsEnabled(IOperation currentOperation, IInvocationOperation logInvocation, int logLevel, IArgumentOperation? logLevelArgumentOperation) + { + var conditional = currentOperation.GetAncestor(OperationKind.Conditional); + + // This is the base case where no ancestor conditional is found. + if (conditional is null) + { + return false; + } + + var conditionInvocations = conditional.Condition + .DescendantsAndSelf() + .OfType(); + + // Check each invocation in the condition to see if it is a valid guard invocation, i.e. same instance and same log level. + // This is not perfect (e.g. 'if (logger.IsEnabled(LogLevel.Debug) || true)' is treated as guarded), but should be good enough to prevent false positives. + if (conditionInvocations.Any(IsValidIsEnabledGuardInvocation)) + { + return true; + } + + // Recursively check the next conditional. + return IsGuardedByIsEnabled(conditional, logInvocation, logLevel, logLevelArgumentOperation); + + bool IsValidIsEnabledGuardInvocation(IInvocationOperation invocation) + { + if (!SymbolEqualityComparer.Default.Equals(_isEnabledMethod, invocation.TargetMethod) && + !invocation.TargetMethod.IsOverrideOrImplementationOfInterfaceMember(_isEnabledMethod)) + { + return false; + } + + return AreInvocationsOnSameInstance(logInvocation, invocation) && IsSameLogLevel(invocation.Arguments[0]); + } + + static bool AreInvocationsOnSameInstance(IInvocationOperation invocation1, IInvocationOperation invocation2) + { + return (invocation1.GetInstance()?.WalkDownConversion(), invocation2.GetInstance()?.WalkDownConversion()) switch + { + (IFieldReferenceOperation fieldRef1, IFieldReferenceOperation fieldRef2) => fieldRef1.Member == fieldRef2.Member, + (IPropertyReferenceOperation propRef1, IPropertyReferenceOperation propRef2) => propRef1.Member == propRef2.Member, + (IParameterReferenceOperation paramRef1, IParameterReferenceOperation paramRef2) => paramRef1.Parameter == paramRef2.Parameter, + (ILocalReferenceOperation localRef1, ILocalReferenceOperation localRef2) => localRef1.Local == localRef2.Local, + _ => false, + }; + } + + bool IsSameLogLevel(IArgumentOperation isEnabledArgument) + { + if (isEnabledArgument.Value.ConstantValue.HasValue) + { + int isEnabledLogLevel = (int)isEnabledArgument.Value.ConstantValue.Value!; + + return logLevel == LogLevelPassedAsParameter + ? logLevelArgumentOperation?.Value.HasConstantValue(isEnabledLogLevel) ?? false + : isEnabledLogLevel == logLevel; + } + + return isEnabledArgument.Value.GetReferencedMemberOrLocalOrParameter() == logLevelArgumentOperation?.Value.GetReferencedMemberOrLocalOrParameter(); + } + } + + private readonly IMethodSymbol _logMethod; + private readonly IMethodSymbol _isEnabledMethod; + private readonly ImmutableDictionary _logExtensionsMethodsAndLevel; + private readonly INamedTypeSymbol? _loggerMessageAttributeType; + } + } +} diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf index cf3c53f482..7958b1f206 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf @@ -67,6 +67,21 @@ Vyhněte se konstantním polím jako argumentům + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Vyhněte se vytvoření nové instance JsonSerializerOptions pro každou operaci serializace. Místo toho ukládejte instance do mezipaměti a znovu je používejte. Instance JsonSerializerOptions pro jedno použití můžou výrazně snížit výkon vaší aplikace. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf index de59d2fbe4..4dc0c4bc5a 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf @@ -67,6 +67,21 @@ Konstantenmatrizen als Argumente vermeiden + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Erstellen Sie für Serialisierungsvorgänge nicht jeweils eine neue JsonSerializerOptions-Instanz. Speichern Sie stattdessen Instanzen zwischen, und verwenden Sie sie wieder. Die einmalige Verwendung von JsonSerializerOptions-Instanzen kann die Leistung Ihrer Anwendung erheblich beeinträchtigen. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf index 0ec6aa48e5..4c0db21d59 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf @@ -67,6 +67,21 @@ Evitar matrices constantes como argumentos + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Evite crear una nueva instancia de "JsonSerializerOptions" para cada operación de serialización. En su lugar, almacene en caché y reutilice instancias. Las instancias "JsonSerializerOptions" de uso único pueden degradar considerablemente el rendimiento de la aplicación. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf index ea1fb007b5..056243131a 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf @@ -67,6 +67,21 @@ Éviter les tableaux constants en tant qu’arguments + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Évitez de créer une instance « JsonSerializerOptions » pour chaque opération de sérialisation. Mettez en cache et réutilisez les instances à la place. Les instances « JsonSerializerOptions » à usage unique peuvent considérablement dégrader les performances de votre application. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf index a0cecdb587..ac42b37e69 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf @@ -67,6 +67,21 @@ Evitare matrici costanti come argomenti + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Evitare di creare una nuova istanza di 'JsonSerializerOptions' per ogni operazione di serializzazione. Memorizzare nella cache le istanze e riutilizzarle. Le istanze monouso di 'JsonSerializerOptions' possono ridurre notevolmente le prestazioni dell'applicazione. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf index 8c9ee9a227..a798262b04 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf @@ -67,6 +67,21 @@ 引数として定数配列を使用しない + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. シリアル化操作ごとに新しい 'JsonSerializerOptions' インスタンスを作成しないでください。代わりにインスタンスをキャッシュして再利用します。'JsonSerializerOptions' インスタンスの単独使用では、アプリケーションのパフォーマンスが大幅に低下する可能性があります。 diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf index b452dcb1b8..58aba402dd 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf @@ -67,6 +67,21 @@ 상수 배열을 인수로 사용하지 않습니다. + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. 모든 직렬화 작업에 대해 새 'JsonSerializerOptions' 인스턴스를 만들지 마세요. 대신 인스턴스를 캐시하고 다시 사용합니다. 단일 사용 'JsonSerializerOptions' 인스턴스는 애플리케이션의 성능을 크게 저하시킬 수 있습니다. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf index 3583b201b6..9f673e1493 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf @@ -67,6 +67,21 @@ Unikaj tablic stałych jako argumentów + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Unikaj tworzenia nowego wystąpienia „JsonSerializerOptions” dla każdej operacji serializacji. Zamiast tego buforuj i ponownie wykorzystuj wystąpienia. Jednorazowe użycie wystąpienia „JsonSerializerOptions” może znacznie obniżyć wydajność aplikacji. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf index 7846c9ceab..d4d9b191ea 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf @@ -67,6 +67,21 @@ Evite matrizes constantes como argumentos + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Evite criar uma nova instância 'JsonSerializerOptions' para cada operação de serialização. Em vez disso, armazene em cache e reutilize instâncias. Instâncias 'JsonSerializerOptions' de uso único podem degradar substancialmente o desempenho de seu aplicativo. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf index 997f93cee2..14637b5ef3 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf @@ -67,6 +67,21 @@ Избегайте использования константных массивов в качестве аргументов + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Избегайте создания нового экземпляра "JsonSerializerOptions" для каждой операции сериализации. Выполняйте кэширование и повторно используйте экземпляры. Однократное использование экземпляров "JsonSerializerOptions" может значительно снизить производительность приложения. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf index 4373b25f18..ee004dc6fb 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf @@ -67,6 +67,21 @@ Sabit dizileri bağımsız değişkenler olarak kullanmaktan sakının + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. Her serileştirme işlemi için yeni bir 'JsonSerializerOptions' örneği oluşturmaktan kaçının. Bunun yerine örnekleri önbelleğe alıp yeniden kullanın. Tek kullanımlık 'JsonSerializerOptions' örnekleri uygulamanızın performansını önemli ölçüde düşürebilir. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf index de4a764cc0..713c2b3f62 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf @@ -67,6 +67,21 @@ 不要将常量数组作为参数 + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. 避免为每个序列化操作创建新的“JsonSerializerOptions”实例。请改为缓存和重用实例。仅使用“JsonSerializerOptions”实例可能会显著降低应用程序的性能。 diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf index 5f5fcd7826..3a945ba69b 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf @@ -67,6 +67,21 @@ 避免常數陣列作為引數 + + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + + + + Evaluation of this argument may be expensive and unnecessary if logging is disabled + Evaluation of this argument may be expensive and unnecessary if logging is disabled + + + + Avoid potentially expensive logging + Avoid potentially expensive logging + + Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application. 避免為每個序列化作業建立新的 'JsonSerializerOptions' 執行個體。改為快取並重新使用執行個體。單一使用 'JsonSerializerOptions' 執行個體可能會大幅降低應用程式的效能。 diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md index b3a6bc05d4..68100f1dae 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md @@ -1860,6 +1860,18 @@ Use 'Convert.ToHexString' or 'Convert.ToHexStringLower' when encoding bytes to a |CodeFix|True| --- +## [CA1873](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1873): Avoid potentially expensive logging + +In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument. + +|Item|Value| +|-|-| +|Category|Performance| +|Enabled|True| +|Severity|Info| +|CodeFix|False| +--- + ## [CA2000](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2000): Dispose objects before losing scope If a disposable object is not explicitly disposed before all references to it are out of scope, the object will be disposed at some indeterminate time when the garbage collector runs the finalizer of the object. Because an exceptional event might occur that will prevent the finalizer of the object from running, the object should be explicitly disposed instead. diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif index 176cbcbd86..8f20065d49 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif @@ -3428,6 +3428,26 @@ ] } }, + "CA1873": { + "id": "CA1873", + "shortDescription": "Avoid potentially expensive logging", + "fullDescription": "In many situations, logging is disabled or set to a log level that results in an unnecessary evaluation for this argument.", + "defaultLevel": "note", + "helpUri": "https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1873", + "properties": { + "category": "Performance", + "isEnabledByDefault": true, + "typeName": "AvoidPotentiallyExpensiveCallWhenLoggingAnalyzer", + "languages": [ + "C#", + "Visual Basic" + ], + "tags": [ + "Telemetry", + "EnabledRuleInAggressiveMode" + ] + } + }, "CA2000": { "id": "CA2000", "shortDescription": "Dispose objects before losing scope", diff --git a/src/NetAnalyzers/RulesMissingDocumentation.md b/src/NetAnalyzers/RulesMissingDocumentation.md index f0385ea23d..e1107d820d 100644 --- a/src/NetAnalyzers/RulesMissingDocumentation.md +++ b/src/NetAnalyzers/RulesMissingDocumentation.md @@ -2,7 +2,6 @@ Rule ID | Missing Help Link | Title | --------|-------------------|-------| -CA1871 | | Do not pass a nullable struct to 'ArgumentNullException.ThrowIfNull' | +CA1873 | | Avoid potentially expensive logging | CA2022 | | Avoid inexact read with 'Stream.Read' | -CA2264 | | Do not pass a non-nullable value to 'ArgumentNullException.ThrowIfNull' | CA2265 | | Do not compare Span\ to 'null' or 'default' | diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs new file mode 100644 index 0000000000..00ea3c7f19 --- /dev/null +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -0,0 +1,3532 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Testing; +using Xunit; + +using VerifyCS = Test.Utilities.CSharpCodeFixVerifier< + Microsoft.NetCore.Analyzers.Performance.AvoidPotentiallyExpensiveCallWhenLoggingAnalyzer, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; +using VerifyVB = Test.Utilities.VisualBasicCodeFixVerifier< + Microsoft.NetCore.Analyzers.Performance.AvoidPotentiallyExpensiveCallWhenLoggingAnalyzer, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + +namespace Microsoft.NetCore.Analyzers.Performance.UnitTests +{ + public class AvoidPotentiallyExpensiveCallWhenLoggingTests + { + public static readonly TheoryData LogLevels = new() + { + "Trace", + "Debug", + "Information", + "Warning", + "Error", + "Critical" + }; + + [Fact] + public async Task LiteralInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Trace, eventId, "literal", exception, formatter); + + logger.Log(LogLevel.Debug, "literal"); + logger.Log(LogLevel.Information, eventId, "literal"); + logger.Log(LogLevel.Warning, exception, "literal"); + logger.Log(LogLevel.Error, eventId, exception, "literal"); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task LiteralInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + logger.Log{{logLevel}}("literal"); + logger.Log{{logLevel}}(eventId, "literal"); + logger.Log{{logLevel}}(exception, "literal"); + logger.Log{{logLevel}}(eventId, exception, "literal"); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task LiteralInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + logger.StaticLogLevel("literal"); + logger.DynamicLogLevel(LogLevel.Debug, "literal"); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task LocalInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + string local = "local"; + + logger.Log(LogLevel.Trace, eventId, local, exception, formatter); + + logger.Log(LogLevel.Debug, local); + logger.Log(LogLevel.Information, eventId, local); + logger.Log(LogLevel.Warning, exception, local); + logger.Log(LogLevel.Error, eventId, exception, local); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task LocalInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + string local = "local"; + + logger.Log{{logLevel}}(local); + logger.Log{{logLevel}}(eventId, local); + logger.Log{{logLevel}}(exception, local); + logger.Log{{logLevel}}(eventId, exception, local); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task LocalInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + string local = "local"; + + logger.StaticLogLevel(local); + logger.DynamicLogLevel(LogLevel.Debug, local); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task FieldInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + private string _field; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Trace, eventId, _field, exception, formatter); + + logger.Log(LogLevel.Debug, _field); + logger.Log(LogLevel.Information, eventId, _field); + logger.Log(LogLevel.Warning, exception, _field); + logger.Log(LogLevel.Error, eventId, exception, _field); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task FieldInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + private string _field; + + void M(ILogger logger, EventId eventId, Exception exception) + { + logger.Log{{logLevel}}(_field); + logger.Log{{logLevel}}(eventId, _field); + logger.Log{{logLevel}}(exception, _field); + logger.Log{{logLevel}}(eventId, exception, _field); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task FieldInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + private static string _field; + + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + logger.StaticLogLevel(_field); + logger.DynamicLogLevel(LogLevel.Debug, _field); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task PropertyInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + public string Property { get; set; } + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Trace, eventId, Property, exception, formatter); + + logger.Log(LogLevel.Debug, Property); + logger.Log(LogLevel.Information, eventId, Property); + logger.Log(LogLevel.Warning, exception, Property); + logger.Log(LogLevel.Error, eventId, exception, Property); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task PropertyInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + public string Property { get; set; } + + void M(ILogger logger, EventId eventId, Exception exception) + { + logger.Log{{logLevel}}(Property); + logger.Log{{logLevel}}(eventId, Property); + logger.Log{{logLevel}}(exception, Property); + logger.Log{{logLevel}}(eventId, exception, Property); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task PropertyInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + public static string Property { get; set; } + + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + logger.StaticLogLevel(Property); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task IndexerInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using System.Collections.Generic; + using Microsoft.Extensions.Logging; + + class C + { + private Dictionary _messages; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Trace, eventId, _messages[LogLevel.Trace], exception, formatter); + + logger.Log(LogLevel.Debug, _messages[LogLevel.Debug]); + logger.Log(LogLevel.Information, eventId, _messages[LogLevel.Information]); + logger.Log(LogLevel.Warning, exception, _messages[LogLevel.Warning]); + logger.Log(LogLevel.Error, eventId, exception, _messages[LogLevel.Error]); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task IndexerInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using System.Collections.Generic; + using Microsoft.Extensions.Logging; + + class C + { + private Dictionary _messages; + + void M(ILogger logger, EventId eventId, Exception exception) + { + logger.Log{{logLevel}}(_messages[LogLevel.{{logLevel}}]); + logger.Log{{logLevel}}(eventId, _messages[LogLevel.{{logLevel}}]); + logger.Log{{logLevel}}(exception, _messages[LogLevel.{{logLevel}}]); + logger.Log{{logLevel}}(eventId, exception, _messages[LogLevel.{{logLevel}}]); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task IndexerInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using System.Collections.Generic; + using Microsoft.Extensions.Logging; + + static partial class C + { + private static Dictionary _messages; + + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + logger.StaticLogLevel(_messages[LogLevel.Information]); + logger.DynamicLogLevel(LogLevel.Debug, _messages[LogLevel.Debug]); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ArrayIndexerInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + private string[] _messages; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Trace, eventId, _messages[0], exception, formatter); + + logger.Log(LogLevel.Debug, _messages[0]); + logger.Log(LogLevel.Information, eventId, _messages[0]); + logger.Log(LogLevel.Warning, exception, _messages[0]); + logger.Log(LogLevel.Error, eventId, exception, _messages[0]); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task ArrayIndexerInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + private string[] _messages; + + void M(ILogger logger, EventId eventId, Exception exception) + { + logger.Log{{logLevel}}(_messages[0]); + logger.Log{{logLevel}}(eventId, _messages[0]); + logger.Log{{logLevel}}(exception, _messages[0]); + logger.Log{{logLevel}}(eventId, exception, _messages[0]); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ArrayIndexerInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + private static string[] _messages; + + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + logger.StaticLogLevel(_messages[0]); + logger.DynamicLogLevel(LogLevel.Debug, _messages[0]); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ConditionalAccessInLog_NoDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + { + logger.Log(LogLevel.Trace, eventId, exception?.Message, exception, formatter); + + logger.Log(LogLevel.Debug, exception?.Message); + logger.Log(LogLevel.Information, eventId, exception?.Message); + logger.Log(LogLevel.Warning, exception, exception?.Message); + logger.Log(LogLevel.Error, eventId, exception, exception?.Message); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task ConditionalAccessInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception) + { + logger.Log{{logLevel}}(exception?.Message); + logger.Log{{logLevel}}(eventId, exception?.Message); + logger.Log{{logLevel}}(exception, exception?.Message); + logger.Log{{logLevel}}(eventId, exception, exception?.Message); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ConditionalAccessInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string? message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string? message); + + static void M(ILogger logger, Exception? exception) + { + logger.StaticLogLevel(exception?.Message); + logger.DynamicLogLevel(LogLevel.Debug, exception?.Message); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task OtherILoggerMethodCalled_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger) + { + logger.BeginScope(ExpensiveMethodCall()); + logger.BeginScope("Processing calculation result {CalculationResult}", ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + // Tests for operations that get flagged. + + [Fact] + public async Task AnonymousObjectCreationOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|new { Test = "42" }|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ArrayCreationOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|new int[10]|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task AwaitOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using System.Threading.Tasks; + using Microsoft.Extensions.Logging; + + class C + { + async void M(ILogger logger, EventId eventId, Exception exception, Func formatter, Task task) + { + logger.Log(LogLevel.Debug, eventId, [|await task|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task BinaryOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|4 + 2|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task CoalesceOperation_ReportsDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|exception ?? new Exception()|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task CollectionExpressionOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|[4, 2]|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source, CodeAnalysis.CSharp.LanguageVersion.CSharp12); + } + + [Fact] + public async Task DefaultValueOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|default|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task IncrementOrDecrementOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) + { + logger.Log(LogLevel.Debug, eventId, [|input++|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task InterpolatedStringOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) + { + logger.Log(LogLevel.Debug, eventId, [|$"{input}"|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task InvocationOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|exception.ToString()|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task IsPatternOperation_ReportsDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|exception is not null|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task IsTypeOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, object input) + { + logger.Log(LogLevel.Debug, eventId, [|input is Exception|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task NameOfOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|nameof(logger)|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ObjectCreationOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|new Exception()|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task SizeOfOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|sizeof(int)|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task TypeOfOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|typeof(int)|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task UnaryOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, bool input) + { + logger.Log(LogLevel.Debug, eventId, [|!input|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WithOperation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + record Point(int X, int Y); + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, Point input) + { + logger.Log(LogLevel.Debug, eventId, [|input with { Y = 42 }|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + // Tests for work done in indexers, array element references or instances of member references. + + [Fact] + public async Task WorkInIndexerInstance_ReportsDiagnostic_CS() + { + string source = """ + using System; + using System.Collections.Generic; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall()[LogLevel.Debug]|], exception, formatter); + } + + Dictionary ExpensiveMethodCall() + { + return default; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInIndexerArgument_ReportsDiagnostic_CS() + { + string source = """ + using System; + using System.Collections.Generic; + using Microsoft.Extensions.Logging; + + class C + { + private Dictionary _messages; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|_messages[ExpensiveMethodCall()]|], exception, formatter); + } + + LogLevel ExpensiveMethodCall() + { + return LogLevel.Debug; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInConditionalAccess_ReportsDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|exception?.ToString()|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInFieldInstance_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + private int _field; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall()._field|], exception, formatter); + } + + C ExpensiveMethodCall() + { + return new C(); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInPropertyInstance_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall().Length|], exception, formatter); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInArrayReference_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + private int _field; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int[] input) + { + logger.Log(LogLevel.Debug, eventId, [|input[ExpensiveMethodCall()]|], exception, formatter); + } + + int ExpensiveMethodCall() + { + return 0; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + // Tests when log call is guarded. + + [Fact] + public async Task GuardedWorkInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + if (logger.IsEnabled(LogLevel.Trace)) + logger.Log(LogLevel.Trace, eventId, ExpensiveMethodCall(), exception, formatter); + + if (logger.IsEnabled(LogLevel.Debug)) + logger.Log(LogLevel.Debug, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Information)) + logger.Log(LogLevel.Information, eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Warning)) + logger.Log(LogLevel.Warning, exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Error)) + logger.Log(LogLevel.Error, eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task GuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + { + if (logger.IsEnabled(level)) + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); + + if (logger.IsEnabled(level)) + logger.Log(level, ExpensiveMethodCall()); + + if (logger.IsEnabled(level)) + logger.Log(level, eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(level)) + logger.Log(level, exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(level)) + logger.Log(level, eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + if (logger.IsEnabled(LogLevel.{{logLevel}})) + { + logger.StaticLogLevel(ExpensiveMethodCall()); + } + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + { + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task NestedGuardedWorkInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + if (logger.IsEnabled(LogLevel.Debug)) + { + if (exception is not null) + { + logger.Log(LogLevel.Debug, eventId, ExpensiveMethodCall(), exception, formatter); + logger.Log(LogLevel.Debug, ExpensiveMethodCall()); + logger.Log(LogLevel.Debug, eventId, ExpensiveMethodCall()); + logger.Log(LogLevel.Debug, exception, ExpensiveMethodCall()); + logger.Log(LogLevel.Debug, eventId, exception, ExpensiveMethodCall()); + } + } + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task NestedGuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + { + if (logger.IsEnabled(level)) + { + if (exception is not null) + { + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); + logger.Log(level, ExpensiveMethodCall()); + logger.Log(level, eventId, ExpensiveMethodCall()); + logger.Log(level, exception, ExpensiveMethodCall()); + logger.Log(level, eventId, exception, ExpensiveMethodCall()); + } + } + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + if (logger.IsEnabled(LogLevel.{{logLevel}})) + if (exception is not null) + logger.Log{{logLevel}}(ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + if (exception is not null) + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + if (exception is not null) + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + if (exception is not null) + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + static bool IsExpensiveComputationEnabled { get; set; } + + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + if (logger.IsEnabled(LogLevel.{{logLevel}})) + { + if (IsExpensiveComputationEnabled) + { + logger.StaticLogLevel(ExpensiveMethodCall()); + } + } + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + { + if (IsExpensiveComputationEnabled) + { + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + } + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task CustomLoggerGuardedWorkInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class CustomLogger : ILogger + { + public IDisposable BeginScope(TState state) { return default; } + public bool IsEnabled(LogLevel logLevel) { return true; } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + } + + class C + { + void M(CustomLogger logger, EventId eventId, Exception exception, Func formatter) + { + if (logger.IsEnabled(LogLevel.Trace)) + logger.Log(LogLevel.Trace, eventId, ExpensiveMethodCall(), exception, formatter); + + if (logger.IsEnabled(LogLevel.Debug)) + logger.Log(LogLevel.Debug, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Information)) + logger.Log(LogLevel.Information, eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Warning)) + logger.Log(LogLevel.Warning, exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Error)) + logger.Log(LogLevel.Error, eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task CustomLoggerGuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class CustomLogger : ILogger + { + public IDisposable BeginScope(TState state) { return default; } + public bool IsEnabled(LogLevel logLevel) { return true; } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + } + + class C + { + void M(CustomLogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + { + if (logger.IsEnabled(level)) + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); + + if (logger.IsEnabled(level)) + logger.Log(level, ExpensiveMethodCall()); + + if (logger.IsEnabled(level)) + logger.Log(level, eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(level)) + logger.Log(level, exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(level)) + logger.Log(level, eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task CustomLoggerGuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class CustomLogger : ILogger + { + public IDisposable BeginScope(TState state) { return default; } + public bool IsEnabled(LogLevel logLevel) { return true; } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + } + + class C + { + void M(CustomLogger logger, EventId eventId, Exception exception) + { + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task CustomLoggerGuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class CustomLogger : ILogger + { + public IDisposable BeginScope(TState state) { return default; } + public bool IsEnabled(LogLevel logLevel) { return true; } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + } + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(CustomLogger logger) + { + if (logger.IsEnabled(LogLevel.{{logLevel}})) + { + logger.StaticLogLevel(ExpensiveMethodCall()); + } + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + { + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WrongLogLevelGuardedWorkInLog_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + if (logger.IsEnabled(LogLevel.Critical)) + logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter); + + if (logger.IsEnabled(LogLevel.Critical)) + logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(LogLevel.Critical)) + logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(LogLevel.Critical)) + logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(LogLevel.Critical)) + logger.Log(LogLevel.Error, eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongLogLevelGuardedWorkInLogNamed_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + if (logger.IsEnabled(LogLevel.None)) + logger.Log{{logLevel}}([|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(LogLevel.None)) + logger.Log{{logLevel}}(eventId, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(LogLevel.None)) + logger.Log{{logLevel}}(exception, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(LogLevel.None)) + logger.Log{{logLevel}}(eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongLogLevelGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + if (logger.IsEnabled(LogLevel.None)) + { + logger.StaticLogLevel([|ExpensiveMethodCall()|]); + } + + if (logger.IsEnabled(LogLevel.None)) + { + logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]); + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WrongDynamicLogLevelGuardedWorkInLog_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + { + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter); + + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Error, eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongDynamicLogLevelGuardedWorkInLogNamed_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, LogLevel level) + { + if (logger.IsEnabled(level)) + logger.Log{{logLevel}}([|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log{{logLevel}}(eventId, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log{{logLevel}}(exception, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log{{logLevel}}(eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongDynamicLogLevelGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger, LogLevel level) + { + if (logger.IsEnabled(level)) + { + logger.StaticLogLevel([|ExpensiveMethodCall()|]); + } + + if (logger.IsEnabled(level)) + { + logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]); + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WrongInstanceGuardedWorkInLog_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + private ILogger _otherLogger; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + if (_otherLogger.IsEnabled(LogLevel.Trace)) + logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter); + + if (_otherLogger.IsEnabled(LogLevel.Debug)) + logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.Information)) + logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.Warning)) + logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.Error)) + logger.Log(LogLevel.Error, eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongInstanceGuardedWorkInLogNamed_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + private ILogger _otherLogger; + + void M(ILogger logger, EventId eventId, Exception exception) + { + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}([|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(exception, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongInstanceGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + private static ILogger _otherLogger; + + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] + static partial void StaticLogLevel(this ILogger logger, string message); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + + static void M(ILogger logger) + { + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + { + logger.StaticLogLevel([|ExpensiveMethodCall()|]); + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + // VB tests + + [Fact] + public async Task LiteralInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, "literal", exception, formatter) + logger.Log(LogLevel.Debug, "literal") + logger.Log(LogLevel.Information, eventId, "literal") + logger.Log(LogLevel.Warning, exception, "literal") + logger.Log(LogLevel.[Error], eventId, exception, "literal") + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task LiteralInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}("literal") + logger.Log{{logLevel}}(eventId, "literal") + logger.Log{{logLevel}}(exception, "literal") + logger.Log{{logLevel}}(eventId, exception, "literal") + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task LiteralInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel("literal") + logger.DynamicLogLevel(LogLevel.Debug, "literal") + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task LocalInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + Dim local As String = "local" + + logger.Log(LogLevel.Trace, eventId, local, exception, formatter) + logger.Log(LogLevel.Debug, local) + logger.Log(LogLevel.Information, eventId, local) + logger.Log(LogLevel.Warning, exception, local) + logger.Log(LogLevel.[Error], eventId, exception, local) + End Sub + End Class + + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task LocalInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + Dim local As String = "local" + + logger.Log{{logLevel}}("literal") + logger.Log{{logLevel}}(eventId, "literal") + logger.Log{{logLevel}}(exception, "literal") + logger.Log{{logLevel}}(eventId, exception, "literal") + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task LocalInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + Dim local As String = "local" + + logger.StaticLogLevel(local) + logger.DynamicLogLevel(LogLevel.Debug, local) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task FieldInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _field As String + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, _field, exception, formatter) + logger.Log(LogLevel.Debug, _field) + logger.Log(LogLevel.Information, eventId, _field) + logger.Log(LogLevel.Warning, exception, _field) + logger.Log(LogLevel.[Error], eventId, exception, _field) + End Sub + End Class + + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task FieldInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _field As String + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}(_field) + logger.Log{{logLevel}}(eventId, _field) + logger.Log{{logLevel}}(exception, _field) + logger.Log{{logLevel}}(eventId, exception, _field) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task FieldInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Private _field As String + + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel(_field) + logger.DynamicLogLevel(LogLevel.Debug, _field) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task PropertyInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Public Property [Property] As String + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, [Property], exception, formatter) + logger.Log(LogLevel.Debug, [Property]) + logger.Log(LogLevel.Information, eventId, [Property]) + logger.Log(LogLevel.Warning, exception, [Property]) + logger.Log(LogLevel.[Error], eventId, exception, [Property]) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task PropertyInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Public Property [Property] As String + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}([Property]) + logger.Log{{logLevel}}(eventId, [Property]) + logger.Log{{logLevel}}(exception, [Property]) + logger.Log{{logLevel}}(eventId, exception, [Property]) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task PropertyInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Public Property [Property] As String + + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel([Property]) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task IndexerInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Collections.Generic + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As Dictionary(Of LogLevel, String) + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, _messages(LogLevel.Trace), exception, formatter) + logger.Log(LogLevel.Debug, _messages(LogLevel.Debug)) + logger.Log(LogLevel.Information, eventId, _messages(LogLevel.Information)) + logger.Log(LogLevel.Warning, exception, _messages(LogLevel.Warning)) + logger.Log(LogLevel.[Error], eventId, exception, _messages(LogLevel.[Error])) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task IndexerInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Collections.Generic + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As Dictionary(Of LogLevel, String) + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}(_messages(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, _messages(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(exception, _messages(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, exception, _messages(LogLevel.{{logLevel}})) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task IndexerInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Collections.Generic + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Private _messages As Dictionary(Of LogLevel, String) + + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel(_messages(LogLevel.Information)) + logger.DynamicLogLevel(LogLevel.Debug, _messages(LogLevel.Debug)) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ArrayIndexerInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As String() + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, _messages(0), exception, formatter) + logger.Log(LogLevel.Debug, _messages(0)) + logger.Log(LogLevel.Information, eventId, _messages(0)) + logger.Log(LogLevel.Warning, exception, _messages(0)) + logger.Log(LogLevel.[Error], eventId, exception, _messages(0)) + End Sub + End Class + + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task ArrayIndexerInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As String() + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}(_messages(0)) + logger.Log{{logLevel}}(eventId, _messages(0)) + logger.Log{{logLevel}}(exception, _messages(0)) + logger.Log{{logLevel}}(eventId, exception, _messages(0)) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ArrayIndexerInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Private _messages As String() + + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel(_messages(0)) + logger.DynamicLogLevel(LogLevel.Debug, _messages(0)) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ConditionalAccessInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, exception?.Message, exception, formatter) + logger.Log(LogLevel.Debug, exception?.Message) + logger.Log(LogLevel.Information, eventId, exception?.Message) + logger.Log(LogLevel.Warning, exception, exception?.Message) + logger.Log(LogLevel.[Error], eventId, exception, exception?.Message) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task ConditionalAccessInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}(exception?.Message) + logger.Log{{logLevel}}(eventId, exception?.Message) + logger.Log{{logLevel}}(exception, exception?.Message) + logger.Log{{logLevel}}(eventId, exception, exception?.Message) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ConditionalAccessInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger, exception As Exception) + logger.StaticLogLevel(exception?.Message) + logger.DynamicLogLevel(LogLevel.Debug, exception?.Message) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task OtherILoggerMethodCalled_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger) + logger.BeginScope(ExpensiveMethodCall()) + logger.BeginScope("Processing calculation result {CalculationResult}", ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + // Tests for operations that get flagged. + + [Fact] + public async Task AnonymousObjectCreationOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|New With {.Test = "42"}|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ArrayCreationOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|New Integer(9) {}|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task AwaitOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Threading.Tasks + Imports Microsoft.Extensions.Logging + + Class C + Async Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String), task As Task(Of String)) + logger.Log(LogLevel.Debug, eventId, [|Await task|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task BinaryOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|4 + 2|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task CoalesceOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|If(exception, New Exception())|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task InterpolatedStringOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), input As Integer) + logger.Log(LogLevel.Debug, eventId, [|$"{input}"|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task InvocationOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|exception.ToString()|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task IsTypeOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String), input As Object) + logger.Log(LogLevel.Debug, eventId, [|TypeOf input Is Exception|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task NameOfOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|NameOf(logger)|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ObjectCreationOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Exception, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|New Exception()|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task TypeOfOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Type, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|GetType(Integer)|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task UnaryOperation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String), input As Boolean) + logger.Log(LogLevel.Debug, eventId, [|Not input|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + // Tests for work done in indexers, array element references or instances of member references. + + [Fact] + public async Task WorkInIndexerInstance_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Collections.Generic + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall()(LogLevel.Debug)|], exception, formatter) + End Sub + + Function ExpensiveMethodCall() As Dictionary(Of LogLevel, String) + Return Nothing + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInIndexerArgument_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Collections.Generic + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As Dictionary(Of LogLevel, String) + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|_messages(ExpensiveMethodCall())|], exception, formatter) + End Sub + + Function ExpensiveMethodCall() As LogLevel + Return LogLevel.Debug + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInConditionalAccess_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|exception?.ToString()|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInFieldInstance_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _field As Integer + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Integer, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall()._field|], exception, formatter) + End Sub + + Function ExpensiveMethodCall() As C + Return New C() + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInPropertyInstance_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Integer, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall().Length|], exception, formatter) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInArrayReference_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _field As Integer + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Integer, Exception, String), input As Integer()) + logger.Log(LogLevel.Debug, eventId, [|input(ExpensiveMethodCall())|], exception, formatter) + End Sub + + Function ExpensiveMethodCall() As Integer + Return 0 + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + // Tests when log call is guarded. + + [Fact] + public async Task GuardedWorkInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If logger.IsEnabled(LogLevel.Trace) Then logger.Log(LogLevel.Trace, eventId, ExpensiveMethodCall(), exception, formatter) + If logger.IsEnabled(LogLevel.Debug) Then logger.Log(LogLevel.Debug, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.Information) Then logger.Log(LogLevel.Information, eventId, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.Warning) Then logger.Log(LogLevel.Warning, exception, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.[Error]) Then logger.Log(LogLevel.[Error], eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task GuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), level As LogLevel) + If logger.IsEnabled(level) Then logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter) + If logger.IsEnabled(level) Then logger.Log(level, ExpensiveMethodCall()) + If logger.IsEnabled(level) Then logger.Log(level, eventId, ExpensiveMethodCall()) + If logger.IsEnabled(level) Then logger.Log(level, exception, ExpensiveMethodCall()) + If logger.IsEnabled(level) Then logger.Log(level, eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.Log{{logLevel}}(ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.Log{{logLevel}}(exception, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkInLoggerMessage_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.StaticLogLevel(ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task NestedGuardedWorkInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If logger.IsEnabled(LogLevel.Debug) + If exception IsNot Nothing + logger.Log(LogLevel.Debug, eventId, ExpensiveMethodCall(), exception, formatter) + logger.Log(LogLevel.Debug, ExpensiveMethodCall()) + logger.Log(LogLevel.Debug, eventId, ExpensiveMethodCall()) + logger.Log(LogLevel.Debug, exception, ExpensiveMethodCall()) + logger.Log(LogLevel.Debug, eventId, exception, ExpensiveMethodCall()) + End If + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task NestedGuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), level As LogLevel) + If logger.IsEnabled(level) Then + If exception IsNot Nothing Then + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter) + logger.Log(level, ExpensiveMethodCall()) + logger.Log(level, eventId, ExpensiveMethodCall()) + logger.Log(level, exception, ExpensiveMethodCall()) + logger.Log(level, eventId, exception, ExpensiveMethodCall()) + End If + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If logger.IsEnabled(LogLevel.{{logLevel}}) + If exception IsNot Nothing + logger.Log{{logLevel}}(ExpensiveMethodCall()) + End If + End If + + If logger.IsEnabled(LogLevel.{{logLevel}}) + If exception IsNot Nothing + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()) + End If + End If + + If logger.IsEnabled(LogLevel.{{logLevel}}) + If exception IsNot Nothing + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()) + End If + End If + + If logger.IsEnabled(LogLevel.{{logLevel}}) + If exception IsNot Nothing + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()) + End If + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkInLoggerMessage_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Public Property IsExpensiveComputationEnabled As Boolean + + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + If logger.IsEnabled(LogLevel.{{logLevel}}) + If IsExpensiveComputationEnabled + logger.StaticLogLevel(ExpensiveMethodCall()) + End If + End If + + If logger.IsEnabled(LogLevel.{{logLevel}}) + If IsExpensiveComputationEnabled + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()) + End If + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task CustomLoggerGuardedWorkInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class CustomLogger + Implements ILogger + + Public Sub Log(Of TState)(logLevel As LogLevel, eventId As EventId, state As TState, exception As Exception, formatter As Func(Of TState, Exception, String)) Implements ILogger.Log + Throw New NotImplementedException() + End Sub + + Public Function IsEnabled(logLevel As LogLevel) As Boolean Implements ILogger.IsEnabled + Throw New NotImplementedException() + End Function + + Public Function BeginScope(Of TState)(state As TState) As IDisposable Implements ILogger.BeginScope + Throw New NotImplementedException() + End Function + End Class + + Class C + Sub M(logger As CustomLogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If logger.IsEnabled(LogLevel.Trace) Then logger.Log(LogLevel.Trace, eventId, ExpensiveMethodCall(), exception, formatter) + If logger.IsEnabled(LogLevel.Debug) Then logger.Log(LogLevel.Debug, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.Information) Then logger.Log(LogLevel.Information, eventId, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.Warning) Then logger.Log(LogLevel.Warning, exception, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.[Error]) Then logger.Log(LogLevel.[Error], eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task CustomLoggerGuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class CustomLogger + Implements ILogger + + Public Sub Log(Of TState)(logLevel As LogLevel, eventId As EventId, state As TState, exception As Exception, formatter As Func(Of TState, Exception, String)) Implements ILogger.Log + Throw New NotImplementedException() + End Sub + + Public Function IsEnabled(logLevel As LogLevel) As Boolean Implements ILogger.IsEnabled + Throw New NotImplementedException() + End Function + + Public Function BeginScope(Of TState)(state As TState) As IDisposable Implements ILogger.BeginScope + Throw New NotImplementedException() + End Function + End Class + + Class C + Sub M(logger As CustomLogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), level As LogLevel) + If logger.IsEnabled(level) Then logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter) + If logger.IsEnabled(level) Then logger.Log(level, ExpensiveMethodCall()) + If logger.IsEnabled(level) Then logger.Log(level, eventId, ExpensiveMethodCall()) + If logger.IsEnabled(level) Then logger.Log(level, exception, ExpensiveMethodCall()) + If logger.IsEnabled(level) Then logger.Log(level, eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task CustomLoggerGuardedWorkInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class CustomLogger + Implements ILogger + + Public Sub Log(Of TState)(logLevel As LogLevel, eventId As EventId, state As TState, exception As Exception, formatter As Func(Of TState, Exception, String)) Implements ILogger.Log + Throw New NotImplementedException() + End Sub + + Public Function IsEnabled(logLevel As LogLevel) As Boolean Implements ILogger.IsEnabled + Throw New NotImplementedException() + End Function + + Public Function BeginScope(Of TState)(state As TState) As IDisposable Implements ILogger.BeginScope + Throw New NotImplementedException() + End Function + End Class + + Class C + Sub M(logger As CustomLogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.Log(LogLevel.{{logLevel}}, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.Log(LogLevel.{{logLevel}}, exception, ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.Log(LogLevel.{{logLevel}}, eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task CustomLoggerGuardedWorkInLoggerMessage_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Class CustomLogger + Implements ILogger + + Public Sub Log(Of TState)(logLevel As LogLevel, eventId As EventId, state As TState, exception As Exception, formatter As Func(Of TState, Exception, String)) Implements ILogger.Log + Throw New NotImplementedException() + End Sub + + Public Function IsEnabled(logLevel As LogLevel) As Boolean Implements ILogger.IsEnabled + Throw New NotImplementedException() + End Function + + Public Function BeginScope(Of TState)(state As TState) As IDisposable Implements ILogger.BeginScope + Throw New NotImplementedException() + End Function + End Class + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As CustomLogger) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.StaticLogLevel(ExpensiveMethodCall()) + If logger.IsEnabled(LogLevel.{{logLevel}}) Then logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WrongLogLevelGuardedWorkInLog_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If logger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter) + If logger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.[Error], eventId, exception, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongLogLevelGuardedWorkInLogNamed_ReportsDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If logger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, eventId, [|ExpensiveMethodCall()|], exception, formatter) + If logger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, eventId, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, exception, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, eventId, exception, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongLogLevelGuardedWorkInLoggerMessage_ReportsDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + If logger.IsEnabled(LogLevel.None) Then logger.StaticLogLevel([|ExpensiveMethodCall()|]) + If logger.IsEnabled(LogLevel.None) Then logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WrongDynamicLogLevelGuardedWorkInLog_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), level As LogLevel) + If logger.IsEnabled(level) Then logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter) + If logger.IsEnabled(level) Then logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(level) Then logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(level) Then logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(level) Then logger.Log(LogLevel.[Error], eventId, exception, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongDynamicLogLevelGuardedWorkInLogNamed_ReportsDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), level As LogLevel) + If logger.IsEnabled(level) Then logger.Log(LogLevel.{{logLevel}}, eventId, [|ExpensiveMethodCall()|], exception, formatter) + If logger.IsEnabled(level) Then logger.Log(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(level) Then logger.Log(LogLevel.{{logLevel}}, eventId, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(level) Then logger.Log(LogLevel.{{logLevel}}, exception, [|ExpensiveMethodCall()|]) + If logger.IsEnabled(level) Then logger.Log(LogLevel.{{logLevel}}, eventId, exception, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongDynamicLogLevelGuardedWorkInLoggerMessage_ReportsDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger, level As LogLevel) + If logger.IsEnabled(level) Then logger.StaticLogLevel([|ExpensiveMethodCall()|]) + If logger.IsEnabled(level) Then logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WrongInstanceGuardedWorkInLog_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _otherLogger As ILogger + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If _otherLogger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter) + If _otherLogger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]) + If _otherLogger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]) + If _otherLogger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]) + If _otherLogger.IsEnabled(LogLevel.Critical) Then logger.Log(LogLevel.[Error], eventId, exception, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongInstanceGuardedWorkInLogNamed_ReportsDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _otherLogger As ILogger + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If _otherLogger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, eventId, [|ExpensiveMethodCall()|], exception, formatter) + If _otherLogger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]) + If _otherLogger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, eventId, [|ExpensiveMethodCall()|]) + If _otherLogger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, exception, [|ExpensiveMethodCall()|]) + If _otherLogger.IsEnabled(LogLevel.None) Then logger.Log(LogLevel.{{logLevel}}, eventId, exception, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongInstanceGuardedWorkInLoggerMessage_ReportsDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Private _otherLogger As ILogger + + + + Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + End Sub + + Sub M(logger As ILogger) + If _otherLogger.IsEnabled(LogLevel.None) Then logger.StaticLogLevel([|ExpensiveMethodCall()|]) + If _otherLogger.IsEnabled(LogLevel.None) Then logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + // Helpers + + private static async Task VerifyCSharpCodeFixAsync(string source, string fixedSource, CodeAnalysis.CSharp.LanguageVersion? languageVersion = null) + { + await new VerifyCS.Test + { + TestCode = source, + FixedCode = fixedSource, + ReferenceAssemblies = Net60WithMELogging, + LanguageVersion = languageVersion ?? CodeAnalysis.CSharp.LanguageVersion.CSharp10 + }.RunAsync(); + } + + private static async Task VerifyBasicCodeFixAsync(string source, string fixedSource, CodeAnalysis.VisualBasic.LanguageVersion? languageVersion = null) + { + await new VerifyVB.Test + { + TestCode = source, + FixedCode = fixedSource, + ReferenceAssemblies = Net60WithMELogging, + LanguageVersion = languageVersion ?? CodeAnalysis.VisualBasic.LanguageVersion.VisualBasic16_9 + }.RunAsync(); + } + + private static readonly ReferenceAssemblies Net60WithMELogging = + ReferenceAssemblies.Net.Net60.AddPackages([new PackageIdentity("Microsoft.Extensions.Logging", "6.0.0")]); + } +} diff --git a/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt b/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt index 0b07345960..763072542d 100644 --- a/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt +++ b/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt @@ -12,7 +12,7 @@ Design: CA2210, CA1000-CA1070 Globalization: CA2101, CA1300-CA1311 Mobility: CA1600-CA1601 -Performance: HA, CA1800-CA1872 +Performance: HA, CA1800-CA1873 Security: CA2100-CA2153, CA2300-CA2330, CA3000-CA3147, CA5300-CA5405 Usage: CA1801, CA1806, CA1816, CA2200-CA2209, CA2211-CA2265 Naming: CA1700-CA1727 diff --git a/src/Utilities/Compiler/WellKnownTypeNames.cs b/src/Utilities/Compiler/WellKnownTypeNames.cs index 9fa8662c79..386ec40b81 100644 --- a/src/Utilities/Compiler/WellKnownTypeNames.cs +++ b/src/Utilities/Compiler/WellKnownTypeNames.cs @@ -79,6 +79,7 @@ internal static class WellKnownTypeNames public const string MicrosoftExtensionsLoggingILogger = "Microsoft.Extensions.Logging.ILogger"; public const string MicrosoftExtensionsLoggingLoggerExtensions = "Microsoft.Extensions.Logging.LoggerExtensions"; public const string MicrosoftExtensionsLoggingLoggerMessage = "Microsoft.Extensions.Logging.LoggerMessage"; + public const string MicrosoftExtensionsLoggingLoggerMessageAttribute = "Microsoft.Extensions.Logging.LoggerMessageAttribute"; public const string MicrosoftIdentityModelTokensAudienceValidator = "Microsoft.IdentityModel.Tokens.AudienceValidator"; public const string MicrosoftIdentityModelTokensLifetimeValidator = "Microsoft.IdentityModel.Tokens.LifetimeValidator"; public const string MicrosoftIdentityModelTokensSecurityToken = "Microsoft.IdentityModel.Tokens.SecurityToken"; From c9be991385d09bbcafa41e93e6e7680c4ca585be Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Fri, 17 Jan 2025 18:56:46 +0100 Subject: [PATCH 02/13] Use loop for IsGuardedByIsEnabled instead of recursion --- ...voidPotentiallyExpensiveCallWhenLogging.cs | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index f4278374a6..ac7058cb61 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -78,7 +78,7 @@ void AnalyzeInvocation(OperationAnalysisContext context) // Check if invocation is a logging invocation and capture the log level (either as IOperation or as int, depending if it is dynamic or not). // Use these to check if the logging invocation is guarded by 'ILogger.IsEnabled' and bail out if it is. if (!symbols.TryGetLogLevel(invocation, out var logLevelArgumentOperation, out var logLevel) || - symbols.IsGuardedByIsEnabled(currentOperation: invocation, logInvocation: invocation, logLevel, logLevelArgumentOperation)) + symbols.IsGuardedByIsEnabled(invocation, logLevel, logLevelArgumentOperation)) { return; } @@ -262,29 +262,26 @@ .Value.Value as int? return true; } - public bool IsGuardedByIsEnabled(IOperation currentOperation, IInvocationOperation logInvocation, int logLevel, IArgumentOperation? logLevelArgumentOperation) + public bool IsGuardedByIsEnabled(IInvocationOperation logInvocation, int logLevel, IArgumentOperation? logLevelArgumentOperation) { - var conditional = currentOperation.GetAncestor(OperationKind.Conditional); - - // This is the base case where no ancestor conditional is found. - if (conditional is null) + var currentAncestorConditional = logInvocation.GetAncestor(OperationKind.Conditional); + while (currentAncestorConditional is not null) { - return false; - } + var conditionInvocations = currentAncestorConditional.Condition + .DescendantsAndSelf() + .OfType(); - var conditionInvocations = conditional.Condition - .DescendantsAndSelf() - .OfType(); + // Check each invocation in the condition to see if it is a valid guard invocation, i.e. same instance and same log level. + // This is not perfect (e.g. 'if (logger.IsEnabled(LogLevel.Debug) || true)' is treated as guarded), but should be good enough to prevent false positives. + if (conditionInvocations.Any(IsValidIsEnabledGuardInvocation)) + { + return true; + } - // Check each invocation in the condition to see if it is a valid guard invocation, i.e. same instance and same log level. - // This is not perfect (e.g. 'if (logger.IsEnabled(LogLevel.Debug) || true)' is treated as guarded), but should be good enough to prevent false positives. - if (conditionInvocations.Any(IsValidIsEnabledGuardInvocation)) - { - return true; + currentAncestorConditional = currentAncestorConditional.GetAncestor(OperationKind.Conditional); } - // Recursively check the next conditional. - return IsGuardedByIsEnabled(conditional, logInvocation, logLevel, logLevelArgumentOperation); + return false; bool IsValidIsEnabledGuardInvocation(IInvocationOperation invocation) { From ee87252dd0b539928a81c775acaa221df41af417 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sat, 18 Jan 2025 01:22:46 +0100 Subject: [PATCH 03/13] Rename TryGetLogLevel to IsLogInvocation --- .../AvoidPotentiallyExpensiveCallWhenLogging.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index ac7058cb61..bc4ba0c202 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -75,9 +75,9 @@ void AnalyzeInvocation(OperationAnalysisContext context) { var invocation = (IInvocationOperation)context.Operation; - // Check if invocation is a logging invocation and capture the log level (either as IOperation or as int, depending if it is dynamic or not). - // Use these to check if the logging invocation is guarded by 'ILogger.IsEnabled' and bail out if it is. - if (!symbols.TryGetLogLevel(invocation, out var logLevelArgumentOperation, out var logLevel) || + // Check if we have a log invocation and capture the log level used, either as IOperation or as int. + // Then, check if the invocation is guarded by 'ILogger.IsEnabled' and bail out if it is. + if (!symbols.IsLogInvocation(invocation, out var logLevel, out var logLevelArgumentOperation) || symbols.IsGuardedByIsEnabled(invocation, logLevel, logLevelArgumentOperation)) { return; @@ -207,10 +207,10 @@ void AddRangeIfNotNull(ImmutableDictionary.Builder builder, } } - public bool TryGetLogLevel(IInvocationOperation invocation, out IArgumentOperation? logLevelArgumentOperation, out int logLevel) + public bool IsLogInvocation(IInvocationOperation invocation, out int logLevel, out IArgumentOperation? logLevelArgumentOperation) { - logLevelArgumentOperation = default; logLevel = LogLevelPassedAsParameter; + logLevelArgumentOperation = default; var method = invocation.TargetMethod.ReducedFrom ?? invocation.TargetMethod; From 696f73e839a5c489fe0dd77a2ce195a739e492e3 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sat, 18 Jan 2025 01:41:49 +0100 Subject: [PATCH 04/13] Rework how expensive argument evaluation is detected Previously, some specific operations were treated as non-expensive and the rest were. This is not really future-proof as the language evolves, and is generally more prone to false positives. So this has now been changed to the opposite: Treat certain operations as expensive, and the rest are not expensive by default and do not trigger a diagnostic. --- ...voidPotentiallyExpensiveCallWhenLogging.cs | 109 +++-- ...otentiallyExpensiveCallWhenLoggingTests.cs | 437 ++++++++++++++++-- 2 files changed, 475 insertions(+), 71 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index bc4ba0c202..d2222f929e 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -7,6 +7,7 @@ using System.Linq; using Analyzer.Utilities; using Analyzer.Utilities.Extensions; +using Analyzer.Utilities.Lightup; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; @@ -85,21 +86,12 @@ void AnalyzeInvocation(OperationAnalysisContext context) var arguments = invocation.Arguments.Skip(invocation.IsExtensionMethodAndHasNoInstance() ? 1 : 0); - // At this stage we have a logging invocation that is not guarded. - // Check each argument if it is potentially expensive. + // Check each argument if it is potentially expensive to evaluate and raise a diagnostic if it is. foreach (var argument in arguments) { - // Check the argument value after conversions to prevent noise (e.g. implicit conversions from null or from int to EventId). - if (IsPotentiallyExpensive(argument.Value.WalkDownConversion())) + if (IsPotentiallyExpensive(argument.Value)) { - // Filter out implicit operations in the case of params arguments. - // If we would report the diagnostic on the implicit argument operation, it would flag the whole invocation. - var explicitDescendants = argument.Value.GetTopmostExplicitDescendants(); - - if (!explicitDescendants.IsEmpty) - { - context.ReportDiagnostic(explicitDescendants[0].CreateDiagnostic(Rule)); - } + context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); } } } @@ -107,41 +99,96 @@ void AnalyzeInvocation(OperationAnalysisContext context) private static bool IsPotentiallyExpensive(IOperation? operation) { - if (operation is null - // Implicit params array creation is treated as not expensive. This would otherwise cause a lot of noise. - or IArrayCreationOperation { IsImplicit: true, Initializer.ElementValues.IsEmpty: true } - or IInstanceReferenceOperation - or IConditionalAccessInstanceOperation - or ILiteralOperation - or ILocalReferenceOperation - or IParameterReferenceOperation) + if (operation is null) { return false; } - if (operation is IPropertyReferenceOperation { Arguments.IsEmpty: false } indexerReference) + if (ICollectionExpressionOperationWrapper.IsInstance(operation) || + operation is IAnonymousObjectCreationOperation or + IAwaitOperation or + IInterpolatedStringOperation { ConstantValue.HasValue: false } or + IInvocationOperation or + IObjectCreationOperation { Type.IsReferenceType: true } or + IWithOperation) { - return IsPotentiallyExpensive(indexerReference.Instance) || - indexerReference.Arguments.Any(a => IsPotentiallyExpensive(a.Value)); + return true; + } + + if (operation is IArrayCreationOperation arrayCreationOperation) + { + return !IsEmptyImplicitParamsArrayCreation(arrayCreationOperation); + } + + if (operation is IConversionOperation conversionOperation) + { + return IsBoxing(conversionOperation) || IsPotentiallyExpensive(conversionOperation.Operand); + } + + if (operation is IArrayElementReferenceOperation arrayElementReferenceOperation) + { + return IsPotentiallyExpensive(arrayElementReferenceOperation.ArrayReference) || + arrayElementReferenceOperation.Indices.Any(IsPotentiallyExpensive); + } + + if (operation is IBinaryOperation binaryOperation) + { + return IsPotentiallyExpensive(binaryOperation.LeftOperand) || + IsPotentiallyExpensive(binaryOperation.RightOperand); + } + + if (operation is ICoalesceOperation coalesceOperation) + { + return IsPotentiallyExpensive(coalesceOperation.Value) || + IsPotentiallyExpensive(coalesceOperation.WhenNull); } - if (operation is IArrayElementReferenceOperation arrayElementReference) + if (operation is IConditionalAccessOperation conditionalAccessOperation) { - return IsPotentiallyExpensive(arrayElementReference.ArrayReference) || - arrayElementReference.Indices.Any(IsPotentiallyExpensive); + return IsPotentiallyExpensive(conditionalAccessOperation.WhenNotNull); } - if (operation is IConditionalAccessOperation conditionalAccess) + if (operation is IIncrementOrDecrementOperation incrementOrDecrementOperation) { - return IsPotentiallyExpensive(conditionalAccess.WhenNotNull); + return IsPotentiallyExpensive(incrementOrDecrementOperation.Target); } - if (operation is IMemberReferenceOperation memberReference) + if (operation is IMemberReferenceOperation memberReferenceOperation) { - return IsPotentiallyExpensive(memberReference.Instance); + if (IsPotentiallyExpensive(memberReferenceOperation.Instance)) + { + return true; + } + + if (memberReferenceOperation is IPropertyReferenceOperation { Arguments.IsEmpty: false } indexerReferenceOperation) + { + return indexerReferenceOperation.Arguments.Any(a => IsPotentiallyExpensive(a.Value)); + } + } + + if (operation is IUnaryOperation unaryOperation) + { + return IsPotentiallyExpensive(unaryOperation.Operand); + } + + return false; + + static bool IsBoxing(IConversionOperation conversionOperation) + { + var targetIsReferenceType = conversionOperation.Type?.IsReferenceType ?? false; + var operandIsValueType = conversionOperation.Operand.Type?.IsValueType ?? false; + + return targetIsReferenceType && operandIsValueType; } - return true; + static bool IsEmptyImplicitParamsArrayCreation(IArrayCreationOperation arrayCreationOperation) + { + return arrayCreationOperation.IsImplicit && + arrayCreationOperation.DimensionSizes.Length == 1 && + arrayCreationOperation.DimensionSizes[0].ConstantValue.HasValue && + arrayCreationOperation.DimensionSizes[0].ConstantValue.Value is int size && + size == 0; + } } internal sealed class RequiredSymbols diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index 00ea3c7f19..4aaca88a34 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -660,7 +660,7 @@ async void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|4 + 2|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, 4 + 2, exception, formatter); } } """; @@ -679,7 +679,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter, string? message) { - logger.Log(LogLevel.Debug, eventId, [|exception ?? new Exception()|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, message ?? "null", exception, formatter); } } """; @@ -719,7 +719,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|default|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, default, exception, formatter); } } """; @@ -738,7 +738,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) { - logger.Log(LogLevel.Debug, eventId, [|input++|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, input++, exception, formatter); } } """; @@ -795,7 +795,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|exception is not null|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, exception is not null, exception, formatter); } } """; @@ -816,7 +816,7 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func formatter, object input) { - logger.Log(LogLevel.Debug, eventId, [|input is Exception|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, input is Exception, exception, formatter); } } """; @@ -835,7 +835,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|nameof(logger)|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, nameof(logger), exception, formatter); } } """; @@ -854,7 +854,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, new TimeSpan(), exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task SizeOfOperation_NoDiagnostic_CS() { string source = """ using System; @@ -883,7 +902,7 @@ class C { void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|sizeof(int)|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, sizeof(int), exception, formatter); } } """; @@ -892,7 +911,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|typeof(int)|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, typeof(int), exception, formatter); } } """; @@ -911,7 +930,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, bool input) { - logger.Log(LogLevel.Debug, eventId, [|!input|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, !input, exception, formatter); } } """; @@ -1084,8 +1103,6 @@ public async Task WorkInArrayReference_ReportsDiagnostic_CS() class C { - private int _field; - void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int[] input) { logger.Log(LogLevel.Debug, eventId, [|input[ExpensiveMethodCall()]|], exception, formatter); @@ -1101,6 +1118,101 @@ int ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } + [Fact] + public async Task WorkInUnaryOperand_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|!ExpensiveMethodCall()|], exception, formatter); + } + + bool ExpensiveMethodCall() + { + return true; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInBinaryOperand_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall() + ExpensiveMethodCall()|], exception, formatter); + } + + int ExpensiveMethodCall() + { + return 0; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInCoalesceOperationValue_ReportsDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall() ?? 0|], exception, formatter); + } + + int? ExpensiveMethodCall() + { + return 0; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInCoalesceOperationWhenNull_ReportsDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|exception ?? new Exception()|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + // Tests when log call is guarded. [Fact] @@ -1915,6 +2027,84 @@ static string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } + // Boxing tests + + [Fact] + public async Task ArgumentIsBoxed_ReportsDiagnostic_CS() + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|42|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ArgumentIsUnboxed_NoDiagnostic_CS() + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, object value) + { + logger.Log(LogLevel.Debug, eventId, (int)value, exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task BinaryOperationWithBoxing_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|"Hello " + 42|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ImplicitBoxingParamsArrayCreation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger) + { + [|logger.LogError("Test: {Number1} and {Number2}", 1, 2)|]; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + // VB tests [Fact] @@ -2526,15 +2716,15 @@ End Class } [Fact] - public async Task BinaryOperation_ReportsDiagnostic_VB() + public async Task BinaryOperation_NoDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) - logger.Log(LogLevel.Debug, eventId, [|4 + 2|], exception, formatter) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Integer, Exception, String)) + logger.Log(LogLevel.Debug, eventId, 4 + 2, exception, formatter) End Sub End Class """; @@ -2543,15 +2733,15 @@ End Class } [Fact] - public async Task CoalesceOperation_ReportsDiagnostic_VB() + public async Task CoalesceOperation_NoDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) - logger.Log(LogLevel.Debug, eventId, [|If(exception, New Exception())|], exception, formatter) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String), message As String) + logger.Log(LogLevel.Debug, eventId, If(message, "null"), exception, formatter) End Sub End Class """; @@ -2594,7 +2784,7 @@ End Class } [Fact] - public async Task IsTypeOperation_ReportsDiagnostic_VB() + public async Task IsTypeOperation_NoDiagnostic_VB() { string source = """ Imports System @@ -2602,7 +2792,7 @@ Imports Microsoft.Extensions.Logging Class C Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String), input As Object) - logger.Log(LogLevel.Debug, eventId, [|TypeOf input Is Exception|], exception, formatter) + logger.Log(LogLevel.Debug, eventId, TypeOf input Is Exception, exception, formatter) End Sub End Class """; @@ -2611,7 +2801,7 @@ End Class } [Fact] - public async Task NameOfOperation_ReportsDiagnostic_VB() + public async Task NameOfOperation_NoDiagnostic_VB() { string source = """ Imports System @@ -2619,7 +2809,7 @@ Imports Microsoft.Extensions.Logging Class C Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log(LogLevel.Debug, eventId, [|NameOf(logger)|], exception, formatter) + logger.Log(LogLevel.Debug, eventId, NameOf(logger), exception, formatter) End Sub End Class """; @@ -2628,7 +2818,7 @@ End Class } [Fact] - public async Task ObjectCreationOperation_ReportsDiagnostic_VB() + public async Task ObjectCreationOperationReferenceType_ReportsDiagnostic_VB() { string source = """ Imports System @@ -2645,7 +2835,24 @@ End Class } [Fact] - public async Task TypeOfOperation_ReportsDiagnostic_VB() + public async Task ObjectCreationOperationValueType_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of TimeSpan, Exception, String)) + logger.Log(LogLevel.Debug, eventId, New TimeSpan(), exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task TypeOfOperation_NoDiagnostic_VB() { string source = """ Imports System @@ -2653,7 +2860,7 @@ Imports Microsoft.Extensions.Logging Class C Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Type, Exception, String)) - logger.Log(LogLevel.Debug, eventId, [|GetType(Integer)|], exception, formatter) + logger.Log(LogLevel.Debug, eventId, GetType(Integer), exception, formatter) End Sub End Class """; @@ -2662,7 +2869,7 @@ End Class } [Fact] - public async Task UnaryOperation_ReportsDiagnostic_VB() + public async Task UnaryOperation_NoDiagnostic_VB() { string source = """ Imports System @@ -2670,7 +2877,7 @@ Imports Microsoft.Extensions.Logging Class C Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String), input As Boolean) - logger.Log(LogLevel.Debug, eventId, [|Not input|], exception, formatter) + logger.Log(LogLevel.Debug, eventId, Not input, exception, formatter) End Sub End Class """; @@ -2810,6 +3017,86 @@ End Class await VerifyBasicCodeFixAsync(source, source); } + [Fact] + public async Task WorkInUnaryOperand_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|Not ExpensiveMethodCall()|], exception, formatter) + End Sub + + Function ExpensiveMethodCall() As Boolean + Return 0 + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInBinaryOperand_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Integer, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall() + ExpensiveMethodCall()|], exception, formatter) + End Sub + + Function ExpensiveMethodCall() As Integer + Return 0 + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInCoalesceOperationValue_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String), message As String) + logger.Log(LogLevel.Debug, eventId, [|If(ExpensiveMethodCall(), exception)|], exception, formatter) + End Sub + + Function ExpensiveMethodCall() As Exception + Return Nothing + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInCoalesceOperationWhenNull_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String), message As String) + logger.Log(LogLevel.Debug, eventId, [|If(exception, new Exception())|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + // Tests when log call is guarded. [Fact] @@ -3502,6 +3789,76 @@ End Module await VerifyBasicCodeFixAsync(source, source); } + // Boxing tests + + [Fact] + public async Task ArgumentIsBoxed_ReportsDiagnostic_VB() + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|42|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ArgumentIsUnboxed_NoDiagnostic_VB() + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Integer, Exception, String), value As Object) + logger.Log(LogLevel.Debug, eventId, CType(value, Integer), exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task BinaryOperationWithBoxing_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Debug, eventId, [|"Hello " + 42|], exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ImplicitBoxingParamsArrayCreation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger) + [|logger.LogError("Test: {Number1} and {Number2}", 1, 2)|] + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + // Helpers private static async Task VerifyCSharpCodeFixAsync(string source, string fixedSource, CodeAnalysis.CSharp.LanguageVersion? languageVersion = null) From 315cdc303f9fc8225f13a17553de408ed5fc6874 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sat, 18 Jan 2025 01:42:47 +0100 Subject: [PATCH 05/13] Fix same instance detection when using conditional access --- ...voidPotentiallyExpensiveCallWhenLogging.cs | 20 +++++++-- ...otentiallyExpensiveCallWhenLoggingTests.cs | 45 +++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index d2222f929e..88304694c3 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -343,16 +343,30 @@ bool IsValidIsEnabledGuardInvocation(IInvocationOperation invocation) static bool AreInvocationsOnSameInstance(IInvocationOperation invocation1, IInvocationOperation invocation2) { - return (invocation1.GetInstance()?.WalkDownConversion(), invocation2.GetInstance()?.WalkDownConversion()) switch + var instance1 = GetInstanceResolvingConditionalAccess(invocation1); + var instance2 = GetInstanceResolvingConditionalAccess(invocation2); + + return (instance1, instance2) switch { - (IFieldReferenceOperation fieldRef1, IFieldReferenceOperation fieldRef2) => fieldRef1.Member == fieldRef2.Member, - (IPropertyReferenceOperation propRef1, IPropertyReferenceOperation propRef2) => propRef1.Member == propRef2.Member, + (IMemberReferenceOperation memberRef1, IMemberReferenceOperation memberRef2) => memberRef1.Member == memberRef2.Member, (IParameterReferenceOperation paramRef1, IParameterReferenceOperation paramRef2) => paramRef1.Parameter == paramRef2.Parameter, (ILocalReferenceOperation localRef1, ILocalReferenceOperation localRef2) => localRef1.Local == localRef2.Local, _ => false, }; } + static IOperation? GetInstanceResolvingConditionalAccess(IInvocationOperation invocation) + { + var instance = invocation.GetInstance()?.WalkDownConversion(); + + if (instance is IConditionalAccessInstanceOperation conditionalAccessInstance) + { + return conditionalAccessInstance.GetConditionalAccess()?.Operation; + } + + return instance; + } + bool IsSameLogLevel(IArgumentOperation isEnabledArgument) { if (isEnabledArgument.Value.ConstantValue.HasValue) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index 4aaca88a34..b86654082f 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -1252,6 +1252,28 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } + [Fact] + public async Task GuardedWorkInLogConditionalAccess_NoDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger? logger, EventId eventId, Exception? exception, Func formatter) + { + if (logger?.IsEnabled(LogLevel.Debug) ?? false) + logger?.Log(LogLevel.Debug, eventId, new Exception(), exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + [Fact] public async Task GuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() { @@ -3124,6 +3146,29 @@ End Class await VerifyBasicCodeFixAsync(source, source); } + [Fact] + public async Task GuardedWorkInLogConditionalAccess_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If (logger?.IsEnabled(LogLevel.Debug)) Then + logger?.Log(LogLevel.Debug, eventId, ExpensiveMethodCall(), exception, formatter) + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + [Fact] public async Task GuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_VB() { From 1e387858933452cc8f18aa55126a3fb6b1783c34 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sat, 18 Jan 2025 01:45:21 +0100 Subject: [PATCH 06/13] Run msbuild pack --- src/NetAnalyzers/RulesMissingDocumentation.md | 1 - 1 file changed, 1 deletion(-) diff --git a/src/NetAnalyzers/RulesMissingDocumentation.md b/src/NetAnalyzers/RulesMissingDocumentation.md index 5f16bcba2c..aca4ac13b4 100644 --- a/src/NetAnalyzers/RulesMissingDocumentation.md +++ b/src/NetAnalyzers/RulesMissingDocumentation.md @@ -4,4 +4,3 @@ Rule ID | Missing Help Link | Title | --------|-------------------|-------| CA1873 | | Avoid potentially expensive logging | CA2023 | | Invalid braces in message template | -CA2024 | | Do not use 'StreamReader.EndOfStream' in async methods | From 9d5191d7c428cc9bcfa8ac20ac4fa13d571d0ee3 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sat, 18 Jan 2025 02:16:33 +0100 Subject: [PATCH 07/13] Reorder tests --- ...otentiallyExpensiveCallWhenLoggingTests.cs | 212 +++++++++--------- 1 file changed, 109 insertions(+), 103 deletions(-) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index b86654082f..e7b2b62d90 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -327,6 +327,7 @@ static partial class C static void M(ILogger logger) { logger.StaticLogLevel(Property); + logger.DynamicLogLevel(LogLevel.Debug, Property); } } """; @@ -599,10 +600,10 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - // Tests for operations that get flagged. + // TODO: Add more variants for new non-flagged tests below [Fact] - public async Task AnonymousObjectCreationOperation_ReportsDiagnostic_CS() + public async Task BinaryOperation_NoDiagnostic_CS() { string source = """ using System; @@ -610,9 +611,9 @@ public async Task AnonymousObjectCreationOperation_ReportsDiagnostic_CS() class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|new { Test = "42" }|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, 4 + 2, exception, formatter); } } """; @@ -621,17 +622,19 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter, string? message) { - logger.Log(LogLevel.Debug, eventId, [|new int[10]|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, message ?? "null", exception, formatter); } } """; @@ -640,18 +643,17 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, Task task) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|await task|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, default, exception, formatter); } } """; @@ -660,7 +662,7 @@ async void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) { - logger.Log(LogLevel.Debug, eventId, 4 + 2, exception, formatter); + logger.Log(LogLevel.Debug, eventId, input++, exception, formatter); } } """; @@ -679,7 +681,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, string? message) + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, message ?? "null", exception, formatter); + logger.Log(LogLevel.Debug, eventId, exception is not null, exception, formatter); } } """; @@ -700,7 +702,7 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, object input) { - logger.Log(LogLevel.Debug, eventId, [|[4, 2]|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, input is Exception, exception, formatter); } } """; - await VerifyCSharpCodeFixAsync(source, source, CodeAnalysis.CSharp.LanguageVersion.CSharp12); + await VerifyCSharpCodeFixAsync(source, source); } [Fact] - public async Task DefaultValueOperation_NoDiagnostic_CS() + public async Task NameOfOperation_NoDiagnostic_CS() { string source = """ using System; @@ -727,9 +729,9 @@ public async Task DefaultValueOperation_NoDiagnostic_CS() class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, default, exception, formatter); + logger.Log(LogLevel.Debug, eventId, nameof(logger), exception, formatter); } } """; @@ -738,7 +740,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, input++, exception, formatter); + logger.Log(LogLevel.Debug, eventId, new TimeSpan(), exception, formatter); } } """; @@ -757,7 +759,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|$"{input}"|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, sizeof(int), exception, formatter); } } """; @@ -776,7 +778,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|exception.ToString()|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, typeof(int), exception, formatter); } } """; @@ -795,19 +797,17 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, bool input) { - logger.Log(LogLevel.Debug, eventId, exception is not null, exception, formatter); + logger.Log(LogLevel.Debug, eventId, !input, exception, formatter); } } """; @@ -815,8 +815,10 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func formatter, object input) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, input is Exception, exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|new { Test = "42" }|], exception, formatter); } } """; @@ -835,7 +837,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, nameof(logger), exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|new int[10]|], exception, formatter); } } """; @@ -854,17 +856,18 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + async void M(ILogger logger, EventId eventId, Exception exception, Func formatter, Task task) { - logger.Log(LogLevel.Debug, eventId, [|new Exception()|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|await task|], exception, formatter); } } """; @@ -873,7 +876,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, new TimeSpan(), exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|[4, 2]|], exception, formatter); } } """; - await VerifyCSharpCodeFixAsync(source, source); + await VerifyCSharpCodeFixAsync(source, source, CodeAnalysis.CSharp.LanguageVersion.CSharp12); } [Fact] - public async Task SizeOfOperation_NoDiagnostic_CS() + public async Task InterpolatedStringOperation_ReportsDiagnostic_CS() { string source = """ using System; @@ -900,9 +903,9 @@ public async Task SizeOfOperation_NoDiagnostic_CS() class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) { - logger.Log(LogLevel.Debug, eventId, sizeof(int), exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|$"{input}"|], exception, formatter); } } """; @@ -911,7 +914,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, typeof(int), exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|exception.ToString()|], exception, formatter); } } """; @@ -930,7 +933,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, bool input) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, !input, exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|new Exception()|], exception, formatter); } } """; @@ -969,7 +972,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func Date: Sat, 18 Jan 2025 02:18:09 +0100 Subject: [PATCH 08/13] Fix constant interpolation string detection --- ...voidPotentiallyExpensiveCallWhenLogging.cs | 6 +++- ...otentiallyExpensiveCallWhenLoggingTests.cs | 36 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index 88304694c3..753aa0f748 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -107,7 +107,6 @@ private static bool IsPotentiallyExpensive(IOperation? operation) if (ICollectionExpressionOperationWrapper.IsInstance(operation) || operation is IAnonymousObjectCreationOperation or IAwaitOperation or - IInterpolatedStringOperation { ConstantValue.HasValue: false } or IInvocationOperation or IObjectCreationOperation { Type.IsReferenceType: true } or IWithOperation) @@ -153,6 +152,11 @@ IInvocationOperation or return IsPotentiallyExpensive(incrementOrDecrementOperation.Target); } + if (operation is IInterpolatedStringOperation interpolatedStringOperation) + { + return interpolatedStringOperation.Parts.All(p => p is IInterpolationOperation); + } + if (operation is IMemberReferenceOperation memberReferenceOperation) { if (IsPotentiallyExpensive(memberReferenceOperation.Instance)) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index e7b2b62d90..eb88a7f969 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -815,6 +815,25 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) + { + logger.Log(LogLevel.Debug, eventId, $"constant", exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + // Tests for operations that get flagged. [Fact] @@ -2808,6 +2827,23 @@ End Class await VerifyBasicCodeFixAsync(source, source); } + [Fact] + public async Task InterpolatedStringOperationConstant_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), input As Integer) + logger.Log(LogLevel.Debug, eventId, $"constant", exception, formatter) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + // Tests for operations that get flagged. [Fact] From 85c71e3d25748c18cd260c6b74d5477152e1e1a2 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sat, 18 Jan 2025 03:51:03 +0100 Subject: [PATCH 09/13] Cleanup and add missing tests --- ...otentiallyExpensiveCallWhenLoggingTests.cs | 2287 +++++++++++------ 1 file changed, 1535 insertions(+), 752 deletions(-) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index eb88a7f969..fcb7bee424 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -81,11 +81,11 @@ public async Task LiteralInLoggerMessage_NoDiagnostic_CS() static partial class C { - [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); static void M(ILogger logger) { @@ -158,11 +158,11 @@ public async Task LocalInLoggerMessage_NoDiagnostic_CS() static partial class C { - [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); static void M(ILogger logger) { @@ -239,11 +239,11 @@ static partial class C { private static string _field; - [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); static void M(ILogger logger) { @@ -318,11 +318,11 @@ static partial class C { public static string Property { get; set; } - [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); static void M(ILogger logger) { @@ -400,11 +400,11 @@ static partial class C { private static Dictionary _messages; - [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); static void M(ILogger logger) { @@ -479,11 +479,11 @@ static partial class C { private static string[] _messages; - [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); static void M(ILogger logger) { @@ -558,11 +558,11 @@ public async Task ConditionalAccessInLoggerMessage_NoDiagnostic_CS() static partial class C { - [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string? message); + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string? argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string? message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string? argument); static void M(ILogger logger, Exception? exception) { @@ -576,7 +576,7 @@ static void M(ILogger logger, Exception? exception) } [Fact] - public async Task OtherILoggerMethodCalled_NoDiagnostic_CS() + public async Task BinaryOperationInLog_NoDiagnostic_CS() { string source = """ using System; @@ -584,15 +584,32 @@ public async Task OtherILoggerMethodCalled_NoDiagnostic_CS() class C { - void M(ILogger logger) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.BeginScope(ExpensiveMethodCall()); - logger.BeginScope("Processing calculation result {CalculationResult}", ExpensiveMethodCall()); + logger.Log(LogLevel.Trace, eventId, 4 + 2, exception, formatter); } + } + """; - string ExpensiveMethodCall() + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task BinaryOperationInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) { - return "very expensive call"; + logger.Log{{logLevel}}("a" + "b"); + logger.Log{{logLevel}}(eventId, "a" + "b"); + logger.Log{{logLevel}}(exception, "a" + "b"); + logger.Log{{logLevel}}(eventId, exception, "a" + "b"); } } """; @@ -600,20 +617,25 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - // TODO: Add more variants for new non-flagged tests below - [Fact] - public async Task BinaryOperation_NoDiagnostic_CS() + public async Task BinaryOperationInLoggerMessage_NoDiagnostic_CS() { string source = """ using System; using Microsoft.Extensions.Logging; - class C + static partial class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) { - logger.Log(LogLevel.Debug, eventId, 4 + 2, exception, formatter); + logger.StaticLogLevel("a" + "b"); + logger.DynamicLogLevel(LogLevel.Debug, "a" + "b"); } } """; @@ -622,7 +644,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, string? message) + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter, string? message) { - logger.Log(LogLevel.Debug, eventId, message ?? "null", exception, formatter); + logger.Log(LogLevel.Trace, eventId, message ?? "null", exception, formatter); + + logger.Log(LogLevel.Debug, message ?? "null"); + logger.Log(LogLevel.Information, eventId, message ?? "null"); + logger.Log(LogLevel.Warning, exception, message ?? "null"); + logger.Log(LogLevel.Error, eventId, exception, message ?? "null"); } } """; @@ -642,18 +669,24 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception? exception, string? message) { - logger.Log(LogLevel.Debug, eventId, default, exception, formatter); + logger.Log{{logLevel}}(message ?? "null"); + logger.Log{{logLevel}}(eventId, message ?? "null"); + logger.Log{{logLevel}}(exception, message ?? "null"); + logger.Log{{logLevel}}(eventId, exception, message ?? "null"); } } """; @@ -662,17 +695,26 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger, string? message) { - logger.Log(LogLevel.Debug, eventId, input++, exception, formatter); + logger.StaticLogLevel(message ?? "null"); + logger.DynamicLogLevel(LogLevel.Debug, message ?? "null"); } } """; @@ -681,19 +723,17 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, exception is not null, exception, formatter); + logger.Log(LogLevel.Debug, eventId, default, exception, formatter); } } """; @@ -701,18 +741,22 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func formatter, object input) + void M(ILogger logger, EventId eventId, Exception exception) { - logger.Log(LogLevel.Debug, eventId, input is Exception, exception, formatter); + logger.Log{{logLevel}}(default); + logger.Log{{logLevel}}(eventId, default); + logger.Log{{logLevel}}(exception, default); + logger.Log{{logLevel}}(eventId, exception, default); } } """; @@ -721,17 +765,24 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) { - logger.Log(LogLevel.Debug, eventId, nameof(logger), exception, formatter); + logger.StaticLogLevel(default); + logger.DynamicLogLevel(LogLevel.Debug, default); } } """; @@ -740,7 +791,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) { - logger.Log(LogLevel.Debug, eventId, new TimeSpan(), exception, formatter); + logger.Log(LogLevel.Debug, eventId, input++, exception, formatter); } } """; @@ -759,17 +810,24 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, int argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, int argument); + + static void M(ILogger logger, int input) { - logger.Log(LogLevel.Debug, eventId, sizeof(int), exception, formatter); + logger.StaticLogLevel(input++); + logger.DynamicLogLevel(LogLevel.Debug, input++); } } """; @@ -778,17 +836,19 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, typeof(int), exception, formatter); + logger.Log(LogLevel.Debug, eventId, exception is not null, exception, formatter); } } """; @@ -797,17 +857,26 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, bool input) + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, bool argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, bool argument); + + static void M(ILogger logger, Exception? exception) { - logger.Log(LogLevel.Debug, eventId, !input, exception, formatter); + logger.StaticLogLevel(exception is not null); + logger.DynamicLogLevel(LogLevel.Debug, exception is not null); } } """; @@ -816,7 +885,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, object input) { - logger.Log(LogLevel.Debug, eventId, $"constant", exception, formatter); + logger.Log(LogLevel.Debug, eventId, input is Exception, exception, formatter); } } """; @@ -834,20 +903,25 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, bool argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, bool argument); + + static void M(ILogger logger, object input) { - logger.Log(LogLevel.Debug, eventId, [|new { Test = "42" }|], exception, formatter); + logger.StaticLogLevel(input is Exception); + logger.DynamicLogLevel(LogLevel.Debug, input is Exception); } } """; @@ -856,7 +930,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|new int[10]|], exception, formatter); + logger.Log(LogLevel.Trace, eventId, nameof(logger), exception, formatter); + + logger.Log(LogLevel.Debug, nameof(logger)); + logger.Log(LogLevel.Information, eventId, nameof(logger)); + logger.Log(LogLevel.Warning, exception, nameof(logger)); + logger.Log(LogLevel.Error, eventId, exception, nameof(logger)); } } """; @@ -874,19 +953,22 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, Task task) + void M(ILogger logger, EventId eventId, Exception exception) { - logger.Log(LogLevel.Debug, eventId, [|await task|], exception, formatter); + logger.Log{{logLevel}}(nameof(logger)); + logger.Log{{logLevel}}(eventId, nameof(logger)); + logger.Log{{logLevel}}(exception, nameof(logger)); + logger.Log{{logLevel}}(eventId, exception, nameof(logger)); } } """; @@ -895,26 +977,33 @@ async void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) { - logger.Log(LogLevel.Debug, eventId, [|[4, 2]|], exception, formatter); + logger.StaticLogLevel(nameof(logger)); + logger.DynamicLogLevel(LogLevel.Debug, nameof(logger)); } } """; - await VerifyCSharpCodeFixAsync(source, source, CodeAnalysis.CSharp.LanguageVersion.CSharp12); + await VerifyCSharpCodeFixAsync(source, source); } [Fact] - public async Task InterpolatedStringOperation_ReportsDiagnostic_CS() + public async Task ObjectCreationOperationValueTypeInLog_NoDiagnostic_CS() { string source = """ using System; @@ -922,9 +1011,9 @@ public async Task InterpolatedStringOperation_ReportsDiagnostic_CS() class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|$"{input}"|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, new TimeSpan(), exception, formatter); } } """; @@ -933,17 +1022,24 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, TimeSpan argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, TimeSpan argument); + + static void M(ILogger logger) { - logger.Log(LogLevel.Debug, eventId, [|exception.ToString()|], exception, formatter); + logger.StaticLogLevel(new TimeSpan()); + logger.DynamicLogLevel(LogLevel.Debug, new TimeSpan()); } } """; @@ -952,7 +1048,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|new Exception()|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, sizeof(int), exception, formatter); } } """; @@ -971,19 +1067,24 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter, Point input) + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, int argument); + + static void M(ILogger logger) { - logger.Log(LogLevel.Debug, eventId, [|input with { Y = 42 }|], exception, formatter); + logger.StaticLogLevel(sizeof(int)); + logger.DynamicLogLevel(LogLevel.Debug, sizeof(int)); } } """; @@ -991,26 +1092,18 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) - { - logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall()[LogLevel.Debug]|], exception, formatter); - } - - Dictionary ExpensiveMethodCall() + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - return default; + logger.Log(LogLevel.Debug, eventId, typeof(int), exception, formatter); } } """; @@ -1019,25 +1112,24 @@ Dictionary ExpensiveMethodCall() } [Fact] - public async Task WorkInIndexerArgument_ReportsDiagnostic_CS() + public async Task TypeOfOperationInLoggerMessage_NoDiagnostic_CS() { string source = """ using System; - using System.Collections.Generic; using Microsoft.Extensions.Logging; - class C + static partial class C { - private Dictionary _messages; + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, Type argument); - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) - { - logger.Log(LogLevel.Debug, eventId, [|_messages[ExpensiveMethodCall()]|], exception, formatter); - } + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, Type argument); - LogLevel ExpensiveMethodCall() + static void M(ILogger logger) { - return LogLevel.Debug; + logger.StaticLogLevel(typeof(int)); + logger.DynamicLogLevel(LogLevel.Debug, typeof(int)); } } """; @@ -1046,19 +1138,17 @@ LogLevel ExpensiveMethodCall() } [Fact] - public async Task WorkInConditionalAccess_ReportsDiagnostic_CS() + public async Task UnaryOperationInLog_NoDiagnostic_CS() { string source = """ - #nullable enable - using System; using Microsoft.Extensions.Logging; class C { - void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, bool input) { - logger.Log(LogLevel.Debug, eventId, [|exception?.ToString()|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, !input, exception, formatter); } } """; @@ -1067,24 +1157,24 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) - { - logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall()._field|], exception, formatter); - } + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, bool argument); - C ExpensiveMethodCall() + static void M(ILogger logger, bool input) { - return new C(); + logger.StaticLogLevel(!input); + logger.DynamicLogLevel(LogLevel.Debug, !input); } } """; @@ -1093,7 +1183,7 @@ C ExpensiveMethodCall() } [Fact] - public async Task WorkInPropertyInstance_ReportsDiagnostic_CS() + public async Task InterpolatedStringOperationConstantInLog_NoDiagnostic_CS() { string source = """ using System; @@ -1101,14 +1191,14 @@ public async Task WorkInPropertyInstance_ReportsDiagnostic_CS() class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall().Length|], exception, formatter); - } + logger.Log(LogLevel.Debug, eventId, $"constant", exception, formatter); - string ExpensiveMethodCall() - { - return "very expensive call"; + logger.Log(LogLevel.Debug, $"constant"); + logger.Log(LogLevel.Information, eventId, $"constant"); + logger.Log(LogLevel.Warning, exception, $"constant"); + logger.Log(LogLevel.Error, eventId, exception, $"constant"); } } """; @@ -1116,23 +1206,22 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - [Fact] - public async Task WorkInArrayReference_ReportsDiagnostic_CS() + [Theory] + [MemberData(nameof(LogLevels))] + public async Task InterpolatedStringOperationConstantInLogNamed_NoDiagnostic_CS(string logLevel) { - string source = """ + string source = $$""" using System; using Microsoft.Extensions.Logging; class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int[] input) - { - logger.Log(LogLevel.Debug, eventId, [|input[ExpensiveMethodCall()]|], exception, formatter); - } - - int ExpensiveMethodCall() + void M(ILogger logger, EventId eventId, Exception exception) { - return 0; + logger.Log{{logLevel}}($"constant"); + logger.Log{{logLevel}}(eventId, $"constant"); + logger.Log{{logLevel}}(exception, $"constant"); + logger.Log{{logLevel}}(eventId, exception, $"constant"); } } """; @@ -1141,22 +1230,24 @@ int ExpensiveMethodCall() } [Fact] - public async Task WorkInUnaryOperand_ReportsDiagnostic_CS() + public async Task InterpolatedStringOperationConstantInLoggerMessage_NoDiagnostic_CS() { string source = """ using System; using Microsoft.Extensions.Logging; - class C + static partial class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) - { - logger.Log(LogLevel.Debug, eventId, [|!ExpensiveMethodCall()|], exception, formatter); - } + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - bool ExpensiveMethodCall() + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) { - return true; + logger.StaticLogLevel($"constant"); + logger.DynamicLogLevel(LogLevel.Debug, $"constant"); } } """; @@ -1165,7 +1256,7 @@ bool ExpensiveMethodCall() } [Fact] - public async Task WorkInBinaryOperand_ReportsDiagnostic_CS() + public async Task OtherILoggerMethodCalled_NoDiagnostic_CS() { string source = """ using System; @@ -1173,14 +1264,15 @@ public async Task WorkInBinaryOperand_ReportsDiagnostic_CS() class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger) { - logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall() + ExpensiveMethodCall()|], exception, formatter); + logger.BeginScope(ExpensiveMethodCall()); + logger.BeginScope("Processing calculation result {CalculationResult}", ExpensiveMethodCall()); } - int ExpensiveMethodCall() + string ExpensiveMethodCall() { - return 0; + return "very expensive call"; } } """; @@ -1188,25 +1280,20 @@ int ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } + // Tests for operations that get flagged. + [Fact] - public async Task WorkInCoalesceOperationValue_ReportsDiagnostic_CS() + public async Task AnonymousObjectCreationOperation_ReportsDiagnostic_CS() { string source = """ - #nullable enable - using System; using Microsoft.Extensions.Logging; - + class C { - void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall() ?? 0|], exception, formatter); - } - - int? ExpensiveMethodCall() - { - return 0; + logger.Log(LogLevel.Debug, eventId, [|new { Test = "42" }|], exception, formatter); } } """; @@ -1215,19 +1302,17 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, [|exception ?? new Exception()|], exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|new int[10]|], exception, formatter); } } """; @@ -1235,38 +1320,19 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) - { - if (logger.IsEnabled(LogLevel.Trace)) - logger.Log(LogLevel.Trace, eventId, ExpensiveMethodCall(), exception, formatter); - - if (logger.IsEnabled(LogLevel.Debug)) - logger.Log(LogLevel.Debug, ExpensiveMethodCall()); - - if (logger.IsEnabled(LogLevel.Information)) - logger.Log(LogLevel.Information, eventId, ExpensiveMethodCall()); - - if (logger.IsEnabled(LogLevel.Warning)) - logger.Log(LogLevel.Warning, exception, ExpensiveMethodCall()); - - if (logger.IsEnabled(LogLevel.Error)) - logger.Log(LogLevel.Error, eventId, exception, ExpensiveMethodCall()); - } - - string ExpensiveMethodCall() + async void M(ILogger logger, EventId eventId, Exception exception, Func formatter, Task task) { - return "very expensive call"; + logger.Log(LogLevel.Debug, eventId, [|await task|], exception, formatter); } } """; @@ -1275,29 +1341,26 @@ string ExpensiveMethodCall() } [Fact] - public async Task GuardedWorkInLogConditionalAccess_NoDiagnostic_CS() + public async Task CollectionExpressionOperation_ReportsDiagnostic_CS() { string source = """ - #nullable enable - using System; using Microsoft.Extensions.Logging; - + class C { - void M(ILogger? logger, EventId eventId, Exception? exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - if (logger?.IsEnabled(LogLevel.Debug) ?? false) - logger?.Log(LogLevel.Debug, eventId, new Exception(), exception, formatter); + logger.Log(LogLevel.Debug, eventId, [|[4, 2]|], exception, formatter); } } """; - await VerifyCSharpCodeFixAsync(source, source); + await VerifyCSharpCodeFixAsync(source, source, CodeAnalysis.CSharp.LanguageVersion.CSharp12); } [Fact] - public async Task GuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() + public async Task InterpolatedStringOperation_ReportsDiagnostic_CS() { string source = """ using System; @@ -1305,27 +1368,9 @@ public async Task GuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) - { - if (logger.IsEnabled(level)) - logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); - - if (logger.IsEnabled(level)) - logger.Log(level, ExpensiveMethodCall()); - - if (logger.IsEnabled(level)) - logger.Log(level, eventId, ExpensiveMethodCall()); - - if (logger.IsEnabled(level)) - logger.Log(level, exception, ExpensiveMethodCall()); - - if (logger.IsEnabled(level)) - logger.Log(level, eventId, exception, ExpensiveMethodCall()); - } - - string ExpensiveMethodCall() + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int input) { - return "very expensive call"; + logger.Log(LogLevel.Debug, eventId, [|$"{input}"|], exception, formatter); } } """; @@ -1333,34 +1378,37 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - [Theory] - [MemberData(nameof(LogLevels))] - public async Task GuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) + [Fact] + public async Task InvocationOperation_ReportsDiagnostic_CS() { - string source = $$""" + string source = """ using System; using Microsoft.Extensions.Logging; class C { - void M(ILogger logger, EventId eventId, Exception exception) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - if (logger.IsEnabled(LogLevel.{{logLevel}})) - logger.Log{{logLevel}}(ExpensiveMethodCall()); - - if (logger.IsEnabled(LogLevel.{{logLevel}})) - logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); + logger.Log(LogLevel.Debug, eventId, [|exception.ToString()|], exception, formatter); + } + } + """; - if (logger.IsEnabled(LogLevel.{{logLevel}})) - logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); + await VerifyCSharpCodeFixAsync(source, source); + } - if (logger.IsEnabled(LogLevel.{{logLevel}})) - logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); - } + [Fact] + public async Task ObjectCreationOperationReferenceType_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; - string ExpensiveMethodCall() + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - return "very expensive call"; + logger.Log(LogLevel.Debug, eventId, [|new Exception()|], exception, formatter); } } """; @@ -1368,38 +1416,20 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - [Theory] - [MemberData(nameof(LogLevels))] - public async Task GuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) + [Fact] + public async Task WithOperation_ReportsDiagnostic_CS() { - string source = $$""" + string source = """ using System; using Microsoft.Extensions.Logging; - static partial class C + class C { - [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); - - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); - - static void M(ILogger logger) - { - if (logger.IsEnabled(LogLevel.{{logLevel}})) - { - logger.StaticLogLevel(ExpensiveMethodCall()); - } - - if (logger.IsEnabled(LogLevel.{{logLevel}})) - { - logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); - } - } + record Point(int X, int Y); - static string ExpensiveMethodCall() + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, Point input) { - return "very expensive call"; + logger.Log(LogLevel.Debug, eventId, [|input with { Y = 42 }|], exception, formatter); } } """; @@ -1407,33 +1437,26 @@ static string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } + // Tests for work done in other operations. + [Fact] - public async Task NestedGuardedWorkInLog_NoDiagnostic_CS() + public async Task WorkInIndexerInstance_ReportsDiagnostic_CS() { string source = """ using System; + using System.Collections.Generic; using Microsoft.Extensions.Logging; class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - if (logger.IsEnabled(LogLevel.Debug)) - { - if (exception is not null) - { - logger.Log(LogLevel.Debug, eventId, ExpensiveMethodCall(), exception, formatter); - logger.Log(LogLevel.Debug, ExpensiveMethodCall()); - logger.Log(LogLevel.Debug, eventId, ExpensiveMethodCall()); - logger.Log(LogLevel.Debug, exception, ExpensiveMethodCall()); - logger.Log(LogLevel.Debug, eventId, exception, ExpensiveMethodCall()); - } - } + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall()[LogLevel.Debug]|], exception, formatter); } - string ExpensiveMethodCall() + Dictionary ExpensiveMethodCall() { - return "very expensive call"; + return default; } } """; @@ -1442,32 +1465,25 @@ string ExpensiveMethodCall() } [Fact] - public async Task NestedGuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() + public async Task WorkInIndexerArgument_ReportsDiagnostic_CS() { string source = """ using System; + using System.Collections.Generic; using Microsoft.Extensions.Logging; class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + private Dictionary _messages; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - if (logger.IsEnabled(level)) - { - if (exception is not null) - { - logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); - logger.Log(level, ExpensiveMethodCall()); - logger.Log(level, eventId, ExpensiveMethodCall()); - logger.Log(level, exception, ExpensiveMethodCall()); - logger.Log(level, eventId, exception, ExpensiveMethodCall()); - } - } + logger.Log(LogLevel.Debug, eventId, [|_messages[ExpensiveMethodCall()]|], exception, formatter); } - string ExpensiveMethodCall() + LogLevel ExpensiveMethodCall() { - return "very expensive call"; + return LogLevel.Debug; } } """; @@ -1475,38 +1491,20 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - [Theory] - [MemberData(nameof(LogLevels))] - public async Task NestedGuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) + [Fact] + public async Task WorkInConditionalAccess_ReportsDiagnostic_CS() { - string source = $$""" + string source = """ + #nullable enable + using System; using Microsoft.Extensions.Logging; class C { - void M(ILogger logger, EventId eventId, Exception exception) - { - if (logger.IsEnabled(LogLevel.{{logLevel}})) - if (exception is not null) - logger.Log{{logLevel}}(ExpensiveMethodCall()); - - if (logger.IsEnabled(LogLevel.{{logLevel}})) - if (exception is not null) - logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); - - if (logger.IsEnabled(LogLevel.{{logLevel}})) - if (exception is not null) - logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); - - if (logger.IsEnabled(LogLevel.{{logLevel}})) - if (exception is not null) - logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); - } - - string ExpensiveMethodCall() + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) { - return "very expensive call"; + logger.Log(LogLevel.Debug, eventId, [|exception?.ToString()|], exception, formatter); } } """; @@ -1514,46 +1512,25 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - [Theory] - [MemberData(nameof(LogLevels))] - public async Task NestedGuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) + [Fact] + public async Task WorkInFieldInstance_ReportsDiagnostic_CS() { - string source = $$""" + string source = """ using System; using Microsoft.Extensions.Logging; - static partial class C + class C { - static bool IsExpensiveComputationEnabled { get; set; } - - [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); - - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + private int _field; - static void M(ILogger logger) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - if (logger.IsEnabled(LogLevel.{{logLevel}})) - { - if (IsExpensiveComputationEnabled) - { - logger.StaticLogLevel(ExpensiveMethodCall()); - } - } - - if (logger.IsEnabled(LogLevel.{{logLevel}})) - { - if (IsExpensiveComputationEnabled) - { - logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); - } - } + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall()._field|], exception, formatter); } - static string ExpensiveMethodCall() + C ExpensiveMethodCall() { - return "very expensive call"; + return new C(); } } """; @@ -1562,22 +1539,160 @@ static string ExpensiveMethodCall() } [Fact] - public async Task CustomLoggerGuardedWorkInLog_NoDiagnostic_CS() + public async Task WorkInPropertyInstance_ReportsDiagnostic_CS() { string source = """ using System; using Microsoft.Extensions.Logging; - class CustomLogger : ILogger + class C { - public IDisposable BeginScope(TState state) { return default; } - public bool IsEnabled(LogLevel logLevel) { return true; } - public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall().Length|], exception, formatter); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInArrayReference_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; class C { - void M(CustomLogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, int[] input) + { + logger.Log(LogLevel.Debug, eventId, [|input[ExpensiveMethodCall()]|], exception, formatter); + } + + int ExpensiveMethodCall() + { + return 0; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInUnaryOperand_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|!ExpensiveMethodCall()|], exception, formatter); + } + + bool ExpensiveMethodCall() + { + return true; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInBinaryOperand_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall() + ExpensiveMethodCall()|], exception, formatter); + } + + int ExpensiveMethodCall() + { + return 0; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInCoalesceOperationValue_ReportsDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|ExpensiveMethodCall() ?? 0|], exception, formatter); + } + + int? ExpensiveMethodCall() + { + return 0; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WorkInCoalesceOperationWhenNull_ReportsDiagnostic_CS() + { + string source = """ + #nullable enable + + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception? exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|exception ?? new Exception()|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + // Tests when log call is guarded. + + [Fact] + public async Task GuardedWorkInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { if (logger.IsEnabled(LogLevel.Trace)) logger.Log(LogLevel.Trace, eventId, ExpensiveMethodCall(), exception, formatter); @@ -1606,22 +1721,37 @@ string ExpensiveMethodCall() } [Fact] - public async Task CustomLoggerGuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() + public async Task GuardedWorkInLogConditionalAccess_NoDiagnostic_CS() { string source = """ + #nullable enable + using System; using Microsoft.Extensions.Logging; - - class CustomLogger : ILogger + + class C { - public IDisposable BeginScope(TState state) { return default; } - public bool IsEnabled(LogLevel logLevel) { return true; } - public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + void M(ILogger? logger, EventId eventId, Exception? exception, Func formatter) + { + if (logger?.IsEnabled(LogLevel.Debug) ?? false) + logger?.Log(LogLevel.Debug, eventId, new Exception(), exception, formatter); + } } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task GuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; class C { - void M(CustomLogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) { if (logger.IsEnabled(level)) logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); @@ -1651,22 +1781,15 @@ string ExpensiveMethodCall() [Theory] [MemberData(nameof(LogLevels))] - public async Task CustomLoggerGuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) + public async Task GuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) { string source = $$""" using System; using Microsoft.Extensions.Logging; - class CustomLogger : ILogger - { - public IDisposable BeginScope(TState state) { return default; } - public bool IsEnabled(LogLevel logLevel) { return true; } - public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } - } - class C { - void M(CustomLogger logger, EventId eventId, Exception exception) + void M(ILogger logger, EventId eventId, Exception exception) { if (logger.IsEnabled(LogLevel.{{logLevel}})) logger.Log{{logLevel}}(ExpensiveMethodCall()); @@ -1693,28 +1816,21 @@ string ExpensiveMethodCall() [Theory] [MemberData(nameof(LogLevels))] - public async Task CustomLoggerGuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) + public async Task GuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) { string source = $$""" using System; using Microsoft.Extensions.Logging; - class CustomLogger : ILogger - { - public IDisposable BeginScope(TState state) { return default; } - public bool IsEnabled(LogLevel logLevel) { return true; } - public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } - } - static partial class C { - [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); - static void M(CustomLogger logger) + static void M(ILogger logger) { if (logger.IsEnabled(LogLevel.{{logLevel}})) { @@ -1737,10 +1853,11 @@ static string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - [Fact] - public async Task WrongLogLevelGuardedWorkInLog_ReportsDiagnostic_CS() + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkInLog_NoDiagnostic_CS(string logLevel) { - string source = """ + string source = $$""" using System; using Microsoft.Extensions.Logging; @@ -1748,20 +1865,17 @@ class C { void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - if (logger.IsEnabled(LogLevel.Critical)) - logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter); - - if (logger.IsEnabled(LogLevel.Critical)) - logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); - - if (logger.IsEnabled(LogLevel.Critical)) - logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]); - - if (logger.IsEnabled(LogLevel.Critical)) - logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]); - - if (logger.IsEnabled(LogLevel.Critical)) - logger.Log(LogLevel.Error, eventId, exception, [|ExpensiveMethodCall()|]); + if (logger.IsEnabled(LogLevel.{{logLevel}})) + { + if (exception is not null) + { + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall(), exception, formatter); + logger.Log(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, exception, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, eventId, exception, ExpensiveMethodCall()); + } + } } string ExpensiveMethodCall() @@ -1774,29 +1888,28 @@ string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - [Theory] - [MemberData(nameof(LogLevels))] - public async Task WrongLogLevelGuardedWorkInLogNamed_ReportsDiagnostic_CS(string logLevel) + [Fact] + public async Task NestedGuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() { - string source = $$""" + string source = """ using System; using Microsoft.Extensions.Logging; class C { - void M(ILogger logger, EventId eventId, Exception exception) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) { - if (logger.IsEnabled(LogLevel.None)) - logger.Log{{logLevel}}([|ExpensiveMethodCall()|]); - - if (logger.IsEnabled(LogLevel.None)) - logger.Log{{logLevel}}(eventId, [|ExpensiveMethodCall()|]); - - if (logger.IsEnabled(LogLevel.None)) - logger.Log{{logLevel}}(exception, [|ExpensiveMethodCall()|]); - - if (logger.IsEnabled(LogLevel.None)) - logger.Log{{logLevel}}(eventId, exception, [|ExpensiveMethodCall()|]); + if (logger.IsEnabled(level)) + { + if (exception is not null) + { + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); + logger.Log(level, ExpensiveMethodCall()); + logger.Log(level, eventId, ExpensiveMethodCall()); + logger.Log(level, exception, ExpensiveMethodCall()); + logger.Log(level, eventId, exception, ExpensiveMethodCall()); + } + } } string ExpensiveMethodCall() @@ -1811,30 +1924,77 @@ string ExpensiveMethodCall() [Theory] [MemberData(nameof(LogLevels))] - public async Task WrongLogLevelGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) + public async Task NestedGuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) { string source = $$""" using System; using Microsoft.Extensions.Logging; - static partial class C + class C { - [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + void M(ILogger logger, EventId eventId, Exception exception) + { + if (logger.IsEnabled(LogLevel.{{logLevel}})) + if (exception is not null) + logger.Log{{logLevel}}(ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + if (exception is not null) + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + if (exception is not null) + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.{{logLevel}})) + if (exception is not null) + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + static bool IsExpensiveComputationEnabled { get; set; } - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); static void M(ILogger logger) { - if (logger.IsEnabled(LogLevel.None)) + if (logger.IsEnabled(LogLevel.{{logLevel}})) { - logger.StaticLogLevel([|ExpensiveMethodCall()|]); + if (IsExpensiveComputationEnabled) + { + logger.StaticLogLevel(ExpensiveMethodCall()); + } } - if (logger.IsEnabled(LogLevel.None)) + if (logger.IsEnabled(LogLevel.{{logLevel}})) { - logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]); + if (IsExpensiveComputationEnabled) + { + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + } } } @@ -1849,30 +2009,81 @@ static string ExpensiveMethodCall() } [Fact] - public async Task WrongDynamicLogLevelGuardedWorkInLog_ReportsDiagnostic_CS() + public async Task CustomLoggerGuardedWorkInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class CustomLogger : ILogger + { + public IDisposable BeginScope(TState state) { return default; } + public bool IsEnabled(LogLevel logLevel) { return true; } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + } + + class C + { + void M(CustomLogger logger, EventId eventId, Exception exception, Func formatter) + { + if (logger.IsEnabled(LogLevel.Trace)) + logger.Log(LogLevel.Trace, eventId, ExpensiveMethodCall(), exception, formatter); + + if (logger.IsEnabled(LogLevel.Debug)) + logger.Log(LogLevel.Debug, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Information)) + logger.Log(LogLevel.Information, eventId, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Warning)) + logger.Log(LogLevel.Warning, exception, ExpensiveMethodCall()); + + if (logger.IsEnabled(LogLevel.Error)) + logger.Log(LogLevel.Error, eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task CustomLoggerGuardedWorkInLogWithDynamicLogLevel_NoDiagnostic_CS() { string source = """ using System; using Microsoft.Extensions.Logging; + class CustomLogger : ILogger + { + public IDisposable BeginScope(TState state) { return default; } + public bool IsEnabled(LogLevel logLevel) { return true; } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + } + class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + void M(CustomLogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) { if (logger.IsEnabled(level)) - logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter); - + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); + if (logger.IsEnabled(level)) - logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); + logger.Log(level, ExpensiveMethodCall()); if (logger.IsEnabled(level)) - logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]); + logger.Log(level, eventId, ExpensiveMethodCall()); if (logger.IsEnabled(level)) - logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]); + logger.Log(level, exception, ExpensiveMethodCall()); if (logger.IsEnabled(level)) - logger.Log(LogLevel.Error, eventId, exception, [|ExpensiveMethodCall()|]); + logger.Log(level, eventId, exception, ExpensiveMethodCall()); } string ExpensiveMethodCall() @@ -1887,27 +2098,34 @@ string ExpensiveMethodCall() [Theory] [MemberData(nameof(LogLevels))] - public async Task WrongDynamicLogLevelGuardedWorkInLogNamed_ReportsDiagnostic_CS(string logLevel) + public async Task CustomLoggerGuardedWorkInLogNamed_NoDiagnostic_CS(string logLevel) { string source = $$""" using System; using Microsoft.Extensions.Logging; + class CustomLogger : ILogger + { + public IDisposable BeginScope(TState state) { return default; } + public bool IsEnabled(LogLevel logLevel) { return true; } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + } + class C { - void M(ILogger logger, EventId eventId, Exception exception, LogLevel level) + void M(CustomLogger logger, EventId eventId, Exception exception) { - if (logger.IsEnabled(level)) - logger.Log{{logLevel}}([|ExpensiveMethodCall()|]); + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(ExpensiveMethodCall()); - if (logger.IsEnabled(level)) - logger.Log{{logLevel}}(eventId, [|ExpensiveMethodCall()|]); + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); - if (logger.IsEnabled(level)) - logger.Log{{logLevel}}(exception, [|ExpensiveMethodCall()|]); + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); - if (logger.IsEnabled(level)) - logger.Log{{logLevel}}(eventId, exception, [|ExpensiveMethodCall()|]); + if (logger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); } string ExpensiveMethodCall() @@ -1922,30 +2140,37 @@ string ExpensiveMethodCall() [Theory] [MemberData(nameof(LogLevels))] - public async Task WrongDynamicLogLevelGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) + public async Task CustomLoggerGuardedWorkInLoggerMessage_NoDiagnostic_CS(string logLevel) { string source = $$""" using System; using Microsoft.Extensions.Logging; + class CustomLogger : ILogger + { + public IDisposable BeginScope(TState state) { return default; } + public bool IsEnabled(LogLevel logLevel) { return true; } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { } + } + static partial class C { - [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); - static void M(ILogger logger, LogLevel level) + static void M(CustomLogger logger) { - if (logger.IsEnabled(level)) + if (logger.IsEnabled(LogLevel.{{logLevel}})) { - logger.StaticLogLevel([|ExpensiveMethodCall()|]); + logger.StaticLogLevel(ExpensiveMethodCall()); } - if (logger.IsEnabled(level)) + if (logger.IsEnabled(LogLevel.{{logLevel}})) { - logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]); + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); } } @@ -1960,7 +2185,7 @@ static string ExpensiveMethodCall() } [Fact] - public async Task WrongInstanceGuardedWorkInLog_ReportsDiagnostic_CS() + public async Task WrongLogLevelGuardedWorkInLog_ReportsDiagnostic_CS() { string source = """ using System; @@ -1968,23 +2193,21 @@ public async Task WrongInstanceGuardedWorkInLog_ReportsDiagnostic_CS() class C { - private ILogger _otherLogger; - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - if (_otherLogger.IsEnabled(LogLevel.Trace)) + if (logger.IsEnabled(LogLevel.Critical)) logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter); - if (_otherLogger.IsEnabled(LogLevel.Debug)) + if (logger.IsEnabled(LogLevel.Critical)) logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); - if (_otherLogger.IsEnabled(LogLevel.Information)) + if (logger.IsEnabled(LogLevel.Critical)) logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]); - if (_otherLogger.IsEnabled(LogLevel.Warning)) + if (logger.IsEnabled(LogLevel.Critical)) logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]); - if (_otherLogger.IsEnabled(LogLevel.Error)) + if (logger.IsEnabled(LogLevel.Critical)) logger.Log(LogLevel.Error, eventId, exception, [|ExpensiveMethodCall()|]); } @@ -2000,7 +2223,7 @@ string ExpensiveMethodCall() [Theory] [MemberData(nameof(LogLevels))] - public async Task WrongInstanceGuardedWorkInLogNamed_ReportsDiagnostic_CS(string logLevel) + public async Task WrongLogLevelGuardedWorkInLogNamed_ReportsDiagnostic_CS(string logLevel) { string source = $$""" using System; @@ -2008,20 +2231,18 @@ public async Task WrongInstanceGuardedWorkInLogNamed_ReportsDiagnostic_CS(string class C { - private ILogger _otherLogger; - void M(ILogger logger, EventId eventId, Exception exception) { - if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + if (logger.IsEnabled(LogLevel.None)) logger.Log{{logLevel}}([|ExpensiveMethodCall()|]); - if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + if (logger.IsEnabled(LogLevel.None)) logger.Log{{logLevel}}(eventId, [|ExpensiveMethodCall()|]); - if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + if (logger.IsEnabled(LogLevel.None)) logger.Log{{logLevel}}(exception, [|ExpensiveMethodCall()|]); - if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + if (logger.IsEnabled(LogLevel.None)) logger.Log{{logLevel}}(eventId, exception, [|ExpensiveMethodCall()|]); } @@ -2037,7 +2258,7 @@ string ExpensiveMethodCall() [Theory] [MemberData(nameof(LogLevels))] - public async Task WrongInstanceGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) + public async Task WrongLogLevelGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) { string source = $$""" using System; @@ -2045,20 +2266,23 @@ public async Task WrongInstanceGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(s static partial class C { - private static ILogger _otherLogger; - - [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{message}`")] - static partial void StaticLogLevel(this ILogger logger, string message); + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); - [LoggerMessage(EventId = 1, Message = "Dynamic log level `{message}`")] - static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string message); + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); static void M(ILogger logger) { - if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + if (logger.IsEnabled(LogLevel.None)) { logger.StaticLogLevel([|ExpensiveMethodCall()|]); } + + if (logger.IsEnabled(LogLevel.None)) + { + logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]); + } } static string ExpensiveMethodCall() @@ -2071,39 +2295,36 @@ static string ExpensiveMethodCall() await VerifyCSharpCodeFixAsync(source, source); } - // Boxing tests - [Fact] - public async Task ArgumentIsBoxed_ReportsDiagnostic_CS() + public async Task WrongDynamicLogLevelGuardedWorkInLog_ReportsDiagnostic_CS() { - string source = $$""" + string source = """ using System; using Microsoft.Extensions.Logging; class C { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) { - logger.Log(LogLevel.Debug, eventId, [|42|], exception, formatter); + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter); + + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log(LogLevel.Error, eventId, exception, [|ExpensiveMethodCall()|]); } - } - """; - - await VerifyCSharpCodeFixAsync(source, source); - } - - [Fact] - public async Task ArgumentIsUnboxed_NoDiagnostic_CS() - { - string source = $$""" - using System; - using Microsoft.Extensions.Logging; - class C - { - void M(ILogger logger, EventId eventId, Exception exception, Func formatter, object value) + string ExpensiveMethodCall() { - logger.Log(LogLevel.Debug, eventId, (int)value, exception, formatter); + return "very expensive call"; } } """; @@ -2111,18 +2332,244 @@ void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + void M(ILogger logger, EventId eventId, Exception exception, LogLevel level) { - logger.Log(LogLevel.Debug, eventId, [|"Hello " + 42|], exception, formatter); + if (logger.IsEnabled(level)) + logger.Log{{logLevel}}([|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log{{logLevel}}(eventId, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log{{logLevel}}(exception, [|ExpensiveMethodCall()|]); + + if (logger.IsEnabled(level)) + logger.Log{{logLevel}}(eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongDynamicLogLevelGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger, LogLevel level) + { + if (logger.IsEnabled(level)) + { + logger.StaticLogLevel([|ExpensiveMethodCall()|]); + } + + if (logger.IsEnabled(level)) + { + logger.DynamicLogLevel(LogLevel.{{logLevel}}, [|ExpensiveMethodCall()|]); + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task WrongInstanceGuardedWorkInLog_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + private ILogger _otherLogger; + + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + if (_otherLogger.IsEnabled(LogLevel.Trace)) + logger.Log(LogLevel.Trace, eventId, [|ExpensiveMethodCall()|], exception, formatter); + + if (_otherLogger.IsEnabled(LogLevel.Debug)) + logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.Information)) + logger.Log(LogLevel.Information, eventId, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.Warning)) + logger.Log(LogLevel.Warning, exception, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.Error)) + logger.Log(LogLevel.Error, eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongInstanceGuardedWorkInLogNamed_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + private ILogger _otherLogger; + + void M(ILogger logger, EventId eventId, Exception exception) + { + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}([|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(exception, [|ExpensiveMethodCall()|]); + + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, exception, [|ExpensiveMethodCall()|]); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task WrongInstanceGuardedWorkInLoggerMessage_ReportsDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + private static ILogger _otherLogger; + + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) + { + if (_otherLogger.IsEnabled(LogLevel.{{logLevel}})) + { + logger.StaticLogLevel([|ExpensiveMethodCall()|]); + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + // Boxing tests + + [Fact] + public async Task ArgumentIsBoxed_ReportsDiagnostic_CS() + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|42|], exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task ArgumentIsUnboxed_NoDiagnostic_CS() + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, object value) + { + logger.Log(LogLevel.Debug, eventId, (int)value, exception, formatter); + } + } + """; + + await VerifyCSharpCodeFixAsync(source, source); + } + + [Fact] + public async Task BinaryOperationWithBoxing_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, [|"Hello " + 42|], exception, formatter); } } """; @@ -2161,6 +2608,7 @@ Imports Microsoft.Extensions.Logging Class C Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) logger.Log(LogLevel.Trace, eventId, "literal", exception, formatter) + logger.Log(LogLevel.Debug, "literal") logger.Log(LogLevel.Information, eventId, "literal") logger.Log(LogLevel.Warning, exception, "literal") @@ -2203,13 +2651,13 @@ Imports Microsoft.Extensions.Logging Partial Module C - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As ILogger) @@ -2234,6 +2682,7 @@ Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter A Dim local As String = "local" logger.Log(LogLevel.Trace, eventId, local, exception, formatter) + logger.Log(LogLevel.Debug, local) logger.Log(LogLevel.Information, eventId, local) logger.Log(LogLevel.Warning, exception, local) @@ -2279,13 +2728,13 @@ Imports Microsoft.Extensions.Logging Partial Module C - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As ILogger) @@ -2312,13 +2761,328 @@ Private _field As String Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) logger.Log(LogLevel.Trace, eventId, _field, exception, formatter) + logger.Log(LogLevel.Debug, _field) logger.Log(LogLevel.Information, eventId, _field) logger.Log(LogLevel.Warning, exception, _field) logger.Log(LogLevel.[Error], eventId, exception, _field) End Sub End Class - + + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task FieldInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _field As String + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}(_field) + logger.Log{{logLevel}}(eventId, _field) + logger.Log{{logLevel}}(exception, _field) + logger.Log{{logLevel}}(eventId, exception, _field) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task FieldInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Private _field As String + + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel(_field) + logger.DynamicLogLevel(LogLevel.Debug, _field) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task PropertyInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Public Property [Property] As String + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, [Property], exception, formatter) + + logger.Log(LogLevel.Debug, [Property]) + logger.Log(LogLevel.Information, eventId, [Property]) + logger.Log(LogLevel.Warning, exception, [Property]) + logger.Log(LogLevel.[Error], eventId, exception, [Property]) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task PropertyInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Public Property [Property] As String + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}([Property]) + logger.Log{{logLevel}}(eventId, [Property]) + logger.Log{{logLevel}}(exception, [Property]) + logger.Log{{logLevel}}(eventId, exception, [Property]) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task PropertyInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Public Property [Property] As String + + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel([Property]) + logger.DynamicLogLevel(LogLevel.Debug, [Property]) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task IndexerInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Collections.Generic + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As Dictionary(Of LogLevel, String) + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, _messages(LogLevel.Trace), exception, formatter) + + logger.Log(LogLevel.Debug, _messages(LogLevel.Debug)) + logger.Log(LogLevel.Information, eventId, _messages(LogLevel.Information)) + logger.Log(LogLevel.Warning, exception, _messages(LogLevel.Warning)) + logger.Log(LogLevel.[Error], eventId, exception, _messages(LogLevel.[Error])) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task IndexerInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Collections.Generic + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As Dictionary(Of LogLevel, String) + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}(_messages(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, _messages(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(exception, _messages(LogLevel.{{logLevel}})) + logger.Log{{logLevel}}(eventId, exception, _messages(LogLevel.{{logLevel}})) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task IndexerInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Collections.Generic + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Private _messages As Dictionary(Of LogLevel, String) + + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel(_messages(LogLevel.Information)) + logger.DynamicLogLevel(LogLevel.Debug, _messages(LogLevel.Debug)) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ArrayIndexerInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As String() + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, _messages(0), exception, formatter) + + logger.Log(LogLevel.Debug, _messages(0)) + logger.Log(LogLevel.Information, eventId, _messages(0)) + logger.Log(LogLevel.Warning, exception, _messages(0)) + logger.Log(LogLevel.[Error], eventId, exception, _messages(0)) + End Sub + End Class + + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task ArrayIndexerInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Private _messages As String() + + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log{{logLevel}}(_messages(0)) + logger.Log{{logLevel}}(eventId, _messages(0)) + logger.Log{{logLevel}}(exception, _messages(0)) + logger.Log{{logLevel}}(eventId, exception, _messages(0)) + End Sub + End Class + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ArrayIndexerInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Private _messages As String() + + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel(_messages(0)) + logger.DynamicLogLevel(LogLevel.Debug, _messages(0)) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } + + [Fact] + public async Task ConditionalAccessInLog_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Trace, eventId, exception?.Message, exception, formatter) + + logger.Log(LogLevel.Debug, exception?.Message) + logger.Log(LogLevel.Information, eventId, exception?.Message) + logger.Log(LogLevel.Warning, exception, exception?.Message) + logger.Log(LogLevel.[Error], eventId, exception, exception?.Message) + End Sub + End Class """; await VerifyBasicCodeFixAsync(source, source); @@ -2326,20 +3090,18 @@ End Class [Theory] [MemberData(nameof(LogLevels))] - public async Task FieldInLogNamed_NoDiagnostic_VB(string logLevel) + public async Task ConditionalAccessInLogNamed_NoDiagnostic_VB(string logLevel) { string source = $$""" Imports System Imports Microsoft.Extensions.Logging Class C - Private _field As String - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log{{logLevel}}(_field) - logger.Log{{logLevel}}(eventId, _field) - logger.Log{{logLevel}}(exception, _field) - logger.Log{{logLevel}}(eventId, exception, _field) + logger.Log{{logLevel}}(exception?.Message) + logger.Log{{logLevel}}(eventId, exception?.Message) + logger.Log{{logLevel}}(exception, exception?.Message) + logger.Log{{logLevel}}(eventId, exception, exception?.Message) End Sub End Class """; @@ -2348,7 +3110,7 @@ End Class } [Fact] - public async Task FieldInLoggerMessage_NoDiagnostic_VB() + public async Task ConditionalAccessInLoggerMessage_NoDiagnostic_VB() { string source = """ Imports System @@ -2356,21 +3118,19 @@ Imports System.Runtime.CompilerServices Imports Microsoft.Extensions.Logging Partial Module C - Private _field As String - - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub - Sub M(logger As ILogger) - logger.StaticLogLevel(_field) - logger.DynamicLogLevel(LogLevel.Debug, _field) + Sub M(logger As ILogger, exception As Exception) + logger.StaticLogLevel(exception?.Message) + logger.DynamicLogLevel(LogLevel.Debug, exception?.Message) End Sub End Module """; @@ -2379,21 +3139,15 @@ End Module } [Fact] - public async Task PropertyInLog_NoDiagnostic_VB() + public async Task BinaryOperationInLog_NoDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Public Property [Property] As String - - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log(LogLevel.Trace, eventId, [Property], exception, formatter) - logger.Log(LogLevel.Debug, [Property]) - logger.Log(LogLevel.Information, eventId, [Property]) - logger.Log(LogLevel.Warning, exception, [Property]) - logger.Log(LogLevel.[Error], eventId, exception, [Property]) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Integer, Exception, String)) + logger.Log(LogLevel.Debug, eventId, 4 + 2, exception, formatter) End Sub End Class """; @@ -2403,20 +3157,18 @@ End Class [Theory] [MemberData(nameof(LogLevels))] - public async Task PropertyInLogNamed_NoDiagnostic_VB(string logLevel) + public async Task BinaryOperationInLogNamed_NoDiagnostic_VB(string logLevel) { string source = $$""" Imports System Imports Microsoft.Extensions.Logging Class C - Public Property [Property] As String - - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log{{logLevel}}([Property]) - logger.Log{{logLevel}}(eventId, [Property]) - logger.Log{{logLevel}}(exception, [Property]) - logger.Log{{logLevel}}(eventId, exception, [Property]) + Sub M(logger As ILogger, eventId As EventId, exception As Exception) + logger.Log{{logLevel}}("4" + "2") + logger.Log{{logLevel}}(eventId, "4" + "2") + logger.Log{{logLevel}}(exception, "4" + "2") + logger.Log{{logLevel}}(eventId, exception, "4" + "2") End Sub End Class """; @@ -2425,7 +3177,7 @@ End Class } [Fact] - public async Task PropertyInLoggerMessage_NoDiagnostic_VB() + public async Task BinaryOperationInLoggerMessage_NoDiagnostic_VB() { string source = """ Imports System @@ -2433,21 +3185,19 @@ Imports System.Runtime.CompilerServices Imports Microsoft.Extensions.Logging Partial Module C - Public Property [Property] As String - - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As ILogger) - logger.StaticLogLevel([Property]) - logger.DynamicLogLevel(LogLevel.Debug, [Property]) + logger.StaticLogLevel("4" + "2") + logger.DynamicLogLevel(LogLevel.Debug, "4" + "2") End Sub End Module """; @@ -2456,22 +3206,20 @@ End Module } [Fact] - public async Task IndexerInLog_NoDiagnostic_VB() + public async Task CoalesceOperationInLog_NoDiagnostic_VB() { string source = """ Imports System - Imports System.Collections.Generic Imports Microsoft.Extensions.Logging Class C - Private _messages As Dictionary(Of LogLevel, String) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String), message As String) + logger.Log(LogLevel.Debug, eventId, If(message, "null"), exception, formatter) - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log(LogLevel.Trace, eventId, _messages(LogLevel.Trace), exception, formatter) - logger.Log(LogLevel.Debug, _messages(LogLevel.Debug)) - logger.Log(LogLevel.Information, eventId, _messages(LogLevel.Information)) - logger.Log(LogLevel.Warning, exception, _messages(LogLevel.Warning)) - logger.Log(LogLevel.[Error], eventId, exception, _messages(LogLevel.[Error])) + logger.Log(LogLevel.Debug, If(message, "null")) + logger.Log(LogLevel.Information, eventId, If(message, "null")) + logger.Log(LogLevel.Warning, exception, If(message, "null")) + logger.Log(LogLevel.[Error], eventId, exception, If(message, "null")) End Sub End Class """; @@ -2481,21 +3229,18 @@ End Class [Theory] [MemberData(nameof(LogLevels))] - public async Task IndexerInLogNamed_NoDiagnostic_VB(string logLevel) + public async Task CoalesceOperationInLogNamed_NoDiagnostic_VB(string logLevel) { string source = $$""" Imports System - Imports System.Collections.Generic Imports Microsoft.Extensions.Logging Class C - Private _messages As Dictionary(Of LogLevel, String) - - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log{{logLevel}}(_messages(LogLevel.{{logLevel}})) - logger.Log{{logLevel}}(eventId, _messages(LogLevel.{{logLevel}})) - logger.Log{{logLevel}}(exception, _messages(LogLevel.{{logLevel}})) - logger.Log{{logLevel}}(eventId, exception, _messages(LogLevel.{{logLevel}})) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, message As String) + logger.Log{{logLevel}}(If(message, "null")) + logger.Log{{logLevel}}(eventId, If(message, "null")) + logger.Log{{logLevel}}(exception, If(message, "null")) + logger.Log{{logLevel}}(eventId, exception, If(message, "null")) End Sub End Class """; @@ -2504,30 +3249,27 @@ End Class } [Fact] - public async Task IndexerInLoggerMessage_NoDiagnostic_VB() + public async Task CoalesceOperationInLoggerMessage_NoDiagnostic_VB() { string source = """ Imports System - Imports System.Collections.Generic Imports System.Runtime.CompilerServices Imports Microsoft.Extensions.Logging Partial Module C - Private _messages As Dictionary(Of LogLevel, String) - - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub - Sub M(logger As ILogger) - logger.StaticLogLevel(_messages(LogLevel.Information)) - logger.DynamicLogLevel(LogLevel.Debug, _messages(LogLevel.Debug)) + Sub M(logger As ILogger, message As String) + logger.StaticLogLevel(If(message, "null")) + logger.DynamicLogLevel(LogLevel.Debug, If(message, "null")) End Sub End Module """; @@ -2536,45 +3278,15 @@ End Module } [Fact] - public async Task ArrayIndexerInLog_NoDiagnostic_VB() + public async Task IsTypeOperationInLog_NoDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Private _messages As String() - - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log(LogLevel.Trace, eventId, _messages(0), exception, formatter) - logger.Log(LogLevel.Debug, _messages(0)) - logger.Log(LogLevel.Information, eventId, _messages(0)) - logger.Log(LogLevel.Warning, exception, _messages(0)) - logger.Log(LogLevel.[Error], eventId, exception, _messages(0)) - End Sub - End Class - - """; - - await VerifyBasicCodeFixAsync(source, source); - } - - [Theory] - [MemberData(nameof(LogLevels))] - public async Task ArrayIndexerInLogNamed_NoDiagnostic_VB(string logLevel) - { - string source = $$""" - Imports System - Imports Microsoft.Extensions.Logging - - Class C - Private _messages As String() - - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log{{logLevel}}(_messages(0)) - logger.Log{{logLevel}}(eventId, _messages(0)) - logger.Log{{logLevel}}(exception, _messages(0)) - logger.Log{{logLevel}}(eventId, exception, _messages(0)) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String), input As Object) + logger.Log(LogLevel.Debug, eventId, TypeOf input Is Exception, exception, formatter) End Sub End Class """; @@ -2583,7 +3295,7 @@ End Class } [Fact] - public async Task ArrayIndexerInLoggerMessage_NoDiagnostic_VB() + public async Task IsTypeOperationInLoggerMessage_NoDiagnostic_VB() { string source = """ Imports System @@ -2591,21 +3303,19 @@ Imports System.Runtime.CompilerServices Imports Microsoft.Extensions.Logging Partial Module C - Private _messages As String() - - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As Boolean) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As Boolean) End Sub - Sub M(logger As ILogger) - logger.StaticLogLevel(_messages(0)) - logger.DynamicLogLevel(LogLevel.Debug, _messages(0)) + Sub M(logger As ILogger, input As Object) + logger.StaticLogLevel(TypeOf input Is Exception) + logger.DynamicLogLevel(LogLevel.Debug, TypeOf input Is Exception) End Sub End Module """; @@ -2614,7 +3324,7 @@ End Module } [Fact] - public async Task ConditionalAccessInLog_NoDiagnostic_VB() + public async Task NameOfOperationInLog_NoDiagnostic_VB() { string source = """ Imports System @@ -2622,11 +3332,12 @@ Imports Microsoft.Extensions.Logging Class C Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log(LogLevel.Trace, eventId, exception?.Message, exception, formatter) - logger.Log(LogLevel.Debug, exception?.Message) - logger.Log(LogLevel.Information, eventId, exception?.Message) - logger.Log(LogLevel.Warning, exception, exception?.Message) - logger.Log(LogLevel.[Error], eventId, exception, exception?.Message) + logger.Log(LogLevel.Debug, eventId, NameOf(logger), exception, formatter) + + logger.Log(LogLevel.Debug, NameOf(logger)) + logger.Log(LogLevel.Information, eventId, NameOf(logger)) + logger.Log(LogLevel.Warning, exception, NameOf(logger)) + logger.Log(LogLevel.[Error], eventId, exception, NameOf(logger)) End Sub End Class """; @@ -2636,18 +3347,18 @@ End Class [Theory] [MemberData(nameof(LogLevels))] - public async Task ConditionalAccessInLogNamed_NoDiagnostic_VB(string logLevel) + public async Task NameOfOperationInLogNamed_NoDiagnostic_VB(string logLevel) { string source = $$""" Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log{{logLevel}}(exception?.Message) - logger.Log{{logLevel}}(eventId, exception?.Message) - logger.Log{{logLevel}}(exception, exception?.Message) - logger.Log{{logLevel}}(eventId, exception, exception?.Message) + Sub M(logger As ILogger, eventId As EventId, exception As Exception) + logger.Log{{logLevel}}(NameOf(logger)) + logger.Log{{logLevel}}(eventId, NameOf(logger)) + logger.Log{{logLevel}}(exception, NameOf(logger)) + logger.Log{{logLevel}}(eventId, exception, NameOf(logger)) End Sub End Class """; @@ -2656,7 +3367,7 @@ End Class } [Fact] - public async Task ConditionalAccessInLoggerMessage_NoDiagnostic_VB() + public async Task NameOfOperationInLoggerMessage_NoDiagnostic_VB() { string source = """ Imports System @@ -2665,18 +3376,18 @@ Imports Microsoft.Extensions.Logging Partial Module C - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub - Sub M(logger As ILogger, exception As Exception) - logger.StaticLogLevel(exception?.Message) - logger.DynamicLogLevel(LogLevel.Debug, exception?.Message) + Sub M(logger As ILogger) + logger.StaticLogLevel(NameOf(logger)) + logger.DynamicLogLevel(LogLevel.Debug, NameOf(logger)) End Sub End Module """; @@ -2685,39 +3396,61 @@ End Module } [Fact] - public async Task OtherILoggerMethodCalled_NoDiagnostic_VB() + public async Task ObjectCreationOperationValueTypeInLog_NoDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger) - logger.BeginScope(ExpensiveMethodCall()) - logger.BeginScope("Processing calculation result {CalculationResult}", ExpensiveMethodCall()) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of TimeSpan, Exception, String)) + logger.Log(LogLevel.Debug, eventId, New TimeSpan(), exception, formatter) End Sub - - Function ExpensiveMethodCall() As String - Return "very expensive call" - End Function End Class """; await VerifyBasicCodeFixAsync(source, source); } - // TODO: Add more variants for new non-flagged tests below + [Fact] + public async Task ObjectCreationOperationValueTypeInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As TimeSpan) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As TimeSpan) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel(New TimeSpan()) + logger.DynamicLogLevel(LogLevel.Debug, New TimeSpan()) + End Sub + End Module + """; + + await VerifyBasicCodeFixAsync(source, source); + } [Fact] - public async Task BinaryOperation_NoDiagnostic_VB() + public async Task TypeOfOperationInLog_NoDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Integer, Exception, String)) - logger.Log(LogLevel.Debug, eventId, 4 + 2, exception, formatter) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Type, Exception, String)) + logger.Log(LogLevel.Debug, eventId, GetType(Integer), exception, formatter) End Sub End Class """; @@ -2726,32 +3459,44 @@ End Class } [Fact] - public async Task CoalesceOperation_NoDiagnostic_VB() + public async Task TypeOfOperationInLoggerMessage_NoDiagnostic_VB() { string source = """ Imports System + Imports System.Runtime.CompilerServices Imports Microsoft.Extensions.Logging - - Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String), message As String) - logger.Log(LogLevel.Debug, eventId, If(message, "null"), exception, formatter) - End Sub - End Class + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As Type) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As Type) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel(GetType(Integer)) + logger.DynamicLogLevel(LogLevel.Debug, GetType(Integer)) + End Sub + End Module """; await VerifyBasicCodeFixAsync(source, source); } [Fact] - public async Task IsTypeOperation_NoDiagnostic_VB() + public async Task UnaryOperationInLog_NoDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String), input As Object) - logger.Log(LogLevel.Debug, eventId, TypeOf input Is Exception, exception, formatter) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String), input As Boolean) + logger.Log(LogLevel.Debug, eventId, Not input, exception, formatter) End Sub End Class """; @@ -2760,32 +3505,49 @@ End Class } [Fact] - public async Task NameOfOperation_NoDiagnostic_VB() + public async Task UnaryOperationInLoggerMessage_NoDiagnostic_VB() { string source = """ Imports System + Imports System.Runtime.CompilerServices Imports Microsoft.Extensions.Logging - - Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log(LogLevel.Debug, eventId, NameOf(logger), exception, formatter) - End Sub - End Class + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As Boolean) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As Boolean) + End Sub + + Sub M(logger As ILogger, input As Boolean) + logger.StaticLogLevel(Not input) + logger.DynamicLogLevel(LogLevel.Debug, Not input) + End Sub + End Module """; await VerifyBasicCodeFixAsync(source, source); } [Fact] - public async Task ObjectCreationOperationValueType_NoDiagnostic_VB() + public async Task InterpolatedStringOperationConstantInLog_ReportsDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of TimeSpan, Exception, String)) - logger.Log(LogLevel.Debug, eventId, New TimeSpan(), exception, formatter) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Debug, eventId, $"constant", exception, formatter) + + logger.Log(LogLevel.Debug, $"constant") + logger.Log(LogLevel.Information, eventId, $"constant") + logger.Log(LogLevel.Warning, exception, $"constant") + logger.Log(LogLevel.[Error], eventId, exception, $"constant") End Sub End Class """; @@ -2793,16 +3555,20 @@ End Class await VerifyBasicCodeFixAsync(source, source); } - [Fact] - public async Task TypeOfOperation_NoDiagnostic_VB() + [Theory] + [MemberData(nameof(LogLevels))] + public async Task InterpolatedStringOperationConstantInLogNamed_NoDiagnostic_VB(string logLevel) { - string source = """ + string source = $$""" Imports System Imports Microsoft.Extensions.Logging - + Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Type, Exception, String)) - logger.Log(LogLevel.Debug, eventId, GetType(Integer), exception, formatter) + Sub M(logger As ILogger, eventId As EventId, exception As Exception) + logger.Log{{logLevel}}($"constant") + logger.Log{{logLevel}}(eventId, $"constant") + logger.Log{{logLevel}}(exception, $"constant") + logger.Log{{logLevel}}(eventId, exception, $"constant") End Sub End Class """; @@ -2811,33 +3577,50 @@ End Class } [Fact] - public async Task UnaryOperation_NoDiagnostic_VB() + public async Task InterpolatedStringOperationConstantInLoggerMessage_NoDiagnostic_VB() { string source = """ Imports System + Imports System.Runtime.CompilerServices Imports Microsoft.Extensions.Logging - - Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Boolean, Exception, String), input As Boolean) - logger.Log(LogLevel.Debug, eventId, Not input, exception, formatter) - End Sub - End Class + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel($"constant") + logger.DynamicLogLevel(LogLevel.Debug, $"constant") + End Sub + End Module """; await VerifyBasicCodeFixAsync(source, source); } [Fact] - public async Task InterpolatedStringOperationConstant_ReportsDiagnostic_VB() + public async Task OtherILoggerMethodCalled_NoDiagnostic_VB() { string source = """ Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), input As Integer) - logger.Log(LogLevel.Debug, eventId, $"constant", exception, formatter) + Sub M(logger As ILogger) + logger.BeginScope(ExpensiveMethodCall()) + logger.BeginScope("Processing calculation result {CalculationResult}", ExpensiveMethodCall()) End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function End Class """; @@ -3272,13 +4055,13 @@ Imports Microsoft.Extensions.Logging Partial Module C - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As ILogger) @@ -3410,13 +4193,13 @@ Partial Module C Public Property IsExpensiveComputationEnabled As Boolean - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As ILogger) @@ -3592,13 +4375,13 @@ End Class Partial Module C - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As CustomLogger) @@ -3677,13 +4460,13 @@ Imports Microsoft.Extensions.Logging Partial Module C - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As ILogger) @@ -3762,13 +4545,13 @@ Imports Microsoft.Extensions.Logging Partial Module C - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As ILogger, level As LogLevel) @@ -3853,13 +4636,13 @@ Partial Module C Private _otherLogger As ILogger - - Partial Private Sub StaticLogLevel(logger As ILogger, message As String) + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) End Sub - - Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, message As String) + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) End Sub Sub M(logger As ILogger) @@ -3886,7 +4669,7 @@ Imports System Imports Microsoft.Extensions.Logging Class C - Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of Object, Exception, String)) logger.Log(LogLevel.Debug, eventId, [|42|], exception, formatter) End Sub End Class From 8efbab64f335182db04834ae3b634cbb31d04429 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Sat, 18 Jan 2025 03:58:57 +0100 Subject: [PATCH 10/13] Add StringSyntax attribute to highlight test source --- ...otentiallyExpensiveCallWhenLoggingTests.cs | 368 +++++++++--------- 1 file changed, 185 insertions(+), 183 deletions(-) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index fcb7bee424..85552e0379 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. +using System.Diagnostics.CodeAnalysis; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; using Xunit; @@ -46,7 +48,7 @@ void M(ILogger logger, EventId eventId, Exception exception, Func ExpensiveMethodCall() } """; - await VerifyCSharpCodeFixAsync(source, source); + await VerifyCSharpDiagnosticAsync(source); } [Fact] @@ -1488,7 +1490,7 @@ LogLevel ExpensiveMethodCall() } """; - await VerifyCSharpCodeFixAsync(source, source); + await VerifyCSharpDiagnosticAsync(source); } [Fact] @@ -1509,7 +1511,7 @@ void M(ILogger logger, EventId eventId, Exception? exception, Func Date: Sat, 18 Jan 2025 04:39:48 +0100 Subject: [PATCH 11/13] Allow nameof, consts and literals in interpolated strings --- ...voidPotentiallyExpensiveCallWhenLogging.cs | 4 +- ...otentiallyExpensiveCallWhenLoggingTests.cs | 346 ++++++++++++++++-- 2 files changed, 327 insertions(+), 23 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index 753aa0f748..55e60e32ac 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -154,7 +154,9 @@ IInvocationOperation or if (operation is IInterpolatedStringOperation interpolatedStringOperation) { - return interpolatedStringOperation.Parts.All(p => p is IInterpolationOperation); + return interpolatedStringOperation.Parts.Any(p => p is + IInterpolationOperation { Expression.ConstantValue.HasValue: false } or + IInterpolatedStringTextOperation { Text.ConstantValue.HasValue: false }); } if (operation is IMemberReferenceOperation memberReferenceOperation) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index 85552e0379..04cddb58f1 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -1184,6 +1184,79 @@ static void M(ILogger logger, bool input) await VerifyCSharpDiagnosticAsync(source); } + [Fact] + public async Task InterpolatedStringOperationLiteralInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, $"literal", exception, formatter); + + logger.Log(LogLevel.Debug, $"literal"); + logger.Log(LogLevel.Information, eventId, $"literal"); + logger.Log(LogLevel.Warning, exception, $"literal"); + logger.Log(LogLevel.Error, eventId, exception, $"literal"); + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task InterpolatedStringOperationLiteralInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + logger.Log{{logLevel}}($"literal"); + logger.Log{{logLevel}}(eventId, $"literal"); + logger.Log{{logLevel}}(exception, $"literal"); + logger.Log{{logLevel}}(eventId, exception, $"literal"); + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Fact] + public async Task InterpolatedStringOperationLiteralInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) + { + logger.StaticLogLevel($"literal"); + logger.DynamicLogLevel(LogLevel.Debug, $"literal"); + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + [Fact] public async Task InterpolatedStringOperationConstantInLog_NoDiagnostic_CS() { @@ -1195,12 +1268,14 @@ class C { void M(ILogger logger, EventId eventId, Exception exception, Func formatter) { - logger.Log(LogLevel.Debug, eventId, $"constant", exception, formatter); + const string constant = "constant"; + + logger.Log(LogLevel.Debug, eventId, $"{constant}", exception, formatter); - logger.Log(LogLevel.Debug, $"constant"); - logger.Log(LogLevel.Information, eventId, $"constant"); - logger.Log(LogLevel.Warning, exception, $"constant"); - logger.Log(LogLevel.Error, eventId, exception, $"constant"); + logger.Log(LogLevel.Debug, $"{constant}"); + logger.Log(LogLevel.Information, eventId, $"{constant}"); + logger.Log(LogLevel.Warning, exception, $"{constant}"); + logger.Log(LogLevel.Error, eventId, exception, $"{constant}"); } } """; @@ -1220,10 +1295,12 @@ class C { void M(ILogger logger, EventId eventId, Exception exception) { - logger.Log{{logLevel}}($"constant"); - logger.Log{{logLevel}}(eventId, $"constant"); - logger.Log{{logLevel}}(exception, $"constant"); - logger.Log{{logLevel}}(eventId, exception, $"constant"); + const string constant = "constant"; + + logger.Log{{logLevel}}($"{constant}"); + logger.Log{{logLevel}}(eventId, $"{constant}"); + logger.Log{{logLevel}}(exception, $"{constant}"); + logger.Log{{logLevel}}(eventId, exception, $"{constant}"); } } """; @@ -1248,8 +1325,83 @@ static partial class C static void M(ILogger logger) { - logger.StaticLogLevel($"constant"); - logger.DynamicLogLevel(LogLevel.Debug, $"constant"); + const string constant = "constant"; + + logger.StaticLogLevel($"{constant}"); + logger.DynamicLogLevel(LogLevel.Debug, $"{constant}"); + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Fact] + public async Task InterpolatedStringOperationNameOfInLog_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + logger.Log(LogLevel.Debug, eventId, $"logger name: {nameof(logger)}", exception, formatter); + + logger.Log(LogLevel.Debug, $"logger name: {nameof(logger)}"); + logger.Log(LogLevel.Information, eventId, $"logger name: {nameof(logger)}"); + logger.Log(LogLevel.Warning, exception, $"logger name: {nameof(logger)}"); + logger.Log(LogLevel.Error, eventId, exception, $"logger name: {nameof(logger)}"); + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task InterpolatedStringOperationNameOfInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + logger.Log{{logLevel}}($"logger name: {nameof(logger)}"); + logger.Log{{logLevel}}(eventId, $"logger name: {nameof(logger)}"); + logger.Log{{logLevel}}(exception, $"logger name: {nameof(logger)}"); + logger.Log{{logLevel}}(eventId, exception, $"logger name: {nameof(logger)}"); + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Fact] + public async Task InterpolatedStringOperationNameOfInLoggerMessage_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) + { + logger.StaticLogLevel($"logger name: {nameof(logger)}"); + logger.DynamicLogLevel(LogLevel.Debug, $"logger name: {nameof(logger)}"); } } """; @@ -3535,6 +3687,78 @@ End Module await VerifyBasicDiagnosticAsync(source); } + [Fact] + public async Task InterpolatedStringOperationLiteralInLog_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Debug, eventId, $"literal", exception, formatter) + + logger.Log(LogLevel.Debug, $"literal") + logger.Log(LogLevel.Information, eventId, $"literal") + logger.Log(LogLevel.Warning, exception, $"literal") + logger.Log(LogLevel.[Error], eventId, exception, $"literal") + End Sub + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task InterpolatedStringOperationLiteralInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception) + logger.Log{{logLevel}}($"literal") + logger.Log{{logLevel}}(eventId, $"literal") + logger.Log{{logLevel}}(exception, $"literal") + logger.Log{{logLevel}}(eventId, exception, $"literal") + End Sub + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Fact] + public async Task InterpolatedStringOperationLiteralInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel($"literal") + logger.DynamicLogLevel(LogLevel.Debug, $"literal") + End Sub + End Module + """; + + await VerifyBasicDiagnosticAsync(source); + } + [Fact] public async Task InterpolatedStringOperationConstantInLog_ReportsDiagnostic_VB() { @@ -3544,12 +3768,14 @@ Imports Microsoft.Extensions.Logging Class C Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) - logger.Log(LogLevel.Debug, eventId, $"constant", exception, formatter) + Const constant As String = "constant" + + logger.Log(LogLevel.Debug, eventId, $"{constant}", exception, formatter) - logger.Log(LogLevel.Debug, $"constant") - logger.Log(LogLevel.Information, eventId, $"constant") - logger.Log(LogLevel.Warning, exception, $"constant") - logger.Log(LogLevel.[Error], eventId, exception, $"constant") + logger.Log(LogLevel.Debug, $"{constant}") + logger.Log(LogLevel.Information, eventId, $"{constant}") + logger.Log(LogLevel.Warning, exception, $"{constant}") + logger.Log(LogLevel.[Error], eventId, exception, $"{constant}") End Sub End Class """; @@ -3567,10 +3793,12 @@ Imports Microsoft.Extensions.Logging Class C Sub M(logger As ILogger, eventId As EventId, exception As Exception) - logger.Log{{logLevel}}($"constant") - logger.Log{{logLevel}}(eventId, $"constant") - logger.Log{{logLevel}}(exception, $"constant") - logger.Log{{logLevel}}(eventId, exception, $"constant") + Const constant As String = "constant" + + logger.Log{{logLevel}}($"{constant}") + logger.Log{{logLevel}}(eventId, $"{constant}") + logger.Log{{logLevel}}(exception, $"{constant}") + logger.Log{{logLevel}}(eventId, exception, $"{constant}") End Sub End Class """; @@ -3598,8 +3826,82 @@ Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argume End Sub Sub M(logger As ILogger) - logger.StaticLogLevel($"constant") - logger.DynamicLogLevel(LogLevel.Debug, $"constant") + Const constant As String = "constant" + + logger.StaticLogLevel($"{constant}") + logger.DynamicLogLevel(LogLevel.Debug, $"{constant}") + End Sub + End Module + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Fact] + public async Task InterpolatedStringOperationNameOfInLog_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + logger.Log(LogLevel.Debug, eventId, $"logger name: {NameOf(logger)}", exception, formatter) + + logger.Log(LogLevel.Debug, $"logger name: {NameOf(logger)}") + logger.Log(LogLevel.Information, eventId, $"logger name: {NameOf(logger)}") + logger.Log(LogLevel.Warning, exception, $"logger name: {NameOf(logger)}") + logger.Log(LogLevel.[Error], eventId, exception, $"logger name: {NameOf(logger)}") + End Sub + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task InterpolatedStringOperationNameOfInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception) + logger.Log{{logLevel}}($"logger name: {NameOf(logger)}") + logger.Log{{logLevel}}(eventId, $"logger name: {NameOf(logger)}") + logger.Log{{logLevel}}(exception, $"logger name: {NameOf(logger)}") + logger.Log{{logLevel}}(eventId, exception, $"logger name: {NameOf(logger)}") + End Sub + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Fact] + public async Task InterpolatedStringOperationNameOfInLoggerMessage_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + logger.StaticLogLevel($"logger name: {NameOf(logger)}") + logger.DynamicLogLevel(LogLevel.Debug, $"logger name: {NameOf(logger)}") End Sub End Module """; From 5c0da56ccda70e92ca29a9c1c4bcfb1ba534b7eb Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Thu, 23 Jan 2025 22:32:16 +0100 Subject: [PATCH 12/13] Use SymbolEqualityComparer to check same instance symbol --- .../AvoidPotentiallyExpensiveCallWhenLogging.cs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index 55e60e32ac..18df0421fe 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -349,16 +349,9 @@ bool IsValidIsEnabledGuardInvocation(IInvocationOperation invocation) static bool AreInvocationsOnSameInstance(IInvocationOperation invocation1, IInvocationOperation invocation2) { - var instance1 = GetInstanceResolvingConditionalAccess(invocation1); - var instance2 = GetInstanceResolvingConditionalAccess(invocation2); - - return (instance1, instance2) switch - { - (IMemberReferenceOperation memberRef1, IMemberReferenceOperation memberRef2) => memberRef1.Member == memberRef2.Member, - (IParameterReferenceOperation paramRef1, IParameterReferenceOperation paramRef2) => paramRef1.Parameter == paramRef2.Parameter, - (ILocalReferenceOperation localRef1, ILocalReferenceOperation localRef2) => localRef1.Local == localRef2.Local, - _ => false, - }; + return SymbolEqualityComparer.Default.Equals( + GetInstanceResolvingConditionalAccess(invocation1).GetReferencedMemberOrLocalOrParameter(), + GetInstanceResolvingConditionalAccess(invocation2).GetReferencedMemberOrLocalOrParameter()); } static IOperation? GetInstanceResolvingConditionalAccess(IInvocationOperation invocation) From b4247c798c35b4e3eb60f56dfea51272dd6854a7 Mon Sep 17 00:00:00 2001 From: Mario Pistrich Date: Tue, 28 Jan 2025 01:02:34 +0100 Subject: [PATCH 13/13] Check all parent blocks for IsEnabled guard --- ...voidPotentiallyExpensiveCallWhenLogging.cs | 60 +- ...otentiallyExpensiveCallWhenLoggingTests.cs | 570 ++++++++++++++++++ 2 files changed, 615 insertions(+), 15 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs index 18df0421fe..8806a6c080 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLogging.cs @@ -317,34 +317,62 @@ .Value.Value as int? public bool IsGuardedByIsEnabled(IInvocationOperation logInvocation, int logLevel, IArgumentOperation? logLevelArgumentOperation) { - var currentAncestorConditional = logInvocation.GetAncestor(OperationKind.Conditional); - while (currentAncestorConditional is not null) + // Check each block for conditionals that contain an 'ILogger.IsEnabled' invocation that guards the log invocation: + // 1. If the 'ILogger.IsEnabled' invocation is negated, the 'WhenTrue' branch must contain a return. + // 2. If the 'ILogger.IsEnabled' invocation is not negated, the 'WhenTrue' branch must contain the log invocation. + // This is also not perfect, but should be good enough to prevent false positives. + var currentBlockAncestor = logInvocation.GetAncestor(OperationKind.Block); + while (currentBlockAncestor is not null) { - var conditionInvocations = currentAncestorConditional.Condition - .DescendantsAndSelf() - .OfType(); - - // Check each invocation in the condition to see if it is a valid guard invocation, i.e. same instance and same log level. - // This is not perfect (e.g. 'if (logger.IsEnabled(LogLevel.Debug) || true)' is treated as guarded), but should be good enough to prevent false positives. - if (conditionInvocations.Any(IsValidIsEnabledGuardInvocation)) + var guardConditionals = currentBlockAncestor.Descendants().OfType(); + if (guardConditionals.Any(IsValidGuardConditional)) { return true; } - currentAncestorConditional = currentAncestorConditional.GetAncestor(OperationKind.Conditional); + currentBlockAncestor = currentBlockAncestor.GetAncestor(OperationKind.Block); } return false; - bool IsValidIsEnabledGuardInvocation(IInvocationOperation invocation) + bool IsValidGuardConditional(IConditionalOperation conditional) { - if (!SymbolEqualityComparer.Default.Equals(_isEnabledMethod, invocation.TargetMethod) && - !invocation.TargetMethod.IsOverrideOrImplementationOfInterfaceMember(_isEnabledMethod)) + if (conditional.Syntax.SpanStart > logInvocation.Syntax.SpanStart) { return false; } - return AreInvocationsOnSameInstance(logInvocation, invocation) && IsSameLogLevel(invocation.Arguments[0]); + var conditionInvocations = conditional.Condition + .DescendantsAndSelf() + .OfType(); + + if (conditionInvocations.Any(IsValidIsEnabledGuardInvocation)) + { + return true; + } + + return false; + + bool IsValidIsEnabledGuardInvocation(IInvocationOperation invocation) + { + if (!IsIsEnabledInvocation(invocation) || + !AreInvocationsOnSameInstance(logInvocation, invocation) || + !IsSameLogLevel(invocation.Arguments[0])) + { + return false; + } + + var isNegated = invocation.Parent is IUnaryOperation { OperatorKind: UnaryOperatorKind.Not }; + var descendants = conditional.WhenTrue.DescendantsAndSelf(); + + return isNegated && descendants.OfType().Any() || !isNegated && descendants.Contains(logInvocation); + } + } + + bool IsIsEnabledInvocation(IInvocationOperation invocation) + { + return SymbolEqualityComparer.Default.Equals(_isEnabledMethod, invocation.TargetMethod) || + invocation.TargetMethod.IsOverrideOrImplementationOfInterfaceMember(_isEnabledMethod); } static bool AreInvocationsOnSameInstance(IInvocationOperation invocation1, IInvocationOperation invocation2) @@ -377,7 +405,9 @@ bool IsSameLogLevel(IArgumentOperation isEnabledArgument) : isEnabledLogLevel == logLevel; } - return isEnabledArgument.Value.GetReferencedMemberOrLocalOrParameter() == logLevelArgumentOperation?.Value.GetReferencedMemberOrLocalOrParameter(); + return SymbolEqualityComparer.Default.Equals( + isEnabledArgument.Value.GetReferencedMemberOrLocalOrParameter(), + logLevelArgumentOperation?.Value.GetReferencedMemberOrLocalOrParameter()); } } diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs index 04cddb58f1..52ef278988 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Performance/AvoidPotentiallyExpensiveCallWhenLoggingTests.cs @@ -2162,6 +2162,278 @@ static string ExpensiveMethodCall() await VerifyCSharpDiagnosticAsync(source); } + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkWithReturnInLog_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + if (!logger.IsEnabled(LogLevel.{{logLevel}})) + return; + + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall(), exception, formatter); + logger.Log(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, exception, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Fact] + public async Task GuardedWorkWithReturnInLogWithDynamicLogLevel_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + { + if (!logger.IsEnabled(level)) + return; + + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); + logger.Log(level, ExpensiveMethodCall()); + logger.Log(level, eventId, ExpensiveMethodCall()); + logger.Log(level, exception, ExpensiveMethodCall()); + logger.Log(level, eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkWithReturnInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + if (!logger.IsEnabled(LogLevel.{{logLevel}})) + return; + + logger.Log{{logLevel}}(ExpensiveMethodCall()); + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkWithReturnInLoggerMessage_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) + { + if (!logger.IsEnabled(LogLevel.{{logLevel}})) + return; + + logger.StaticLogLevel(ExpensiveMethodCall()); + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkWithReturnInLog_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter) + { + if (!logger.IsEnabled(LogLevel.{{logLevel}})) + return; + + if (exception is not null) + { + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall(), exception, formatter); + logger.Log(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, exception, ExpensiveMethodCall()); + logger.Log(LogLevel.{{logLevel}}, eventId, exception, ExpensiveMethodCall()); + } + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Fact] + public async Task NestedGuardedWorkWithReturnInLogWithDynamicLogLevel_NoDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception, Func formatter, LogLevel level) + { + if (!logger.IsEnabled(level)) + return; + + if (exception is not null) + { + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter); + logger.Log(level, ExpensiveMethodCall()); + logger.Log(level, eventId, ExpensiveMethodCall()); + logger.Log(level, exception, ExpensiveMethodCall()); + logger.Log(level, eventId, exception, ExpensiveMethodCall()); + } + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkWithReturnInLogNamed_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger, EventId eventId, Exception exception) + { + if (!logger.IsEnabled(LogLevel.{{logLevel}})) + return; + + if (exception is not null) + { + logger.Log{{logLevel}}(ExpensiveMethodCall()); + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()); + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()); + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()); + } + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkWithReturnInLoggerMessage_NoDiagnostic_CS(string logLevel) + { + string source = $$""" + using System; + using Microsoft.Extensions.Logging; + + static partial class C + { + static bool IsExpensiveComputationEnabled { get; set; } + + [LoggerMessage(EventId = 0, Level = LogLevel.{{logLevel}}, Message = "Static log level `{argument}`")] + static partial void StaticLogLevel(this ILogger logger, string argument); + + [LoggerMessage(EventId = 1, Message = "Dynamic log level `{argument}`")] + static partial void DynamicLogLevel(this ILogger logger, LogLevel level, string argument); + + static void M(ILogger logger) + { + if (!logger.IsEnabled(LogLevel.{{logLevel}})) + return; + + if (IsExpensiveComputationEnabled) + { + logger.StaticLogLevel(ExpensiveMethodCall()); + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()); + } + } + + static string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + [Fact] public async Task CustomLoggerGuardedWorkInLog_NoDiagnostic_CS() { @@ -2672,6 +2944,35 @@ static string ExpensiveMethodCall() await VerifyCSharpDiagnosticAsync(source); } + [Fact] + public async Task GuardAfterLogInvocation_ReportsDiagnostic_CS() + { + string source = """ + using System; + using Microsoft.Extensions.Logging; + + class C + { + void M(ILogger logger) + { + logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]); + + if (!logger.IsEnabled(LogLevel.Debug)) + { + return; + } + } + + string ExpensiveMethodCall() + { + return "very expensive call"; + } + } + """; + + await VerifyCSharpDiagnosticAsync(source); + } + // Boxing tests [Fact] @@ -4529,6 +4830,252 @@ End Module await VerifyBasicDiagnosticAsync(source); } + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkWithReturnInLog_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If Not logger.IsEnabled(LogLevel.{{logLevel}}) Then Return + + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall(), exception, formatter) + logger.Log(LogLevel.{{logLevel}}, ExpensiveMethodCall()) + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall()) + logger.Log(LogLevel.{{logLevel}}, exception, ExpensiveMethodCall()) + logger.Log(LogLevel.{{logLevel}}, eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Fact] + public async Task GuardedWorkWithReturnInLogWithDynamicLogLevel_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), level As LogLevel) + If Not logger.IsEnabled(level) Then Return + + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter) + logger.Log(level, ExpensiveMethodCall()) + logger.Log(level, eventId, ExpensiveMethodCall()) + logger.Log(level, exception, ExpensiveMethodCall()) + logger.Log(level, eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkWithReturnInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If Not logger.IsEnabled(LogLevel.{{logLevel}}) Then Return + + logger.Log{{logLevel}}(ExpensiveMethodCall()) + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()) + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()) + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task GuardedWorkWithReturnInLoggerMessage_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + If Not logger.IsEnabled(LogLevel.{{logLevel}}) Then Return + + logger.StaticLogLevel(ExpensiveMethodCall()) + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()) + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Module + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkWithReturnInLog_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If Not logger.IsEnabled(LogLevel.{{logLevel}}) Then Return + + If exception IsNot Nothing + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall(), exception, formatter) + logger.Log(LogLevel.{{logLevel}}, ExpensiveMethodCall()) + logger.Log(LogLevel.{{logLevel}}, eventId, ExpensiveMethodCall()) + logger.Log(LogLevel.{{logLevel}}, exception, ExpensiveMethodCall()) + logger.Log(LogLevel.{{logLevel}}, eventId, exception, ExpensiveMethodCall()) + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Fact] + public async Task NestedGuardedWorkWithReturnInLogWithDynamicLogLevel_NoDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String), level As LogLevel) + If Not logger.IsEnabled(level) Then Return + + If exception IsNot Nothing Then + logger.Log(level, eventId, ExpensiveMethodCall(), exception, formatter) + logger.Log(level, ExpensiveMethodCall()) + logger.Log(level, eventId, ExpensiveMethodCall()) + logger.Log(level, exception, ExpensiveMethodCall()) + logger.Log(level, eventId, exception, ExpensiveMethodCall()) + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkWithReturnInLogNamed_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger, eventId As EventId, exception As Exception, formatter As Func(Of String, Exception, String)) + If Not logger.IsEnabled(LogLevel.{{logLevel}}) Then Return + + If exception IsNot Nothing + logger.Log{{logLevel}}(ExpensiveMethodCall()) + logger.Log{{logLevel}}(eventId, ExpensiveMethodCall()) + logger.Log{{logLevel}}(exception, ExpensiveMethodCall()) + logger.Log{{logLevel}}(eventId, exception, ExpensiveMethodCall()) + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + + [Theory] + [MemberData(nameof(LogLevels))] + public async Task NestedGuardedWorkWithReturnInLoggerMessage_NoDiagnostic_VB(string logLevel) + { + string source = $$""" + Imports System + Imports System.Runtime.CompilerServices + Imports Microsoft.Extensions.Logging + + Partial Module C + Public Property IsExpensiveComputationEnabled As Boolean + + + + Partial Private Sub StaticLogLevel(logger As ILogger, argument As String) + End Sub + + + + Partial Private Sub DynamicLogLevel(logger As ILogger, level As LogLevel, argument As String) + End Sub + + Sub M(logger As ILogger) + If Not logger.IsEnabled(LogLevel.{{logLevel}}) Then Return + + If IsExpensiveComputationEnabled + logger.StaticLogLevel(ExpensiveMethodCall()) + logger.DynamicLogLevel(LogLevel.{{logLevel}}, ExpensiveMethodCall()) + End If + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Module + """; + + await VerifyBasicDiagnosticAsync(source); + } + [Fact] public async Task CustomLoggerGuardedWorkInLog_NoDiagnostic_VB() { @@ -4963,6 +5510,29 @@ End Module await VerifyBasicDiagnosticAsync(source); } + [Fact] + public async Task GuardAfterLogInvocation_ReportsDiagnostic_VB() + { + string source = """ + Imports System + Imports Microsoft.Extensions.Logging + + Class C + Sub M(logger As ILogger) + logger.Log(LogLevel.Debug, [|ExpensiveMethodCall()|]) + + If logger.IsEnabled(LogLevel.Debug) Then Return + End Sub + + Function ExpensiveMethodCall() As String + Return "very expensive call" + End Function + End Class + """; + + await VerifyBasicDiagnosticAsync(source); + } + // Boxing tests [Fact]