diff --git a/.editorconfig b/.editorconfig index f2384a03b..df65e8879 100644 --- a/.editorconfig +++ b/.editorconfig @@ -405,6 +405,9 @@ dotnet_diagnostic.CA2016.severity = error # Disabled because it's common to use a named argument when passing `null` or bool arguments to make the parameter's purpose clear dotnet_diagnostic.AV1555.severity = none +# AV1500: Methods should not exceed 7 statements +dotnet_diagnostic.AV1500.severity = silent + #### Handling TODOs #### # This is a popular rule in analyzers. Everyone has an opinion and # some of the severity levels conflict. We don't need all of these diff --git a/src/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs b/src/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs index 887c9a783..5c941962a 100644 --- a/src/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs +++ b/src/Moq.Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs @@ -1,3 +1,5 @@ +using Microsoft.CodeAnalysis.Operations; + namespace Moq.Analyzers; /// @@ -10,8 +12,6 @@ public class AsShouldBeUsedOnlyForInterfaceAnalyzer : DiagnosticAnalyzer private const string Title = "Moq: Invalid As type parameter"; private const string Message = "Mock.As() should take interfaces only"; - private static readonly MoqMethodDescriptorBase MoqAsMethodDescriptor = new MoqAsMethodDescriptor(); - private static readonly DiagnosticDescriptor Rule = new( RuleId, Title, @@ -29,43 +29,58 @@ public override void Initialize(AnalysisContext context) { context.EnableConcurrentExecution(); context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); - context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression); - } - [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")] - private static void Analyze(SyntaxNodeAnalysisContext context) - { - if (context.Node is not InvocationExpressionSyntax invocationExpression) + context.RegisterCompilationStartAction(static context => { - return; - } + // Ensure Moq is referenced in the compilation + ImmutableArray mockTypes = context.Compilation.GetMoqMock(); + if (mockTypes.IsEmpty) + { + return; + } - if (invocationExpression.Expression is not MemberAccessExpressionSyntax memberAccessSyntax) - { - return; - } + // Look for the Mock.As() method and provide it to Analyze to avoid looking it up multiple times. + ImmutableArray asMethods = mockTypes + .SelectMany(mockType => mockType.GetMembers("As")) + .OfType() + .Where(method => method.IsGenericMethod) + .ToImmutableArray(); + if (asMethods.IsEmpty) + { + return; + } + + context.RegisterOperationAction(context => Analyze(context, asMethods), OperationKind.Invocation); + }); + } - if (!MoqAsMethodDescriptor.IsMatch(context.SemanticModel, memberAccessSyntax, context.CancellationToken)) + private static void Analyze(OperationAnalysisContext context, ImmutableArray wellKnownAsMethods) + { + if (context.Operation is not IInvocationOperation invocationOperation) { return; } - if (!memberAccessSyntax.Name.TryGetGenericArguments(out SeparatedSyntaxList typeArguments)) + IMethodSymbol targetMethod = invocationOperation.TargetMethod; + if (!targetMethod.IsInstanceOf(wellKnownAsMethods)) { return; } - if (typeArguments.Count != 1) + ImmutableArray typeArguments = targetMethod.TypeArguments; + if (typeArguments.Length != 1) { return; } - TypeSyntax typeArgument = typeArguments[0]; - SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(typeArgument, context.CancellationToken); - - if (symbolInfo.Symbol is ITypeSymbol { TypeKind: not TypeKind.Interface }) + if (typeArguments[0] is ITypeSymbol { TypeKind: not TypeKind.Interface }) { - context.ReportDiagnostic(Diagnostic.Create(Rule, typeArgument.GetLocation())); + // Try to locate the type argument in the syntax tree to report the diagnostic at the correct location. + // If that fails for any reason, report the diagnostic on the operation itself. + NameSyntax? memberName = context.Operation.Syntax.DescendantNodes().OfType().Select(mae => mae.Name).DefaultIfNotSingle(); + Location location = memberName?.GetLocation() ?? invocationOperation.Syntax.GetLocation(); + + context.ReportDiagnostic(Diagnostic.Create(Rule, location)); } } } diff --git a/src/Moq.Analyzers/CompilationExtensions.cs b/src/Moq.Analyzers/CompilationExtensions.cs new file mode 100644 index 000000000..d0ec9baff --- /dev/null +++ b/src/Moq.Analyzers/CompilationExtensions.cs @@ -0,0 +1,41 @@ +using Microsoft.CodeAnalysis.Operations; + +namespace Moq.Analyzers; + +internal static class CompilationExtensions +{ + /// + /// An extension method that performs for multiple metadata names. + /// + /// The to inspect. + /// A list of type names to query. + /// if the type can't be found or there was an ambiguity during lookup. + public static ImmutableArray GetTypesByMetadataNames(this Compilation compilation, ReadOnlySpan metadataNames) + { + ImmutableArray.Builder builder = ImmutableArray.CreateBuilder(metadataNames.Length); + + foreach (string metadataName in metadataNames) + { + INamedTypeSymbol? type = compilation.GetTypeByMetadataName(metadataName); + if (type is not null) + { + builder.Add(type); + } + } + + return builder.ToImmutable(); + } + + /// + /// Get the Moq.Mock and Moq.Mock`1 type symbols (if part of the compilation). + /// + /// The to inspect. + /// + /// s for the Moq.Mock symbols that are part of the compilation. + /// An empty array if none (never ). + /// + public static ImmutableArray GetMoqMock(this Compilation compilation) + { + return compilation.GetTypesByMetadataNames([WellKnownTypeNames.MoqMock, WellKnownTypeNames.MoqMock1]); + } +} diff --git a/src/Moq.Analyzers/EnumerableExtensions.cs b/src/Moq.Analyzers/EnumerableExtensions.cs new file mode 100644 index 000000000..f4d09b8f9 --- /dev/null +++ b/src/Moq.Analyzers/EnumerableExtensions.cs @@ -0,0 +1,43 @@ +namespace Moq.Analyzers; + +internal static class EnumerableExtensions +{ + /// + public static TSource? DefaultIfNotSingle(this IEnumerable source) + { + return source.DefaultIfNotSingle(_ => true); + } + + /// + /// Returns the only element of a sequence that satisfies a specified condition or default if no such element exists or more than one element satisfies the condition. + /// + /// The type of the collection. + /// The collection to enumerate. + /// A function to test each element for a condition. + /// + /// The single element that satisfies the condition, or default if no such element exists or more than one element satisfies the condition. + /// + /// + /// This should be equivalent to calling + /// combined with a catch that returns . + /// + public static TSource? DefaultIfNotSingle(this IEnumerable source, Func predicate) + { + bool isFound = false; + TSource? item = default; + + foreach (TSource element in source.Where(predicate)) + { + if (isFound) + { + // We already found an element, thus there's multiple matches; return default. + return default; + } + + isFound = true; + item = element; + } + + return item; + } +} diff --git a/src/Moq.Analyzers/MoqAsMethodDescriptor.cs b/src/Moq.Analyzers/MoqAsMethodDescriptor.cs deleted file mode 100644 index 46265a136..000000000 --- a/src/Moq.Analyzers/MoqAsMethodDescriptor.cs +++ /dev/null @@ -1,33 +0,0 @@ -namespace Moq.Analyzers; - -/// -/// A class that, given a and a , determines if -/// it is a call to the Moq `Mock.As()` method. -/// -internal class MoqAsMethodDescriptor : MoqMethodDescriptorBase -{ - private const string MethodName = "As"; - - [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")] - public override bool IsMatch(SemanticModel semanticModel, MemberAccessExpressionSyntax memberAccessSyntax, CancellationToken cancellationToken) - { - if (!IsFastMatch(memberAccessSyntax, MethodName.AsSpan())) - { - return false; - } - - ISymbol? symbol = semanticModel.GetSymbolInfo(memberAccessSyntax, cancellationToken).Symbol; - - if (symbol is not IMethodSymbol methodSymbol) - { - return false; - } - - if (!IsContainedInMockType(methodSymbol)) - { - return false; - } - - return methodSymbol.Name.AsSpan().SequenceEqual(MethodName.AsSpan()) && methodSymbol.IsGenericMethod; - } -} diff --git a/src/Moq.Analyzers/SymbolExtensions.cs b/src/Moq.Analyzers/SymbolExtensions.cs new file mode 100644 index 000000000..64a585589 --- /dev/null +++ b/src/Moq.Analyzers/SymbolExtensions.cs @@ -0,0 +1,44 @@ +namespace Moq.Analyzers; + +internal static class SymbolExtensions +{ + /// + /// Determines whether the symbol is an instance of the specified symbol. + /// + /// The type of the to compare. + /// The symbol to compare. + /// The symbol to compare to. + /// The to use for equality. + /// + /// if is an instance of , either as a direct match, + /// or as a specialaization; otherwise, . + /// + /// + /// As an example, Type.Method<int>() is an instance of Type.Method<T>(). + /// + public static bool IsInstanceOf(this ISymbol symbol, TSymbol other, SymbolEqualityComparer? symbolEqualityComparer = null) + where TSymbol : class, ISymbol + { + symbolEqualityComparer ??= SymbolEqualityComparer.Default; + + return symbol switch + { + IMethodSymbol methodSymbol => symbolEqualityComparer.Equals(methodSymbol.OriginalDefinition, other), + _ => symbolEqualityComparer.Equals(symbol, other), + }; + } + + /// + /// The symbol to compare. + /// + /// The symbols to compare to. Returns if matches any of others. + /// + /// The to use for equality. + public static bool IsInstanceOf(this ISymbol symbol, IEnumerable others, SymbolEqualityComparer? symbolEqualityComparer = null) + where TSymbol : class, ISymbol + { + symbolEqualityComparer ??= SymbolEqualityComparer.Default; + + return others.Any(other => symbol.IsInstanceOf(other, symbolEqualityComparer)); + } +} diff --git a/src/Moq.Analyzers/WellKnownTypeNames.cs b/src/Moq.Analyzers/WellKnownTypeNames.cs new file mode 100644 index 000000000..ffd4b598e --- /dev/null +++ b/src/Moq.Analyzers/WellKnownTypeNames.cs @@ -0,0 +1,7 @@ +namespace Moq.Analyzers; + +internal static class WellKnownTypeNames +{ + public const string MoqMock = "Moq.Mock"; + public const string MoqMock1 = "Moq.Mock`1"; +} diff --git a/tests/Moq.Analyzers.Benchmarks/Moq1300Benchmarks.cs b/tests/Moq.Analyzers.Benchmarks/Moq1300Benchmarks.cs index c8006602f..81505b4dc 100644 --- a/tests/Moq.Analyzers.Benchmarks/Moq1300Benchmarks.cs +++ b/tests/Moq.Analyzers.Benchmarks/Moq1300Benchmarks.cs @@ -38,6 +38,7 @@ internal class {name} private void Test() {{ new Mock().As(); + _ = new SampleClass{index}().Calculate(); // Add an expression that looks similar but does not match }} }} ")); diff --git a/tests/Moq.Analyzers.Test/AsAcceptOnlyInterfaceAnalyzerTests.cs b/tests/Moq.Analyzers.Test/AsAcceptOnlyInterfaceAnalyzerTests.cs index 870c03832..16d1aa470 100644 --- a/tests/Moq.Analyzers.Test/AsAcceptOnlyInterfaceAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/AsAcceptOnlyInterfaceAnalyzerTests.cs @@ -1,4 +1,3 @@ -using Microsoft.CodeAnalysis.Testing; using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; namespace Moq.Analyzers.Test; @@ -9,8 +8,8 @@ public static IEnumerable TestData() { return new object[][] { - ["""new Mock().As<{|Moq1300:BaseSampleClass|}>();"""], - ["""new Mock().As<{|Moq1300:SampleClass|}>();"""], + ["""new Mock().{|Moq1300:As|}();"""], + ["""new Mock().{|Moq1300:As|}();"""], ["""new Mock().As();"""], ["""new Mock().As().Setup(x => x.Calculate(It.IsAny(), It.IsAny())).Returns(10);"""], }.WithNamespaces().WithReferenceAssemblyGroups();