feature: Add analysis for camel-cased format arguments in LoggerMessageAttribute #50766
Conversation
|
This PR is targeting |
There was a problem hiding this comment.
Pull Request Overview
This PR adds analysis for camel-cased format arguments in LoggerMessageAttribute to the CA1727 rule, extending its current coverage beyond just LoggerMessage.Define calls to also include modern source generator-based logging methods.
- Adds symbol analysis for methods decorated with LoggerMessageAttribute to detect camel-cased format placeholders
- Refactors existing analysis logic into a shared core method to support both operation and symbol-based analysis contexts
- Includes comprehensive test coverage for both positional and named LoggerMessageAttribute constructors
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| LoggerMessageDefineAnalyzer.cs | Adds symbol analysis registration and core refactoring to analyze LoggerMessageAttribute message templates |
| LoggerMessageDefineTests.cs | Adds test cases for CA1727 rule validation with LoggerMessageAttribute scenarios |
Comments suppressed due to low confidence (1)
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Runtime/LoggerMessageDefineAnalyzer.cs:1
- The catch block only handles ArgumentException, but LogValuesFormatter constructor may throw other exceptions like IndexOutOfRangeException or FormatException for malformed templates. Consider catching a broader exception type or multiple specific exceptions to ensure all invalid template scenarios are handled properly.
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
| 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)); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Each call like this will allocate a closure and several delegates, which could harm performance.
There was a problem hiding this comment.
hmm, I'll see how I could make these static lambdas, that should fix the issue, right?
closes dotnet/roslyn-analyzers#6051