feat: add Moq1003 analyzer for internal types requiring InternalsVisibleTo#958
feat: add Moq1003 analyzer for internal types requiring InternalsVisibleTo#958
Conversation
) Add Moq1003 analyzer that detects when Mock<T> is used where T is an internal type and the assembly does not have [InternalsVisibleTo("DynamicProxyGenAssembly2")]. Handles new Mock<T>(), Mock.Of<T>(), and MockRepository.Create<T>() patterns. Checks effective accessibility for nested types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Moq.Analyzers suite by introducing a new diagnostic (Moq1003) that proactively identifies potential runtime issues when attempting to mock internal types. By flagging these cases early, it helps developers ensure their mocking setups are valid and prevents unexpected failures during testing, ultimately improving the reliability of Moq-based tests. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (9)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
DeepSource Code ReviewWe reviewed changes in See full review on DeepSource ↗ Code Review Summary
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
There was a problem hiding this comment.
Code Review
This pull request introduces a new analyzer, Moq1003, to detect when an internal type is mocked without the necessary [InternalsVisibleTo] attribute. The implementation is well-structured and includes comprehensive documentation and tests. I've found a potential bug in the analyzer's logic for detecting MockRepository.Create<T>() and noted that the test suite is missing coverage for this specific case. Addressing these points will ensure the new analyzer is fully functional as described.
|
|
||
| private static bool IsMockRepositoryCreateMethod(IMethodSymbol targetMethod, MoqKnownSymbols knownSymbols) | ||
| { | ||
| return targetMethod.IsInstanceOf(knownSymbols.MockRepositoryCreate); |
There was a problem hiding this comment.
The IsInstanceOf extension method typically expects a single ISymbol (like INamedTypeSymbol) as an argument, but knownSymbols.MockRepositoryCreate is an ImmutableArray<IMethodSymbol>. This is likely a bug that will prevent detection of MockRepository.Create<T> usages.
To correctly check if targetMethod is one of the Create methods from MockRepository or its base types, you should check if the original definition of the target method is contained within the MockRepositoryCreate collection.
return knownSymbols.MockRepositoryCreate.Contains(targetMethod.OriginalDefinition, SymbolEqualityComparer.Default);| public static IEnumerable<object[]> InternalTypeWithoutAttributeTestData() | ||
| { |
There was a problem hiding this comment.
The tests are very thorough but appear to be missing coverage for the MockRepository.Create<T>() pattern, which is mentioned in the pull request description as a supported case. Adding tests for this pattern will ensure it is handled correctly by the analyzer and would help catch related issues.
For example, you could add a case to InternalTypeWithoutAttributeTestData:
// In InternalTypeWithoutAttributeTestData:
["""new MockRepository(MockBehavior.Default).Create<{|Moq1003:InternalClass|}>()"""]You would also want to add corresponding passing cases to PublicTypeTestData and other relevant test data collections.
There was a problem hiding this comment.
Pull request overview
Adds a new Roslyn analyzer rule (Moq1003) to warn when Mock<T> (and related factory APIs) are used with effectively-internal types without the required InternalsVisibleTo("DynamicProxyGenAssembly2"), plus initial documentation and tests.
Changes:
- Introduces
InternalTypeMustHaveInternalsVisibleToAnalyzer(Moq1003) to detect internal/effectively-internal mocked types missingInternalsVisibleTo. - Adds Moq1003 documentation and registers the new rule ID in release notes and rule tables.
- Adds analyzer tests for internal/public/interface/nested scenarios and InternalsVisibleTo attribute variations.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs | New tests covering core Moq1003 scenarios and attribute variations. |
| src/Common/DiagnosticIds.cs | Adds the Moq1003 diagnostic ID constant. |
| src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs | New analyzer implementation for Moq1003. |
| src/Analyzers/AnalyzerReleases.Unshipped.md | Adds Moq1003 to the “New Rules” table. |
| docs/rules/README.md | Registers Moq1003 in the rules index table. |
| docs/rules/Moq1003.md | New rule documentation page for Moq1003. |
| [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); | ||
| } |
There was a problem hiding this comment.
ShouldNotAnalyzeWhenMoqNotReferenced still uses ReferenceAssemblyCatalog.Net80WithOldMoq, which does reference Moq. As written, this test can pass even if the analyzer doesn’t correctly short-circuit when Moq isn’t referenced, because there’s no Mock<T> usage to trigger analysis anyway. To actually validate the intended behavior, use a reference-assemblies group without the Moq package (or add one to ReferenceAssemblyCatalog) and keep the source free of Moq types.
| if (current.DeclaredAccessibility == Accessibility.Internal || | ||
| current.DeclaredAccessibility == Accessibility.Friend) | ||
| { | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
IsEffectivelyInternal only checks Accessibility.Internal/Friend. For nested types, Roslyn can report ProtectedOrInternal / ProtectedAndInternal as the declared accessibility; these types still rely on the internal portion for access from DynamicProxyGenAssembly2, so they also require InternalsVisibleTo to be mockable. Consider treating these accessibility values as effectively-internal as well to avoid false negatives.
| 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); |
There was a problem hiding this comment.
IsMockOfMethod uses a string-based method-name check (targetMethod.Name == "Of"). This repo has MoqKnownSymbols.MockOf specifically to do symbol-based matching; using the name is fragile (and violates the symbol-based detection rule) and can lead to false positives/negatives across Moq versions. Consider replacing this with targetMethod.IsInstanceOf(knownSymbols.MockOf) (or IsInstanceOf(..., out match) if you need the matched overload).
| 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); | |
| return targetMethod.IsInstanceOf(knownSymbols.MockOf); |
| if (!string.Equals( | ||
| attribute.AttributeClass.ToDisplayString(), | ||
| "System.Runtime.CompilerServices.InternalsVisibleToAttribute", | ||
| StringComparison.Ordinal)) | ||
| { |
There was a problem hiding this comment.
HasInternalsVisibleToDynamicProxy matches InternalsVisibleToAttribute by comparing AttributeClass.ToDisplayString() to a hard-coded string. This allocates strings and is sensitive to display format. Prefer resolving the InternalsVisibleToAttribute symbol once (e.g., via Compilation.GetTypeByMetadataName) and comparing with SymbolEqualityComparer.Default.
| /// 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. | ||
| /// </summary> | ||
| private static bool AssemblyNameStartsWithDynamicProxy(string assemblyName) | ||
| { | ||
| return assemblyName.StartsWith(DynamicProxyAssemblyName, StringComparison.Ordinal); |
There was a problem hiding this comment.
AssemblyNameStartsWithDynamicProxy uses StartsWith("DynamicProxyGenAssembly2"). This will incorrectly treat values like "DynamicProxyGenAssembly2SomethingElse" as valid and suppress the diagnostic (false negative). Consider parsing the InternalsVisibleTo value up to the first comma (and trimming) and then comparing the simple name for exact equality.
| /// 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. | |
| /// </summary> | |
| private static bool AssemblyNameStartsWithDynamicProxy(string assemblyName) | |
| { | |
| return assemblyName.StartsWith(DynamicProxyAssemblyName, StringComparison.Ordinal); | |
| /// Checks if the simple assembly name matches the DynamicProxy assembly name. | |
| /// The InternalsVisibleTo attribute value can include a public key and other | |
| /// metadata after a comma, so we compare only the simple name (before the | |
| /// first comma) for exact equality. | |
| /// </summary> | |
| private static bool AssemblyNameStartsWithDynamicProxy(string assemblyName) | |
| { | |
| int commaIndex = assemblyName.IndexOf(','); | |
| string simpleName = commaIndex >= 0 | |
| ? assemblyName[..commaIndex] | |
| : assemblyName; | |
| simpleName = simpleName.Trim(); | |
| return string.Equals(simpleName, DynamicProxyAssemblyName, StringComparison.Ordinal); |
| 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; | ||
| } |
There was a problem hiding this comment.
HasInternalsVisibleToDynamicProxy is called per matched mock usage and iterates assembly.GetAttributes() each time. In a large test suite this could become a hot path (especially with many mocks). Consider caching the computed result per IAssemblySymbol in the compilation-start scope (thread-safe) so each assembly’s attributes are scanned at most once.
| public static IEnumerable<object[]> 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(); |
There was a problem hiding this comment.
The PR description says Moq1003 handles MockRepository.Create<T>(), but the test data here doesn’t include any MockRepository.Create<T>() pattern. Add at least one positive and one negative case exercising Create<T>() so the invocation-path logic is covered (and covered for both old/new Moq reference groups).
src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs
Outdated
Show resolved
Hide resolved
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 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<T>() 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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ock 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
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs
Outdated
Show resolved
Hide resolved
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
Moq1003 (InternalTypeMustHaveInternalsVisibleToAnalyzer) is declared in the unshipped release tracking, DiagnosticIds, and docs but has no analyzer class in this branch. This causes RS2002: "Rule 'Moq1003' is part of the next unshipped analyzer release, but is not a supported diagnostic for any analyzer." Remove these entries; they belong in the PR that introduces the actual Moq1003 analyzer (#958). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Misleading diagnostic for private protected types
- Removed Accessibility.ProtectedAndInternal from IsEffectivelyInternal since InternalsVisibleTo cannot satisfy the inheritance requirement for private protected types.
Preview (0507c7497c)
diff --git a/docs/rules/Moq1003.md b/docs/rules/Moq1003.md
new file mode 100644
--- /dev/null
+++ b/docs/rules/Moq1003.md
@@ -1,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<T>`, 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<MyService>(); // 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<MyService>(); // 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<MyService>(); // 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
--- 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
--- a/src/Analyzers/AnalyzerReleases.Unshipped.md
+++ b/src/Analyzers/AnalyzerReleases.Unshipped.md
@@ -7,6 +7,7 @@
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
--- /dev/null
+++ b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs
@@ -1,0 +1,241 @@
+using Microsoft.CodeAnalysis.Operations;
+using Moq.Analyzers.Common;
+
+namespace Moq.Analyzers;
+
+/// <summary>
+/// Detects when <c>Mock<T></c> is used where <c>T</c> is an <see langword="internal"/> type
+/// and the assembly containing <c>T</c> does not have
+/// <c>[InternalsVisibleTo("DynamicProxyGenAssembly2")]</c>.
+/// </summary>
+[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");
+
+ /// <inheritdoc />
+ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+
+ /// <inheritdoc />
+ 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 &&
+ MockDetectionHelpers.IsValidMockCreation(creation, knownSymbols, out mockedType))
+ {
+ diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, creation.Syntax);
+ }
+ else if (context.Operation is IInvocationOperation invocation &&
+ IsValidMockInvocation(invocation, knownSymbols, out mockedType))
+ {
+ diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, invocation.Syntax);
+ }
+ else
+ {
+ return;
+ }
+
+ if (mockedType != null && diagnosticLocation != null &&
+ ShouldReportDiagnostic(mockedType, knownSymbols.InternalsVisibleToAttribute))
+ {
+ context.ReportDiagnostic(diagnosticLocation.CreateDiagnostic(
+ Rule,
+ mockedType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat)));
+ }
+ }
+
+ private static bool IsValidMockInvocation(
+ IInvocationOperation invocation,
+ MoqKnownSymbols knownSymbols,
+ [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out ITypeSymbol? mockedType)
+ {
+ mockedType = null;
+
+ IMethodSymbol targetMethod = invocation.TargetMethod;
+
+ // Mock.Of<T>() -- 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<T>()
+ if (targetMethod.IsInstanceOf(knownSymbols.MockRepositoryCreate))
+ {
+ if (targetMethod.TypeArguments.Length == 1)
+ {
+ mockedType = targetMethod.TypeArguments[0];
+ return true;
+ }
+
+ return false;
+ }
+
+ return false;
+ }
+
+ /// <summary>
+ /// Determines whether the mocked type is effectively internal and its assembly
+ /// lacks InternalsVisibleTo for DynamicProxy.
+ /// </summary>
+ private static bool ShouldReportDiagnostic(
+ ITypeSymbol mockedType,
+ INamedTypeSymbol? internalsVisibleToAttribute)
+ {
+ if (!IsEffectivelyInternal(mockedType))
+ {
+ return false;
+ }
+
+ return !HasInternalsVisibleToDynamicProxy(mockedType.ContainingAssembly, internalsVisibleToAttribute);
+ }
+
+ /// <summary>
+ /// 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:
+ /// <list type="bullet">
+ /// <item><see cref="Accessibility.Internal"/> (internal)</item>
+ /// <item><see cref="Accessibility.ProtectedOrInternal"/> (protected internal) on
+ /// a containing type, because DynamicProxy does not derive from the container</item>
+ /// </list>
+ /// Note: <see cref="Accessibility.Private"/>, <see cref="Accessibility.Protected"/>,
+ /// and <see cref="Accessibility.ProtectedAndInternal"/> (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.
+ /// </summary>
+ private static bool IsEffectivelyInternal(ITypeSymbol type)
+ {
+ ITypeSymbol? current = type;
+ while (current != null)
+ {
+ switch (current.DeclaredAccessibility)
+ {
+ case Accessibility.Internal:
+ case Accessibility.ProtectedOrInternal:
+ return true;
+ }
+
+ current = current.ContainingType;
+ }
+
+ return false;
+ }
+
+ /// <summary>
+ /// Checks the assembly's attributes for InternalsVisibleTo targeting DynamicProxy,
+ /// using symbol-based comparison for the attribute type.
+ /// </summary>
+ 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)
+ {
+ continue;
+ }
+
+ // 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 &&
+ IsDynamicProxyAssemblyName(assemblyName))
+ {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ /// <summary>
+ /// 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.
+ /// </summary>
+ private static bool IsDynamicProxyAssemblyName(string assemblyName)
+ {
+ 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] == ',';
+ }
+}
diff --git a/src/Analyzers/NoSealedClassMocksAnalyzer.cs b/src/Analyzers/NoSealedClassMocksAnalyzer.cs
--- 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 @@
// 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;
}
@@ -94,29 +93,17 @@
}
/// <summary>
- /// Determines if the operation is a valid Mock{T} object creation and extracts the mocked type.
- /// </summary>
- 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);
- }
-
- /// <summary>
/// Determines if the operation is a valid Mock.Of{T}() invocation and extracts the mocked type.
/// </summary>
- 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;
}
@@ -132,41 +119,6 @@
}
/// <summary>
- /// Checks if the method symbol represents a static Mock.Of{T}() method.
- /// </summary>
- 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);
- }
-
- /// <summary>
- /// Attempts to extract the mocked type argument from a generic Mock{T} type.
- /// </summary>
- 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;
- }
-
- /// <summary>
/// Determines whether a diagnostic should be reported for the mocked type based on its characteristics.
/// </summary>
/// <param name="mockedType">The type being mocked.</param>
@@ -198,38 +150,4 @@
// For reference types, report if sealed
return mockedType.IsSealed;
}
-
- /// <summary>
- /// Gets the diagnostic location for a Mock{T} object creation.
- /// </summary>
- private static Location GetDiagnosticLocationForObjectCreation(IOperation operation, IObjectCreationOperation creation)
- {
- return GetDiagnosticLocation(operation, creation.Syntax);
- }
-
- /// <summary>
- /// Gets the diagnostic location for a Mock.Of{T}() invocation.
- /// </summary>
- private static Location GetDiagnosticLocationForInvocation(IOperation operation, IInvocationOperation invocation)
- {
- return GetDiagnosticLocation(operation, invocation.Syntax);
- }
-
- /// <summary>
- /// Attempts to locate the type argument in the syntax tree for precise diagnostic reporting.
- /// </summary>
- 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<GenericNameSyntax>()
- .FirstOrDefault()?
- .TypeArgumentList?
- .Arguments
- .FirstOrDefault();
-
- return typeArgument?.GetLocation() ?? fallbackSyntax.GetLocation();
- }
}
diff --git a/src/Common/DiagnosticIds.cs b/src/Common/DiagnosticIds.cs
--- a/src/Common/DiagnosticIds.cs
+++ b/src/Common/DiagnosticIds.cs
@@ -7,6 +7,7 @@
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/src/Common/MockDetectionHelpers.cs b/src/Common/MockDetectionHelpers.cs
new file mode 100644
--- /dev/null
+++ b/src/Common/MockDetectionHelpers.cs
@@ -1,0 +1,92 @@
+using System.Diagnostics.CodeAnalysis;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Moq.Analyzers.Common;
+
+/// <summary>
+/// Shared helper methods for detecting Moq mock creation patterns across analyzers.
+/// </summary>
+internal static class MockDetectionHelpers
+{
+ /// <summary>
+ /// Determines if the operation is a valid <c>Mock{T}</c> object creation and extracts the mocked type.
+ /// </summary>
+ /// <param name="creation">The object creation operation to check.</param>
+ /// <param name="knownSymbols">The Moq known symbols.</param>
+ /// <param name="mockedType">When successful, contains the mocked type.</param>
+ /// <returns><see langword="true"/> if this is a valid <c>Mock{T}</c> creation; otherwise, <see langword="false"/>.</returns>
+ 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);
+ }
+
+ /// <summary>
+ /// Attempts to extract the mocked type argument from a generic <c>Mock{T}</c> type.
+ /// </summary>
+ /// <param name="type">The type to extract from.</param>
+ /// <param name="mockedType">When successful, contains the mocked type.</param>
+ /// <returns><see langword="true"/> if the mocked type was extracted; otherwise, <see langword="false"/>.</returns>
+ 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;
+ }
+
+ /// <summary>
+ /// Checks if the method symbol represents a static <c>Mock.Of{T}()</c> method.
+ /// </summary>
+ /// <param name="targetMethod">The method symbol to check.</param>
+ /// <param name="knownSymbols">The Moq known symbols.</param>
+ /// <returns><see langword="true"/> if this is the <c>Mock.Of</c> method; otherwise, <see langword="false"/>.</returns>
+ 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);
+ }
+
+ /// <summary>
+ /// Attempts to locate the type argument in the syntax tree for precise diagnostic reporting.
+ /// </summary>
+ /// <param name="operation">The operation being analyzed.</param>
+ /// <param name="fallbackSyntax">The fallback syntax node if the type argument cannot be found.</param>
+ /// <returns>The location of the type argument, or the fallback syntax location.</returns>
+ public static Location GetDiagnosticLocation(IOperation operation, SyntaxNode fallbackSyntax)
+ {
+ TypeSyntax? typeArgument = operation.Syntax
+ .DescendantNodes()
+ .OfType<GenericNameSyntax>()
+ .FirstOrDefault()?
+ .TypeArgumentList?
+ .Arguments
+ .FirstOrDefault();
+
+ return typeArgument?.GetLocation() ?? fallbackSyntax.GetLocation();
+ }
+}
diff --git a/src/Common/WellKnown/KnownSymbols.cs b/src/Common/WellKnown/KnownSymbols.cs
--- a/src/Common/WellKnown/KnownSymbols.cs
+++ b/src/Common/WellKnown/KnownSymbols.cs
@@ -57,5 +57,10 @@
/// </summary>
public INamedTypeSymbol? Action1 => TypeProvider.GetOrCreateTypeByMetadataName("System.Action`1");
+ /// <summary>
+ /// Gets the class <see cref="System.Runtime.CompilerServices.InternalsVisibleToAttribute"/>.
+ /// </summary>
+ 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
new file mode 100644
--- /dev/null
+++ b/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs
@@ -1,0 +1,339 @@
+using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.InternalTypeMustHaveInternalsVisibleToAnalyzer>;
+
+namespace Moq.Analyzers.Test;
+
+public class InternalTypeMustHaveInternalsVisibleToAnalyzerTests
+{
+ public static IEnumerable<object[]> 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<object[]> PublicTypeTestData()
+ {
+ return new object[][]
+ {
+ ["""new Mock<PublicClass>()"""],
+ ["""new Mock<PublicClass>(MockBehavior.Strict)"""],
+ ["""Mock.Of<PublicClass>()"""],
+ ["""var mock = new Mock<PublicClass>()"""],
+ }.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ public static IEnumerable<object[]> InterfaceTestData()
+ {
+ return new object[][]
+ {
+ // Internal interfaces also need InternalsVisibleTo
+ ["""new Mock<{|Moq1003:IInternalInterface|}>()"""],
+ ["""Mock.Of<{|Moq1003:IInternalInterface|}>()"""],
+
+ // Public interfaces should not trigger
+ ["""new Mock<IPublicInterface>()"""],
+ ["""Mock.Of<IPublicInterface>()"""],
+ }.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ public static IEnumerable<object[]> 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<PublicOuter.PublicNested>()"""],
+ }.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<InternalClass>();
+ var of = Mock.Of<InternalClass>();
+ }
+ }
+ """,
+ referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq);
+ }
+
+ [Fact]
+ public async Task ShouldFlagInternalTypeWithWrongAssemblyName()
+ {
+ await Verifier.VerifyAnalyzerAsync(
... diff truncated: showing 800 of 957 lines
src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs
Outdated
Show resolved
Hide resolved
…nostic 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.
…oq1003 Add MockRepository.Create<T>() 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 <noreply@anthropic.com>
rjmurillo-bot
left a comment
There was a problem hiding this comment.
All required CI checks pass. Moq1003 InternalsVisibleTo analyzer is ready.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicated mock invocation detection logic across analyzers
- Consolidated the duplicated IsValidMockInvocation method from both analyzers into a shared public method in MockDetectionHelpers.
Preview (7ce02ad196)
diff --git a/.markdownlint.json b/.markdownlint.json
--- 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/docs/rules/Moq1003.md b/docs/rules/Moq1003.md
new file mode 100644
--- /dev/null
+++ b/docs/rules/Moq1003.md
@@ -1,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<T>`, 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<MyService>(); // 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<MyService>(); // 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<MyService>(); // 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
--- 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) |
| [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) |
diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md
--- a/src/Analyzers/AnalyzerReleases.Unshipped.md
+++ b/src/Analyzers/AnalyzerReleases.Unshipped.md
@@ -1,12 +1,14 @@
; Unshipped analyzer release
-; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md
+; <https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md>
### 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)
+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)
diff --git a/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs
new file mode 100644
--- /dev/null
+++ b/src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs
@@ -1,0 +1,205 @@
+using Microsoft.CodeAnalysis.Operations;
+using Moq.Analyzers.Common;
+
+namespace Moq.Analyzers;
+
+/// <summary>
+/// Detects when <c>Mock<T></c> is used where <c>T</c> is an <see langword="internal"/> type
+/// and the assembly containing <c>T</c> does not have
+/// <c>[InternalsVisibleTo("DynamicProxyGenAssembly2")]</c>.
+/// </summary>
+[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");
+
+ /// <inheritdoc />
+ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+
+ /// <inheritdoc />
+ 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 &&
+ MockDetectionHelpers.IsValidMockCreation(creation, knownSymbols, out mockedType))
+ {
+ diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, creation.Syntax);
+ }
+ else if (context.Operation is IInvocationOperation invocation &&
+ MockDetectionHelpers.IsValidMockInvocation(invocation, knownSymbols, out mockedType))
+ {
+ diagnosticLocation = MockDetectionHelpers.GetDiagnosticLocation(context.Operation, invocation.Syntax);
+ }
+ else
+ {
+ return;
+ }
+
+ if (mockedType != null && diagnosticLocation != null &&
+ ShouldReportDiagnostic(mockedType, knownSymbols.InternalsVisibleToAttribute))
+ {
+ context.ReportDiagnostic(diagnosticLocation.CreateDiagnostic(
+ Rule,
+ mockedType.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat)));
+ }
+ }
+
+ /// <summary>
+ /// Determines whether the mocked type is effectively internal and its assembly
+ /// lacks InternalsVisibleTo for DynamicProxy.
+ /// </summary>
+ private static bool ShouldReportDiagnostic(
+ ITypeSymbol mockedType,
+ INamedTypeSymbol? internalsVisibleToAttribute)
+ {
+ if (!IsEffectivelyInternal(mockedType))
+ {
+ return false;
+ }
+
+ return !HasInternalsVisibleToDynamicProxy(mockedType.ContainingAssembly, internalsVisibleToAttribute);
+ }
+
+ /// <summary>
+ /// 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:
+ /// <list type="bullet">
+ /// <item><see cref="Accessibility.Internal"/> (internal)</item>
+ /// <item><see cref="Accessibility.ProtectedOrInternal"/> (protected internal) on
+ /// a containing type, because DynamicProxy does not derive from the container</item>
+ /// </list>
+ /// Note: <see cref="Accessibility.Private"/>, <see cref="Accessibility.Protected"/>,
+ /// and <see cref="Accessibility.ProtectedAndInternal"/> (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.
+ /// </summary>
+ private static bool IsEffectivelyInternal(ITypeSymbol type)
+ {
+ ITypeSymbol? current = type;
+ while (current != null)
+ {
+ switch (current.DeclaredAccessibility)
+ {
+ case Accessibility.Internal:
+ case Accessibility.ProtectedOrInternal:
+ return true;
+ }
+
+ current = current.ContainingType;
+ }
+
+ return false;
+ }
+
+ /// <summary>
+ /// Checks the assembly's attributes for InternalsVisibleTo targeting DynamicProxy,
+ /// using symbol-based comparison for the attribute type.
+ /// </summary>
+ 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)
+ {
+ continue;
+ }
+
+ // 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 &&
+ IsDynamicProxyAssemblyName(assemblyName))
+ {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ /// <summary>
+ /// 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.
+ /// </summary>
+ private static bool IsDynamicProxyAssemblyName(string assemblyName)
+ {
+ 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] == ',';
+ }
+}
diff --git a/src/Analyzers/NoMockOfLoggerAnalyzer.cs b/src/Analyzers/NoMockOfLoggerAnalyzer.cs
--- a/src/Analyzers/NoMockOfLoggerAnalyzer.cs
+++ b/src/Analyzers/NoMockOfLoggerAnalyzer.cs
@@ -84,7 +84,7 @@
// 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);
}
@@ -101,32 +101,6 @@
}
/// <summary>
- /// Determines if the operation is a valid Mock.Of{T}() or MockRepository.Create{T}() invocation
- /// and extracts the mocked type.
- /// </summary>
- 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;
- }
-
- /// <summary>
/// Determines whether the mocked type is ILogger or ILogger{T} using symbol-based comparison.
/// </summary>
private static bool IsLoggerType(ITypeSymbol mockedType, MoqKnownSymbols knownSymbols)
diff --git a/src/Common/DiagnosticIds.cs b/src/Common/DiagnosticIds.cs
--- a/src/Common/DiagnosticIds.cs
+++ b/src/Common/DiagnosticIds.cs
@@ -7,6 +7,7 @@
internal const string SealedClassCannotBeMocked = "Moq1000";
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";
diff --git a/src/Common/MockDetectionHelpers.cs b/src/Common/MockDetectionHelpers.cs
--- a/src/Common/MockDetectionHelpers.cs
+++ b/src/Common/MockDetectionHelpers.cs
@@ -54,6 +54,37 @@
}
/// <summary>
+ /// Determines if the operation is a valid Mock.Of{T}() or MockRepository.Create{T}() invocation
+ /// and extracts the mocked type.
+ /// </summary>
+ /// <param name="invocation">The invocation operation.</param>
+ /// <param name="knownSymbols">The known Moq symbols.</param>
+ /// <param name="mockedType">When successful, the mocked type; otherwise, null.</param>
+ /// <returns>True if this is a valid mock invocation; otherwise, false.</returns>
+ 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;
+ }
+
+ /// <summary>
/// Checks if the method symbol represents a static Mock.Of{T}() method.
/// </summary>
/// <param name="targetMethod">The method symbol to check.</param>
diff --git a/src/Common/WellKnown/KnownSymbols.cs b/src/Common/WellKnown/KnownSymbols.cs
--- a/src/Common/WellKnown/KnownSymbols.cs
+++ b/src/Common/WellKnown/KnownSymbols.cs
@@ -57,5 +57,10 @@
/// </summary>
public INamedTypeSymbol? Action1 => TypeProvider.GetOrCreateTypeByMetadataName("System.Action`1");
+ /// <summary>
+ /// Gets the class <see cref="System.Runtime.CompilerServices.InternalsVisibleToAttribute"/>.
+ /// </summary>
+ public INamedTypeSymbol? InternalsVisibleToAttribute => TypeProvider.GetOrCreateTypeByMetadataName("System.Runtime.CompilerServices.InternalsVisibleToAttribute");
+
protected WellKnownTypeProvider TypeProvider { get; }
}
diff --git a/tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs b/tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs
--- a/tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs
+++ b/tests/Moq.Analyzers.Test/Helpers/ReferenceAssemblyCatalog.cs
@@ -30,10 +30,16 @@
public static string Net80WithNewMoqAndLogging => nameof(Net80WithNewMoqAndLogging);
/// <summary>
+ /// Gets the name of the reference assembly group for .NET 8.0 without Moq.
+ /// </summary>
+ public static string Net80 => nameof(Net80);
+
+ /// <summary>
/// Gets the catalog of reference assemblies.
/// </summary>
/// <remarks>
- /// The key is the name of the reference assembly group (<see cref="Net80WithOldMoq"/> and <see cref="Net80WithNewMoq"/>).
+ /// The key is the name of the reference assembly group (<see cref="Net80WithOldMoq"/>, <see cref="Net80WithNewMoq"/>,
+ /// <see cref="Net80WithNewMoqAndLogging"/>, and <see cref="Net80"/>).
/// </remarks>
public static IReadOnlyDictionary<string, ReferenceAssemblies> Catalog { get; } = new Dictionary<string, ReferenceAssemblies>(StringComparer.Ordinal)
{
@@ -54,5 +60,8 @@
new PackageIdentity("Microsoft.Extensions.Logging.Abstractions", "8.0.0"),
])
},
+
+ // .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
new file mode 100644
--- /dev/null
+++ b/tests/Moq.Analyzers.Test/InternalTypeMustHaveInternalsVisibleToAnalyzerTests.cs
@@ -1,0 +1,348 @@
+using Microsoft.CodeAnalysis.Testing;
+using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.InternalTypeMustHaveInternalsVisibleToAnalyzer>;
+
+namespace Moq.Analyzers.Test;
+
+public class InternalTypeMustHaveInternalsVisibleToAnalyzerTests
+{
+ public static IEnumerable<object[]> InternalTypeWithoutAttributeTestData()
+ {
+ return new object[][]
+ {
+ ["""new Mock<{|Moq1003:InternalClass|}>()"""],
+ ["""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();
+ }
+
+ public static IEnumerable<object[]> PublicTypeTestData()
+ {
+ return new object[][]
+ {
+ ["""new Mock<PublicClass>()"""],
+ ["""new Mock<PublicClass>(MockBehavior.Strict)"""],
+ ["""Mock.Of<PublicClass>()"""],
+ ["""var mock = new Mock<PublicClass>()"""],
+ ["""var repo = new MockRepository(MockBehavior.Strict); repo.Create<PublicClass>()"""],
+ }.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ public static IEnumerable<object[]> InterfaceTestData()
+ {
+ return new object[][]
+ {
+ // Internal interfaces also need InternalsVisibleTo
+ ["""new Mock<{|Moq1003:IInternalInterface|}>()"""],
+ ["""Mock.Of<{|Moq1003:IInternalInterface|}>()"""],
+
+ // Public interfaces should not trigger
+ ["""new Mock<IPublicInterface>()"""],
+ ["""Mock.Of<IPublicInterface>()"""],
+ }.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ public static IEnumerable<object[]> 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<PublicOuter.PublicNested>()"""],
+ }.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<InternalClass>();
+ var of = Mock.Of<InternalClass>();
+ var repo = new MockRepository(MockBehavior.Strict);
+ repo.Create<InternalClass>();
+ }
+ }
+ """,
+ 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<InternalClass>();
+ }
+ }
+ """,
+ referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80WithOldMoq);
+ }
+
+ [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
+ {
+ internal class InternalClass { }
+
+ internal class UnitTest
+ {
+ private void Test()
+ {
+ var instance = new InternalClass();
+ }
+ }
+ }
+ """,
+ referenceAssemblyGroup: ReferenceAssemblyCatalog.Net80,
+ CompilerDiagnostics.None);
+ }
+
+ [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<PublicAbstractClass>();
+ }
+ }
+ """,
+ 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<InternalClass>();
+ }
+ }
+ """,
+ 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.
... diff truncated: showing 800 of 820 lines
src/Analyzers/InternalTypeMustHaveInternalsVisibleToAnalyzer.cs
Outdated
Show resolved
Hide resolved
Moves duplicated IsValidMockInvocation method from InternalTypeMustHaveInternalsVisibleToAnalyzer and NoMockOfLoggerAnalyzer into MockDetectionHelpers as a shared method that handles both Mock.Of<T>() and MockRepository.Create<T>() invocations.
rjmurillo-bot
left a comment
There was a problem hiding this comment.
All required checks passing. Merge conflicts resolved. Code changes verified. LGTM.
## Summary Main is broken. PRs #955 and #958 each independently added a `Net80` property and dictionary entry to `ReferenceAssemblyCatalog`. The merge created duplicates, failing with `CS0102: The type 'ReferenceAssemblyCatalog' already contains a definition for 'Net80'`. ## Changes Removed the duplicate property (lines 38-41) and duplicate dictionary entry (lines 73-74) from `ReferenceAssemblyCatalog.cs`. Kept the first copies which have the more descriptive doc comment. ## Test Plan - [x] `dotnet build` passes - [x] 2786 tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Removed .NET 8.0 reference assembly support from test infrastructure. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Mock<T>is used whereTis aninternaltype and the containing assembly lacks[InternalsVisibleTo("DynamicProxyGenAssembly2")]new Mock<T>(),Mock.Of<T>(), andMockRepository.Create<T>()patternsInternalsVisibleTovalues with public key suffixes (prefix match)Test plan
Fixes #110
🤖 Generated with Claude Code