From 8570e006ed74f8ffc047477724430e7b9758ca50 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Tue, 1 Oct 2019 10:50:40 -0700 Subject: [PATCH] Fix NRE in recently added AssigningSymbolAndItsMemberInSameStatement analyzer Handle null operation type, which is possible for OperationKind.None Found while dogfooding the latest package --- ...igningSymbolAndItsMemberInSameStatement.cs | 28 +++++++------------ ...gSymbolAndItsMemberInSameStatementTests.cs | 15 ++++++++++ 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatement.cs b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatement.cs index aa73798712..bc790d7d45 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatement.cs +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatement.cs @@ -50,29 +50,21 @@ private void AnalyzeAssignment(OperationAnalysisContext context) } // This analyzer makes sense only for reference type objects - if (operationTarget.Instance?.Type.IsValueType == true) + if (operationTarget.Instance?.Type?.IsReferenceType != true) { return; } - // Search for object equal to operationTarget.Instance further in assignment chain - bool isViolationFound = false; - if (operationTarget.Instance is ILocalReferenceOperation localInstance) + bool isViolationFound = operationTarget.Instance switch { - isViolationFound = AnalyzeAssignmentToMember(assignmentOperation, localInstance, (a, b) => a.Local.Equals(b.Local)); - } - else if (operationTarget.Instance is IMemberReferenceOperation memberInstance) - { - isViolationFound = AnalyzeAssignmentToMember(assignmentOperation, memberInstance, (a, b) => a.Member.Equals(b.Member) && a.Instance?.Syntax.ToString() == b.Instance?.Syntax.ToString()); - } - else if (operationTarget.Instance is IParameterReferenceOperation parameterInstance) - { - isViolationFound = AnalyzeAssignmentToMember(assignmentOperation, parameterInstance, (a, b) => a.Parameter.Equals(b.Parameter)); - } - else - { - return; - } + ILocalReferenceOperation localInstance => + AnalyzeAssignmentToMember(assignmentOperation, localInstance, (a, b) => a.Local.Equals(b.Local)), + IMemberReferenceOperation memberInstance => + AnalyzeAssignmentToMember(assignmentOperation, memberInstance, (a, b) => a.Member.Equals(b.Member) && a.Instance?.Syntax.ToString() == b.Instance?.Syntax.ToString()), + IParameterReferenceOperation parameterInstance => + AnalyzeAssignmentToMember(assignmentOperation, parameterInstance, (a, b) => a.Parameter.Equals(b.Parameter)), + _ => false, + }; if (isViolationFound) { diff --git a/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatementTests.cs b/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatementTests.cs index e9dfdf21a1..b340637b9e 100644 --- a/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatementTests.cs +++ b/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatementTests.cs @@ -316,5 +316,20 @@ public void Method() } ", TestValidationMode.AllowCompileErrors); } + + [Fact] + public void CSharpAssignmentInCodeWithOperationNone() + { + VerifyCSharpUnsafeCode(@" +public struct Test +{ + public System.IntPtr PtrField; + public unsafe void Method(Test a, Test *b) + { + b->PtrField = a.PtrField; + } +} +"); + } } }