Use WellKnownTypeProvider + KnownSymbols pattern to simplify analyzers

[MattKotsenas]

Change adds two related types and refactors the analyzers such that the string-based WellKnownMockTypes can be deleted. These changes unlock additional refactoring, but I stopped to get feedback.

The main changes are:

WellKnownTypeProvider

WellKnownTypeProvider comes from roslyn-analyzers. It caches ISymbol lookups for a given Compilation to amortize costs across all the analyzers running for a given compilation. It also enforces some best practices when looking for "well known" symbols in the cases of name collisions.

KnownSymbols

KnownSymbols comes from typeshape-csharp to simplify looking up types in the WellKnownTypeProvider. In this change only types are cached, the member lookups (methods, fields, etc.) are not. However, Roslyn internally caches, so it isn't clear that's needed.

Using the KnownSymbols pattern cleans up the code quite a bit. It does push us away from SyntaxNodes and towards Symbols, however I don't think that's a bad thing because we also want to migrate to the IOperation-based analysis, which already has the corresponding ISymbol available at the start of analysis.

The existing benchmarks show the new and old code (v0.1.0) to be within the margin of error.

Summary by CodeRabbit

• New Features

  • Added new diagnostic rules to enhance code quality checks, including IDs for ambiguous references and XML comment issues.
  • Introduced a new caching mechanism for analyzers with BoundedCacheWithFactory.

• Bug Fixes

  • Improved logic in various analyzers for better handling of mock-related methods and symbols.

• Documentation

  • Updated documentation for new diagnostic rules and methods.

• Chores

  • Removed deprecated diagnostic IDs to streamline the analysis process.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

📝 Walkthrough

Walkthrough

The pull request introduces several changes across various files, primarily focusing on enhancements to code analysis capabilities. Key modifications include the addition of a new package reference for Microsoft.CodeAnalysis.PerformanceSensitiveAnalyzers, updates to various analyzer classes to streamline symbol resolution and improve diagnostic reporting, and the introduction of new diagnostic rules in YAML configuration files. Additionally, a new caching class is introduced to optimize symbol lookups, and several deprecated files are removed to clean up the codebase.

Changes

File Path	Change Summary
build/targets/codeanalysis/CodeAnalysis.props	Added package reference for Microsoft.CodeAnalysis.PerformanceSensitiveAnalyzers.
build/targets/codeanalysis/Packages.props	Added package version entry for Microsoft.CodeAnalysis.PerformanceSensitiveAnalyzers.
build/targets/compiler/Compiler.props	Updated package reference from PolySharp to Polyfill.
build/targets/compiler/Packages.props	Removed PolySharp version entry and added Polyfill version entry.
src/Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs	Significant logic modifications to streamline the analyzer's implementation.
src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs	Updated to utilize MoqKnownSymbols for enhanced symbol handling.
src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs	Added MoqKnownSymbols parameter to multiple methods for improved symbol resolution.
src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs	Introduced MoqKnownSymbols to enhance functionality and simplify logic.
src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs	Restructured logic and added SuppressMessage attribute.
src/Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs	Refactored logic to improve clarity and structure.
src/Analyzers/SquiggleCop.Baseline.yaml	Added new diagnostic rules and updated existing ones for better analysis.
src/BannedSymbols.txt	Updated diagnostic creation methods for consistency and clarity.
src/CodeFixes/CallbackSignatureShouldMatchMockedMethodCodeFix.cs	Enhanced logic for semantic models and method invocations.
src/Common/Caching/BoundedCacheWithFactory.cs	Introduced a new bounded cache class for managing cached values.
src/Common/GlobalUsings.cs	Added global using directive for Moq.Analyzers.Common.WellKnown.
src/Common/ISymbolExtensions.cs	Updated method signatures to enhance nullability support and added new visibility method.
src/Common/ITypeSymbolExtensions.cs	Introduced a method to retrieve base types of a specified type.
src/Common/WellKnown/KnownSymbols.cs	Added a class for accessing well-known symbols related to Moq.
src/Common/WellKnown/MoqKnownSymbols.cs	Introduced a class for accessing Moq-specific symbols.
src/Common/WellKnown/WellKnownTypeProvider.cs	Added a class for managing and caching well-known types within a compilation context.
tests/Moq.Analyzers.Benchmarks/SquiggleCop.Baseline.yaml	Added new diagnostic IDs and updated existing ones.
tests/Moq.Analyzers.Test.Analyzers/SquiggleCop.Baseline.yaml	Added new diagnostic IDs and removed deprecated entries.
tests/Moq.Analyzers.Test/SquiggleCop.Baseline.yaml	Added new diagnostic IDs and removed deprecated entries.

Possibly related PRs

• Update SDK to get newer version of Source Link #202: This PR updates the SDK and introduces new analyzer rules, which may relate to the changes in package references in the main PR.
• Bug/fix issue 143 #221: This PR modifies the ISymbolExtensions class, which could be relevant as it deals with symbol analysis, similar to the changes in the main PR regarding analyzers.
• Bug/fix issue 144 #224: This PR updates the SetupShouldNotIncludeAsyncResultAnalyzer, which is related to the analysis of mock setups, potentially connecting to the changes in the main PR.
• Add rule to explicitly pick MockBehavior #226: This PR adds a rule to explicitly pick MockBehavior, which aligns with the focus on analyzers in the main PR.
• Add BannedApiAnalyzer and ban direct use of Diagnostic.Create #239: This PR introduces a new package reference for Microsoft.CodeAnalysis.BannedApiAnalyzers, which is directly related to the changes in package references in the main PR.
• Remove dead suppressions and dead code #242: This PR removes suppressions and dead code, which may relate to the overall improvements in code quality and analysis capabilities reflected in the main PR.

Suggested labels

feature, analyzers

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[MattKotsenas]

Reviewers note: Reviewing by commit makes each refactor clearer. I do however understand this is a larger change, so if there's things to pull out into separate PRs let me know.

[coderabbitai]

Actionable comments posted: 31

🧹 Outside diff range comments (4)

src/CodeFixes/CallbackSignatureShouldMatchMockedMethodCodeFix.cs (1)

Line range hint 67-71: Strengthen null handling and array bounds checking

There are several potential issues in this code segment:

1. The Debug.Assert won't protect against null in release builds
2. The array access at index 0 could throw if the array is empty

Consider this more defensive approach:

         InvocationExpressionSyntax? setupMethodInvocation = semanticModel.FindSetupMethodFromCallbackInvocation(knownSymbols, callbackInvocation, cancellationToken);
-        Debug.Assert(setupMethodInvocation != null, nameof(setupMethodInvocation) + " != null");
+        if (setupMethodInvocation is null)
+        {
+            return document;
+        }
         IMethodSymbol[] matchingMockedMethods = semanticModel.GetAllMatchingMockedMethodSymbolsFromSetupMethodInvocation(setupMethodInvocation).ToArray();

-        if (matchingMockedMethods.Length != 1)
+        if (matchingMockedMethods.Length == 0 || matchingMockedMethods.Length > 1)
         {
             return document;
         }

src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs (2)

Line range hint 100-116: Consider extracting the "Result" string as a constant.

The property name "Result" appears to be a well-known value. Consider extracting it as a private constant to improve maintainability.

+    private const string TaskResultPropertyName = "Result";
+
     private static bool IsTaskResultProperty(IPropertySymbol propertySymbol, MoqKnownSymbols knownSymbols)
     {
         // Check if the property is named "Result"
-        if (!string.Equals(propertySymbol.Name, "Result", StringComparison.Ordinal))
+        if (!string.Equals(propertySymbol.Name, TaskResultPropertyName, StringComparison.Ordinal))
         {
             return false;
         }

---

Line range hint 109-116: Consider reordering the null checks.

The null check for taskOfTType could be moved before the property name check to fail fast and improve readability.

     private static bool IsTaskResultProperty(IPropertySymbol propertySymbol, MoqKnownSymbols knownSymbols)
     {
+        // Check if Task<T> type is available
+        INamedTypeSymbol? taskOfTType = knownSymbols.Task1;
+        if (taskOfTType == null)
+        {
+            return false; // If Task<T> type cannot be found, we skip it
+        }
+
         // Check if the property is named "Result"
         if (!string.Equals(propertySymbol.Name, "Result", StringComparison.Ordinal))
         {
             return false;
         }
 
-        // Check if the containing type is Task<T>
-        INamedTypeSymbol? taskOfTType = knownSymbols.Task1;
-
-        if (taskOfTType == null)
-        {
-            return false; // If Task<T> type cannot be found, we skip it
-        }
-
         return SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingType, taskOfTType);
     }

src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs (1)

Line range hint 283-338: Refactor AnyConstructorsFound method for better maintainability

The AnyConstructorsFound method is lengthy and complex, which can make it hard to read and maintain. Breaking it down into smaller helper methods can improve readability and facilitate testing.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between b939a59 and b1074f1.

📒 Files selected for processing (32)

• build/targets/codeanalysis/CodeAnalysis.props (1 hunks)
• build/targets/codeanalysis/Packages.props (1 hunks)
• build/targets/compiler/Compiler.props (1 hunks)
• build/targets/compiler/Packages.props (1 hunks)
• src/Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs (1 hunks)
• src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs (2 hunks)
• src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs (5 hunks)
• src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs (1 hunks)
• src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs (5 hunks)
• src/Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs (1 hunks)
• src/Analyzers/SquiggleCop.Baseline.yaml (5 hunks)
• src/BannedSymbols.txt (1 hunks)
• src/CodeFixes/CallbackSignatureShouldMatchMockedMethodCodeFix.cs (1 hunks)
• src/CodeFixes/SquiggleCop.Baseline.yaml (5 hunks)
• src/Common/Caching/BoundedCacheWithFactory.cs (1 hunks)
• src/Common/CompilationExtensions.cs (0 hunks)
• src/Common/EnumerableExtensions.cs (1 hunks)
• src/Common/GlobalUsings.cs (1 hunks)
• src/Common/ISymbolExtensions.cs (2 hunks)
• src/Common/ITypeSymbolExtensions.cs (1 hunks)
• src/Common/MoqMethodDescriptorBase.cs (0 hunks)
• src/Common/MoqSetupMethodDescriptor.cs (0 hunks)
• src/Common/SemanticModelExtensions.cs (2 hunks)
• src/Common/SymbolVisibility.cs (1 hunks)
• src/Common/WellKnown/KnownSymbols.cs (1 hunks)
• src/Common/WellKnown/MoqKnownSymbolExtensions.cs (1 hunks)
• src/Common/WellKnown/MoqKnownSymbols.cs (1 hunks)
• src/Common/WellKnown/WellKnownTypeProvider.cs (1 hunks)
• src/Common/WellKnownMoqNames.cs (0 hunks)
• tests/Moq.Analyzers.Benchmarks/SquiggleCop.Baseline.yaml (2 hunks)
• tests/Moq.Analyzers.Test.Analyzers/SquiggleCop.Baseline.yaml (4 hunks)
• tests/Moq.Analyzers.Test/SquiggleCop.Baseline.yaml (2 hunks)

💤 Files with no reviewable changes (4)

• src/Common/CompilationExtensions.cs
• src/Common/MoqMethodDescriptorBase.cs
• src/Common/MoqSetupMethodDescriptor.cs
• src/Common/WellKnownMoqNames.cs

🧰 Additional context used

📓 Path-based instructions (18)

src/Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/CodeFixes/CallbackSignatureShouldMatchMockedMethodCodeFix.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/Caching/BoundedCacheWithFactory.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/EnumerableExtensions.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/GlobalUsings.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/ISymbolExtensions.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/ITypeSymbolExtensions.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/SemanticModelExtensions.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/SymbolVisibility.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/WellKnown/KnownSymbols.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/WellKnown/MoqKnownSymbolExtensions.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/WellKnown/MoqKnownSymbols.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

src/Common/WellKnown/WellKnownTypeProvider.cs (1)

Pattern **/*.cs: I need your help tracking down and fixing some bugs that have been reported in this codebase.

I suspect the bugs are related to:

• Incorrect handling of edge cases
• Off-by-one errors in loops or array indexing
• Unexpected data types
• Uncaught exceptions
• Concurrency issues
• Improper configuration settings

To diagnose:

1. Review the code carefully and systematically
2. Trace the relevant code paths
3. Consider boundary conditions and potential error states
4. Look for antipatterns that tend to cause bugs
5. Run the code mentally with example inputs
6. Think about interactions between components

When you find potential bugs, for each one provide:

1. File path and line number(s)
2. Description of the issue and why it's a bug
3. Example input that would trigger the bug
4. Suggestions for how to fix it

After analysis, please update the code with your proposed fixes. Try to match the existing code style. Add regression tests if possible to prevent the bugs from recurring.

I appreciate your diligence and attention to detail! Let me know if you need any clarification on the intended behavior of the code.

📓 Learnings (2)

src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs (1)

Learnt from: MattKotsenas
PR: rjmurillo/moq.analyzers#226
File: src/Moq.Analyzers/SetExplicitMockBehaviorAnalyzer.cs:45-49
Timestamp: 2024-10-15T20:25:09.079Z
Learning: In this codebase, `WellKnownTypeNames` is included via global usings.

src/BannedSymbols.txt (1)

Learnt from: MattKotsenas
PR: rjmurillo/moq.analyzers#239
File: src/BannedSymbols.txt:1-8
Timestamp: 2024-10-24T21:06:43.546Z
Learning: All direct uses of `Diagnostic.Create` should be banned, even if `DiagnosticExtensions` doesn't currently wrap them. Developers should add an extension to `DiagnosticExtensions` if they need to use any of the `Diagnostic.Create` APIs.

🪛 GitHub Check: Codacy Static Code Analysis

src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs

[failure] 185-185: src/Analyzers/ConstructorArgumentsShouldMatchAnalyzer.cs#L185
Add the 'const' modifier to 'hasMockBehavior'.

src/Common/Caching/BoundedCacheWithFactory.cs

[failure] 1-1: src/Common/Caching/BoundedCacheWithFactory.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

---

[warning] 12-12: src/Common/Caching/BoundedCacheWithFactory.cs#L12
Types should not have members with visibility set higher than the type's visibility

src/Common/ISymbolExtensions.cs

[failure] 88-88: src/Common/ISymbolExtensions.cs#L88
Add a 'default' clause to this 'switch' statement.

---

[failure] 106-106: src/Common/ISymbolExtensions.cs#L106
Add a 'default' clause to this 'switch' statement.

src/Common/ITypeSymbolExtensions.cs

[failure] 1-1: src/Common/ITypeSymbolExtensions.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

---

[warning] 3-3: src/Common/ITypeSymbolExtensions.cs#L3
Types should not have members with visibility set higher than the type's visibility

src/Common/SymbolVisibility.cs

[failure] 1-1: src/Common/SymbolVisibility.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

src/Common/WellKnown/KnownSymbols.cs

[failure] 1-1: src/Common/WellKnown/KnownSymbols.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

---

[warning] 11-11: src/Common/WellKnown/KnownSymbols.cs#L11
Types should not have members with visibility set higher than the type's visibility

src/Common/WellKnown/MoqKnownSymbolExtensions.cs

[failure] 1-1: src/Common/WellKnown/MoqKnownSymbolExtensions.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

---

[warning] 3-3: src/Common/WellKnown/MoqKnownSymbolExtensions.cs#L3
Types should not have members with visibility set higher than the type's visibility

src/Common/WellKnown/MoqKnownSymbols.cs

[failure] 1-1: src/Common/WellKnown/MoqKnownSymbols.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

---

[warning] 5-5: src/Common/WellKnown/MoqKnownSymbols.cs#L5
Types should not have members with visibility set higher than the type's visibility

src/Common/WellKnown/WellKnownTypeProvider.cs

[failure] 1-1: src/Common/WellKnown/WellKnownTypeProvider.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

---

[warning] 15-15: src/Common/WellKnown/WellKnownTypeProvider.cs#L15
Types should not have members with visibility set higher than the type's visibility

---

[failure] 124-124: src/Common/WellKnown/WellKnownTypeProvider.cs#L124
Add the missing 'else' clause with either the appropriate action or a suitable comment as to why no action is taken.

---

[failure] 137-137: src/Common/WellKnown/WellKnownTypeProvider.cs#L137
The Cyclomatic Complexity of this method is 19 which is greater than 10 authorized.

---

[notice] 176-176: src/Common/WellKnown/WellKnownTypeProvider.cs#L176
Replace this 'switch' expression with a ternary conditional operator to increase readability.

---

[failure] 179-179: src/Common/WellKnown/WellKnownTypeProvider.cs#L179
Reduce the number of conditional operators (10) used in the expression (maximum allowed 3).

---

[failure] 213-213: src/Common/WellKnown/WellKnownTypeProvider.cs#L213
Refactor this method to reduce its Cognitive Complexity from 26 to the 15 allowed.

🔇 Additional comments (41)

build/targets/compiler/Packages.props (1)

3-3: Verify compatibility with WellKnownTypeProvider implementation.

The switch to Polyfill 7.1.2 looks good, but we should ensure it's compatible with the new WellKnownTypeProvider pattern being introduced.

✅ Verification successful

Based on the output, I can now generate the final response since I have enough context. The search revealed that Polyfill is properly configured in the build system with appropriate PrivateAssets and IncludeAssets settings, and there are no direct dependencies on PolySharp in the codebase.

The package switch to Polyfill is safe and compatible

The change from PolySharp to Polyfill is properly configured in the build system:

• It's set up as a build-time dependency with PrivateAssets="all"
• Includes appropriate analyzer assets via IncludeAssets="runtime; build; native; contentfiles; analyzers"
• No direct code dependencies were found that could conflict with WellKnownTypeProvider

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any potential conflicts or dependencies
rg -g '*.cs' -g '*.csproj' -i 'polysharp|polyfill' 

Length of output: 52

---

Script:

#!/bin/bash
# Let's try a broader search to understand the package references and their usage
fd -e props -e csproj -e config . | xargs cat

Length of output: 18091

src/Common/WellKnown/MoqKnownSymbolExtensions.cs (1)

1-4: LGTM! Namespace and class structure are well-defined.

The internal visibility is appropriate for analyzer implementation, and the static class is correctly used for extension methods.

🧰 Tools

🪛 GitHub Check: Codacy Static Code Analysis

[failure] 1-1: src/Common/WellKnown/MoqKnownSymbolExtensions.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

---

[warning] 3-3: src/Common/WellKnown/MoqKnownSymbolExtensions.cs#L3
Types should not have members with visibility set higher than the type's visibility

src/Common/GlobalUsings.cs (1)

7-7: LGTM! The global using directive aligns with architectural improvements.

The addition of global using Moq.Analyzers.Common.WellKnown supports the transition to the WellKnownTypeProvider pattern, which will help improve symbol resolution and caching across analyzers.

build/targets/compiler/Compiler.props (1)

Line range hint 9-13: Verify the package migration from PolySharp to Polyfill.

This represents a significant package change with a major version jump (1.14.1 → 7.1.2). While the asset configuration remains correct, this change could have broader implications.

Let's verify the package details and any migration guides:

Consider adding a comment in the props file documenting this package migration and any notable differences between PolySharp and Polyfill.

✅ Verification successful

Let me try another verification approach to understand the package usage better.

---

Package change from PolySharp to Polyfill is properly justified

The commit message clearly explains the motivation for this change: the Polyfill package was chosen because it provides additional polyfills, specifically ConcurrentDictionary.GetOrAdd<T> which was needed. The change is well-documented in the commit history, and the package configuration (PrivateAssets and IncludeAssets) remains appropriate for a polyfill generator.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for migration documentation and package usage

# Test 1: Look for migration documentation
echo "Checking for migration documentation..."
rg -l -i "polyfill|polysharp" README.md docs/ 2>/dev/null || true

# Test 2: Check for any remaining references to PolySharp
echo "Checking for remaining PolySharp references..."
rg -l "PolySharp" . --type xml --type cs 2>/dev/null || true

Length of output: 294

---

Script:

#!/bin/bash
# Search for Polyfill package references and configurations
echo "Checking Polyfill package references..."
rg -l "Polyfill" . --type xml --type props --type config

# Search for any build configuration or documentation mentioning polyfills
echo "Checking build configurations and docs for polyfill mentions..."
rg -i "polyfill" . --type md --type xml --type props --type config -C 2

# Check commit history for related changes
echo "Checking recent commits for package changes..."
git log -p --grep="polyfill\|polysharp" -i

Length of output: 1759

src/Common/SymbolVisibility.cs (1)

3-24: LGTM! Clean and well-documented enum implementation.

The enum is well-structured with clear documentation and explicit value assignments.

src/Common/ITypeSymbolExtensions.cs (1)

1-1: Verify assembly version configuration.

The static analysis tool indicates missing assembly version. This should be verified at the project level.

✅ Verification successful

Let me gather more information about version configuration in the project by checking the project files and Directory.Build.props which commonly contain version settings in .NET projects.

---

Let me check the content of the versioning props file referenced in Directory.Build.props since it appears to handle versioning configuration.

---

Assembly version is properly configured through Nerdbank.GitVersioning

The project uses Nerdbank.GitVersioning package which automatically handles assembly and package versioning through Git version control. This is a recommended approach that provides consistent versioning across all assemblies in the solution. The warning can be safely ignored as the versioning is managed at the solution level through this package.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for assembly version in project files and assembly info
echo "Checking for AssemblyVersion in project files:"
find . -name "*.csproj" -exec grep -l "AssemblyVersion" {} \;

echo -e "\nChecking for AssemblyVersion in AssemblyInfo files:"
find . -name "AssemblyInfo.cs" -exec grep -l "AssemblyVersion" {} \;

Length of output: 356

---

Script:

#!/bin/bash
# Check project files content and Directory.Build.props
echo "Project files content:"
find . -name "*.csproj" -exec cat {} \;

echo -e "\nDirectory.Build.props content:"
find . -name "Directory.Build.props" -exec cat {} \;

echo -e "\nChecking for Version tags in project files:"
find . -name "*.csproj" -exec grep -l "Version" {} \;

Length of output: 8698

---

Script:

#!/bin/bash
# Check versioning configuration
echo "Checking versioning configuration:"
find . -name "Versioning.props" -exec cat {} \;

Length of output: 339

🧰 Tools

🪛 GitHub Check: Codacy Static Code Analysis

[failure] 1-1: src/Common/ITypeSymbolExtensions.cs#L1
Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

build/targets/codeanalysis/Packages.props (1)

6-6: LGTM! Package addition aligns with performance goals.

The addition of Microsoft.CodeAnalysis.PerformanceSensitiveAnalyzers is appropriate given the PR's focus on optimizing symbol lookups and maintaining performance. While it's a beta version, it's consistent with other Microsoft.CodeAnalysis packages in use.

src/Common/WellKnown/KnownSymbols.cs (4)

3-10: LGTM! Well-documented class purpose and usage.

The documentation clearly explains the caching mechanism and provides guidance on symbol comparison with SyntaxNodes.

---

13-21: LGTM! Well-designed constructor overloads.

The constructors provide good flexibility in initialization while maintaining a single source of truth through constructor chaining.

---

43-43: LGTM! Appropriate protection level for inheritance.

The protected property allows derived classes (like MoqKnownSymbols) to access the type provider while maintaining encapsulation.

---

26-41: Verify caching behavior of repeated type lookups.

While the class is designed for caching, let's verify that repeated property access doesn't trigger multiple type lookups.

✅ Verification successful

Type lookups are properly cached, no performance concerns

The implementation shows robust caching at multiple levels:

• Static ProviderCache for compilation-level caching
• Instance-level _fullNameToTypeMap (ConcurrentDictionary) caches individual type lookups
• Thread-safe implementation using concurrent collections

The Task-related properties will only perform the actual type lookup once, with subsequent accesses retrieving the cached value from _fullNameToTypeMap.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for any tests verifying the caching behavior of WellKnownTypeProvider

# Test: Look for test files related to WellKnownTypeProvider
rg -t cs "WellKnownTypeProvider" --glob "*Test*.cs" -A 5

# Test: Look for any benchmarks measuring type lookup performance
rg -t cs "WellKnownTypeProvider.*Benchmark" -A 5

Length of output: 107

---

Script:

#!/bin/bash
# Let's check the implementation of TypeProvider to understand the caching behavior

# First, find the TypeProvider implementation
rg -t cs "class.*TypeProvider" -A 5

# Also look for GetOrCreateTypeByMetadataName implementation
ast-grep --pattern 'GetOrCreateTypeByMetadataName($_) {
  $$$
}'

# Check if there's any dictionary or cache field
rg -t cs "Dictionary|Cache" --glob "*TypeProvider*.cs"

Length of output: 1427

src/Common/EnumerableExtensions.cs (1)

44-54: Verify usage patterns of WhereNotNull methods.

Let's analyze how these extension methods are being used in the codebase to ensure they align with the PR objectives of improving analyzers.

✅ Verification successful

Let me gather more context about the usage patterns, particularly focusing on the type parameters and nullable contexts.

---

Based on the gathered information, I can now provide a final response about the WhereNotNull extension methods:

Extension methods are correctly implemented and used appropriately

The WhereNotNull extension methods are properly implemented with specific constraints for both reference types (class) and value types (struct), and their only usage is in SetExplicitMockBehaviorAnalyzer.cs for handling nullable Mock types in a type-safe manner. This aligns with the PR's objective of improving analyzer robustness through better null handling.

• The implementation in src/Common/EnumerableExtensions.cs provides specialized handling for both reference and value types
• The usage in SetExplicitMockBehaviorAnalyzer.cs safely processes nullable Mock types when building the array of mock types
• No other usages found in the codebase, indicating focused and intentional usage

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find usages of WhereNotNull to understand implementation patterns

echo "Searching for WhereNotNull usage patterns..."
rg -l "WhereNotNull" | while read -r file; do
    echo "=== $file ==="
    # Show 3 lines before and after each usage for context
    rg -B 3 -A 3 "WhereNotNull" "$file"
done

Length of output: 1203

---

Script:

#!/bin/bash
# Search for Mock-related types and their usage with nullable contexts
echo "=== Searching for Mock type declarations and usage ==="
ast-grep --pattern 'class Mock$_'

echo -e "\n=== Searching for MockRepository declarations ==="
ast-grep --pattern 'class MockRepository$_'

echo -e "\n=== Checking nullable context settings ==="
rg "#nullable" -A 2

echo -e "\n=== Checking other potential WhereNotNull usages with IEnumerable ==="
rg "IEnumerable.*\?" -B 1 -A 1

Length of output: 25165

build/targets/codeanalysis/CodeAnalysis.props (1)

29-32: LGTM! Addition of performance-sensitive analyzers aligns with PR objectives.

The inclusion of Microsoft.CodeAnalysis.PerformanceSensitiveAnalyzers is particularly relevant given this PR's focus on optimizing symbol lookups and caching. The package configuration follows best practices with appropriate asset inclusion settings.

src/BannedSymbols.txt (2)

Line range hint 1-8: LGTM! Comprehensive coverage of Diagnostic.Create bans.

The banned signatures cover all overloads of Diagnostic.Create, which aligns with the requirement to use DiagnosticExtensions.CreateDiagnostic for better readability and consistency.

---

10-12: LGTM! Verify impact of banning GetType methods.*

The banned signatures align with the PR's goal of using KnownSymbols pattern for better type resolution and caching. This will help reduce computational costs by caching ISymbol lookups.

Let's verify the current usage of these banned methods to ensure all cases are covered by KnownSymbols:

✅ Verification successful

Potential issue: WellKnownTypeProvider still uses GetTypeByMetadataName internally

The verification reveals that while we're banning direct usage of GetTypeByMetadataName, the WellKnownTypeProvider itself internally uses this method in its implementation. Let's verify if this is an intentional implementation detail or a potential issue:

---

Internal usage of GetTypeByMetadataName in WellKnownTypeProvider is correct

The usage of GetTypeByMetadataName within WellKnownTypeProvider is intentional and correct. This class serves as the caching layer that implements the KnownSymbols pattern, where:

• It maintains a concurrent dictionary to cache type lookups
• It optimizes the lookup process by first checking namespace names
• It handles assembly visibility and type resolution across referenced assemblies
• It's designed to be the centralized place for type resolution, which is exactly what the ban on direct GetTypeByMetadataName usage aims to encourage

The ban in BannedSymbols.txt correctly pushes developers to use the KnownSymbols pattern instead of directly calling these methods, while the internal implementation in WellKnownTypeProvider appropriately encapsulates the actual type resolution logic.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find any remaining usages of GetTypeByMetadataName or GetTypesByMetadataName
# that might need to be updated to use KnownSymbols

echo "Searching for GetTypeByMetadataName usages..."
rg "GetTypeByMetadataName" -A 2

echo -e "\nSearching for GetTypesByMetadataName usages..."
rg "GetTypesByMetadataName" -A 2

Length of output: 2055

---

Script:

#!/bin/bash
# Get more context around the WellKnownTypeProvider implementation
ast-grep --pattern 'class WellKnownTypeProvider {
  $$$
}'

Length of output: 60

---

Script:

#!/bin/bash
# Let's try to get more context around the WellKnownTypeProvider implementation using ripgrep
echo "Getting context around WellKnownTypeProvider usage of GetTypeByMetadataName..."
rg -B 10 -A 10 "GetTypeByMetadataName" "src/Common/WellKnown/WellKnownTypeProvider.cs"

echo -e "\nChecking class definition..."
rg "class WellKnownTypeProvider" -B 2 -A 2 "src/Common/WellKnown/WellKnownTypeProvider.cs"

Length of output: 4127

src/Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs (2)

47-51: LGTM! Clean pattern matching with early return.

The code effectively uses pattern matching to handle non-member access expressions, improving readability and maintainability.

---

Line range hint 25-30: Verify thread safety for concurrent execution.

The analyzer enables concurrent execution but will use shared state with the proposed _knownSymbols caching. Ensure thread safety by using appropriate synchronization mechanisms.

Consider using a thread-safe initialization pattern:

 public class SetupShouldNotIncludeAsyncResultAnalyzer : DiagnosticAnalyzer
 {
-    private MoqKnownSymbols? _knownSymbols;
+    private volatile MoqKnownSymbols? _knownSymbols;
+    private readonly object _lock = new object();
     // ... other fields ...

     private static void Analyze(SyntaxNodeAnalysisContext context)
     {
-        _knownSymbols ??= new MoqKnownSymbols(context.SemanticModel.Compilation);
+        if (_knownSymbols == null)
+        {
+            lock (_lock)
+            {
+                _knownSymbols ??= new MoqKnownSymbols(context.SemanticModel.Compilation);
+            }
+        }

src/CodeFixes/CallbackSignatureShouldMatchMockedMethodCodeFix.cs (2)

55-58: LGTM: Improved null check using pattern matching

The change to use is null pattern matching is a good modernization that aligns with C# best practices.

---

60-67: Verify integration with WellKnownTypeProvider pattern

The changes introduce the WellKnownTypeProvider pattern through MoqKnownSymbols. Let's verify the integration with other components.

✅ Verification successful

Integration with WellKnownTypeProvider pattern is properly implemented

The verification shows that MoqKnownSymbols is consistently integrated across the codebase:

• Properly inherits from base KnownSymbols class and uses WellKnownTypeProvider
• Used consistently across 10 different analyzer and code fix files
• No legacy string-based type lookups were found
• The semantic model extensions are properly utilizing the new pattern

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for consistent usage of MoqKnownSymbols across the codebase
echo "Checking for MoqKnownSymbols usage patterns..."
rg -p "new MoqKnownSymbols\(" -A 3

# Check for any remaining string-based type lookups that should use MoqKnownSymbols
echo "Checking for potential missed conversions to MoqKnownSymbols..."
rg "WellKnownMockTypes\." 

Length of output: 304

---

Script:

#!/bin/bash
# Let's try a more comprehensive search for MoqKnownSymbols usage
echo "Searching for MoqKnownSymbols class definition and usage..."
rg -p "class MoqKnownSymbols" -A 5

echo "Searching for files containing MoqKnownSymbols..."
rg -l "MoqKnownSymbols"

echo "Checking semantic model usage patterns..."
rg -p "semanticModel\.FindSetupMethodFromCallbackInvocation" -A 3

# Let's also check for any related well-known type providers
echo "Checking for other well-known type providers..."
rg -p "class.*KnownSymbols" -A 5

Length of output: 2615

src/Common/WellKnown/MoqKnownSymbols.cs (4)

17-30: LGTM! Robust implementation with proper null handling.

The implementation correctly handles null cases and follows the pattern described in the PR objectives of not caching member lookups.

---

32-45: LGTM! Consistent implementation with non-generic Mock properties.

---

64-83: LGTM! Appropriate use of SingleOrDefault for enum field lookups.

---

61-62: Verify performance impact of boxing in MockRepositoryCreate.

The suppression of ECS0900 suggests potential performance implications. While likely negligible, consider benchmarking this specific lookup against high-frequency usage patterns.

✅ Verification successful

Let me check the usage context to better assess the performance impact.

---

Boxing performance impact is negligible in this context

The property MockRepositoryCreate is only used once in a control flow condition within ConstructorArgumentsShouldMatchAnalyzer, which is a static analysis scenario. Since this is not in a performance-critical path and is used for one-time type checking rather than in a tight loop, the boxing overhead from SelectMany and OfType operations is acceptable.

The existing suppression message accurately reflects this - it's a minor performance issue that doesn't warrant immediate optimization given the usage pattern.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of MockRepositoryCreate to assess impact
rg -l "MockRepositoryCreate" --type cs

Length of output: 135

---

Script:

#!/bin/bash
# Check how MockRepositoryCreate is used in the analyzer
rg "MockRepositoryCreate" -B 2 -A 2 --type cs

Length of output: 1095

src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs (1)

39-47: Verify cancellation token propagation.

The code correctly uses the cancellation token for GetSymbolInfo, but we should verify it's properly propagated to all semantic model operations to prevent potential race conditions.

✅ Verification successful

Let me search for semantic model operations more precisely to verify cancellation token usage.

---

Cancellation token is properly propagated in all semantic model operations

The verification shows that all semantic model operations in SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs correctly use the cancellation token:

• GetSymbolInfo(memberAccessExpression, context.CancellationToken)
• IsMoqSetupMethod(..., context.CancellationToken)
• GetSymbolInfo(mockedMemberExpression, context.CancellationToken)

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for semantic model operations without cancellation token
rg -A 2 "SemanticModel\.(Get|Is|Has)" | grep -v "CancellationToken"

Length of output: 8536

---

Script:

#!/bin/bash
# Search for semantic model operations in the specific analyzer file
rg -A 2 "SemanticModel\." "src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs"

Length of output: 774

src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs (1)

34-35: LGTM! Consider verifying performance impact.

The introduction of MoqKnownSymbols aligns with the PR objectives and appears thread-safe for concurrent analysis. The implementation correctly scopes the instance to each analysis context.

Let's verify the performance impact:

✅ Verification successful

Let's examine the benchmark implementations to verify if they cover the MoqKnownSymbols usage:

---

Let's check if there are any performance-related comments or documentation in the relevant files:

---

Let's check the analyzer's concurrent execution configuration and the base class implementation:

---

Let's check the actual implementation of concurrent execution and MoqKnownSymbols usage in the analyzer:

---

Implementation is thread-safe and follows performance best practices

The implementation is verified to be correct and performant:

• The analyzer properly enables concurrent execution via EnableConcurrentExecution()
• MoqKnownSymbols is correctly scoped to each analysis context
• The implementation is immutable and thread-safe, using ImmutableArray<T> for collections
• Caching is handled by the underlying WellKnownTypeProvider
• Performance considerations are acknowledged (see SuppressMessage("Performance"...) annotation)

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any performance regression tests or benchmarks

# Look for benchmark files
echo "Searching for benchmark files..."
fd -e cs -e csproj ".*[Bb]enchmark.*"

# Look for performance tests
echo "Searching for performance tests..."
rg -l "Performance|Benchmark" -g "*.cs"

Length of output: 1547

---

Script:

#!/bin/bash
# Check benchmark implementations for MoqKnownSymbols coverage
rg -A 10 "MoqKnownSymbols" tests/Moq.Analyzers.Benchmarks/

Length of output: 58

---

Script:

#!/bin/bash
# Check for performance-related comments in MoqKnownSymbols and related files
echo "Checking MoqKnownSymbols implementation:"
rg -C 5 "performance|concurrent|thread|safe" src/Common/WellKnown/MoqKnownSymbols.cs

echo -e "\nChecking WellKnownTypeProvider implementation:"
rg -C 5 "performance|concurrent|thread|safe" src/Common/WellKnown/WellKnownTypeProvider.cs

echo -e "\nChecking analyzer implementation:"
rg -C 5 "performance|concurrent|thread|safe" src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

Length of output: 575

---

Script:

#!/bin/bash
# Check analyzer's concurrent execution setup
echo "Checking analyzer's concurrent execution:"
ast-grep --pattern 'class CallbackSignatureShouldMatchMockedMethodAnalyzer : DiagnosticAnalyzer {
  $$$
  public override void Initialize(AnalysisContext context) {
    $$$
  }
  $$$
}'

echo -e "\nChecking for EnableConcurrentExecution calls:"
rg -l "EnableConcurrentExecution" --type cs

Length of output: 882

---

Script:

#!/bin/bash
# Check the Initialize method implementation and MoqKnownSymbols usage
echo "Checking Initialize method implementation:"
rg -A 10 "Initialize\(AnalysisContext context\)" src/Analyzers/CallbackSignatureShouldMatchMockedMethodAnalyzer.cs

echo -e "\nChecking MoqKnownSymbols implementation:"
cat src/Common/WellKnown/MoqKnownSymbols.cs

Length of output: 4271

tests/Moq.Analyzers.Test.Analyzers/SquiggleCop.Baseline.yaml (2)

310-318: LGTM! Good addition of compiler diagnostics.

The new compiler warning rules will help catch common issues around documentation, field initialization, reference ambiguity, and locking. All rules are appropriately configured with Warning severity.

---

1607-1607: 🛠️ Refactor suggestion

Consider enabling VSTHRD105 rule.

While adding the VSTHRD105 rule is good, it's currently marked as suppressed (IsEverSuppressed: true). This rule helps prevent threading issues by warning against method overloads that make assumptions about TaskScheduler.Current. Consider enabling it to catch potential threading issues early.

src/Analyzers/SquiggleCop.Baseline.yaml (2)

310-318: LGTM! Good addition of compiler diagnostic rules.

The added compiler diagnostic rules (CS0419, CS0649, etc.) will help catch common issues with:

• XML documentation formatting and validation
• Unassigned fields
• Parameter documentation
• Nullable reference type usage

---

913-913: LGTM! Appropriate severity level adjustments.

The severity level changes are well-considered:

• RS1035 (banned APIs) as Error - Critical for analyzer correctness
• SA1101 (this prefix) as Note - Style preference
• VSTHRD105 (TaskScheduler.Current) as Error - Important for thread safety

Also applies to: 1431-1431, 1608-1608

src/CodeFixes/SquiggleCop.Baseline.yaml (3)

310-318: LGTM! Added compiler diagnostic rules for better code quality.

The new compiler diagnostic rules cover important areas:

• XML comment validation (CS1570-CS1574)
• Field initialization warnings (CS0649)
• Nullable reference type handling (CS8669)

---

Line range hint 1608-1620: LGTM! Added comprehensive threading-related diagnostic rules.

The new VSTHRD rules enforce important async/await best practices:

• VSTHRD105: Avoids TaskScheduler.Current assumptions
• VSTHRD106-114: Proper async event handling and resource cleanup
• VSTHRD200: Consistent async method naming

---

1431-1431: Verify impact of SA1101 rule modification.

The SA1101 rule (prefix local calls with this) now includes 'Note' severity alongside 'Error'. This mixed severity configuration is unusual and might lead to inconsistent enforcement.

✅ Verification successful

The previous search was too narrow. Let's broaden it to:

1. Check YAML files for any SA1101 configurations to understand the current setup
2. Look for StyleCop suppressions in C# files to understand the impact

---

Mixed severity configuration for SA1101 is consistent across the codebase

The SA1101 rule configuration with mixed severities (Error, Note) is consistently defined across all configuration files in the codebase:

• src/Analyzers/SquiggleCop.Baseline.yaml
• src/CodeFixes/SquiggleCop.Baseline.yaml
• tests/Moq.Analyzers.Test/SquiggleCop.Baseline.yaml
• tests/Moq.Analyzers.Test.Analyzers/SquiggleCop.Baseline.yaml
• tests/Moq.Analyzers.Benchmarks/SquiggleCop.Baseline.yaml

This appears to be an intentional configuration pattern, and since there are no StyleCop suppressions found in the C# files, the mixed severity setup is unlikely to cause issues.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for existing SA1101 suppressions that might be affected
rg -l "SA1101" --type cs

Length of output: 24

---

Script:

#!/bin/bash
# Search for SA1101 rule configurations in all YAML files
rg "SA1101.*this" --type yaml -A 2 -B 2

# Search for any StyleCop rule suppressions in C# files
rg "\[SuppressMessage.*StyleCop" --type cs

Length of output: 6908

tests/Moq.Analyzers.Benchmarks/SquiggleCop.Baseline.yaml (2)

310-318: LGTM! New compiler diagnostic rules are well-configured.

The added compiler diagnostics enhance code quality by enforcing:

• Proper XML documentation formatting and references
• Correct nullable reference type usage
• Safe threading practices

---

342-357: LGTM! New performance diagnostic rules enhance allocation tracking.

The added performance diagnostics help identify:

• Unnecessary array allocations in params
• Value type boxing scenarios
• Closure and delegate allocations
• String concatenation allocations

These rules align well with the PR's focus on performance optimization.

tests/Moq.Analyzers.Test/SquiggleCop.Baseline.yaml (1)

310-317: LGTM! The new diagnostic rules enhance code quality checks.

The added diagnostic rules improve detection of:

• XML documentation issues (CS0419, CS1572, CS1573, CS1574, CS1734)
• Nullable reference type safety (CS8669)
• Threading safety concerns (CS9216)

src/Common/SemanticModelExtensions.cs (1)

10-29: Ensure all calls to FindSetupMethodFromCallbackInvocation are updated with the new MoqKnownSymbols parameter

The method FindSetupMethodFromCallbackInvocation now includes an additional parameter MoqKnownSymbols knownSymbols. Please verify that all invocations of this method within the codebase have been updated to pass the knownSymbols argument. Missing this update could lead to compilation errors or runtime issues.

You can run the following script to locate all usages of FindSetupMethodFromCallbackInvocation that may need updating:

✅ Verification successful

Let me check the actual implementation of these calls to verify if they include the new parameter.

---

All calls to FindSetupMethodFromCallbackInvocation are correctly updated with the new parameter

The verification shows that all invocations of the method include the required knownSymbols parameter:

• In CallbackSignatureShouldMatchMockedMethodCodeFix.cs: semanticModel.FindSetupMethodFromCallbackInvocation(knownSymbols, callbackInvocation, cancellationToken)
• In CallbackSignatureShouldMatchMockedMethodAnalyzer.cs: context.SemanticModel.FindSetupMethodFromCallbackInvocation(knownSymbols, callbackOrReturnsInvocation, context.CancellationToken)
• In SemanticModelExtensions.cs: The recursive call also includes knownSymbols: semanticModel.FindSetupMethodFromCallbackInvocation(knownSymbols, method.Expression, cancellationToken)

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all calls to FindSetupMethodFromCallbackInvocation that need to include the MoqKnownSymbols parameter.

rg -tcs --no-heading -o 'FindSetupMethodFromCallbackInvocation\(' -A 1

Length of output: 882

---

Script:

#!/bin/bash
# Get the context of the calls to FindSetupMethodFromCallbackInvocation
rg -tcs --no-heading -B 2 -A 2 'FindSetupMethodFromCallbackInvocation\('

Length of output: 2455

src/Analyzers/SetExplicitMockBehaviorAnalyzer.cs (4)

37-38: Proper instantiation of MoqKnownSymbols.

The MoqKnownSymbols class is correctly instantiated with the current compilation context.

---

40-42: Efficient check for Moq reference.

The use of IsMockReferenced() to determine if Moq is referenced in the compilation is appropriate and prevents unnecessary analysis when Moq is not present.

---

46-48: Null check for MockBehavior symbol is appropriate.

Ensuring that mockBehaviorSymbol is not null before proceeding avoids potential null reference exceptions.

---

53-54: Retrieval of Mock.Of() methods.

The ofMethods are correctly retrieved from knownSymbols.MockOf to be used in further analysis.

src/Common/WellKnown/WellKnownTypeProvider.cs (1)

119-129: ⚠️ Potential issue

Add an 'else' clause or comment to clarify loop control flow

In the loop starting at line 119, there's an if, an else if, but no else. For better readability and to explicitly handle all conditions, consider adding an else clause or a comment explaining why no action is taken when neither condition is met.

Consider adding an else block or a comment:

            for (int i = 0; i < fullTypeName.Length; i++)
            {
                if (fullTypeName[i] == '.')
                {
                    namespaceNamesBuilder.Add(fullTypeName[prevStartIndex..i]);
                    prevStartIndex = i + 1;
                }
                else if (!IsIdentifierPartCharacter(fullTypeName[i]))
                {
                    break;
+               }
+               else
+               {
+                   // Continue processing the next character
                }
            }

Likely invalid or redundant comment.

🧰 Tools

🪛 GitHub Check: Codacy Static Code Analysis

[failure] 124-124: src/Common/WellKnown/WellKnownTypeProvider.cs#L124
Add the missing 'else' clause with either the appropriate action or a suitable comment as to why no action is taken.

[qlty-cloud-legacy]

Code Climate has analyzed commit a9833ca and detected 0 issues on this pull request.

View more on Code Climate.