Merge pull request dotnet#29852 from mavasani/DeadCodeAnalysis_FollowUp

Enable DeadCodeAnalysis rules and address design/review feedback.
alatas · Sep 14, 2018 · eb489b7 · eb489b7
2 parents 4de0585 + a025d9d
commit eb489b7
Show file tree

Hide file tree

Showing 15 changed files with 551 additions and 129 deletions.
diff --git a/src/Compilers/Test/Utilities/VisualBasic/ParserTestUtilities.vb b/src/Compilers/Test/Utilities/VisualBasic/ParserTestUtilities.vb
@@ -49,11 +49,11 @@ Friend Module ParserTestUtilities
     End Function
 
     Public Function ParseAndVerify(code As XCData, ParamArray expectedDiagnostics() As DiagnosticDescription) As SyntaxTree
-        Return ParseAndVerify(TestBase.NormalizeNewLines(code), VisualBasicParseOptions.Default, expectedDiagnostics, errorCodesOnly:=False)
+        Return ParseAndVerify(TestHelpers.NormalizeNewLines(code), VisualBasicParseOptions.Default, expectedDiagnostics, errorCodesOnly:=False)
     End Function
 
     Public Function ParseAndVerify(code As XCData, options As VisualBasicParseOptions, ParamArray expectedDiagnostics() As DiagnosticDescription) As SyntaxTree
-        Return ParseAndVerify(TestBase.NormalizeNewLines(code), options, expectedDiagnostics, errorCodesOnly:=False)
+        Return ParseAndVerify(TestHelpers.NormalizeNewLines(code), options, expectedDiagnostics, errorCodesOnly:=False)
     End Function
 
     Public Function ParseAndVerify(source As String, ParamArray expectedDiagnostics() As DiagnosticDescription) As SyntaxTree

diff --git a/src/Compilers/VisualBasic/Test/Emit/ExpressionTrees/CodeGenExprLambda.vb b/src/Compilers/VisualBasic/Test/Emit/ExpressionTrees/CodeGenExprLambda.vb
@@ -1849,7 +1849,7 @@ End Structure
                                         Optional latestReferences As Boolean = False,
                                         Optional addXmlReferences As Boolean = False,
                                         Optional diagnostics() As DiagnosticDescription = Nothing)
-            TestExpressionTrees(sourceFile, TestBase.NormalizeNewLines(result), checked, optimize, latestReferences, addXmlReferences, diagnostics)
+            TestExpressionTrees(sourceFile, TestHelpers.NormalizeNewLines(result), checked, optimize, latestReferences, addXmlReferences, diagnostics)
         End Sub
 
         Private Class ExpressionTreeTest

diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/FixAllProvider/BatchFixerTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/FixAllProvider/BatchFixerTests.cs
@@ -43,7 +43,7 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
                     var symbol = context.SemanticModel.GetSymbolInfo(node).Symbol;
                     if (symbol != null && symbol.Kind == SymbolKind.Field)
                     {
-                        var diagnostic = CodeAnalysis.Diagnostic.Create(Descriptor, node.GetLocation());
+                        var diagnostic = Diagnostic.Create(Descriptor, node.GetLocation());
                         context.ReportDiagnostic(diagnostic);
                     }
                 }

diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/RemoveSuppressionTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/RemoveSuppressionTests.cs
@@ -47,7 +47,7 @@ public void AnalyzeNode(SyntaxNodeAnalysisContext context)
             {
                 var classDecl = (ClassDeclarationSyntax)context.Node;
                 var location = _reportDiagnosticsWithoutLocation ? Location.None : classDecl.Identifier.GetLocation();
-                context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Decsciptor, location));
+                context.ReportDiagnostic(Diagnostic.Create(Decsciptor, location));
             }
         }
 

diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTest_FixMultipleTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTest_FixMultipleTests.cs
@@ -47,8 +47,8 @@ public void AnalyzeNode(SyntaxNodeAnalysisContext context)
                 {
                     var classDecl = (ClassDeclarationSyntax)context.Node;
                     var location = classDecl.Identifier.GetLocation();
-                    context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Decsciptor1, location));
-                    context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Decsciptor2, location));
+                    context.ReportDiagnostic(Diagnostic.Create(Decsciptor1, location));
+                    context.ReportDiagnostic(Diagnostic.Create(Decsciptor2, location));
                 }
             }
 

diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTests.cs
@@ -468,7 +468,7 @@ public override void Initialize(AnalysisContext context)
                     public void AnalyzeNode(SyntaxNodeAnalysisContext context)
                     {
                         var classDecl = (ClassDeclarationSyntax)context.Node;
-                        context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Decsciptor, classDecl.Identifier.GetLocation()));
+                        context.ReportDiagnostic(Diagnostic.Create(Decsciptor, classDecl.Identifier.GetLocation()));
                     }
                 }
 
@@ -530,7 +530,7 @@ public override void Initialize(AnalysisContext context)
                     public void AnalyzeNode(SyntaxNodeAnalysisContext context)
                     {
                         var classDecl = (ClassDeclarationSyntax)context.Node;
-                        context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(_descriptor, classDecl.Identifier.GetLocation()));
+                        context.ReportDiagnostic(Diagnostic.Create(_descriptor, classDecl.Identifier.GetLocation()));
                     }
                 }
 
@@ -595,7 +595,7 @@ public override void Initialize(AnalysisContext context)
                     public void AnalyzeNode(SyntaxNodeAnalysisContext context)
                     {
                         var classDecl = (ClassDeclarationSyntax)context.Node;
-                        context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(_descriptor, classDecl.Identifier.GetLocation()));
+                        context.ReportDiagnostic(Diagnostic.Create(_descriptor, classDecl.Identifier.GetLocation()));
                     }
                 }
 
@@ -647,7 +647,7 @@ public override void Initialize(AnalysisContext context)
                 public void AnalyzeNode(SyntaxNodeAnalysisContext context)
                 {
                     var classDecl = (ClassDeclarationSyntax)context.Node;
-                    context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Decsciptor, classDecl.GetLocation()));
+                    context.ReportDiagnostic(Diagnostic.Create(Decsciptor, classDecl.GetLocation()));
                 }
             }
 
@@ -760,39 +760,39 @@ public void AnalyzeNode(SyntaxNodeAnalysisContext context)
                         {
                             case SyntaxKind.ClassDeclaration:
                                 var classDecl = (ClassDeclarationSyntax)context.Node;
-                                context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Descriptor, classDecl.Identifier.GetLocation()));
+                                context.ReportDiagnostic(Diagnostic.Create(Descriptor, classDecl.Identifier.GetLocation()));
                                 break;
 
                             case SyntaxKind.NamespaceDeclaration:
                                 var ns = (NamespaceDeclarationSyntax)context.Node;
-                                context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Descriptor, ns.Name.GetLocation()));
+                                context.ReportDiagnostic(Diagnostic.Create(Descriptor, ns.Name.GetLocation()));
                                 break;
 
                             case SyntaxKind.MethodDeclaration:
                                 var method = (MethodDeclarationSyntax)context.Node;
-                                context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Descriptor, method.Identifier.GetLocation()));
+                                context.ReportDiagnostic(Diagnostic.Create(Descriptor, method.Identifier.GetLocation()));
                                 break;
 
                             case SyntaxKind.PropertyDeclaration:
                                 var property = (PropertyDeclarationSyntax)context.Node;
-                                context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Descriptor, property.Identifier.GetLocation()));
+                                context.ReportDiagnostic(Diagnostic.Create(Descriptor, property.Identifier.GetLocation()));
                                 break;
 
                             case SyntaxKind.FieldDeclaration:
                                 var field = (FieldDeclarationSyntax)context.Node;
-                                context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Descriptor, field.Declaration.Variables.First().Identifier.GetLocation()));
+                                context.ReportDiagnostic(Diagnostic.Create(Descriptor, field.Declaration.Variables.First().Identifier.GetLocation()));
                                 break;
 
                             case SyntaxKind.EventDeclaration:
                                 var e = (EventDeclarationSyntax)context.Node;
-                                context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Descriptor, e.Identifier.GetLocation()));
+                                context.ReportDiagnostic(Diagnostic.Create(Descriptor, e.Identifier.GetLocation()));
                                 break;
 
                             case SyntaxKind.EnumDeclaration:
                                 // Report diagnostic on each descendant comment trivia
                                 foreach (var trivia in context.Node.DescendantTrivia().Where(t => t.Kind() == SyntaxKind.SingleLineCommentTrivia || t.Kind() == SyntaxKind.MultiLineCommentTrivia))
                                 {
-                                    context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Descriptor, trivia.GetLocation()));
+                                    context.ReportDiagnostic(Diagnostic.Create(Descriptor, trivia.GetLocation()));
                                 }
                                 break;
                         }
@@ -1571,7 +1571,7 @@ public override void Initialize(AnalysisContext context)
 
                 public void AnalyzeNode(SyntaxNodeAnalysisContext context)
                 {
-                    context.ReportDiagnostic(CodeAnalysis.Diagnostic.Create(Descriptor, Location.None));
+                    context.ReportDiagnostic(Diagnostic.Create(Descriptor, Location.None));
                 }
             }
 

diff --git a/src/EditorFeatures/CSharpTest/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/EditorFeatures/CSharpTest/RemoveUnusedMembers/RemoveUnusedMembersTests.cs
@@ -5,17 +5,16 @@
 using Microsoft.CodeAnalysis.CSharp.RemoveUnusedMembers;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Diagnostics;
-using Microsoft.CodeAnalysis.RemoveUnusedMembers;
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Xunit;
+using static Roslyn.Test.Utilities.TestHelpers;
 
 namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RemoveUnusedMembers
 {
     public class RemoveUnusedMembersTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest
     {
         internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
-            => (new CSharpRemoveUnusedMembersDiagnosticAnalyzer(forceEnableRules: true),
-            new CSharpRemoveUnusedMembersCodeFixProvider());
+            => (new CSharpRemoveUnusedMembersDiagnosticAnalyzer(), new CSharpRemoveUnusedMembersCodeFixProvider());
 
         [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
         [InlineData("public")]
@@ -32,6 +31,37 @@ await TestMissingInRegularAndScriptAsync(
 }}");
         }
 
+        [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        [InlineData("public")]
+        [InlineData("internal")]
+        [InlineData("protected")]
+        [InlineData("protected internal")]
+        [InlineData("private protected")]
+        public async Task NonPrivateFieldWithConstantInitializer(string accessibility)
+        {
+            await TestMissingInRegularAndScriptAsync(
+$@"class MyClass
+{{
+    {accessibility} int [|_goo|] = 0;
+}}");
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        [InlineData("public")]
+        [InlineData("internal")]
+        [InlineData("protected")]
+        [InlineData("protected internal")]
+        [InlineData("private protected")]
+        public async Task NonPrivateFieldWithNonConstantInitializer(string accessibility)
+        {
+            await TestMissingInRegularAndScriptAsync(
+$@"class MyClass
+{{
+    {accessibility} int [|_goo|] = _goo2;
+    private static readonly int _goo2 = 0;
+}}");
+        }
+
         [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
         [InlineData("public")]
         [InlineData("internal")]
@@ -120,6 +150,76 @@ await TestInRegularAndScriptAsync(
 }");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task GenericMethodIsUnused()
+        {
+            await TestInRegularAndScriptAsync(
+@"class MyClass
+{
+    private int [|M|]<T>() => 0;
+}",
+@"class MyClass
+{
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task MethodInGenericTypeIsUnused()
+        {
+            await TestInRegularAndScriptAsync(
+@"class MyClass<T>
+{
+    private int [|M|]() => 0;
+}",
+@"class MyClass<T>
+{
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task InstanceConstructorIsUnused_NoArguments()
+        {
+            // We only flag constructors with arguments.
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass
+{
+    private [|MyClass()|] { }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task InstanceConstructorIsUnused_WithArguments()
+        {
+            await TestInRegularAndScriptAsync(
+@"class MyClass
+{
+    private [|MyClass(int i)|] { }
+}",
+@"class MyClass
+{
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task StaticConstructorIsNotFlagged()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass
+{
+    static [|MyClass()|] { }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task DestructorIsNotFlagged()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass
+{
+    ~[|MyClass()|] { }
+}");
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
         public async Task PropertyIsUnused()
         {
@@ -571,6 +671,83 @@ public void M2()
 }");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task GenericMethodIsInvoked_ExplicitTypeArguments()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass
+{
+    private int [|M1|]<T>() => 0;
+    private int M2() => M1<int>();
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task GenericMethodIsInvoked_ImplicitTypeArguments()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass
+{
+    private T [|M1|]<T>(T t) => t;
+    private int M2() => M1(0);
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task MethodInGenericTypeIsInvoked_NoTypeArguments()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass<T>
+{
+    private int [|M1|]() => 0;
+    private int M2() => M1();
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task MethodInGenericTypeIsInvoked_NonConstructedType()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass<T>
+{
+    private int [|M1|]() => 0;
+    private int M2(MyClass<T> m) => m.M1();
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task MethodInGenericTypeIsInvoked_ConstructedType()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass<T>
+{
+    private int [|M1|]() => 0;
+    private int M2(MyClass<int> m) => m.M1();
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task InstanceConstructorIsUsed_NoArguments()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass
+{
+    private [|MyClass()|] { }
+    public static readonly MyClass Instance = new MyClass();
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
+        public async Task InstanceConstructorIsUsed_WithArguments()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class MyClass
+{
+    private [|MyClass(int i)|] { }
+    public static readonly MyClass Instance = new MyClass(0);
+}");
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedMembers)]
         public async Task PropertyIsRead()
         {

diff --git a/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs b/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs
@@ -26,7 +26,7 @@
 namespace Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions
 {
     [UseExportProvider]
-    public abstract class AbstractCodeActionOrUserDiagnosticTest : TestBase
+    public abstract class AbstractCodeActionOrUserDiagnosticTest
     {
         public struct TestParameters
         {