From 968ab0748c073e4792b37d57d2b452f21f34b106 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 14:16:09 -0600 Subject: [PATCH 01/11] feat: add analyzer for internal mocks requiring InternalsVisibleTo (#110) Add Moq1003 analyzer that detects when Mock is used where T is an internal type and the assembly does not have [InternalsVisibleTo("DynamicProxyGenAssembly2")]. Handles new Mock(), Mock.Of(), and MockRepository.Create() patterns. Checks effective accessibility for nested types. Co-Authored-By: Claude Opus 4.6 --- docs/rules/Moq1003.md | 60 ++++ docs/rules/README.md | 1 + src/Analyzers/AnalyzerReleases.Unshipped.md | 1 + ...lTypeMustHaveInternalsVisibleToAnalyzer.cs | 262 +++++++++++++++++ src/Common/DiagnosticIds.cs | 1 + ...MustHaveInternalsVisibleToAnalyzerTests.cs | 266 ++++++++++++++++++ 6 files changed, 591 insertions(+) create mode 100644 docs/rules/Moq1003.md create mode 100644 src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs create mode 100644 tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs diff --git a/docs/rules/Moq1003.md b/docs/rules/Moq1003.md new file mode 100644 index 000000000..f827c64b7 --- /dev/null +++ b/docs/rules/Moq1003.md @@ -0,0 +1,60 @@ +# Moq1003: Internal type requires InternalsVisibleTo for DynamicProxy + +| Item | Value | +| -------- | ------- | +| Enabled | True | +| Severity | Warning | +| CodeFix | False | + +--- + +When mocking an `internal` type with `Mock`, Castle DynamicProxy needs access to the type's internals to generate a proxy at runtime. Without `[InternalsVisibleTo("DynamicProxyGenAssembly2")]` on the assembly containing the internal type, the mock will fail at runtime. + +To fix: + +- Add `[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]` to the assembly containing the internal type +- Make the type `public` if appropriate +- Introduce a public interface and mock that instead + +## Examples of patterns that are flagged by this analyzer + +```csharp +// Assembly without InternalsVisibleTo +internal class MyService { } + +var mock = new Mock(); // Moq1003: Internal type requires InternalsVisibleTo +``` + +## Solution + +```csharp +// Add to AssemblyInfo.cs or any file in the project containing the internal type +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("DynamicProxyGenAssembly2")] + +internal class MyService { } + +var mock = new Mock(); // OK +``` + +## Suppress a warning + +If you just want to suppress a single violation, add preprocessor directives to +your source file to disable and then re-enable the rule. + +```csharp +#pragma warning disable Moq1003 +var mock = new Mock(); // Moq1003: Internal type requires InternalsVisibleTo +#pragma warning restore Moq1003 +``` + +To disable the rule for a file, folder, or project, set its severity to `none` +in the +[configuration file](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files). + +```ini +[*.{cs,vb}] +dotnet_diagnostic.Moq1003.severity = none +``` + +For more information, see +[How to suppress code analysis warnings](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings). diff --git a/docs/rules/README.md b/docs/rules/README.md index 4fea2fc44..1a674b1b3 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -5,6 +5,7 @@ | [Moq1000](./Moq1000.md) | Usage | Sealed classes cannot be mocked | [NoSealedClassMocksAnalyzer.cs](../../src/Analyzers/NoSealedClassMocksAnalyzer.cs) | | [Moq1001](./Moq1001.md) | Usage | Mocked interfaces cannot have constructor parameters | [ConstructorArgumentsShouldMatchAnalyzer.cs](../../src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs) | | [Moq1002](./Moq1002.md) | Usage | Parameters provided into mock do not match any existing constructors | [ConstructorArgumentsShouldMatchAnalyzer.cs](../../src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs) | +| [Moq1003](./Moq1003.md) | Usage | Internal type requires InternalsVisibleTo for DynamicProxy | [InternalTypeMustHaveInternalsVisibleToAnalyzer.cs](../../src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs) | | [Moq1100](./Moq1100.md) | Correctness | Callback signature must match the signature of the mocked method | [CallbackSignatureShouldMatchMockedMethodAnalyzer.cs](../../src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs) | | [Moq1101](./Moq1101.md) | Usage | SetupGet/SetupSet/SetupProperty should be used for properties, not for methods | [NoMethodsInPropertySetupAnalyzer.cs](../../src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs) | | [Moq1200](./Moq1200.md) | Correctness | Setup should be used only for overridable members | [SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs](../../src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs) | diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index 80da4abd8..c25ae7a25 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -7,6 +7,7 @@ Rule ID | Category | Severity | Notes Moq1000 | Usage | Warning | NoSealedClassMocksAnalyzer (updated category from Moq to Usage) Moq1001 | Usage | Warning | NoConstructorArgumentsForInterfaceMockRuleId (updated category from Moq to Usage) Moq1002 | Usage | Warning | NoMatchingConstructorRuleId (updated category from Moq to Usage) +Moq1003 | Usage | Warning | InternalTypeMustHaveInternalsVisibleToAnalyzer Moq1100 | Usage | Warning | CallbackSignatureShouldMatchMockedMethodAnalyzer (updated category from Moq to Usage) Moq1101 | Usage | Warning | NoMethodsInPropertySetupAnalyzer (updated category from Moq to Usage) Moq1200 | Usage | Error | SetupShouldBeUsedOnlyForOverridableMembersAnalyzer (updated category from Moq to Usage) diff --git a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs new file mode 100644 index 000000000..4ec7f2c54 --- /dev/null +++ b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs @@ -0,0 +1,262 @@ +using System.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; + +namespace Moq.Analyzers; + +/// +/// Detects when Mock<T> is used where T is an type +/// and the assembly containing T does not have +/// [InternalsVisibleTo("DynamicProxyGenAssembly2")]. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class InternalTypeMustHaveInternalsVisibleToAnalyzer : DiagnosticAnalyzer +{ + private static readonly string DynamicProxyAssemblyName = "DynamicProxyGenAssembly2"; + + private static readonly LocalizableString Title = "Moq: Internal type requires InternalsVisibleTo"; + private static readonly LocalizableString Message = "Internal type '{0}' requires [InternalsVisibleTo(\"DynamicProxyGenAssembly2\")] in its assembly to be mocked"; + private static readonly LocalizableString Description = "Mocking internal types requires the assembly to grant access to Castle DynamicProxy via InternalsVisibleTo."; + + private static readonly DiagnosticDescriptor Rule = new( + DiagnosticIds.InternalTypeMustHaveInternalsVisibleTo, + Title, + Message, + DiagnosticCategory.Usage, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: Description, + helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.InternalTypeMustHaveInternalsVisibleTo}.md"); + + /// + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(RegisterCompilationStartAction); + } + + private static void RegisterCompilationStartAction(CompilationStartAnalysisContext context) + { + MoqKnownSymbols knownSymbols = new(context.Compilation); + + if (!knownSymbols.IsMockReferenced()) + { + return; + } + + if (knownSymbols.Mock1 is null) + { + return; + } + + context.RegisterOperationAction( + operationAnalysisContext => Analyze(operationAnalysisContext, knownSymbols), + OperationKind.ObjectCreation, + OperationKind.Invocation); + } + + private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols knownSymbols) + { + ITypeSymbol? mockedType = null; + Location? diagnosticLocation = null; + + if (context.Operation is IObjectCreationOperation creation && + IsValidMockCreation(creation, knownSymbols, out mockedType)) + { + diagnosticLocation = GetDiagnosticLocation(context.Operation, creation.Syntax); + } + else if (context.Operation is IInvocationOperation invocation && + IsValidMockInvocation(invocation, knownSymbols, out mockedType)) + { + diagnosticLocation = GetDiagnosticLocation(context.Operation, invocation.Syntax); + } + else + { + return; + } + + if (mockedType != null && diagnosticLocation != null && ShouldReportDiagnostic(mockedType)) + { + context.ReportDiagnostic(diagnosticLocation.CreateDiagnostic(Rule, mockedType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + } + } + + private static bool IsValidMockCreation( + IObjectCreationOperation creation, + MoqKnownSymbols knownSymbols, + [NotNullWhen(true)] out ITypeSymbol? mockedType) + { + mockedType = null; + + if (creation.Type is null || creation.Constructor is null || !creation.Type.IsInstanceOf(knownSymbols.Mock1)) + { + return false; + } + + return TryGetMockedTypeFromGeneric(creation.Type, out mockedType); + } + + private static bool IsValidMockInvocation( + IInvocationOperation invocation, + MoqKnownSymbols knownSymbols, + [NotNullWhen(true)] out ITypeSymbol? mockedType) + { + mockedType = null; + + IMethodSymbol targetMethod = invocation.TargetMethod; + + // Mock.Of() + if (IsMockOfMethod(targetMethod, knownSymbols)) + { + if (targetMethod.TypeArguments.Length == 1) + { + mockedType = targetMethod.TypeArguments[0]; + return true; + } + + return false; + } + + // MockRepository.Create() + if (IsMockRepositoryCreateMethod(targetMethod, knownSymbols)) + { + if (targetMethod.TypeArguments.Length == 1) + { + mockedType = targetMethod.TypeArguments[0]; + return true; + } + + return false; + } + + return false; + } + + private static bool IsMockOfMethod(IMethodSymbol targetMethod, MoqKnownSymbols knownSymbols) + { + if (!targetMethod.IsStatic) + { + return false; + } + + if (!string.Equals(targetMethod.Name, "Of", StringComparison.Ordinal)) + { + return false; + } + + return targetMethod.ContainingType is not null && + targetMethod.ContainingType.Equals(knownSymbols.Mock, SymbolEqualityComparer.Default); + } + + private static bool IsMockRepositoryCreateMethod(IMethodSymbol targetMethod, MoqKnownSymbols knownSymbols) + { + return targetMethod.IsInstanceOf(knownSymbols.MockRepositoryCreate); + } + + private static bool TryGetMockedTypeFromGeneric(ITypeSymbol type, [NotNullWhen(true)] out ITypeSymbol? mockedType) + { + mockedType = null; + + if (type is not INamedTypeSymbol namedType || namedType.TypeArguments.Length != 1) + { + return false; + } + + mockedType = namedType.TypeArguments[0]; + return true; + } + + /// + /// Determines whether the mocked type is internal and its assembly lacks InternalsVisibleTo for DynamicProxy. + /// + private static bool ShouldReportDiagnostic(ITypeSymbol mockedType) + { + if (!IsEffectivelyInternal(mockedType)) + { + return false; + } + + return !HasInternalsVisibleToDynamicProxy(mockedType.ContainingAssembly); + } + + /// + /// Checks if the type has effective accessibility. + /// A nested public type inside an internal type is also effectively internal. + /// + private static bool IsEffectivelyInternal(ITypeSymbol type) + { + ITypeSymbol? current = type; + while (current != null) + { + if (current.DeclaredAccessibility == Accessibility.Internal || + current.DeclaredAccessibility == Accessibility.Friend) + { + return true; + } + + current = current.ContainingType; + } + + return false; + } + + private static bool HasInternalsVisibleToDynamicProxy(IAssemblySymbol? assembly) + { + if (assembly is null) + { + return false; + } + + foreach (AttributeData attribute in assembly.GetAttributes()) + { + if (attribute.AttributeClass is null) + { + continue; + } + + if (!string.Equals( + attribute.AttributeClass.ToDisplayString(), + "System.Runtime.CompilerServices.InternalsVisibleToAttribute", + StringComparison.Ordinal)) + { + continue; + } + + if (attribute.ConstructorArguments.Length == 1 && + attribute.ConstructorArguments[0].Value is string assemblyName && + AssemblyNameStartsWithDynamicProxy(assemblyName)) + { + return true; + } + } + + return false; + } + + /// + /// Checks if the assembly name starts with the DynamicProxy assembly name. + /// The InternalsVisibleTo attribute value can include a public key, so we + /// check for a prefix match rather than an exact match. + /// + private static bool AssemblyNameStartsWithDynamicProxy(string assemblyName) + { + return assemblyName.StartsWith(DynamicProxyAssemblyName, StringComparison.Ordinal); + } + + private static Location GetDiagnosticLocation(IOperation operation, SyntaxNode fallbackSyntax) + { + TypeSyntax? typeArgument = operation.Syntax + .DescendantNodes() + .OfType() + .FirstOrDefault()? + .TypeArgumentList? + .Arguments + .FirstOrDefault(); + + return typeArgument?.GetLocation() ?? fallbackSyntax.GetLocation(); + } +} diff --git a/src/Common/DiagnosticIds.cs b/src/Common/DiagnosticIds.cs index 9af16ce95..172efca89 100644 --- a/src/Common/DiagnosticIds.cs +++ b/src/Common/DiagnosticIds.cs @@ -7,6 +7,7 @@ internal static class DiagnosticIds internal const string SealedClassCannotBeMocked = "Moq1000"; internal const string NoConstructorArgumentsForInterfaceMockRuleId = "Moq1001"; internal const string NoMatchingConstructorRuleId = "Moq1002"; + internal const string InternalTypeMustHaveInternalsVisibleTo = "Moq1003"; internal const string BadCallbackParameters = "Moq1100"; internal const string PropertySetupUsedForMethod = "Moq1101"; internal const string SetupOnlyUsedForOverridableMembers = "Moq1200"; diff --git a/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs b/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs new file mode 100644 index 000000000..07a6ad1de --- /dev/null +++ b/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs @@ -0,0 +1,266 @@ +using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; + +namespace Moq.Analyzers.Test; + +public class InternalTypeMustHaveInternalsVisibleToAnalyzerTests +{ + public static IEnumerable InternalTypeWithoutAttributeTestData() + { + return new object[][] + { + ["""new Mock<{|Moq1003:InternalClass|}>()"""], + ["""new Mock<{|Moq1003:InternalClass|}>(MockBehavior.Strict)"""], + ["""Mock.Of<{|Moq1003:InternalClass|}>()"""], + ["""var mock = new Mock<{|Moq1003:InternalClass|}>()"""], + }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + public static IEnumerable PublicTypeTestData() + { + return new object[][] + { + ["""new Mock()"""], + ["""new Mock(MockBehavior.Strict)"""], + ["""Mock.Of()"""], + ["""var mock = new Mock()"""], + }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + public static IEnumerable InterfaceTestData() + { + return new object[][] + { + // Internal interfaces also need InternalsVisibleTo + ["""new Mock<{|Moq1003:IInternalInterface|}>()"""], + ["""Mock.Of<{|Moq1003:IInternalInterface|}>()"""], + + // Public interfaces should not trigger + ["""new Mock()"""], + ["""Mock.Of()"""], + }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + public static IEnumerable NestedTypeTestData() + { + return new object[][] + { + // Public type nested inside internal type is effectively internal + ["""new Mock<{|Moq1003:InternalOuter.PublicNested|}>()"""], + + // Internal type nested inside public type + ["""new Mock<{|Moq1003:PublicOuter.InternalNested|}>()"""], + + // Public nested in public should not trigger + ["""new Mock()"""], + }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + [Theory] + [MemberData(nameof(InternalTypeWithoutAttributeTestData))] + public async Task ShouldDetectInternalTypeWithoutInternalsVisibleTo(string referenceAssemblyGroup, string @namespace, string mock) + { + await Verifier.VerifyAnalyzerAsync( + $$""" + {{@namespace}} + + internal class InternalClass { public virtual void DoWork() { } } + + public class PublicClass { public virtual void DoWork() { } } + + internal class UnitTest + { + private void Test() + { + {{mock}}; + } + } + """, + referenceAssemblyGroup); + } + + [Theory] + [MemberData(nameof(PublicTypeTestData))] + public async Task ShouldNotFlagPublicType(string referenceAssemblyGroup, string @namespace, string mock) + { + await Verifier.VerifyAnalyzerAsync( + $$""" + {{@namespace}} + + public class PublicClass { public virtual void DoWork() { } } + + internal class UnitTest + { + private void Test() + { + {{mock}}; + } + } + """, + referenceAssemblyGroup); + } + + [Theory] + [MemberData(nameof(InterfaceTestData))] + public async Task ShouldHandleInterfaces(string referenceAssemblyGroup, string @namespace, string mock) + { + await Verifier.VerifyAnalyzerAsync( + $$""" + {{@namespace}} + + internal interface IInternalInterface { void DoWork(); } + + public interface IPublicInterface { void DoWork(); } + + internal class UnitTest + { + private void Test() + { + {{mock}}; + } + } + """, + referenceAssemblyGroup); + } + + [Theory] + [MemberData(nameof(NestedTypeTestData))] + public async Task ShouldHandleNestedTypes(string referenceAssemblyGroup, string @namespace, string mock) + { + await Verifier.VerifyAnalyzerAsync( + $$""" + {{@namespace}} + + internal class InternalOuter + { + public class PublicNested { public virtual void DoWork() { } } + } + + public class PublicOuter + { + internal class InternalNested { public virtual void DoWork() { } } + public class PublicNested { public virtual void DoWork() { } } + } + + internal class UnitTest + { + private void Test() + { + {{mock}}; + } + } + """, + referenceAssemblyGroup); + } + + [Fact] + public async Task ShouldNotFlagInternalTypeWithCorrectInternalsVisibleTo() + { + await Verifier.VerifyAnalyzerAsync( + """ + using Moq; + using System.Runtime.CompilerServices; + + [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] + + internal class InternalClass { public virtual void DoWork() { } } + + internal class UnitTest + { + private void Test() + { + var mock = new Mock(); + var of = Mock.Of(); + } + } + """, + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + } + + [Fact] + public async Task ShouldFlagInternalTypeWithWrongAssemblyName() + { + await Verifier.VerifyAnalyzerAsync( + """ + using Moq; + using System.Runtime.CompilerServices; + + [assembly: InternalsVisibleTo("SomeOtherAssembly")] + + internal class InternalClass { public virtual void DoWork() { } } + + internal class UnitTest + { + private void Test() + { + var mock = new Mock<{|Moq1003:InternalClass|}>(); + var of = Mock.Of<{|Moq1003:InternalClass|}>(); + } + } + """, + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + } + + [Fact] + public async Task ShouldNotFlagInternalTypeWithPublicKeyInAttribute() + { + await Verifier.VerifyAnalyzerAsync( + """ + using Moq; + using System.Runtime.CompilerServices; + + [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")] + + internal class InternalClass { public virtual void DoWork() { } } + + internal class UnitTest + { + private void Test() + { + var mock = new Mock(); + } + } + """, + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + } + + [Fact] + public async Task ShouldNotAnalyzeWhenMoqNotReferenced() + { + await Verifier.VerifyAnalyzerAsync( + """ + namespace Test + { + internal class InternalClass { } + + internal class UnitTest + { + private void Test() + { + var instance = new InternalClass(); + } + } + } + """, + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + } + + [Fact] + public async Task ShouldNotFlagAbstractPublicType() + { + await Verifier.VerifyAnalyzerAsync( + """ + using Moq; + + public abstract class PublicAbstractClass { public abstract void DoWork(); } + + internal class UnitTest + { + private void Test() + { + var mock = new Mock(); + } + } + """, + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + } +} From fcb9ab5f7843b2862f93f50b70cad4a630b1590d Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 14:24:47 -0600 Subject: [PATCH 02/11] fix: add Moq1004 entry for merge compatibility with PR #957 Co-Authored-By: Claude Opus 4.6 --- docs/rules/README.md | 1 + src/Analyzers/AnalyzerReleases.Unshipped.md | 1 + src/Common/DiagnosticIds.cs | 1 + 3 files changed, 3 insertions(+) diff --git a/docs/rules/README.md b/docs/rules/README.md index 1a674b1b3..99527f875 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -6,6 +6,7 @@ | [Moq1001](./Moq1001.md) | Usage | Mocked interfaces cannot have constructor parameters | [ConstructorArgumentsShouldMatchAnalyzer.cs](../../src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs) | | [Moq1002](./Moq1002.md) | Usage | Parameters provided into mock do not match any existing constructors | [ConstructorArgumentsShouldMatchAnalyzer.cs](../../src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs) | | [Moq1003](./Moq1003.md) | Usage | Internal type requires InternalsVisibleTo for DynamicProxy | [InternalTypeMustHaveInternalsVisibleToAnalyzer.cs](../../src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs) | +| [Moq1004](./Moq1004.md) | Usage | ILogger should not be mocked | [NoMockOfLoggerAnalyzer.cs](../../src/Analyzers/NoMockOfLoggerAnalyzer.cs) | | [Moq1100](./Moq1100.md) | Correctness | Callback signature must match the signature of the mocked method | [CallbackSignatureShouldMatchMockedMethodAnalyzer.cs](../../src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs) | | [Moq1101](./Moq1101.md) | Usage | SetupGet/SetupSet/SetupProperty should be used for properties, not for methods | [NoMethodsInPropertySetupAnalyzer.cs](../../src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs) | | [Moq1200](./Moq1200.md) | Correctness | Setup should be used only for overridable members | [SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs](../../src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs) | diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index c25ae7a25..58b81b6d5 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -8,6 +8,7 @@ Moq1000 | Usage | Warning | NoSealedClassMocksAnalyzer (updated category from Mo Moq1001 | Usage | Warning | NoConstructorArgumentsForInterfaceMockRuleId (updated category from Moq to Usage) Moq1002 | Usage | Warning | NoMatchingConstructorRuleId (updated category from Moq to Usage) Moq1003 | Usage | Warning | InternalTypeMustHaveInternalsVisibleToAnalyzer +Moq1004 | Usage | Warning | NoMockOfLoggerAnalyzer Moq1100 | Usage | Warning | CallbackSignatureShouldMatchMockedMethodAnalyzer (updated category from Moq to Usage) Moq1101 | Usage | Warning | NoMethodsInPropertySetupAnalyzer (updated category from Moq to Usage) Moq1200 | Usage | Error | SetupShouldBeUsedOnlyForOverridableMembersAnalyzer (updated category from Moq to Usage) diff --git a/src/Common/DiagnosticIds.cs b/src/Common/DiagnosticIds.cs index 172efca89..f46c498c3 100644 --- a/src/Common/DiagnosticIds.cs +++ b/src/Common/DiagnosticIds.cs @@ -8,6 +8,7 @@ internal static class DiagnosticIds internal const string NoConstructorArgumentsForInterfaceMockRuleId = "Moq1001"; internal const string NoMatchingConstructorRuleId = "Moq1002"; internal const string InternalTypeMustHaveInternalsVisibleTo = "Moq1003"; + internal const string LoggerShouldNotBeMocked = "Moq1004"; internal const string BadCallbackParameters = "Moq1100"; internal const string PropertySetupUsedForMethod = "Moq1101"; internal const string SetupOnlyUsedForOverridableMembers = "Moq1200"; From d4522a3fa23300c905b2cd058560f7d8a42483c6 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 14:30:55 -0600 Subject: [PATCH 03/11] fix: improve Moq1003 analyzer quality to BCL standards - Replace string-based ToDisplayString() attribute matching with symbol-based SymbolEqualityComparer for InternalsVisibleToAttribute - Add InternalsVisibleToAttribute to KnownSymbols for proper symbol resolution via WellKnownTypeProvider (avoids banned API) - Use IsInstanceOf with MoqKnownSymbols.MockOf instead of manual name + containing type string checks for Mock.Of() detection - Remove redundant IsMockOfMethod and IsMockRepositoryCreateMethod wrapper methods - Fix IsDynamicProxyAssemblyName to reject prefix-only matches like "DynamicProxyGenAssembly2Extra" (require exact name or comma) - Add ProtectedOrInternal and Private to IsEffectivelyInternal check since DynamicProxy cannot access these from another assembly - Bail out conservatively when InternalsVisibleToAttribute symbol cannot be resolved (avoid false positives) - Remove stale Moq1004 entries from DiagnosticIds and Unshipped.md - Add tests: multiple InternalsVisibleTo attributes, similar-but-wrong assembly name, protected internal nested types Fixes #110 Co-Authored-By: Claude Opus 4.6 --- src/Analyzers/AnalyzerReleases.Unshipped.md | 1 - ...lTypeMustHaveInternalsVisibleToAnalyzer.cs | 114 +++++++++++------- src/Common/DiagnosticIds.cs | 1 - src/Common/WellKnown/KnownSymbols.cs | 5 + ...MustHaveInternalsVisibleToAnalyzerTests.cs | 73 +++++++++++ 5 files changed, 146 insertions(+), 48 deletions(-) diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index 58b81b6d5..c25ae7a25 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -8,7 +8,6 @@ Moq1000 | Usage | Warning | NoSealedClassMocksAnalyzer (updated category from Mo Moq1001 | Usage | Warning | NoConstructorArgumentsForInterfaceMockRuleId (updated category from Moq to Usage) Moq1002 | Usage | Warning | NoMatchingConstructorRuleId (updated category from Moq to Usage) Moq1003 | Usage | Warning | InternalTypeMustHaveInternalsVisibleToAnalyzer -Moq1004 | Usage | Warning | NoMockOfLoggerAnalyzer Moq1100 | Usage | Warning | CallbackSignatureShouldMatchMockedMethodAnalyzer (updated category from Moq to Usage) Moq1101 | Usage | Warning | NoMethodsInPropertySetupAnalyzer (updated category from Moq to Usage) Moq1200 | Usage | Error | SetupShouldBeUsedOnlyForOverridableMembersAnalyzer (updated category from Moq to Usage) diff --git a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs index 4ec7f2c54..4949f85df 100644 --- a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs +++ b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs @@ -59,7 +59,9 @@ private static void RegisterCompilationStartAction(CompilationStartAnalysisConte OperationKind.Invocation); } - private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols knownSymbols) + private static void Analyze( + OperationAnalysisContext context, + MoqKnownSymbols knownSymbols) { ITypeSymbol? mockedType = null; Location? diagnosticLocation = null; @@ -79,9 +81,12 @@ private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols kn return; } - if (mockedType != null && diagnosticLocation != null && ShouldReportDiagnostic(mockedType)) + if (mockedType != null && diagnosticLocation != null && + ShouldReportDiagnostic(mockedType, knownSymbols.InternalsVisibleToAttribute)) { - context.ReportDiagnostic(diagnosticLocation.CreateDiagnostic(Rule, mockedType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); + context.ReportDiagnostic(diagnosticLocation.CreateDiagnostic( + Rule, + mockedType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat))); } } @@ -109,8 +114,8 @@ private static bool IsValidMockInvocation( IMethodSymbol targetMethod = invocation.TargetMethod; - // Mock.Of() - if (IsMockOfMethod(targetMethod, knownSymbols)) + // Mock.Of() -- use symbol-based comparison via MoqKnownSymbols.MockOf + if (targetMethod.IsInstanceOf(knownSymbols.MockOf)) { if (targetMethod.TypeArguments.Length == 1) { @@ -122,7 +127,7 @@ private static bool IsValidMockInvocation( } // MockRepository.Create() - if (IsMockRepositoryCreateMethod(targetMethod, knownSymbols)) + if (targetMethod.IsInstanceOf(knownSymbols.MockRepositoryCreate)) { if (targetMethod.TypeArguments.Length == 1) { @@ -136,27 +141,6 @@ private static bool IsValidMockInvocation( return false; } - private static bool IsMockOfMethod(IMethodSymbol targetMethod, MoqKnownSymbols knownSymbols) - { - if (!targetMethod.IsStatic) - { - return false; - } - - if (!string.Equals(targetMethod.Name, "Of", StringComparison.Ordinal)) - { - return false; - } - - return targetMethod.ContainingType is not null && - targetMethod.ContainingType.Equals(knownSymbols.Mock, SymbolEqualityComparer.Default); - } - - private static bool IsMockRepositoryCreateMethod(IMethodSymbol targetMethod, MoqKnownSymbols knownSymbols) - { - return targetMethod.IsInstanceOf(knownSymbols.MockRepositoryCreate); - } - private static bool TryGetMockedTypeFromGeneric(ITypeSymbol type, [NotNullWhen(true)] out ITypeSymbol? mockedType) { mockedType = null; @@ -171,31 +155,47 @@ private static bool TryGetMockedTypeFromGeneric(ITypeSymbol type, [NotNullWhen(t } /// - /// Determines whether the mocked type is internal and its assembly lacks InternalsVisibleTo for DynamicProxy. + /// Determines whether the mocked type is effectively internal and its assembly + /// lacks InternalsVisibleTo for DynamicProxy. /// - private static bool ShouldReportDiagnostic(ITypeSymbol mockedType) + private static bool ShouldReportDiagnostic( + ITypeSymbol mockedType, + INamedTypeSymbol? internalsVisibleToAttribute) { if (!IsEffectivelyInternal(mockedType)) { return false; } - return !HasInternalsVisibleToDynamicProxy(mockedType.ContainingAssembly); + return !HasInternalsVisibleToDynamicProxy(mockedType.ContainingAssembly, internalsVisibleToAttribute); } /// - /// Checks if the type has effective accessibility. - /// A nested public type inside an internal type is also effectively internal. + /// Checks if the type (or any containing type) has accessibility that requires + /// InternalsVisibleTo for DynamicProxy to access it. DynamicProxy resides in a + /// separate assembly and does not derive from containing types, so it relies on + /// assembly-level access. Any of the following accessibility levels on the type + /// or its containers make it inaccessible to DynamicProxy without InternalsVisibleTo: + /// + /// (internal) + /// (private protected) + /// (protected internal) on + /// a containing type, because DynamicProxy does not derive from the container + /// on a nested type + /// /// private static bool IsEffectivelyInternal(ITypeSymbol type) { ITypeSymbol? current = type; while (current != null) { - if (current.DeclaredAccessibility == Accessibility.Internal || - current.DeclaredAccessibility == Accessibility.Friend) + switch (current.DeclaredAccessibility) { - return true; + case Accessibility.Internal: + case Accessibility.ProtectedAndInternal: + case Accessibility.ProtectedOrInternal: + case Accessibility.Private: + return true; } current = current.ContainingType; @@ -204,13 +204,26 @@ private static bool IsEffectivelyInternal(ITypeSymbol type) return false; } - private static bool HasInternalsVisibleToDynamicProxy(IAssemblySymbol? assembly) + /// + /// Checks the assembly's attributes for InternalsVisibleTo targeting DynamicProxy, + /// using symbol-based comparison for the attribute type. + /// + private static bool HasInternalsVisibleToDynamicProxy( + IAssemblySymbol? assembly, + INamedTypeSymbol? internalsVisibleToAttribute) { if (assembly is null) { return false; } + // If we cannot resolve InternalsVisibleToAttribute (highly unlikely), bail out + // conservatively by not reporting a diagnostic (avoiding false positives). + if (internalsVisibleToAttribute is null) + { + return true; + } + foreach (AttributeData attribute in assembly.GetAttributes()) { if (attribute.AttributeClass is null) @@ -218,17 +231,15 @@ private static bool HasInternalsVisibleToDynamicProxy(IAssemblySymbol? assembly) continue; } - if (!string.Equals( - attribute.AttributeClass.ToDisplayString(), - "System.Runtime.CompilerServices.InternalsVisibleToAttribute", - StringComparison.Ordinal)) + // Symbol-based comparison instead of string-based ToDisplayString() + if (!SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, internalsVisibleToAttribute)) { continue; } if (attribute.ConstructorArguments.Length == 1 && attribute.ConstructorArguments[0].Value is string assemblyName && - AssemblyNameStartsWithDynamicProxy(assemblyName)) + IsDynamicProxyAssemblyName(assemblyName)) { return true; } @@ -238,15 +249,26 @@ attribute.ConstructorArguments[0].Value is string assemblyName && } /// - /// Checks if the assembly name starts with the DynamicProxy assembly name. - /// The InternalsVisibleTo attribute value can include a public key, so we - /// check for a prefix match rather than an exact match. + /// Checks if the assembly name matches DynamicProxy. The InternalsVisibleTo attribute + /// value can be either the simple name ("DynamicProxyGenAssembly2") or include a + /// public key token ("DynamicProxyGenAssembly2, PublicKey=..."). We match the exact + /// name followed by either end-of-string or a comma separator. /// - private static bool AssemblyNameStartsWithDynamicProxy(string assemblyName) + private static bool IsDynamicProxyAssemblyName(string assemblyName) { - return assemblyName.StartsWith(DynamicProxyAssemblyName, StringComparison.Ordinal); + if (!assemblyName.StartsWith(DynamicProxyAssemblyName, StringComparison.Ordinal)) + { + return false; + } + + // Must be exact match or followed by comma (for public key suffix) + return assemblyName.Length == DynamicProxyAssemblyName.Length || + assemblyName[DynamicProxyAssemblyName.Length] == ','; } + /// + /// Attempts to locate the type argument in the syntax tree for precise diagnostic reporting. + /// private static Location GetDiagnosticLocation(IOperation operation, SyntaxNode fallbackSyntax) { TypeSyntax? typeArgument = operation.Syntax diff --git a/src/Common/DiagnosticIds.cs b/src/Common/DiagnosticIds.cs index f46c498c3..172efca89 100644 --- a/src/Common/DiagnosticIds.cs +++ b/src/Common/DiagnosticIds.cs @@ -8,7 +8,6 @@ internal static class DiagnosticIds internal const string NoConstructorArgumentsForInterfaceMockRuleId = "Moq1001"; internal const string NoMatchingConstructorRuleId = "Moq1002"; internal const string InternalTypeMustHaveInternalsVisibleTo = "Moq1003"; - internal const string LoggerShouldNotBeMocked = "Moq1004"; internal const string BadCallbackParameters = "Moq1100"; internal const string PropertySetupUsedForMethod = "Moq1101"; internal const string SetupOnlyUsedForOverridableMembers = "Moq1200"; diff --git a/src/Common/WellKnown/KnownSymbols.cs b/src/Common/WellKnown/KnownSymbols.cs index 70e5dbb16..7007b3d35 100644 --- a/src/Common/WellKnown/KnownSymbols.cs +++ b/src/Common/WellKnown/KnownSymbols.cs @@ -57,5 +57,10 @@ public KnownSymbols(Compilation compilation) /// public INamedTypeSymbol? Action1 => TypeProvider.GetOrCreateTypeByMetadataName("System.Action`1"); + /// + /// Gets the class . + /// + public INamedTypeSymbol? InternalsVisibleToAttribute => TypeProvider.GetOrCreateTypeByMetadataName("System.Runtime.CompilerServices.InternalsVisibleToAttribute"); + protected WellKnownTypeProvider TypeProvider { get; } } diff --git a/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs b/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs index 07a6ad1de..f99802ae8 100644 --- a/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs @@ -263,4 +263,77 @@ private void Test() """, referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); } + + [Fact] + public async Task ShouldNotFlagWhenMultipleAttributesIncludeDynamicProxy() + { + await Verifier.VerifyAnalyzerAsync( + """ + using Moq; + using System.Runtime.CompilerServices; + + [assembly: InternalsVisibleTo("SomeOtherAssembly")] + [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] + + internal class InternalClass { public virtual void DoWork() { } } + + internal class UnitTest + { + private void Test() + { + var mock = new Mock(); + } + } + """, + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + } + + [Fact] + public async Task ShouldFlagInternalTypeWithSimilarButWrongAssemblyName() + { + await Verifier.VerifyAnalyzerAsync( + """ + using Moq; + using System.Runtime.CompilerServices; + + [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2Extra")] + + internal class InternalClass { public virtual void DoWork() { } } + + internal class UnitTest + { + private void Test() + { + var mock = new Mock<{|Moq1003:InternalClass|}>(); + } + } + """, + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + } + + [Fact] + public async Task ShouldFlagProtectedInternalNestedType() + { + // protected internal nested type requires InternalsVisibleTo because + // DynamicProxy does not derive from the containing type and cannot + // access the type via the protected path. + await Verifier.VerifyAnalyzerAsync( + """ + using Moq; + + public class PublicBase + { + protected internal class ProtectedInternalNested { public virtual void DoWork() { } } + } + + internal class UnitTest + { + private void Test() + { + var mock = new Mock<{|Moq1003:PublicBase.ProtectedInternalNested|}>(); + } + } + """, + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + } } From 85d73f0a423545d9c826b8aa3902612fad61e153 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 14:32:35 -0600 Subject: [PATCH 04/11] fix: restore Moq1004 entry for merge compatibility with PR #957 Co-Authored-By: Claude Opus 4.6 --- src/Analyzers/AnalyzerReleases.Unshipped.md | 1 + src/Common/DiagnosticIds.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index c25ae7a25..58b81b6d5 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -8,6 +8,7 @@ Moq1000 | Usage | Warning | NoSealedClassMocksAnalyzer (updated category from Mo Moq1001 | Usage | Warning | NoConstructorArgumentsForInterfaceMockRuleId (updated category from Moq to Usage) Moq1002 | Usage | Warning | NoMatchingConstructorRuleId (updated category from Moq to Usage) Moq1003 | Usage | Warning | InternalTypeMustHaveInternalsVisibleToAnalyzer +Moq1004 | Usage | Warning | NoMockOfLoggerAnalyzer Moq1100 | Usage | Warning | CallbackSignatureShouldMatchMockedMethodAnalyzer (updated category from Moq to Usage) Moq1101 | Usage | Warning | NoMethodsInPropertySetupAnalyzer (updated category from Moq to Usage) Moq1200 | Usage | Error | SetupShouldBeUsedOnlyForOverridableMembersAnalyzer (updated category from Moq to Usage) diff --git a/src/Common/DiagnosticIds.cs b/src/Common/DiagnosticIds.cs index 172efca89..f46c498c3 100644 --- a/src/Common/DiagnosticIds.cs +++ b/src/Common/DiagnosticIds.cs @@ -8,6 +8,7 @@ internal static class DiagnosticIds internal const string NoConstructorArgumentsForInterfaceMockRuleId = "Moq1001"; internal const string NoMatchingConstructorRuleId = "Moq1002"; internal const string InternalTypeMustHaveInternalsVisibleTo = "Moq1003"; + internal const string LoggerShouldNotBeMocked = "Moq1004"; internal const string BadCallbackParameters = "Moq1100"; internal const string PropertySetupUsedForMethod = "Moq1101"; internal const string SetupOnlyUsedForOverridableMembers = "Moq1200"; From 28d6e4089d98de472305859fccf352015665ee0e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 1 Mar 2026 20:31:48 +0000 Subject: [PATCH 05/11] fix(analyzers): add missing accessibility checks and extract shared mock detection helpers - Add ProtectedOrInternal and ProtectedAndInternal accessibility checks to IsEffectivelyInternal method in InternalTypeMustHaveInternalsVisibleToAnalyzer - Extract duplicated mock detection methods (IsValidMockCreation, TryGetMockedTypeFromGeneric, IsMockOfMethod, GetDiagnosticLocation) into MockDetectionHelpers common utility class - Update both InternalTypeMustHaveInternalsVisibleToAnalyzer and NoSealedClassMocksAnalyzer to use the shared helpers --- ...lTypeMustHaveInternalsVisibleToAnalyzer.cs | 54 +--------- src/Analyzers/NoSealedClassMocksAnalyzer.cs | 100 ++---------------- src/Common/MockDetectionHelpers.cs | 92 ++++++++++++++++ 3 files changed, 106 insertions(+), 140 deletions(-) create mode 100644 src/Common/MockDetectionHelpers.cs diff --git a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs index 4949f85df..5a39a6f0d 100644 --- a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs +++ b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs @@ -1,5 +1,5 @@ -using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.Operations; +using Moq.Analyzers.Common; namespace Moq.Analyzers; @@ -67,14 +67,14 @@ private static void Analyze( Location? diagnosticLocation = null; if (context.Operation is IObjectCreationOperation creation && - IsValidMockCreation(creation, knownSymbols, out mockedType)) + MockDetectionHelpers.IsValidMockCreation(creation, knownSymbols, out mockedType)) { - diagnosticLocation = GetDiagnosticLocation(context.Operation, creation.Syntax); + diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, creation.Syntax); } else if (context.Operation is IInvocationOperation invocation && IsValidMockInvocation(invocation, knownSymbols, out mockedType)) { - diagnosticLocation = GetDiagnosticLocation(context.Operation, invocation.Syntax); + diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, invocation.Syntax); } else { @@ -90,25 +90,10 @@ private static void Analyze( } } - private static bool IsValidMockCreation( - IObjectCreationOperation creation, - MoqKnownSymbols knownSymbols, - [NotNullWhen(true)] out ITypeSymbol? mockedType) - { - mockedType = null; - - if (creation.Type is null || creation.Constructor is null || !creation.Type.IsInstanceOf(knownSymbols.Mock1)) - { - return false; - } - - return TryGetMockedTypeFromGeneric(creation.Type, out mockedType); - } - private static bool IsValidMockInvocation( IInvocationOperation invocation, MoqKnownSymbols knownSymbols, - [NotNullWhen(true)] out ITypeSymbol? mockedType) + [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out ITypeSymbol? mockedType) { mockedType = null; @@ -141,19 +126,6 @@ private static bool IsValidMockInvocation( return false; } - private static bool TryGetMockedTypeFromGeneric(ITypeSymbol type, [NotNullWhen(true)] out ITypeSymbol? mockedType) - { - mockedType = null; - - if (type is not INamedTypeSymbol namedType || namedType.TypeArguments.Length != 1) - { - return false; - } - - mockedType = namedType.TypeArguments[0]; - return true; - } - /// /// Determines whether the mocked type is effectively internal and its assembly /// lacks InternalsVisibleTo for DynamicProxy. @@ -265,20 +237,4 @@ private static bool IsDynamicProxyAssemblyName(string assemblyName) return assemblyName.Length == DynamicProxyAssemblyName.Length || assemblyName[DynamicProxyAssemblyName.Length] == ','; } - - /// - /// Attempts to locate the type argument in the syntax tree for precise diagnostic reporting. - /// - private static Location GetDiagnosticLocation(IOperation operation, SyntaxNode fallbackSyntax) - { - TypeSyntax? typeArgument = operation.Syntax - .DescendantNodes() - .OfType() - .FirstOrDefault()? - .TypeArgumentList? - .Arguments - .FirstOrDefault(); - - return typeArgument?.GetLocation() ?? fallbackSyntax.GetLocation(); - } } diff --git a/src/Analyzers/NoSealedClassMocksAnalyzer.cs b/src/Analyzers/NoSealedClassMocksAnalyzer.cs index 1078ab7eb..cd4b33428 100644 --- a/src/Analyzers/NoSealedClassMocksAnalyzer.cs +++ b/src/Analyzers/NoSealedClassMocksAnalyzer.cs @@ -1,5 +1,5 @@ -using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.Operations; +using Moq.Analyzers.Common; namespace Moq.Analyzers; @@ -70,20 +70,19 @@ private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols kn // Handle object creation: new Mock{T}() if (context.Operation is IObjectCreationOperation creation && - IsValidMockCreation(creation, knownSymbols, out mockedType)) + MockDetectionHelpers.IsValidMockCreation(creation, knownSymbols, out mockedType)) { - diagnosticLocation = GetDiagnosticLocationForObjectCreation(context.Operation, creation); + diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, creation.Syntax); } // Handle static method invocation: Mock.Of{T}() else if (context.Operation is IInvocationOperation invocation && IsValidMockOfInvocation(invocation, knownSymbols, out mockedType)) { - diagnosticLocation = GetDiagnosticLocationForInvocation(context.Operation, invocation); + diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, invocation.Syntax); } else { - // Operation is neither a Mock object creation nor a Mock.Of invocation that we need to analyze return; } @@ -93,30 +92,18 @@ private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols kn } } - /// - /// Determines if the operation is a valid Mock{T} object creation and extracts the mocked type. - /// - private static bool IsValidMockCreation(IObjectCreationOperation creation, MoqKnownSymbols knownSymbols, [NotNullWhen(true)] out ITypeSymbol? mockedType) - { - mockedType = null; - - if (creation.Type is null || creation.Constructor is null || !creation.Type.IsInstanceOf(knownSymbols.Mock1)) - { - return false; - } - - return TryGetMockedTypeFromGeneric(creation.Type, out mockedType); - } - /// /// Determines if the operation is a valid Mock.Of{T}() invocation and extracts the mocked type. /// - private static bool IsValidMockOfInvocation(IInvocationOperation invocation, MoqKnownSymbols knownSymbols, [NotNullWhen(true)] out ITypeSymbol? mockedType) + private static bool IsValidMockOfInvocation( + IInvocationOperation invocation, + MoqKnownSymbols knownSymbols, + [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out ITypeSymbol? mockedType) { mockedType = null; // Check if this is a static method call to Mock.Of{T}() - if (!IsValidMockOfMethod(invocation.TargetMethod, knownSymbols)) + if (!MockDetectionHelpers.IsMockOfMethod(invocation.TargetMethod, knownSymbols)) { return false; } @@ -131,41 +118,6 @@ private static bool IsValidMockOfInvocation(IInvocationOperation invocation, Moq return false; } - /// - /// Checks if the method symbol represents a static Mock.Of{T}() method. - /// - private static bool IsValidMockOfMethod(IMethodSymbol? targetMethod, MoqKnownSymbols knownSymbols) - { - if (targetMethod is null || !targetMethod.IsStatic) - { - return false; - } - - if (!string.Equals(targetMethod.Name, "Of", StringComparison.Ordinal)) - { - return false; - } - - return targetMethod.ContainingType is not null && - targetMethod.ContainingType.Equals(knownSymbols.Mock, SymbolEqualityComparer.Default); - } - - /// - /// Attempts to extract the mocked type argument from a generic Mock{T} type. - /// - private static bool TryGetMockedTypeFromGeneric(ITypeSymbol type, [NotNullWhen(true)] out ITypeSymbol? mockedType) - { - mockedType = null; - - if (type is not INamedTypeSymbol namedType || namedType.TypeArguments.Length != 1) - { - return false; - } - - mockedType = namedType.TypeArguments[0]; - return true; - } - /// /// Determines whether a diagnostic should be reported for the mocked type based on its characteristics. /// @@ -198,38 +150,4 @@ private static bool ShouldReportDiagnostic(ITypeSymbol mockedType) // For reference types, report if sealed return mockedType.IsSealed; } - - /// - /// Gets the diagnostic location for a Mock{T} object creation. - /// - private static Location GetDiagnosticLocationForObjectCreation(IOperation operation, IObjectCreationOperation creation) - { - return GetDiagnosticLocation(operation, creation.Syntax); - } - - /// - /// Gets the diagnostic location for a Mock.Of{T}() invocation. - /// - private static Location GetDiagnosticLocationForInvocation(IOperation operation, IInvocationOperation invocation) - { - return GetDiagnosticLocation(operation, invocation.Syntax); - } - - /// - /// Attempts to locate the type argument in the syntax tree for precise diagnostic reporting. - /// - private static Location GetDiagnosticLocation(IOperation operation, SyntaxNode fallbackSyntax) - { - // 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 fallback syntax. - TypeSyntax? typeArgument = operation.Syntax - .DescendantNodes() - .OfType() - .FirstOrDefault()? - .TypeArgumentList? - .Arguments - .FirstOrDefault(); - - return typeArgument?.GetLocation() ?? fallbackSyntax.GetLocation(); - } } diff --git a/src/Common/MockDetectionHelpers.cs b/src/Common/MockDetectionHelpers.cs new file mode 100644 index 000000000..6496c5d1c --- /dev/null +++ b/src/Common/MockDetectionHelpers.cs @@ -0,0 +1,92 @@ +using System.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; + +namespace Moq.Analyzers.Common; + +/// +/// Shared helper methods for detecting Moq mock creation patterns across analyzers. +/// +internal static class MockDetectionHelpers +{ + /// + /// Determines if the operation is a valid Mock{T} object creation and extracts the mocked type. + /// + /// The object creation operation to check. + /// The Moq known symbols. + /// When successful, contains the mocked type. + /// if this is a valid Mock{T} creation; otherwise, . + public static bool IsValidMockCreation( + IObjectCreationOperation creation, + MoqKnownSymbols knownSymbols, + [NotNullWhen(true)] out ITypeSymbol? mockedType) + { + mockedType = null; + + if (creation.Type is null || creation.Constructor is null || !creation.Type.IsInstanceOf(knownSymbols.Mock1)) + { + return false; + } + + return TryGetMockedTypeFromGeneric(creation.Type, out mockedType); + } + + /// + /// Attempts to extract the mocked type argument from a generic Mock{T} type. + /// + /// The type to extract from. + /// When successful, contains the mocked type. + /// if the mocked type was extracted; otherwise, . + public static bool TryGetMockedTypeFromGeneric(ITypeSymbol type, [NotNullWhen(true)] out ITypeSymbol? mockedType) + { + mockedType = null; + + if (type is not INamedTypeSymbol namedType || namedType.TypeArguments.Length != 1) + { + return false; + } + + mockedType = namedType.TypeArguments[0]; + return true; + } + + /// + /// Checks if the method symbol represents a static Mock.Of{T}() method. + /// + /// The method symbol to check. + /// The Moq known symbols. + /// if this is the Mock.Of method; otherwise, . + public static bool IsMockOfMethod(IMethodSymbol? targetMethod, MoqKnownSymbols knownSymbols) + { + if (targetMethod is null || !targetMethod.IsStatic) + { + return false; + } + + if (!string.Equals(targetMethod.Name, "Of", StringComparison.Ordinal)) + { + return false; + } + + return targetMethod.ContainingType is not null && + targetMethod.ContainingType.Equals(knownSymbols.Mock, SymbolEqualityComparer.Default); + } + + /// + /// Attempts to locate the type argument in the syntax tree for precise diagnostic reporting. + /// + /// The operation being analyzed. + /// The fallback syntax node if the type argument cannot be found. + /// The location of the type argument, or the fallback syntax location. + public static Location GetDiagnosticLocation(IOperation operation, SyntaxNode fallbackSyntax) + { + TypeSyntax? typeArgument = operation.Syntax + .DescendantNodes() + .OfType() + .FirstOrDefault()? + .TypeArgumentList? + .Arguments + .FirstOrDefault(); + + return typeArgument?.GetLocation() ?? fallbackSyntax.GetLocation(); + } +} From 0de6c9bb9217b19f569907e266e1d0f56ffedfe4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 1 Mar 2026 20:42:51 +0000 Subject: [PATCH 06/11] fix: remove accidentally committed Moq1004 references Remove references to Moq1004 (NoMockOfLoggerAnalyzer) which does not exist: - Remove row from docs/rules/README.md (broken link to Moq1004.md) - Remove entry from AnalyzerReleases.Unshipped.md (premature release entry) - Remove unused LoggerShouldNotBeMocked constant from DiagnosticIds.cs --- docs/rules/README.md | 1 - src/Analyzers/AnalyzerReleases.Unshipped.md | 3 +-- src/Common/DiagnosticIds.cs | 3 +-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/docs/rules/README.md b/docs/rules/README.md index 99527f875..1a674b1b3 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -6,7 +6,6 @@ | [Moq1001](./Moq1001.md) | Usage | Mocked interfaces cannot have constructor parameters | [ConstructorArgumentsShouldMatchAnalyzer.cs](../../src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs) | | [Moq1002](./Moq1002.md) | Usage | Parameters provided into mock do not match any existing constructors | [ConstructorArgumentsShouldMatchAnalyzer.cs](../../src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs) | | [Moq1003](./Moq1003.md) | Usage | Internal type requires InternalsVisibleTo for DynamicProxy | [InternalTypeMustHaveInternalsVisibleToAnalyzer.cs](../../src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs) | -| [Moq1004](./Moq1004.md) | Usage | ILogger should not be mocked | [NoMockOfLoggerAnalyzer.cs](../../src/Analyzers/NoMockOfLoggerAnalyzer.cs) | | [Moq1100](./Moq1100.md) | Correctness | Callback signature must match the signature of the mocked method | [CallbackSignatureShouldMatchMockedMethodAnalyzer.cs](../../src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs) | | [Moq1101](./Moq1101.md) | Usage | SetupGet/SetupSet/SetupProperty should be used for properties, not for methods | [NoMethodsInPropertySetupAnalyzer.cs](../../src/Analyzers/NoMethodsInPropertySetupAnalyzer.cs) | | [Moq1200](./Moq1200.md) | Correctness | Setup should be used only for overridable members | [SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs](../../src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs) | diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index 58b81b6d5..0af3eabdf 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -1,4 +1,4 @@ -; Unshipped analyzer release +; Unshipped analyzer release ; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md ### New Rules @@ -8,7 +8,6 @@ Moq1000 | Usage | Warning | NoSealedClassMocksAnalyzer (updated category from Mo Moq1001 | Usage | Warning | NoConstructorArgumentsForInterfaceMockRuleId (updated category from Moq to Usage) Moq1002 | Usage | Warning | NoMatchingConstructorRuleId (updated category from Moq to Usage) Moq1003 | Usage | Warning | InternalTypeMustHaveInternalsVisibleToAnalyzer -Moq1004 | Usage | Warning | NoMockOfLoggerAnalyzer Moq1100 | Usage | Warning | CallbackSignatureShouldMatchMockedMethodAnalyzer (updated category from Moq to Usage) Moq1101 | Usage | Warning | NoMethodsInPropertySetupAnalyzer (updated category from Moq to Usage) Moq1200 | Usage | Error | SetupShouldBeUsedOnlyForOverridableMembersAnalyzer (updated category from Moq to Usage) diff --git a/src/Common/DiagnosticIds.cs b/src/Common/DiagnosticIds.cs index f46c498c3..7d6e893f6 100644 --- a/src/Common/DiagnosticIds.cs +++ b/src/Common/DiagnosticIds.cs @@ -1,4 +1,4 @@ -namespace Moq.Analyzers.Common; +namespace Moq.Analyzers.Common; #pragma warning disable ECS0200 // Consider using readonly instead of const for flexibility @@ -8,7 +8,6 @@ internal static class DiagnosticIds internal const string NoConstructorArgumentsForInterfaceMockRuleId = "Moq1001"; internal const string NoMatchingConstructorRuleId = "Moq1002"; internal const string InternalTypeMustHaveInternalsVisibleTo = "Moq1003"; - internal const string LoggerShouldNotBeMocked = "Moq1004"; internal const string BadCallbackParameters = "Moq1100"; internal const string PropertySetupUsedForMethod = "Moq1101"; internal const string SetupOnlyUsedForOverridableMembers = "Moq1200"; From ba7194a7a9eeef5c5322022ccf8bf4727be4a8b5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 1 Mar 2026 20:57:57 +0000 Subject: [PATCH 07/11] fix(analyzer): exclude private types from InternalsVisibleTo diagnostic Private types are only accessible within their declaring type, and InternalsVisibleTo cannot help with that. Reporting a diagnostic suggesting to add InternalsVisibleTo for private types provides incorrect guidance. Fixes: 0c611947-6089-4d27-b35c-90039eed5e53 --- .../InternalTypeMustHaveInternalsVisibleToAnalyzer.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs index 5a39a6f0d..59997d883 100644 --- a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs +++ b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs @@ -153,8 +153,10 @@ private static bool ShouldReportDiagnostic( /// (private protected) /// (protected internal) on /// a containing type, because DynamicProxy does not derive from the container - /// on a nested type /// + /// Note: and + /// are excluded because InternalsVisibleTo cannot help with those - private types are + /// only accessible within their declaring type, and protected types require inheritance. /// private static bool IsEffectivelyInternal(ITypeSymbol type) { @@ -166,7 +168,6 @@ private static bool IsEffectivelyInternal(ITypeSymbol type) case Accessibility.Internal: case Accessibility.ProtectedAndInternal: case Accessibility.ProtectedOrInternal: - case Accessibility.Private: return true; } From 0507c7497cde23e291cddf4d38252df22583ec04 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 1 Mar 2026 21:11:57 +0000 Subject: [PATCH 08/11] fix(analyzer): exclude private protected from InternalsVisibleTo diagnostic Remove Accessibility.ProtectedAndInternal from IsEffectivelyInternal check. For private protected types, InternalsVisibleTo satisfies only the assembly requirement but not the inheritance requirement from the containing type, which DynamicProxy does not provide. Suggesting InternalsVisibleTo for such types is misleading since it alone won't make the mock work. --- .../InternalTypeMustHaveInternalsVisibleToAnalyzer.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs index 59997d883..688788a70 100644 --- a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs +++ b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs @@ -150,13 +150,14 @@ private static bool ShouldReportDiagnostic( /// or its containers make it inaccessible to DynamicProxy without InternalsVisibleTo: /// /// (internal) - /// (private protected) /// (protected internal) on /// a containing type, because DynamicProxy does not derive from the container /// - /// Note: and - /// are excluded because InternalsVisibleTo cannot help with those - private types are - /// only accessible within their declaring type, and protected types require inheritance. + /// Note: , , + /// and (private protected) are excluded + /// because InternalsVisibleTo cannot help with those - private types are only accessible + /// within their declaring type, and protected/private protected types require inheritance + /// from the containing type, which DynamicProxy does not provide. /// private static bool IsEffectivelyInternal(ITypeSymbol type) { @@ -166,7 +167,6 @@ private static bool IsEffectivelyInternal(ITypeSymbol type) switch (current.DeclaredAccessibility) { case Accessibility.Internal: - case Accessibility.ProtectedAndInternal: case Accessibility.ProtectedOrInternal: return true; } From 49aaedd91e48bf911942297cf4b5311f49117245 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 15:13:14 -0600 Subject: [PATCH 09/11] test: add MockRepository.Create coverage and fix no-Moq test for Moq1003 Add MockRepository.Create() test cases to both positive (internal type triggers Moq1003) and negative (public type does not trigger) parameterized test data sets. Also add the MockRepository path to the InternalsVisibleTo attribute verification test. Fix ShouldNotAnalyzeWhenMoqNotReferenced to use Net80 (no Moq package) instead of Net80WithOldMoq, so IsMockReferenced() actually returns false and the early-return path gets covered. Add CompilerDiagnostics.None to suppress errors from the global "using Moq;" injected by the test harness. Add Net80 entry to ReferenceAssemblyCatalog for .NET 8.0 without Moq, enabling tests that verify analyzer behavior when Moq is absent. Co-Authored-By: Claude Opus 4.6 --- .../Helpers/ReferenceAssemblyCatalog.cs | 10 +++++++++- ...rnalTypeMustHaveInternalsVisibleToAnalyzerTests.cs | 11 ++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs b/tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs index 3d6af60ca..ae8f5a2e8 100644 --- a/tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs +++ b/tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs @@ -24,11 +24,16 @@ public static class ReferenceAssemblyCatalog /// public static string Net80WithNewMoq => nameof(Net80WithNewMoq); + /// + /// Gets the name of the reference assembly group for .NET 8.0 without Moq. + /// + public static string Net80 => nameof(Net80); + /// /// Gets the catalog of reference assemblies. /// /// - /// The key is the name of the reference assembly group ( and ). + /// The key is the name of the reference assembly group (, , and ). /// public static IReadOnlyDictionary Catalog { get; } = new Dictionary(StringComparer.Ordinal) { @@ -39,5 +44,8 @@ public static class ReferenceAssemblyCatalog // This must be 4.12.0 or later in order to have the new `Mock.Of(MockBehavior)` method (see https://github.com/devlooped/moq/commit/1561c006c87a0894c5257a1e541da44e40e33dd3). // 4.18.4 is currently the most downloaded version of Moq. { nameof(Net80WithNewMoq), ReferenceAssemblies.Net.Net80.AddPackages([new PackageIdentity("Moq", "4.18.4")]) }, + + // .NET 8.0 without Moq, used to verify analyzers bail out gracefully when Moq is not referenced. + { nameof(Net80), ReferenceAssemblies.Net.Net80 }, }; } diff --git a/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs b/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs index f99802ae8..cd8701353 100644 --- a/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs @@ -1,3 +1,4 @@ +using Microsoft.CodeAnalysis.Testing; using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; namespace Moq.Analyzers.Test; @@ -12,6 +13,7 @@ public static IEnumerable InternalTypeWithoutAttributeTestData() ["""new Mock<{|Moq1003:InternalClass|}>(MockBehavior.Strict)"""], ["""Mock.Of<{|Moq1003:InternalClass|}>()"""], ["""var mock = new Mock<{|Moq1003:InternalClass|}>()"""], + ["""var repo = new MockRepository(MockBehavior.Strict); repo.Create<{|Moq1003:InternalClass|}>()"""], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); } @@ -23,6 +25,7 @@ public static IEnumerable PublicTypeTestData() ["""new Mock(MockBehavior.Strict)"""], ["""Mock.Of()"""], ["""var mock = new Mock()"""], + ["""var repo = new MockRepository(MockBehavior.Strict); repo.Create()"""], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); } @@ -170,6 +173,8 @@ private void Test() { var mock = new Mock(); var of = Mock.Of(); + var repo = new MockRepository(MockBehavior.Strict); + repo.Create(); } } """, @@ -226,6 +231,9 @@ private void Test() [Fact] public async Task ShouldNotAnalyzeWhenMoqNotReferenced() { + // Use Net80 (no Moq) so IsMockReferenced() returns false and the analyzer + // bails out early. CompilerDiagnostics.None suppresses errors caused by the + // global "using Moq;" that the test harness injects. await Verifier.VerifyAnalyzerAsync( """ namespace Test @@ -241,7 +249,8 @@ private void Test() } } """, - referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq); + referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80, + CompilerDiagnostics.None); } [Fact] From 437c69d3f414dac53ef11656b8e9ee182bba186f Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 15:58:25 -0600 Subject: [PATCH 10/11] fix: revert separator spacing and disable MD060 for Roslyn compatibility The Roslyn RS2007 analyzer requires AnalyzerReleases.md separator rows without spaces around pipes. Disable MD060 (table-column-style) globally since the format is controlled by Roslyn's release tracking analyzer. Co-Authored-By: Claude Opus 4.6 --- .markdownlint.json | 3 ++- src/Analyzers/AnalyzerReleases.Unshipped.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.markdownlint.json b/.markdownlint.json index 484d3db4c..df6139201 100644 --- a/.markdownlint.json +++ b/.markdownlint.json @@ -2,5 +2,6 @@ "MD013": false, "MD024": false, "MD033": false, - "MD041": false + "MD041": false, + "MD060": false } diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index 5c41b6889..3589f1c95 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -4,7 +4,7 @@ ### New Rules Rule ID | Category | Severity | Notes --------- | ---------- | ---------- | ------- +--------|----------|----------|------- Moq1000 | Usage | Warning | NoSealedClassMocksAnalyzer (updated category from Moq to Usage) Moq1001 | Usage | Warning | NoConstructorArgumentsForInterfaceMockRuleId (updated category from Moq to Usage) Moq1002 | Usage | Warning | NoMatchingConstructorRuleId (updated category from Moq to Usage) From 7ce02ad196158c689a829544900452dbca7609fa Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 1 Mar 2026 22:19:49 +0000 Subject: [PATCH 11/11] refactor: consolidate mock invocation detection into shared helper Moves duplicated IsValidMockInvocation method from InternalTypeMustHaveInternalsVisibleToAnalyzer and NoMockOfLoggerAnalyzer into MockDetectionHelpers as a shared method that handles both Mock.Of() and MockRepository.Create() invocations. --- ...lTypeMustHaveInternalsVisibleToAnalyzer.cs | 38 +------------------ src/Analyzers/NoMockOfLoggerAnalyzer.cs | 28 +------------- src/Common/MockDetectionHelpers.cs | 31 +++++++++++++++ 3 files changed, 33 insertions(+), 64 deletions(-) diff --git a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs index 688788a70..b96cf3eb4 100644 --- a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs +++ b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs @@ -72,7 +72,7 @@ private static void Analyze( diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, creation.Syntax); } else if (context.Operation is IInvocationOperation invocation && - IsValidMockInvocation(invocation, knownSymbols, out mockedType)) + MockDetectionHelpers.IsValidMockInvocation(invocation, knownSymbols, out mockedType)) { diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, invocation.Syntax); } @@ -90,42 +90,6 @@ private static void Analyze( } } - private static bool IsValidMockInvocation( - IInvocationOperation invocation, - MoqKnownSymbols knownSymbols, - [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out ITypeSymbol? mockedType) - { - mockedType = null; - - IMethodSymbol targetMethod = invocation.TargetMethod; - - // Mock.Of() -- use symbol-based comparison via MoqKnownSymbols.MockOf - if (targetMethod.IsInstanceOf(knownSymbols.MockOf)) - { - if (targetMethod.TypeArguments.Length == 1) - { - mockedType = targetMethod.TypeArguments[0]; - return true; - } - - return false; - } - - // MockRepository.Create() - if (targetMethod.IsInstanceOf(knownSymbols.MockRepositoryCreate)) - { - if (targetMethod.TypeArguments.Length == 1) - { - mockedType = targetMethod.TypeArguments[0]; - return true; - } - - return false; - } - - return false; - } - /// /// Determines whether the mocked type is effectively internal and its assembly /// lacks InternalsVisibleTo for DynamicProxy. diff --git a/src/Analyzers/NoMockOfLoggerAnalyzer.cs b/src/Analyzers/NoMockOfLoggerAnalyzer.cs index 0b79715bf..f946a1ec2 100644 --- a/src/Analyzers/NoMockOfLoggerAnalyzer.cs +++ b/src/Analyzers/NoMockOfLoggerAnalyzer.cs @@ -84,7 +84,7 @@ private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols kn // Handle static method invocation: Mock.Of{T}() or MockRepository.Create{T}() else if (context.Operation is IInvocationOperation invocation && - IsValidMockInvocation(invocation, knownSymbols, out mockedType)) + MockDetectionHelpers.IsValidMockInvocation(invocation, knownSymbols, out mockedType)) { diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, invocation.Syntax); } @@ -100,32 +100,6 @@ private static void Analyze(OperationAnalysisContext context, MoqKnownSymbols kn } } - /// - /// Determines if the operation is a valid Mock.Of{T}() or MockRepository.Create{T}() invocation - /// and extracts the mocked type. - /// - private static bool IsValidMockInvocation(IInvocationOperation invocation, MoqKnownSymbols knownSymbols, [NotNullWhen(true)] out ITypeSymbol? mockedType) - { - mockedType = null; - - bool isMockOf = MockDetectionHelpers.IsValidMockOfMethod(invocation.TargetMethod, knownSymbols); - bool isMockRepositoryCreate = !isMockOf && invocation.TargetMethod.IsInstanceOf(knownSymbols.MockRepositoryCreate); - - if (!isMockOf && !isMockRepositoryCreate) - { - return false; - } - - // Both Mock.Of{T}() and MockRepository.Create{T}() use a single type argument - if (invocation.TargetMethod.TypeArguments.Length == 1) - { - mockedType = invocation.TargetMethod.TypeArguments[0]; - return true; - } - - return false; - } - /// /// Determines whether the mocked type is ILogger or ILogger{T} using symbol-based comparison. /// diff --git a/src/Common/MockDetectionHelpers.cs b/src/Common/MockDetectionHelpers.cs index b9770aa63..32c0656ba 100644 --- a/src/Common/MockDetectionHelpers.cs +++ b/src/Common/MockDetectionHelpers.cs @@ -53,6 +53,37 @@ public static bool IsValidMockOfInvocation(IInvocationOperation invocation, MoqK return false; } + /// + /// Determines if the operation is a valid Mock.Of{T}() or MockRepository.Create{T}() invocation + /// and extracts the mocked type. + /// + /// The invocation operation. + /// The known Moq symbols. + /// When successful, the mocked type; otherwise, null. + /// True if this is a valid mock invocation; otherwise, false. + public static bool IsValidMockInvocation(IInvocationOperation invocation, MoqKnownSymbols knownSymbols, [NotNullWhen(true)] out ITypeSymbol? mockedType) + { + mockedType = null; + + IMethodSymbol targetMethod = invocation.TargetMethod; + + bool isMockOf = IsValidMockOfMethod(targetMethod, knownSymbols); + bool isMockRepositoryCreate = !isMockOf && targetMethod.IsInstanceOf(knownSymbols.MockRepositoryCreate); + + if (!isMockOf && !isMockRepositoryCreate) + { + return false; + } + + if (targetMethod.TypeArguments.Length == 1) + { + mockedType = targetMethod.TypeArguments[0]; + return true; + } + + return false; + } + /// /// Checks if the method symbol represents a static Mock.Of{T}() method. ///