From 9f687171568a856c5c9d4678f233f441cde32bc5 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Mon, 24 Jun 2019 06:27:17 -0700 Subject: [PATCH] Handle indexers in CA2245 (AvoidPropertySelfAssignment) Found false positives during dogfooding --- .../AvoidPropertySelfAssignment.cs | 50 ++++++++++++- .../AvoidPropertySelfAssignmentTests.cs | 74 +++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AvoidPropertySelfAssignment.cs b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AvoidPropertySelfAssignment.cs index 8db77d3ad4..1264d7a8f0 100644 --- 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 + } } } } \ No newline at end of file diff --git a/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AvoidPropertySelfAssignmentTests.cs b/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AvoidPropertySelfAssignmentTests.cs index 67b8d974f7..003b92d278 100644 --- a/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AvoidPropertySelfAssignmentTests.cs +++ b/src/Microsoft.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() {