Merge pull request #1450 from mavasani/ImmutableCollection

Do not flag RS0012 for ToImmutable invocations if the contents of the…
dotnet · Dec 6, 2017 · f43577b · f43577b
2 parents 7597eb5 + b71a93e
commit f43577b
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 8 deletions.
diff --git a/...s/Core/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValue.cs b/...s/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/...s/ImmutableCollections/DoNotCallToImmutableCollectionOnAnImmutableCollectionValueTests.cs b/...s/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}<TSource>);
      }}
+
+     public static {collectionName}<TSource> To{collectionName}<TSource>(this IEnumerable<TSource> items, IEqualityComparer<TSource> comparer)
+     {{
+         return default({collectionName}<TSource>);
+     }}
 }}
 
 class C
 {{
-    public void M(IEnumerable<int> p1, List<int> p2, {collectionName}<int> p3)
+    public void M(IEnumerable<int> p1, List<int> p2, {collectionName}<int> p3, IEqualityComparer<int> comparer)
     {{
         // Allowed
         p1.To{collectionName}();
         p2.To{collectionName}();
+        p3.To{collectionName}(comparer); // Potentially modifies the collection
 
         // No dataflow
         IEnumerable<int> 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
+
+	<System.Runtime.CompilerServices.Extension> _
+	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}<TKey, TValue>);
      }}
+
+    public static {collectionName}<TKey, TValue> To{collectionName}<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> items, IEqualityComparer<TKey> keyComparer)
+     {{
+         return default({collectionName}<TKey, TValue>);
+     }}
 }}
 
 class C
 {{
-    public void M(IEnumerable<KeyValuePair<int, int>> p1, List<KeyValuePair<int, int>> p2, {collectionName}<int, int> p3)
+    public void M(IEnumerable<KeyValuePair<int, int>> p1, List<KeyValuePair<int, int>> p2, {collectionName}<int, int> p3, IEqualityComparer<int> keyComparer)
     {{
         // Allowed
         p1.To{collectionName}();
         p2.To{collectionName}();
+        p3.To{collectionName}(keyComparer); // Potentially modifies the collection
 
         // No dataflow
         IEnumerable<KeyValuePair<int, int>> 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
+
+	<System.Runtime.CompilerServices.Extension> _
+	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)
         {