diff --git a/README.md b/README.md index 74e813b8..6ec34f3a 100755 --- a/README.md +++ b/README.md @@ -195,6 +195,7 @@ If you are already using other analyzers, you can check [which rules are duplica |[MA0177](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0177.md)|Style|Use single-line XML comment syntax when possible|ℹ️|❌|✔️| |[MA0178](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0178.md)|Design|Use TimeSpan.Zero instead of TimeSpan.FromXXX(0)|ℹ️|✔️|✔️| |[MA0179](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0179.md)|Performance|Use Attribute.IsDefined instead of GetCustomAttribute(s)|ℹ️|✔️|✔️| +|[MA0180](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0180.md)|Design|ILogger type parameter should match containing type|⚠️|❌|✔️| diff --git a/docs/README.md b/docs/README.md index 2128a625..5660d81e 100755 --- a/docs/README.md +++ b/docs/README.md @@ -179,6 +179,7 @@ |[MA0177](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0177.md)|Style|Use single-line XML comment syntax when possible|ℹ️|❌|✔️| |[MA0178](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0178.md)|Design|Use TimeSpan.Zero instead of TimeSpan.FromXXX(0)|ℹ️|✔️|✔️| |[MA0179](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0179.md)|Performance|Use Attribute.IsDefined instead of GetCustomAttribute(s)|ℹ️|✔️|✔️| +|[MA0180](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0180.md)|Design|ILogger type parameter should match containing type|⚠️|❌|✔️| |Id|Suppressed rule|Justification| |--|---------------|-------------| @@ -724,6 +725,9 @@ dotnet_diagnostic.MA0178.severity = suggestion # MA0179: Use Attribute.IsDefined instead of GetCustomAttribute(s) dotnet_diagnostic.MA0179.severity = suggestion + +# MA0180: ILogger type parameter should match containing type +dotnet_diagnostic.MA0180.severity = none ``` # .editorconfig - all rules disabled @@ -1262,4 +1266,7 @@ dotnet_diagnostic.MA0178.severity = none # MA0179: Use Attribute.IsDefined instead of GetCustomAttribute(s) dotnet_diagnostic.MA0179.severity = none + +# MA0180: ILogger type parameter should match containing type +dotnet_diagnostic.MA0180.severity = none ``` diff --git a/docs/Rules/MA0180.md b/docs/Rules/MA0180.md new file mode 100644 index 00000000..3de288a7 --- /dev/null +++ b/docs/Rules/MA0180.md @@ -0,0 +1,45 @@ +# MA0180 - ILogger type parameter should match containing type + +Sources: [ILoggerParameterTypeShouldMatchContainingTypeAnalyzer.cs](https://github.com/meziantou/Meziantou.Analyzer/blob/main/src/Meziantou.Analyzer/Rules/ILoggerParameterTypeShouldMatchContainingTypeAnalyzer.cs), [ILoggerParameterTypeShouldMatchContainingTypeFixer.cs](https://github.com/meziantou/Meziantou.Analyzer/blob/main/src/Meziantou.Analyzer.CodeFixers/Rules/ILoggerParameterTypeShouldMatchContainingTypeFixer.cs) + + +## Description + +When using `ILogger` in a class constructor, the type parameter should match the containing class. This helps ensure proper categorization of log messages and makes it easier to filter logs by component. + +## Non-compliant code + +```csharp +using Microsoft.Extensions.Logging; + +class A(ILogger logger) +{ +} + +class B +{ +} +``` + +## Compliant code + +```csharp +using Microsoft.Extensions.Logging; + +class A(ILogger logger) +{ +} + +class B +{ +} +``` + +## Configuration + +This rule is disabled by default. You can enable it by setting the severity in your `.editorconfig` file: + +```editorconfig +# MA0180: ILogger type parameter should match containing type +dotnet_diagnostic.MA0180.severity = warning +``` diff --git a/src/Meziantou.Analyzer.CodeFixers/Rules/ILoggerParameterTypeShouldMatchContainingTypeFixer.cs b/src/Meziantou.Analyzer.CodeFixers/Rules/ILoggerParameterTypeShouldMatchContainingTypeFixer.cs new file mode 100644 index 00000000..e7e139a5 --- /dev/null +++ b/src/Meziantou.Analyzer.CodeFixers/Rules/ILoggerParameterTypeShouldMatchContainingTypeFixer.cs @@ -0,0 +1,76 @@ +using System.Collections.Immutable; +using System.Composition; +using Meziantou.Analyzer.Internals; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; + +namespace Meziantou.Analyzer.Rules; + +[ExportCodeFixProvider(LanguageNames.CSharp), Shared] +public sealed class ILoggerParameterTypeShouldMatchContainingTypeFixer : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(RuleIdentifiers.ILoggerParameterTypeShouldMatchContainingType); + + public override FixAllProvider GetFixAllProvider() + { + return WellKnownFixAllProviders.BatchFixer; + } + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true); + if (nodeToFix is null) + return; + + var title = "Use ILogger with matching type parameter"; + var codeAction = CodeAction.Create( + title, + ct => FixAsync(context.Document, nodeToFix, ct), + equivalenceKey: title); + + context.RegisterCodeFix(codeAction, context.Diagnostics); + } + + private static async Task FixAsync(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + var semanticModel = editor.SemanticModel; + + // Find the parameter that contains the node + var parameter = nodeToFix.AncestorsAndSelf().OfType().FirstOrDefault(); + if (parameter is null) + return document; + + // Get the ILogger type + var parameterType = parameter.Type; + if (parameterType is not GenericNameSyntax genericType) + return document; + + // Find the containing type by traversing up the syntax tree + var containingTypeDeclaration = parameter.Ancestors().OfType().FirstOrDefault(); + if (containingTypeDeclaration is null) + return document; + + // Get the symbol for the containing type + var containingTypeSymbol = semanticModel.GetDeclaredSymbol(containingTypeDeclaration, cancellationToken); + if (containingTypeSymbol is null) + return document; + + // Create new type argument + var generator = editor.Generator; + var newTypeArgument = generator.TypeExpression(containingTypeSymbol); + + // Create new generic type + var newGenericType = genericType.WithTypeArgumentList( + SyntaxFactory.TypeArgumentList( + SyntaxFactory.SingletonSeparatedList((TypeSyntax)newTypeArgument))); + + editor.ReplaceNode(genericType, newGenericType); + return editor.GetChangedDocument(); + } +} diff --git a/src/Meziantou.Analyzer.Pack/configuration/default.editorconfig b/src/Meziantou.Analyzer.Pack/configuration/default.editorconfig index 18284e80..5c205277 100644 --- a/src/Meziantou.Analyzer.Pack/configuration/default.editorconfig +++ b/src/Meziantou.Analyzer.Pack/configuration/default.editorconfig @@ -535,3 +535,6 @@ dotnet_diagnostic.MA0178.severity = suggestion # MA0179: Use Attribute.IsDefined instead of GetCustomAttribute(s) dotnet_diagnostic.MA0179.severity = suggestion + +# MA0180: ILogger type parameter should match containing type +dotnet_diagnostic.MA0180.severity = none diff --git a/src/Meziantou.Analyzer.Pack/configuration/none.editorconfig b/src/Meziantou.Analyzer.Pack/configuration/none.editorconfig index 179fd62c..329ddd66 100644 --- a/src/Meziantou.Analyzer.Pack/configuration/none.editorconfig +++ b/src/Meziantou.Analyzer.Pack/configuration/none.editorconfig @@ -535,3 +535,6 @@ dotnet_diagnostic.MA0178.severity = none # MA0179: Use Attribute.IsDefined instead of GetCustomAttribute(s) dotnet_diagnostic.MA0179.severity = none + +# MA0180: ILogger type parameter should match containing type +dotnet_diagnostic.MA0180.severity = none diff --git a/src/Meziantou.Analyzer/RuleIdentifiers.cs b/src/Meziantou.Analyzer/RuleIdentifiers.cs index 3ce1f4f8..42ba859f 100755 --- a/src/Meziantou.Analyzer/RuleIdentifiers.cs +++ b/src/Meziantou.Analyzer/RuleIdentifiers.cs @@ -180,6 +180,7 @@ internal static class RuleIdentifiers public const string UseSingleLineXmlCommentSyntaxWhenPossible = "MA0177"; public const string UseTimeSpanZero = "MA0178"; public const string UseAttributeIsDefined = "MA0179"; + public const string ILoggerParameterTypeShouldMatchContainingType = "MA0180"; public static string GetHelpUri(string identifier) { diff --git a/src/Meziantou.Analyzer/Rules/ILoggerParameterTypeShouldMatchContainingTypeAnalyzer.cs b/src/Meziantou.Analyzer/Rules/ILoggerParameterTypeShouldMatchContainingTypeAnalyzer.cs new file mode 100644 index 00000000..0e7d9fba --- /dev/null +++ b/src/Meziantou.Analyzer/Rules/ILoggerParameterTypeShouldMatchContainingTypeAnalyzer.cs @@ -0,0 +1,76 @@ +using System.Collections.Immutable; +using Meziantou.Analyzer.Internals; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Meziantou.Analyzer.Rules; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ILoggerParameterTypeShouldMatchContainingTypeAnalyzer : DiagnosticAnalyzer +{ + private static readonly DiagnosticDescriptor Rule = new( + RuleIdentifiers.ILoggerParameterTypeShouldMatchContainingType, + title: "ILogger type parameter should match containing type", + messageFormat: "ILogger type parameter should be '{0}' instead of '{1}'", + RuleCategories.Design, + DiagnosticSeverity.Warning, + isEnabledByDefault: false, + description: "", + helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.ILoggerParameterTypeShouldMatchContainingType)); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(compilationContext => + { + var iloggerSymbol = compilationContext.Compilation.GetBestTypeByMetadataName("Microsoft.Extensions.Logging.ILogger`1"); + if (iloggerSymbol is null) + return; + + compilationContext.RegisterSymbolAction(context => AnalyzeNamedType(context, iloggerSymbol), SymbolKind.NamedType); + }); + } + + private static void AnalyzeNamedType(SymbolAnalysisContext context, INamedTypeSymbol iloggerSymbol) + { + var namedType = (INamedTypeSymbol)context.Symbol; + + // Skip interfaces, abstract classes, etc. - only check concrete types + if (namedType.TypeKind != TypeKind.Class || namedType.IsAbstract) + return; + + // Check all constructors (including primary constructors) + foreach (var constructor in namedType.Constructors) + { + // Skip implicitly declared constructors + if (constructor.IsImplicitlyDeclared) + continue; + + foreach (var parameter in constructor.Parameters) + { + // Check if parameter type is ILogger + if (parameter.Type is INamedTypeSymbol { IsGenericType: true } parameterType && + parameterType.OriginalDefinition.IsEqualTo(iloggerSymbol)) + { + // Get the type argument of ILogger + var typeArgument = parameterType.TypeArguments[0]; + + // Check if it matches the containing type + if (!typeArgument.IsEqualTo(namedType)) + { + context.ReportDiagnostic( + Rule, + parameter, + DiagnosticParameterReportOptions.ReportOnType, + namedType.Name, + typeArgument.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat)); + } + } + } + } + } +} diff --git a/tests/Meziantou.Analyzer.Test/Rules/ILoggerParameterTypeShouldMatchContainingTypeAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/ILoggerParameterTypeShouldMatchContainingTypeAnalyzerTests.cs new file mode 100644 index 00000000..eba14890 --- /dev/null +++ b/tests/Meziantou.Analyzer.Test/Rules/ILoggerParameterTypeShouldMatchContainingTypeAnalyzerTests.cs @@ -0,0 +1,283 @@ +using Meziantou.Analyzer.Rules; +using Meziantou.Analyzer.Test.Helpers; +using TestHelper; + +namespace Meziantou.Analyzer.Test.Rules; + +public sealed class ILoggerParameterTypeShouldMatchContainingTypeAnalyzerTests +{ + private static ProjectBuilder CreateProjectBuilder() + { + return new ProjectBuilder() + .WithAnalyzer(id: "MA0180") + .WithCodeFixProvider() + .WithTargetFramework(TargetFramework.Net8_0) + .AddNuGetReference("Microsoft.Extensions.Logging.Abstractions", "8.0.0", "lib/net8.0"); + } + +#if CSHARP12_OR_GREATER + [Fact] + public async Task PrimaryConstructor_Mismatch_ShouldReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + class A([|ILogger|] logger) + { + } + + class B + { + } + """; + + var fix = """ + using Microsoft.Extensions.Logging; + + class A(ILogger logger) + { + } + + class B + { + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ShouldFixCodeWith(fix) + .ValidateAsync(); + } +#endif + + [Fact] + public async Task RegularConstructor_Mismatch_ShouldReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + class A + { + public A([|ILogger|] logger) + { + } + } + + class B + { + } + """; + + var fix = """ + using Microsoft.Extensions.Logging; + + class A + { + public A(ILogger logger) + { + } + } + + class B + { + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ShouldFixCodeWith(fix) + .ValidateAsync(); + } + +#if CSHARP12_OR_GREATER + [Fact] + public async Task PrimaryConstructor_Match_ShouldNotReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + class A(ILogger logger) + { + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } +#endif + + [Fact] + public async Task RegularConstructor_Match_ShouldNotReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + class A + { + public A(ILogger logger) + { + } + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } + +#if CSHARP12_OR_GREATER + [Fact] + public async Task NonGenericILogger_ShouldNotReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + class A(ILogger logger) + { + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } +#endif + +#if CSHARP12_OR_GREATER + [Fact] + public async Task AbstractClass_ShouldNotReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + abstract class A(ILogger logger) + { + } + + class B + { + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } +#endif + + [Fact] + public async Task Interface_ShouldNotReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + interface IA + { + } + + class B + { + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task MultipleConstructors_Mismatch_ShouldReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + class A + { + public A([|ILogger|] logger) + { + } + + public A(string name, [|ILogger|] logger) + { + } + } + + class B + { + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } + +#if CSHARP12_OR_GREATER + [Fact] + public async Task NestedClass_Mismatch_ShouldReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + class Outer + { + class Inner([|ILogger|] logger) + { + } + } + """; + + var fix = """ + using Microsoft.Extensions.Logging; + + class Outer + { + class Inner(ILogger logger) + { + } + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ShouldFixCodeWith(fix) + .ValidateAsync(); + } +#endif + +#if CSHARP12_OR_GREATER + [Fact] + public async Task GenericClass_Mismatch_ShouldReportDiagnostic() + { + var sourceCode = """ + using Microsoft.Extensions.Logging; + + class A([|ILogger|] logger) + { + } + + class B + { + } + """; + + var fix = """ + using Microsoft.Extensions.Logging; + + class A(ILogger> logger) + { + } + + class B + { + } + """; + + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ShouldFixCodeWith(fix) + .ValidateAsync(); + } +#endif +}