Merge pull request #2625 from mavasani/CA2245_Indexers

Handle indexers in CA2245 (AvoidPropertySelfAssignment)
dotnet · Jun 24, 2019 · 3b22a6a · 3b22a6a
2 parents fc74193 + 9f68717
commit 3b22a6a
Show file tree

Hide file tree

Showing 2 changed files with 123 additions and 1 deletion.
diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AvoidPropertySelfAssignment.cs b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AvoidPropertySelfAssignment.cs
@@ -49,11 +49,24 @@ public override void Initialize(AnalysisContext analysisContext)
                     return;
                 }
 
-                if (!Equals(operationTarget.Property, operationValue.Property))
+                if (!Equals(operationTarget.Property, operationValue.Property) ||
+                    operationTarget.Arguments.Length != operationValue.Arguments.Length)
                 {
                     return;
                 }
 
+                if (operationTarget.Arguments.Length > 0)
+                {
+                    // Indexers - compare if all the arguments are identical.
+                    for (int i = 0; i < operationTarget.Arguments.Length; i++)
+                    {
+                        if (!IsArgumentValueEqual(operationTarget.Arguments[i].Value, operationValue.Arguments[i].Value))
+                        {
+                            return;
+                        }
+                    }
+                }
+
                 if (operationTarget.Instance is IInstanceReferenceOperation targetInstanceReference &&
                     targetInstanceReference.ReferenceKind == InstanceReferenceKind.ContainingTypeInstance &&
                     operationValue.Instance is IInstanceReferenceOperation valueInstanceReference &&
@@ -63,6 +76,41 @@ operationValue.Instance is IInstanceReferenceOperation valueInstanceReference &&
                     operationContext.ReportDiagnostic(diagnostic);
                 }
             }, OperationKind.SimpleAssignment);
+
+            return;
+
+            // Local functions
+            static bool IsArgumentValueEqual(IOperation targetArg, IOperation valueArg)
+            {
+                // Check if arguments are identical constant/local/parameter reference operations.
+                //   1. Not identical: 'this[i] = this[j]'
+                //   2. Identical: 'this[i] = this[i]', 'this[0] = this[0]'
+                if (targetArg.Kind != valueArg.Kind)
+                {
+                    return false;
+                }
+
+                if (targetArg.ConstantValue.HasValue != valueArg.ConstantValue.HasValue)
+                {
+                    return false;
+                }
+
+                if (targetArg.ConstantValue.HasValue)
+                {
+                    return Equals(targetArg.ConstantValue.Value, valueArg.ConstantValue.Value);
+                }
+
+#pragma warning disable IDE0055 // Fix formatting - Does not seem to be handling switch expressions.
+                return targetArg switch
+                {
+                    ILocalReferenceOperation targetLocalReference =>
+                        Equals(targetLocalReference.Local, ((ILocalReferenceOperation)valueArg).Local),
+                    IParameterReferenceOperation targetParameterReference =>
+                        Equals(targetParameterReference.Parameter, ((IParameterReferenceOperation)valueArg).Parameter),
+                    _ => false,
+                };
+#pragma warning restore IDE0055 // Fix formatting
+            }
         }
     }
 }
diff --git a/...oft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AvoidPropertySelfAssignmentTests.cs b/...oft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AvoidPropertySelfAssignmentTests.cs
@@ -183,6 +183,80 @@ public C()
 ");
         }
 
+        [Fact]
+        public void CSharpIndexerAssignmentDoesNotCauseDiagnosticToAppear()
+        {
+            VerifyCSharp(@"
+internal class A
+{
+    private int[] _a;
+    public int this[int i] { get => _a[i]; set => _a[i] = value; }
+
+    public void ExchangeValue(int i, int j)
+    {
+        var temp = this[i];
+        this[i] = this[j];
+        this[j] = temp;
+    }
+}
+");
+        }
+
+        [Fact]
+        public void CSharpIndexerAssignmentWithSameConstantIndexCausesDiagnosticToAppear()
+        {
+            VerifyCSharp(@"
+internal class A
+{
+    private int[] _a;
+    public int this[int i] { get => _a[i]; set => _a[i] = value; }
+
+    public void M()
+    {
+        this[0] = this[0];
+    }
+}
+",
+            GetCSharpResultAt(9, 19, "this[]"));
+        }
+
+        [Fact]
+        public void CSharpIndexerAssignmentWithSameLocalReferenceIndexCausesDiagnosticToAppear()
+        {
+            VerifyCSharp(@"
+internal class A
+{
+    private int[] _a;
+    public int this[int i] { get => _a[i]; set => _a[i] = value; }
+
+    public void M()
+    {
+        int local = 0;
+        this[local] = this[local];
+    }
+}
+",
+            GetCSharpResultAt(10, 23, "this[]"));
+        }
+
+        [Fact]
+        public void CSharpIndexerAssignmentWithSameParameterReferenceIndexCausesDiagnosticToAppear()
+        {
+            VerifyCSharp(@"
+internal class A
+{
+    private int[] _a;
+    public int this[int i] { get => _a[i]; set => _a[i] = value; }
+
+    public void M(int param)
+    {
+        this[param] = this[param];
+    }
+}
+",
+            GetCSharpResultAt(9, 23, "this[]"));
+        }
+
         [Fact]
         public void VbAssignmentInConstructorWithNoArguments()
         {