Merge pull request #4268 from lgolding/master

Protect against nulls in EquatableAnalyzer and add unit tests

src/Diagnostics/Roslyn/Core/Performance/EquatableAnalyzer.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-using System;
 using System.Collections.Immutable;
+using System.Diagnostics;
 using System.Linq;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.Diagnostics;
 
 namespace Roslyn.Diagnostics.Analyzers
@@ -94,18 +93,24 @@ private void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol objec
 
         private bool IsObjectEqualsOverride(IMethodSymbol methodSymbol, INamedTypeSymbol objectType)
         {
+            Debug.Assert(methodSymbol != null);
+            if (methodSymbol == null)
+            {
+                return false;
+            }
+
             if (!methodSymbol.IsOverride)
             {
                 return false;
             }
 
             if (methodSymbol.Parameters.Length != 1 ||
-                !methodSymbol.Parameters[0].Type.Equals(objectType))
+                methodSymbol.Parameters[0]?.Type?.Equals(objectType) != true)
             {
                 return false;
             }
 
-            if (methodSymbol.ReturnType.SpecialType != SpecialType.System_Boolean)
+            if (methodSymbol.ReturnType?.SpecialType != SpecialType.System_Boolean)
             {
                 return false;
             }
@@ -114,9 +119,9 @@ private bool IsObjectEqualsOverride(IMethodSymbol methodSymbol, INamedTypeSymbol
             {
                 methodSymbol = methodSymbol.OverriddenMethod;
             }
-            while (methodSymbol.IsOverride);
+            while (methodSymbol?.IsOverride == true);
 
-            return methodSymbol.ContainingType.Equals(objectType);
+            return methodSymbol.ContainingType?.Equals(objectType) == true;
         }
     }
 }

src/Diagnostics/Roslyn/Test/Performance/EquatableAnalyzerTests.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using Microsoft.CodeAnalysis.Diagnostics;
+using Roslyn.Diagnostics.Analyzers;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.UnitTests.Performance
+{
+    public class EquatableAnalyzerTests : DiagnosticAnalyzerTestBase
+    {
+        [Fact]
+        public void NoDiagnosticForStructWithNoEqualsOverrideAndNoIEquatableImplementation()
+        {
+            var code = @"
+struct S
+{
+}
+";
+            VerifyCSharp(code);
+        }
+
+        [Fact]
+        public void NoDiagnosticForClassWithNoEqualsOverrideAndNoIEquatableImplementation()
+        {
+            var code = @"
+class C
+{
+}
+";
+            VerifyCSharp(code);
+        }
+
+        [Fact]
+        public void DiagnosticForStructWithEqualsOverrideButNoIEquatableImplementation()
+        {
+            var code = @"
+struct S
+{
+    public override bool Equals(object other)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.ImplementIEquatableMessage, "S");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 26, RoslynDiagnosticIds.ImplementIEquatableRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void NoDiagnosticForClassWithEqualsOverrideAndNoIEquatableImplementation()
+        {
+            var code = @"
+class C
+{
+    public override bool Equals(object other)
+    {
+        return true;
+    }
+}
+";
+            VerifyCSharp(code);
+        }
+
+        [Fact]
+        public void DiagnosticForStructWithIEquatableImplementationButNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+struct S : IEquatable<S>
+{
+    public bool Equals(S other)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "S");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 8, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationButNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public bool Equals(C other)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationWithNoParameterListAndNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public bool Equals
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationWithMalformedParameterListAndNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public bool Equals(
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationWithMalformedParameterListAndNoEqualsOverride2()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public bool Equals)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationWithNoParametersAndNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public bool Equals()
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationWithMalformedParameterDeclarationAndNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public bool Equals(x)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationWithWrongReturnTypeAndNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public int Equals(C x)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationWithNoBodyAndNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public bool Equals(C other)
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithIEquatableImplementationWithNoReturnTypeAndNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public Equals(C other)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void NoDiagnosticForClassWithEqualsOverrideWithWrongSignatureAndNoIEquatableImplementation()
+        {
+            var code = @"
+using System;
+
+class C
+{
+    public override Equals(object other, int n)
+    {
+        return true;
+    }
+}
+";
+            VerifyCSharp(code);
+        }
+
+        [Fact]
+        public void DiagnosticForClassWithExplicitIEquatableImplementationAndNoEqualsOverride()
+        {
+            var code = @"
+using System;
+
+class C : IEquatable<C>
+{
+    public bool IEquatable<C>.Equals(object other)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage = string.Format(RoslynDiagnosticsResources.OverrideObjectEqualsMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt(4, 7, RoslynDiagnosticIds.OverrideObjectEqualsRuleId, expectedMessage));
+        }
+
+        [Fact]
+        public void DiagnosticForDerivedStructWithEqualsOverrideAndNoIEquatableImplementation()
+        {
+            var code = @"
+using System;
+
+struct B
+{
+    public override bool Equals(object other)
+    {
+        return false;
+    }
+}
+
+struct C : B
+{
+    public override bool Equals(object other)
+    {
+        return true;
+    }
+}
+";
+            string expectedMessage1 = string.Format(RoslynDiagnosticsResources.ImplementIEquatableMessage, "B");
+            string expectedMessage2 = string.Format(RoslynDiagnosticsResources.ImplementIEquatableMessage, "C");
+            VerifyCSharp(code,
+                GetCSharpResultAt( 6, 26, RoslynDiagnosticIds.ImplementIEquatableRuleId, expectedMessage1),
+                GetCSharpResultAt(14, 26, RoslynDiagnosticIds.ImplementIEquatableRuleId, expectedMessage2));
+        }
+
+        protected override DiagnosticAnalyzer GetBasicDiagnosticAnalyzer()
+            => new EquatableAnalyzer();
+
+        protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()
+            => new EquatableAnalyzer();
+    }
+}

src/Diagnostics/Roslyn/Test/RoslynDiagnosticAnalyzersTest.csproj

@@ -86,6 +86,7 @@
   <ItemGroup>
     <Compile Include="ApiDesign\CancellationTokenMustBeLastTests.cs" />
     <Compile Include="Performance\EmptyArrayDiagnosticAnalyzerTests.cs" />
+    <Compile Include="Performance\EquatableAnalyzerTests.cs" />
     <Compile Include="Performance\LinqAnalyzerTests.cs" />
     <Compile Include="Performance\SpecializedEnumerableCreationAnalyzerTests.cs" />
     <Compile Include="Reliability\ImmutableCollectionAnalyzerTests.cs" />
@@ -98,4 +99,4 @@
     <Import Project="..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
     <Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
   </ImportGroup>
-</Project>
+</Project>
\ No newline at end of file