Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ public override void Initialize(AnalysisContext context)
}

context.RegisterOperationAction(context => AnalyzeInvocation(context, loggerType, loggerExtensionsType, loggerMessageType), OperationKind.Invocation);

if (wellKnownTypeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftExtensionsLoggingLoggerMessageAttribute, out var loggerMessageAttributeType))
{
context.RegisterSymbolAction(context => AnalyzeMethodSymbol(context, loggerMessageAttributeType), SymbolKind.Method);
}
});
}

Expand Down Expand Up @@ -191,6 +196,60 @@ private void AnalyzeInvocation(OperationAnalysisContext context, INamedTypeSymbo
}
}

private void AnalyzeMethodSymbol(SymbolAnalysisContext context, INamedTypeSymbol loggerMessageAttributeType)
{
var method = (IMethodSymbol)context.Symbol;

// Look for LoggerMessageAttribute on the method
var attribute = method.GetAttribute(loggerMessageAttributeType);
if (attribute == null)
return;

// Extract the message template from the attribute
var messageValue = ExtractMessageFromAttribute(attribute);
if (messageValue == null)
return;

// Analyze the message template for CA1727, CA2253, and CA2023 rules
AnalyzeMessageTemplateFromSymbol(context, messageValue, method);
}

private string? ExtractMessageFromAttribute(AttributeData attribute)
{
// Check constructor arguments (positional) - typically: LoggerMessage(eventId, level, message)
if (attribute.ConstructorArguments.Length >= 3 &&
attribute.ConstructorArguments[2].Value is string constructorMessage)
{
return constructorMessage;
}

// Check named arguments - Message = "..."
var messageArg = attribute.NamedArguments.FirstOrDefault(arg => arg.Key == "Message");
if (messageArg.Value.Value is string namedMessage)
{
return namedMessage;
}

return null;
}

private void AnalyzeMessageTemplateFromSymbol(SymbolAnalysisContext context, string text, IMethodSymbol methodSymbol)
{
// Get the first syntax reference for reporting location
var syntaxReference = methodSymbol.DeclaringSyntaxReferences.FirstOrDefault();
if (syntaxReference == null)
return;

var location = syntaxReference.GetSyntax().GetLocation();

// Use the common analysis logic but report diagnostics directly to the symbol context
AnalyzeMessageTemplateCore(text,
onInvalidTemplate: () => context.ReportDiagnostic(Diagnostic.Create(CA2023Rule, location)),
onNumericPlaceholder: () => context.ReportDiagnostic(Diagnostic.Create(CA2253Rule, location)),
onCamelCasePlaceholder: () => context.ReportDiagnostic(Diagnostic.Create(CA1727Rule, location)),
onParameterCountMismatch: null); // Skip parameter count validation for LoggerMessageAttribute methods
}

private void AnalyzeFormatArgument(OperationAnalysisContext context, IOperation formatExpression, int paramsCount, bool argsIsArray, bool usingLoggerExtensionsTypes, IMethodSymbol methodSymbol)
{
var text = TryGetFormatText(formatExpression);
Expand All @@ -204,6 +263,26 @@ private void AnalyzeFormatArgument(OperationAnalysisContext context, IOperation
return;
}

// Use the common analysis logic but report diagnostics to the operation context
AnalyzeMessageTemplateCore(text,
onInvalidTemplate: () => context.ReportDiagnostic(formatExpression.CreateDiagnostic(CA2023Rule)),
onNumericPlaceholder: () => context.ReportDiagnostic(formatExpression.CreateDiagnostic(CA2253Rule)),
onCamelCasePlaceholder: () => context.ReportDiagnostic(formatExpression.CreateDiagnostic(CA1727Rule)),
onParameterCountMismatch: (expectedCount) =>
{
var argsPassedDirectly = argsIsArray && paramsCount == 1;
if (!argsPassedDirectly && paramsCount != expectedCount)
{
context.ReportDiagnostic(formatExpression.CreateDiagnostic(CA2017Rule));
}
});
Comment on lines +267 to +278
Copy link
Contributor

Choose a reason for hiding this comment

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

Each call like this will allocate a closure and several delegates, which could harm performance.

Copy link
Author

Choose a reason for hiding this comment

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

hmm, I'll see how I could make these static lambdas, that should fix the issue, right?

}

/// <summary>
/// Core logic for analyzing message templates. Uses callbacks to report diagnostics to different contexts.
/// </summary>
private void AnalyzeMessageTemplateCore(string text, Action onInvalidTemplate, Action onNumericPlaceholder, Action onCamelCasePlaceholder, Action<int>? onParameterCountMismatch)
{
LogValuesFormatter formatter;
try
{
Expand All @@ -218,27 +297,24 @@ private void AnalyzeFormatArgument(OperationAnalysisContext context, IOperation

if (!IsValidMessageTemplate(formatter.OriginalFormat))
{
context.ReportDiagnostic(formatExpression.CreateDiagnostic(CA2023Rule));
onInvalidTemplate();
return;
}

foreach (var valueName in formatter.ValueNames)
{
if (int.TryParse(valueName, out _))
{
context.ReportDiagnostic(formatExpression.CreateDiagnostic(CA2253Rule));
onNumericPlaceholder();
}
else if (!string.IsNullOrEmpty(valueName) && char.IsLower(valueName[0]))
{
context.ReportDiagnostic(formatExpression.CreateDiagnostic(CA1727Rule));
onCamelCasePlaceholder();
}
}

var argsPassedDirectly = argsIsArray && paramsCount == 1;
if (!argsPassedDirectly && paramsCount != formatter.ValueNames.Count)
{
context.ReportDiagnostic(formatExpression.CreateDiagnostic(CA2017Rule));
}
// Parameter count validation (only for operation-based analysis)
onParameterCountMismatch?.Invoke(formatter.ValueNames.Count);
}

private static SymbolDisplayFormat GetLanguageSpecificFormat(IOperation operation) =>
Expand Down Expand Up @@ -353,4 +429,4 @@ private static bool FindLogParameters(IMethodSymbol methodSymbol, [NotNullWhen(t
return message != null;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,52 @@ public async Task CA1727IsProducedForCamelCasedFormatArgumentAsync(string format
await TriggerCodeAsync(format);
}

[Fact]
public async Task CA1727IsProducedForCamelCasedFormatArgumentInLoggerMessageAttributeAsync()
{
string code = @"
using Microsoft.Extensions.Logging;
public static partial class C
{
[LoggerMessage(1, LogLevel.Error, ""Unsuccessful status code {|CA1727:{statusCode}|}"")]
static partial void LogUnsuccessfulStatusCode(HttpStatusCode statusCode);

[LoggerMessage(2, LogLevel.Information, ""User {|CA1727:{userName}|} logged in"")]
static partial void LogUserAction(string userName);

// This should not trigger CA1727 - PascalCase is correct
[LoggerMessage(3, LogLevel.Debug, ""Processing {ItemCount} items"")]
static partial void LogProcessing(int itemCount);
}";
await new VerifyCS.Test
{
LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp9,
TestCode = code,
ReferenceAssemblies = AdditionalMetadataReferences.DefaultWithMELogging,
}.RunAsync();
}

[Fact]
public async Task CA1727IsProducedForCamelCasedFormatArgumentInLoggerMessageAttributeWithNamedArgumentAsync()
{
string code = @"
using Microsoft.Extensions.Logging;
public static partial class C
{
[LoggerMessage(EventId = 1, Level = LogLevel.Error, Message = ""Error occurred: {|CA1727:{errorMessage}|}"")]
static partial void LogError(string errorMessage);

[LoggerMessage(EventId = 2, Message = ""Processing {|CA1727:{itemName}|}"")]
static partial void LogProcessingItem(LogLevel level, string itemName);
}";
await new VerifyCS.Test
{
LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp9,
TestCode = code,
ReferenceAssemblies = AdditionalMetadataReferences.DefaultWithMELogging,
}.RunAsync();
}

[Theory]
// Concat would be optimized by compiler
[MemberData(nameof(GenerateTemplateAndDefineUsageIgnoresCA1848ForBeginScope), @"nameof(ILogger) + "" string""", "")]
Expand Down
Loading