Fix MSTEST0037 to detect LINQ Enumerable.Contains on interface types#7487
Fix MSTEST0037 to detect LINQ Enumerable.Contains on interface types#7487Evangelink wants to merge 2 commits intomainfrom
Conversation
MSTEST0037 (UseProperAssertMethodsAnalyzer) was not detecting Assert.IsTrue(collection.Contains(item)) when .Contains() resolved to the LINQ extension method System.Linq.Enumerable.Contains<T>() instead of an instance method on the collection type. This affected interface types like IReadOnlyList<T>, IReadOnlyCollection<T>, and IEnumerable<T>. Root cause: RecognizeCollectionMethodCheck only handled instance methods (Parameters.Length == 1, ContainingType is the collection). For LINQ calls, Roslyn represents them with ContainingType == Enumerable and Arguments includes the 'this' parameter (Length == 2). Fix: Add a second detection path that matches Enumerable.Contains calls by checking ContainingType against the Enumerable type symbol, matching the pattern already used by TryMatchLinqMethod for Any()/Count(). Fixes #7486
There was a problem hiding this comment.
Pull request overview
This PR fixes MSTEST0037 (UseProperAssertMethodsAnalyzer) to detect Assert.IsTrue(collection.Contains(item)) when .Contains() resolves to the LINQ extension method System.Linq.Enumerable.Contains<T>() instead of an instance method. This was previously missed for interface types like IReadOnlyList<T>, IReadOnlyCollection<T>, IEnumerable<T>, and custom non-BCL types that don't define their own Contains.
Changes:
- Adds a new LINQ detection path in
RecognizeCollectionMethodCheckthat matchesEnumerable.Containscalls by checkingContainingType == enumerableTypeSymbolandArguments.Length == 2 - Propagates
enumerableTypeSymbolinto theRecognizeCollectionMethodCheckmethod signature - Adds 6 new unit tests covering
IReadOnlyList<T>,IEnumerable<T>,IReadOnlyCollection<T>, custom non-BCL collections,IsFalsevariant, and a case with a message parameter
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Analyzers/MSTest.Analyzers/UseProperAssertMethodsAnalyzer.cs |
Adds LINQ Enumerable.Contains detection path in RecognizeCollectionMethodCheck by checking for ContainingType == enumerableTypeSymbol with 2-argument form |
test/UnitTests/MSTest.Analyzers.UnitTests/UseProperAssertMethodsAnalyzerTests.cs |
Adds 6 unit tests verifying the new LINQ Contains detection and code fix for various interface types and scenarios |
| } | ||
|
|
||
| [TestMethod] | ||
| public async Task WhenAssertIsTrueWithLinqContainsOnCustomCollection_StillReports() |
There was a problem hiding this comment.
The test method name WhenAssertIsTrueWithLinqContainsOnCustomCollection_StillReports is misleading. The "StillReports" suffix implies pre-existing behavior, but this is actually a new scenario enabled by the fix. For non-BCL custom types, the old instance method path would NOT have reported a diagnostic (it requires IsBCLCollectionType), but now the new LINQ Enumerable.Contains path fires for ANY type. A more accurate name would be WhenAssertIsTrueWithLinqContainsOnCustomNonBCLCollection_Reports or similar.
MSTEST0037 (UseProperAssertMethodsAnalyzer) was not detecting Assert.IsTrue(collection.Contains(item)) when .Contains() resolved to the LINQ extension method System.Linq.Enumerable.Contains() instead of an instance method on the collection type. This affected interface types like IReadOnlyList, IReadOnlyCollection, and IEnumerable.
Root cause: RecognizeCollectionMethodCheck only handled instance methods (Parameters.Length == 1, ContainingType is the collection). For LINQ calls, Roslyn represents them with ContainingType == Enumerable and Arguments includes the 'this' parameter (Length == 2).
Fix: Add a second detection path that matches Enumerable.Contains calls by checking ContainingType against the Enumerable type symbol, matching the pattern already used by TryMatchLinqMethod for Any()/Count().
Fixes #7486