diff --git a/src/Moq.Analyzers/Common/ISymbolExtensions.cs b/src/Moq.Analyzers/Common/ISymbolExtensions.cs index 4fcb98b11..a0cfc1e0b 100644 --- a/src/Moq.Analyzers/Common/ISymbolExtensions.cs +++ b/src/Moq.Analyzers/Common/ISymbolExtensions.cs @@ -1,4 +1,6 @@ -namespace Moq.Analyzers.Common; +using System.Runtime.CompilerServices; + +namespace Moq.Analyzers.Common; internal static class ISymbolExtensions { @@ -47,4 +49,20 @@ public static bool IsConstructor(this ISymbol symbol) return symbol.DeclaredAccessibility != Accessibility.Private && symbol is IMethodSymbol { MethodKind: MethodKind.Constructor } and { IsStatic: false }; } + + public static bool IsMethodReturnTypeTask(this ISymbol methodSymbol) + { + string type = methodSymbol.ToDisplayString(); + return string.Equals(type, "System.Threading.Tasks.Task", StringComparison.Ordinal) + || string.Equals(type, "System.Threading.ValueTask", StringComparison.Ordinal) + || type.StartsWith("System.Threading.Tasks.Task<", StringComparison.Ordinal) + || (type.StartsWith("System.Threading.Tasks.ValueTask<", StringComparison.Ordinal) + && type.EndsWith(".Result", StringComparison.Ordinal)); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool IsOverridable(this ISymbol symbol) + { + return !symbol.IsSealed && (symbol.IsVirtual || symbol.IsAbstract || symbol.IsOverride); + } } diff --git a/src/Moq.Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs b/src/Moq.Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs index 9e0584401..88ad01210 100644 --- a/src/Moq.Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs +++ b/src/Moq.Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs @@ -1,3 +1,6 @@ +using System.Runtime.CompilerServices; +using ISymbolExtensions = Microsoft.CodeAnalysis.ISymbolExtensions; + namespace Moq.Analyzers; /// @@ -34,26 +37,81 @@ private static void Analyze(SyntaxNodeAnalysisContext context) { InvocationExpressionSyntax setupInvocation = (InvocationExpressionSyntax)context.Node; - if (setupInvocation.Expression is MemberAccessExpressionSyntax memberAccessExpression && context.SemanticModel.IsMoqSetupMethod(memberAccessExpression, context.CancellationToken)) + if (setupInvocation.Expression is not MemberAccessExpressionSyntax memberAccessExpression + || !context.SemanticModel.IsMoqSetupMethod(memberAccessExpression, context.CancellationToken)) + { + return; + } + + ExpressionSyntax? mockedMemberExpression = setupInvocation.FindMockedMemberExpressionFromSetupMethod(); + if (mockedMemberExpression == null) + { + return; + } + + SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(mockedMemberExpression, context.CancellationToken); + ISymbol? symbol = symbolInfo.Symbol; + + if (symbol is null) + { + return; + } + + // Skip if it's part of an interface + if (symbol.ContainingType.TypeKind == TypeKind.Interface) + { + return; + } + + switch (symbol) { - ExpressionSyntax? mockedMemberExpression = setupInvocation.FindMockedMemberExpressionFromSetupMethod(); - if (mockedMemberExpression == null) - { - return; - } - - SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(mockedMemberExpression, context.CancellationToken); - if (symbolInfo.Symbol is IPropertySymbol or IMethodSymbol - && !IsMethodOverridable(symbolInfo.Symbol)) - { - Diagnostic diagnostic = mockedMemberExpression.CreateDiagnostic(Rule); - context.ReportDiagnostic(diagnostic); - } + case IPropertySymbol propertySymbol: + // Check if the property is Task.Result and skip diagnostic if it is + if (IsTaskResultProperty(propertySymbol, context)) + { + return; + } + + if (propertySymbol.IsOverridable()) + { + return; + } + + if (propertySymbol.IsMethodReturnTypeTask()) + { + return; + } + + break; + case IMethodSymbol methodSymbol: + if (methodSymbol.IsOverridable() || methodSymbol.IsMethodReturnTypeTask()) + { + return; + } + + break; } + + Diagnostic diagnostic = mockedMemberExpression.CreateDiagnostic(Rule); + context.ReportDiagnostic(diagnostic); } - private static bool IsMethodOverridable(ISymbol methodSymbol) + private static bool IsTaskResultProperty(IPropertySymbol propertySymbol, SyntaxNodeAnalysisContext context) { - return !methodSymbol.IsSealed && (methodSymbol.IsVirtual || methodSymbol.IsAbstract || methodSymbol.IsOverride); + // Check if the property is named "Result" + if (!string.Equals(propertySymbol.Name, "Result", StringComparison.Ordinal)) + { + return false; + } + + // Check if the containing type is Task + INamedTypeSymbol? taskOfTType = context.SemanticModel.Compilation.GetTypeByMetadataName("System.Threading.Tasks.Task`1"); + + if (taskOfTType == null) + { + return false; // If Task type cannot be found, we skip it + } + + return SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingType, taskOfTType); } } diff --git a/src/Moq.Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs b/src/Moq.Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs index 8489fabd5..e12fb5704 100644 --- a/src/Moq.Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs +++ b/src/Moq.Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs @@ -44,28 +44,12 @@ private static void Analyze(SyntaxNodeAnalysisContext context) SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(mockedMemberExpression, context.CancellationToken); if ((symbolInfo.Symbol is IPropertySymbol || symbolInfo.Symbol is IMethodSymbol) - && !IsMethodOverridable(symbolInfo.Symbol) - && IsMethodReturnTypeTask(symbolInfo.Symbol)) + && !symbolInfo.Symbol.IsOverridable() + && symbolInfo.Symbol.IsMethodReturnTypeTask()) { Diagnostic diagnostic = mockedMemberExpression.GetLocation().CreateDiagnostic(Rule); context.ReportDiagnostic(diagnostic); } } } - - private static bool IsMethodOverridable(ISymbol methodSymbol) - { - return !methodSymbol.IsSealed - && (methodSymbol.IsVirtual || methodSymbol.IsAbstract || methodSymbol.IsOverride); - } - - private static bool IsMethodReturnTypeTask(ISymbol methodSymbol) - { - string type = methodSymbol.ToDisplayString(); - return string.Equals(type, "System.Threading.Tasks.Task", StringComparison.Ordinal) - || string.Equals(type, "System.Threading.ValueTask", StringComparison.Ordinal) - || type.StartsWith("System.Threading.Tasks.Task<", StringComparison.Ordinal) - || (type.StartsWith("System.Threading.Tasks.ValueTask<", StringComparison.Ordinal) - && type.EndsWith(".Result", StringComparison.Ordinal)); - } } diff --git a/src/Moq.Analyzers/SquiggleCop.Baseline.yaml b/src/Moq.Analyzers/SquiggleCop.Baseline.yaml index 783cade44..b2eac85e9 100644 --- a/src/Moq.Analyzers/SquiggleCop.Baseline.yaml +++ b/src/Moq.Analyzers/SquiggleCop.Baseline.yaml @@ -331,7 +331,7 @@ - {Id: EM0105, Title: Duplicate Case Type, Category: Logic, DefaultSeverity: Error, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: EnableGenerateDocumentationFile, Title: Set MSBuild property 'GenerateDocumentationFile' to 'true', Category: Style, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false} - {Id: IDE0004, Title: Remove Unnecessary Cast, Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true} -- {Id: IDE0005, Title: Using directive is unnecessary., Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false} +- {Id: IDE0005, Title: Using directive is unnecessary., Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true} - {Id: IDE0005_gen, Title: Using directive is unnecessary., Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true} - {Id: IDE0007, Title: Use implicit type, Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false} - {Id: IDE0008, Title: Use explicit type, Category: Style, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false} diff --git a/tests/Moq.Analyzers.Test/SetupShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs b/tests/Moq.Analyzers.Test/SetupShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs index 849b3571c..c067a2fdb 100644 --- a/tests/Moq.Analyzers.Test/SetupShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/SetupShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs @@ -1,8 +1,9 @@ +using Xunit.Abstractions; using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; namespace Moq.Analyzers.Test; -public class SetupShouldBeUsedOnlyForOverridableMembersAnalyzerTests +public class SetupShouldBeUsedOnlyForOverridableMembersAnalyzerTests(ITestOutputHelper output) { public static IEnumerable TestData() { @@ -16,6 +17,7 @@ public static IEnumerable TestData() ["""new Mock().Setup(x => x.TestProperty);"""], ["""new Mock().Setup(x => x.Calculate(It.IsAny(), It.IsAny()));"""], ["""new Mock().Setup(x => x.DoSth());"""], + ["""new Mock().Setup(x => x.DoSomethingAsync().Result).Returns(true);"""], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); } @@ -23,39 +25,48 @@ public static IEnumerable TestData() [MemberData(nameof(TestData))] public async Task ShouldAnalyzeSetupForOverridableMembers(string referenceAssemblyGroup, string @namespace, string mock) { + string source = $$""" + {{@namespace}} + + public interface ISampleInterface + { + int Calculate(int a, int b); + int TestProperty { get; set; } + } + + public interface IParameterlessAsyncMethod + { + Task DoSomethingAsync(); + } + + public abstract class BaseSampleClass + { + public int Calculate() => 0; + public abstract int Calculate(int a, int b); + public abstract int Calculate(int a, int b, int c); + } + + public class SampleClass : BaseSampleClass + { + public override int Calculate(int a, int b) => 0; + public sealed override int Calculate(int a, int b, int c) => 0; + public virtual int DoSth() => 0; + public int Property { get; set; } + } + + internal class UnitTest + { + private void Test() + { + {{mock}} + } + } + """; + + output.WriteLine(source); + await Verifier.VerifyAnalyzerAsync( - $$""" - {{@namespace}} - - public interface ISampleInterface - { - int Calculate(int a, int b); - int TestProperty { get; set; } - } - - public abstract class BaseSampleClass - { - public int Calculate() => 0; - public abstract int Calculate(int a, int b); - public abstract int Calculate(int a, int b, int c); - } - - public class SampleClass : BaseSampleClass - { - public override int Calculate(int a, int b) => 0; - public sealed override int Calculate(int a, int b, int c) => 0; - public virtual int DoSth() => 0; - public int Property { get; set; } - } - - internal class UnitTest - { - private void Test() - { - {{mock}} - } - } - """, + source, referenceAssemblyGroup); } }