diff --git a/src/Microsoft.NetCore.Analyzers/Core/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValue.cs b/src/Microsoft.NetCore.Analyzers/Core/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValue.cs index 6ee1dd7a97..cffd08a91a 100644 --- a/src/Microsoft.NetCore.Analyzers/Core/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValue.cs +++ b/src/Microsoft.NetCore.Analyzers/Core/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValue.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Linq; using Analyzer.Utilities; using Analyzer.Utilities.Extensions; using Microsoft.CodeAnalysis; @@ -67,6 +68,16 @@ public override void Initialize(AnalysisContext context) } Debug.Assert(!string.IsNullOrEmpty(metadataName)); + + // Do not flag invocations that take any explicit argument (comparer, converter, etc.) + // as they can potentially modify the contents of the resulting collection. + // See https://github.com/dotnet/roslyn/issues/23625 for language specific implementation below. + var argumentsToSkip = targetMethod.IsExtensionMethod && invocation.Language != LanguageNames.VisualBasic ? 1 : 0; + if (invocation.Arguments.Skip(argumentsToSkip).Any(arg => arg.ArgumentKind == ArgumentKind.Explicit)) + { + return; + } + var immutableCollectionType = compilation.GetTypeByMetadataName(metadataName); if (immutableCollectionType == null) { diff --git a/src/Microsoft.NetCore.Analyzers/UnitTests/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValueTests.cs b/src/Microsoft.NetCore.Analyzers/UnitTests/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValueTests.cs index 52aa12b422..d931108965 100644 --- a/src/Microsoft.NetCore.Analyzers/UnitTests/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValueTests.cs +++ b/src/Microsoft.NetCore.Analyzers/UnitTests/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValueTests.cs @@ -35,7 +35,7 @@ protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() #region No Diagnostic Tests - [Theory] + [Theory, WorkItem(1432, "https://github.com/dotnet/roslyn-analyzers/issues/1432")] [MemberData(nameof(CollectionNames_Arity1))] public void NoDiagnosticCases_Arity1(string collectionName) { @@ -50,15 +50,21 @@ static class Extensions {{ return default({collectionName}); }} + + public static {collectionName} To{collectionName}(this IEnumerable items, IEqualityComparer comparer) + {{ + return default({collectionName}); + }} }} class C {{ - public void M(IEnumerable p1, List p2, {collectionName} p3) + public void M(IEnumerable p1, List p2, {collectionName} p3, IEqualityComparer comparer) {{ // Allowed p1.To{collectionName}(); p2.To{collectionName}(); + p3.To{collectionName}(comparer); // Potentially modifies the collection // No dataflow IEnumerable l1 = p3; @@ -76,13 +82,19 @@ Module Extensions Public Function To{collectionName}(Of TSource)(items As IEnumerable(Of TSource)) As {collectionName}(Of TSource) Return Nothing End Function + + _ + Public Function To{collectionName}(Of TSource)(items As IEnumerable(Of TSource), comparer as IEqualityComparer(Of TSource)) As {collectionName}(Of TSource) + Return Nothing + End Function End Module Class C - Public Sub M(p1 As IEnumerable(Of Integer), p2 As List(Of Integer), p3 As {collectionName}(Of Integer)) + Public Sub M(p1 As IEnumerable(Of Integer), p2 As List(Of Integer), p3 As {collectionName}(Of Integer), comparer As IEqualityComparer(Of Integer)) ' Allowed p1.To{collectionName}() p2.To{collectionName}() + p3.To{collectionName}(comparer) ' Potentially modifies the collection ' No dataflow Dim l1 As IEnumerable(Of Integer) = p3 @@ -92,7 +104,7 @@ End Class "); } - [Theory] + [Theory, WorkItem(1432, "https://github.com/dotnet/roslyn-analyzers/issues/1432")] [MemberData(nameof(CollectionNames_Arity2))] public void NoDiagnosticCases_Arity2(string collectionName) { @@ -107,15 +119,21 @@ static class Extensions {{ return default({collectionName}); }} + + public static {collectionName} To{collectionName}(this IEnumerable> items, IEqualityComparer keyComparer) + {{ + return default({collectionName}); + }} }} class C {{ - public void M(IEnumerable> p1, List> p2, {collectionName} p3) + public void M(IEnumerable> p1, List> p2, {collectionName} p3, IEqualityComparer keyComparer) {{ // Allowed p1.To{collectionName}(); p2.To{collectionName}(); + p3.To{collectionName}(keyComparer); // Potentially modifies the collection // No dataflow IEnumerable> l1 = p3; @@ -133,13 +151,19 @@ Module Extensions Public Function To{collectionName}(Of TKey, TValue)(items As IEnumerable(Of KeyValuePair(Of TKey, TValue))) As {collectionName}(Of TKey, TValue) Return Nothing End Function + + _ + Public Function To{collectionName}(Of TKey, TValue)(items As IEnumerable(Of KeyValuePair(Of TKey, TValue)), keyComparer As IEqualityComparer(Of TKey)) As {collectionName}(Of TKey, TValue) + Return Nothing + End Function End Module Class C - Public Sub M(p1 As IEnumerable(Of KeyValuePair(Of Integer, Integer)), p2 As List(Of KeyValuePair(Of Integer, Integer)), p3 As {collectionName}(Of Integer, Integer)) + Public Sub M(p1 As IEnumerable(Of KeyValuePair(Of Integer, Integer)), p2 As List(Of KeyValuePair(Of Integer, Integer)), p3 As {collectionName}(Of Integer, Integer), keyComparer As IEqualityComparer(Of Integer)) ' Allowed p1.To{collectionName}() p2.To{collectionName}() + p3.To{collectionName}(keyComparer) ' Potentially modifies the collection ' No dataflow Dim l1 As IEnumerable(Of KeyValuePair(Of Integer, Integer)) = p3 @@ -153,7 +177,7 @@ End Class #region Diagnostic Tests - [Theory(Skip = "https://github.com/dotnet/roslyn-analyzers/issues/1318")] + [Theory] [MemberData(nameof(CollectionNames_Arity1))] public void DiagnosticCases_Arity1(string collectionName) { @@ -208,7 +232,7 @@ End Class GetBasicResultAt(15, 3, collectionName)); } - [Theory(Skip = "https://github.com/dotnet/roslyn-analyzers/issues/1318")] + [Theory] [MemberData(nameof(CollectionNames_Arity2))] public void DiagnosticCases_Arity2(string collectionName) {