Merge pull request #2258 from srivatsn/finalizers

Cleaned up CA1821 (Remove empty finalizers) and fixed a bug in the fixer

src/Diagnostics/FxCop/CSharp/CSharpFxCopRulesDiagnosticAnalyzers.csproj

@@ -78,7 +78,7 @@
     <Compile Include="Design\CodeFixes\CA1052CSharpCodeFixProvider.cs" />
     <Compile Include="Design\CSharpCA1003DiagnosticAnalyzer.cs" />
     <Compile Include="Design\CSharpCA1024DiagnosticAnalyzer.cs" />
-    <Compile Include="Performance\CSharpCA1821DiagnosticAnalyzer.cs" />
+    <Compile Include="Performance\RemoveEmptyFinalizers.cs" />
     <Compile Include="Reliability\CSharpCA2002DiagnosticAnalyzer.cs" />
     <Compile Include="Usage\CodeFixes\CA2235CSharpCodeFixProvider.cs" />
     <Compile Include="Usage\CSharpCA2200DiagnosticAnalyzer.cs" />

src/Diagnostics/FxCop/CSharp/Performance/CSharpCA1821DiagnosticAnalyzer.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.Linq;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
-using Microsoft.CodeAnalysis.Diagnostics;
-using Microsoft.CodeAnalysis.FxCopAnalyzers.Performance;
-using Microsoft.CodeAnalysis.FxCopAnalyzers.Utilities;
-
-namespace Microsoft.CodeAnalysis.CSharp.FxCopAnalyzers.Performance
-{
-    [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public class CSharpCA1821DiagnosticAnalyzer : CA1821DiagnosticAnalyzer<SyntaxKind>
-    {
-        protected override AbstractCodeBlockEndedAnalyzer GetCodeBlockEndedAnalyzer()
-        {
-            return new CodeBlockEndedAnalyzer();
-        }
-
-        private sealed class CodeBlockEndedAnalyzer : AbstractCodeBlockEndedAnalyzer
-        {
-            protected override bool IsEmptyFinalizer(SyntaxNode node, SemanticModel model)
-            {
-                foreach (var exp in node.DescendantNodes().OfType<StatementSyntax>().Where(n => !n.IsKind(SyntaxKind.Block) && !n.IsKind(SyntaxKind.EmptyStatement)))
-                {
-                    // NOTE: FxCop only checks if there is any method call within a given destructor to decide an empty finalizer.
-                    // Here in order to minimize false negatives, we conservatively treat it as non-empty finalizer if its body contains any statements.
-                    // But, still conditional methods like Debug.Fail() will be considered as being empty as FxCop currently does.
-
-                    var method = exp as ExpressionStatementSyntax;
-                    if (method != null && HasConditionalAttribute(method.Expression, model))
-                    {
-                        continue;
-                    }
-
-                    return false;
-                }
-
-                return true;
-            }
-
-            private bool HasConditionalAttribute(SyntaxNode root, SemanticModel model)
-            {
-                var node = root as InvocationExpressionSyntax;
-                if (node != null)
-                {
-                    var exp = node.Expression as MemberAccessExpressionSyntax;
-                    if (exp != null)
-                    {
-                        var symbol = model.GetSymbolInfo(exp.Name).Symbol;
-                        if (symbol != null && symbol.GetAttributes().Any(n => n.AttributeClass.Equals(WellKnownTypes.ConditionalAttribute(model.Compilation))))
-                        {
-                            return true;
-                        }
-                    }
-                }
-
-                return false;
-            }
-        }
-    }
-}

src/Diagnostics/FxCop/CSharp/Performance/RemoveEmptyFinalizers.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.Linq;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.FxCopAnalyzers.Performance;
+using Microsoft.CodeAnalysis.FxCopAnalyzers.Utilities;
+
+namespace Microsoft.CodeAnalysis.CSharp.FxCopAnalyzers.Performance
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public class CSharpRemoveEmptyFinalizers : RemoveEmptyFinalizers<SyntaxKind>
+    {
+        protected override bool IsEmptyFinalizer(SyntaxNode node, SemanticModel model)
+        {
+            foreach (var exp in node.DescendantNodes().OfType<StatementSyntax>().Where(n => !n.IsKind(SyntaxKind.Block) && !n.IsKind(SyntaxKind.EmptyStatement)))
+            {
+                // NOTE: FxCop only checks if there is any method call within a given destructor to decide an empty finalizer.
+                // Here in order to minimize false negatives, we conservatively treat it as non-empty finalizer if its body contains any statements.
+                // But, still conditional methods like Debug.Fail() will be considered as being empty as FxCop currently does.
+
+                var method = exp as ExpressionStatementSyntax;
+                if (method != null && HasConditionalAttribute(method.Expression, model))
+                {
+                    continue;
+                }
+
+                return false;
+            }
+
+            return true;
+        }
+
+        private bool HasConditionalAttribute(SyntaxNode root, SemanticModel model)
+        {
+            var node = root as InvocationExpressionSyntax;
+            if (node != null)
+            {
+                var exp = node.Expression as MemberAccessExpressionSyntax;
+                if (exp != null)
+                {
+                    var symbol = model.GetSymbolInfo(exp.Name).Symbol;
+                    if (symbol != null && symbol.GetAttributes().Any(n => n.AttributeClass.Equals(WellKnownTypes.ConditionalAttribute(model.Compilation))))
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
+    }
+}

src/Diagnostics/FxCop/Core/FxCopRulesDiagnosticAnalyzers.csproj

@@ -126,9 +126,8 @@
     <Compile Include="MultipleCodeFixProviderBase.cs" />
     <Compile Include="Naming\CA1708DiagnosticAnalyzer.cs" />
     <Compile Include="Naming\CA1715DiagnosticAnalyzer.cs" />
-    <Compile Include="Performance\CA1821DiagnosticAnalyzer.cs" />
-    <Compile Include="Performance\CA1821DiagnosticAnalyzerRule.cs" />
-    <Compile Include="Performance\CodeFixes\CA1821CodeFixProvider.cs" />
+    <Compile Include="Performance\RemoveEmptyFinalizers.cs" />
+    <Compile Include="Performance\RemoveEmptyFinalizers.Fixer.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Reliability\CA2002DiagnosticAnalyzer.cs" />
     <Compile Include="Shared\CommonAccessibilityUtilities.cs" />

src/Diagnostics/FxCop/Core/Performance/CA1821DiagnosticAnalyzer.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.Collections.Immutable;
-using Microsoft.CodeAnalysis.Diagnostics;
-using Microsoft.CodeAnalysis.FxCopAnalyzers.Utilities;
-
-namespace Microsoft.CodeAnalysis.FxCopAnalyzers.Performance
-{
-    public abstract class CA1821DiagnosticAnalyzer<TLanguageKindEnum> : DiagnosticAnalyzer where TLanguageKindEnum : struct
-    {
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
-        {
-            get
-            {
-                return ImmutableArray.Create(CA1821DiagnosticAnalyzerRule.Rule);
-            }
-        }
-
-        public override void Initialize(AnalysisContext analysisContext)
-        {
-            analysisContext.RegisterCodeBlockStartAction<TLanguageKindEnum>(
-                (context) =>
-                {
-                    var method = context.OwningSymbol as IMethodSymbol;
-                    if (method == null)
-                    {
-                        return;
-                    }
-
-                    if (!IsDestructor(method))
-                    {
-                        return;
-                    }
-
-                    context.RegisterCodeBlockEndAction(GetCodeBlockEndedAnalyzer().AnalyzeCodeBlock);
-                });
-        }
-
-        protected abstract AbstractCodeBlockEndedAnalyzer GetCodeBlockEndedAnalyzer();
-
-        private static bool IsDestructor(IMethodSymbol method)
-        {
-            if (method.MethodKind == MethodKind.Destructor)
-            {
-                return true; // for C#
-            }
-
-            if (method.Name != "Finalize" || method.Parameters.Length != 0 || !method.ReturnsVoid)
-            {
-                return false;
-            }
-
-            var overridden = method.OverriddenMethod;
-            if (overridden == null)
-            {
-                return false;
-            }
-
-            for (var o = overridden.OverriddenMethod; o != null; o = o.OverriddenMethod)
-            {
-                overridden = o;
-            }
-
-            return overridden.ContainingType.SpecialType == SpecialType.System_Object; // it is object.Finalize
-        }
-
-        protected abstract class AbstractCodeBlockEndedAnalyzer
-        {
-            public ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
-            {
-                get
-                {
-                    return ImmutableArray.Create(CA1821DiagnosticAnalyzerRule.Rule);
-                }
-            }
-
-            public void AnalyzeCodeBlock(CodeBlockAnalysisContext context)
-            {
-                if (IsEmptyFinalizer(context.CodeBlock, context.SemanticModel))
-                {
-                    context.ReportDiagnostic(context.OwningSymbol.CreateDiagnostic(CA1821DiagnosticAnalyzerRule.Rule));
-                }
-            }
-
-            protected abstract bool IsEmptyFinalizer(SyntaxNode node, SemanticModel model);
-        }
-    }
-}

src/Diagnostics/FxCop/Core/Performance/CodeFixes/CA1821CodeFixProvider.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.Collections.Immutable;
-using System.Composition;
-using System.Threading;
-using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.CodeFixes;
-
-namespace Microsoft.CodeAnalysis.FxCopAnalyzers.Performance
-{
-    /// <summary>
-    /// CA1821: Remove empty finalizers
-    /// </summary>
-    [ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic, Name = CA1821DiagnosticAnalyzerRule.RuleId), Shared]
-    public sealed class CA1821CodeFixProvider : CodeFixProviderBase
-    {
-        public sealed override ImmutableArray<string> FixableDiagnosticIds
-        {
-            get { return ImmutableArray.Create(CA1821DiagnosticAnalyzerRule.RuleId); }
-        }
-
-        protected sealed override string GetCodeFixDescription(Diagnostic diagnostic)
-        {
-            return FxCopFixersResources.RemoveEmptyFinalizers;
-        }
-
-        internal override Task<Document> GetUpdatedDocumentAsync(Document document, SemanticModel model, SyntaxNode root, SyntaxNode nodeToFix, Diagnostic diagnostic, CancellationToken cancellationToken)
-        {
-            return Task.FromResult(document.WithSyntaxRoot(root.RemoveNode(nodeToFix, SyntaxRemoveOptions.KeepNoTrivia)));
-        }
-    }
-}

src/Diagnostics/FxCop/Core/Performance/RemoveEmptyFinalizers.Fixer.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.Composition;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.Editing;
+
+namespace Microsoft.CodeAnalysis.FxCopAnalyzers.Performance
+{
+    /// <summary>
+    /// CA1821: Remove empty finalizers
+    /// </summary>
+    [ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic, Name = RuleId), Shared]
+    public sealed class RemoveEmptyFinalizersFixer : CodeFixProvider
+    {
+        public const string RuleId = "CA1821";
+
+        public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(RuleId);
+
+        public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+        {
+            var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+            var node = root.FindNode(context.Span);
+
+            if (node == null)
+            {
+                return;
+            }
+
+            // We cannot have multiple overlapping diagnostics of this id.
+            var diagnostic = context.Diagnostics.Single();
+            context.RegisterCodeFix(new MyCodeAction(FxCopFixersResources.RemoveEmptyFinalizers,
+                             async ct => await RemoveFinalizer(context.Document, node, ct).ConfigureAwait(false)),
+                        diagnostic);
+            return;
+        }
+
+        private async Task<Document> RemoveFinalizer(Document document, SyntaxNode node, CancellationToken cancellationToken)
+        {
+            var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
+
+            // Get the declaration so that we step up to the methodblocksyntax and not the methodstatementsyntax for VB.
+            node = editor.Generator.GetDeclaration(node);
+            editor.RemoveNode(node);
+            return editor.GetChangedDocument();
+        }
+
+        private class MyCodeAction : DocumentChangeAction
+        {
+            public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument)
+                : base(title, createChangedDocument)
+            {
+            }
+        }
+    }
+}

src/Diagnostics/FxCop/Core/Performance/CA1821DiagnosticAnalyzerRule.cs → src/Diagnostics/FxCop/Core/Performance/RemoveEmptyFinalizers.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.Collections.Immutable;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.FxCopAnalyzers.Utilities;
 
 namespace Microsoft.CodeAnalysis.FxCopAnalyzers.Performance
 {
-    internal class CA1821DiagnosticAnalyzerRule
+    public abstract class RemoveEmptyFinalizers<TLanguageKindEnum> : DiagnosticAnalyzer where TLanguageKindEnum : struct
     {
         public const string RuleId = "CA1821";
         private static LocalizableString s_localizableMessageAndTitle = new LocalizableResourceString(nameof(FxCopRulesResources.RemoveEmptyFinalizers), FxCopRulesResources.ResourceManager, typeof(FxCopRulesResources));
@@ -19,5 +21,62 @@ internal class CA1821DiagnosticAnalyzerRule
                                                                          description: s_localizableDescription,
                                                                          helpLinkUri: "http://msdn.microsoft.com/library/bb264476.aspx",
                                                                          customTags: DiagnosticCustomTags.Microsoft);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+        public override void Initialize(AnalysisContext analysisContext)
+        {
+            analysisContext.RegisterCodeBlockStartAction<TLanguageKindEnum>(
+                (context) =>
+                {
+                    var method = context.OwningSymbol as IMethodSymbol;
+                    if (method == null)
+                    {
+                        return;
+                    }
+
+                    if (!IsFinalizer(method))
+                    {
+                        return;
+                    }
+                    
+                    context.RegisterCodeBlockEndAction(codeBlockContext =>
+                    {
+                        if (IsEmptyFinalizer(codeBlockContext.CodeBlock, context.SemanticModel))
+                        {
+                            codeBlockContext.ReportDiagnostic(codeBlockContext.OwningSymbol.CreateDiagnostic(Rule));
+                        }
+                    });
+                });
+        }
+
+        
+        private static bool IsFinalizer(IMethodSymbol method)
+        {
+            if (method.MethodKind == MethodKind.Destructor)
+            {
+                return true; // for C#
+            }
+
+            if (method.Name != "Finalize" || method.Parameters.Length != 0 || !method.ReturnsVoid)
+            {
+                return false;
+            }
+
+            var overridden = method.OverriddenMethod;
+            if (overridden == null)
+            {
+                return false;
+            }
+
+            for (var o = overridden.OverriddenMethod; o != null; o = o.OverriddenMethod)
+            {
+                overridden = o;
+            }
+
+            return overridden.ContainingType.SpecialType == SpecialType.System_Object; // it is object.Finalize
+        }
+
+        protected abstract bool IsEmptyFinalizer(SyntaxNode node, SemanticModel model);
     }
 }

src/Diagnostics/FxCop/Test/FxCopRulesDiagnosticAnalyzersTest.csproj

@@ -92,8 +92,8 @@
     <Compile Include="HardeningAnalyzer\HardeningAnalyzerTests.cs" />
     <Compile Include="Naming\CA1708Tests.cs" />
     <Compile Include="Naming\CA1715Tests.cs" />
-    <Compile Include="Performance\CA1821Tests.cs" />
-    <Compile Include="Performance\CodeFixes\CA1821FixerTests.cs" />
+    <Compile Include="Performance\RemoveEmptyFinalizersTests.cs" />
+    <Compile Include="Performance\RemoveEmptyFinalizersTests.Fixer.cs" />
     <Compile Include="Reliability\CA2002Tests.cs" />
     <Compile Include="Usage\CA2200Tests.cs" />
     <Compile Include="Usage\CA2214Tests.cs" />

src/Diagnostics/FxCop/Test/Performance/CodeFixes/CA1821FixerTests.cs → src/Diagnostics/FxCop/Test/Performance/RemoveEmptyFinalizersTests.Fixer.cs

@@ -10,26 +10,26 @@
 
 namespace Microsoft.CodeAnalysis.UnitTests
 {
-    public partial class CA1821FixerTests : CodeFixTestBase
+    public partial class RemoveEmptyFinalizersFixerTests : CodeFixTestBase
     {
         protected override DiagnosticAnalyzer GetBasicDiagnosticAnalyzer()
         {
-            return new BasicCA1821DiagnosticAnalyzer();
+            return new BasicRemoveEmptyFinalizers();
         }
 
         protected override CodeFixProvider GetBasicCodeFixProvider()
         {
-            return new CA1821CodeFixProvider();
+            return new RemoveEmptyFinalizersFixer();
         }
 
         protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()
         {
-            return new CSharpCA1821DiagnosticAnalyzer();
+            return new CSharpRemoveEmptyFinalizers();
         }
 
         protected override CodeFixProvider GetCSharpCodeFixProvider()
         {
-            return new CA1821CodeFixProvider();
+            return new RemoveEmptyFinalizersFixer();
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
@@ -51,7 +51,7 @@ public class Class1
 ");
         }
 
-        [Fact(Skip = "Bug 902686"), Trait(Traits.Feature, Traits.Features.Diagnostics)]
+        [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
         public void CA1821BasicCodeFixTestRemoveEmptyFinalizers()
         {
             VerifyBasicFix(@"

src/Diagnostics/FxCop/Test/Performance/CA1821Tests.cs → src/Diagnostics/FxCop/Test/Performance/RemoveEmptyFinalizersTests.cs

@@ -9,16 +9,16 @@
 
 namespace Microsoft.CodeAnalysis.UnitTests
 {
-    public partial class CA1821Tests : DiagnosticAnalyzerTestBase
+    public partial class RemoveEmptyFinalizersTests : DiagnosticAnalyzerTestBase
     {
         protected override DiagnosticAnalyzer GetBasicDiagnosticAnalyzer()
         {
-            return new BasicCA1821DiagnosticAnalyzer();
+            return new BasicRemoveEmptyFinalizers();
         }
 
         protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()
         {
-            return new CSharpCA1821DiagnosticAnalyzer();
+            return new CSharpRemoveEmptyFinalizers();
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]

src/Diagnostics/FxCop/VisualBasic/BasicFxCopRulesDiagnosticAnalyzers.vbproj

@@ -100,7 +100,7 @@
     <Compile Include="Design\BasicCA1024DiagnosticAnalyzer.vb" />
     <Compile Include="Design\CodeFixes\CA1008BasicCodeFixProvider.vb" />
     <Compile Include="Design\CodeFixes\CA1052BasicCodeFixProvider.vb" />
-    <Compile Include="Performance\BasicCA1821DiagnosticAnalyzer.vb" />
+    <Compile Include="Performance\RemoveEmptyFinalizers.vb" />
     <Compile Include="Reliability\BasicCA2002DiagnosticAnalyzer.vb" />
     <Compile Include="Usage\BasicCA2200DiagnosticAnalyzer.vb" />
     <Compile Include="Usage\BasicCA2214DiagnosticAnalyzer.vb" />

src/Diagnostics/FxCop/VisualBasic/Performance/BasicCA1821DiagnosticAnalyzer.vb

-' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
-
-Imports Microsoft.CodeAnalysis.Diagnostics
-Imports Microsoft.CodeAnalysis.FxCopAnalyzers.Performance
-Imports Microsoft.CodeAnalysis.FxCopAnalyzers.Utilities
-Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
-
-Namespace Microsoft.CodeAnalysis.VisualBasic.FxCopAnalyzers.Performance
-    <DiagnosticAnalyzer(LanguageNames.VisualBasic)>
-    Public Class BasicCA1821DiagnosticAnalyzer
-        Inherits CA1821DiagnosticAnalyzer(Of SyntaxKind)
-
-        Protected Overrides Function GetCodeBlockEndedAnalyzer() As AbstractCodeBlockEndedAnalyzer
-            Return New CodeBlockEndedAnalyzer()
-        End Function
-
-        Private NotInheritable Class CodeBlockEndedAnalyzer
-            Inherits AbstractCodeBlockEndedAnalyzer
-
-            Protected Overrides Function IsEmptyFinalizer(node As SyntaxNode, model As SemanticModel) As Boolean
-                Dim symbol = model.GetDeclaredSymbol(node)
-                Dim method As IMethodSymbol = TryCast(symbol, IMethodSymbol)
-
-                ' First, validate the signature
-                If (symbol.IsStatic OrElse method.IsAbstract OrElse method.ReturnType.SpecialType <> SpecialType.System_Void OrElse
-                method.Parameters.Length > 0 OrElse Not CaseInsensitiveComparison.Equals(method.Name, "Finalize")) Then
-                    Return False
-                End If
-
-                ' Second, validate that the method overrides Object.Finalize
-                ' Otherwise, it is not a genuine finalizer.
-                While Not (method.OverriddenMethod Is Nothing)
-                    method = method.OverriddenMethod
-                End While
-
-                If method.ContainingType.SpecialType = SpecialType.System_Object Then
-                    For Each exp In node.DescendantNodes(descendIntoTrivia:=False).OfType(Of StatementSyntax)() _
-                        .Where(Function(n) Not n.IsKind(SyntaxKind.SubStatement) AndAlso Not n.IsKind(SyntaxKind.EndSubStatement))
-
-                        ' NOTE: FxCop only checks if there Is any method call within a given destructor to decide an empty finalizer.
-                        ' Here in order to minimize false negatives, we conservatively treat it as non-empty finalizer if its body contains any statements.
-                        ' But, still conditional methods Like Debug.Fail() will be considered as being empty as FxCop currently does.
-
-                        ' If a method has conditional attributes (e.g., Debug.Assert), Continue.
-                        Dim stmt As ExpressionStatementSyntax = TryCast(exp, ExpressionStatementSyntax)
-                        If stmt IsNot Nothing AndAlso HasConditionalAttribute(stmt.Expression, model) Then
-                            Continue For
-                        End If
-
-                        ' TODO: check if a method is reachable.
-                        Return False
-                    Next
-
-                    Return True
-                End If
-
-                Return False
-            End Function
-
-            Protected Function HasConditionalAttribute(root As SyntaxNode, model As SemanticModel) As Boolean
-                Dim node = TryCast(root, InvocationExpressionSyntax)
-                If node IsNot Nothing Then
-                    Dim exp = TryCast(node.Expression, MemberAccessExpressionSyntax)
-                    If exp IsNot Nothing Then
-                        Dim symbol = model.GetSymbolInfo(exp.Name).Symbol
-                        If symbol IsNot Nothing AndAlso symbol.GetAttributes().Any(Function(n) n.AttributeClass.Equals(WellKnownTypes.ConditionalAttribute(model.Compilation))) Then
-                            Return True
-                        End If
-                    End If
-                End If
-
-                Return False
-            End Function
-        End Class
-    End Class
-End Namespace

src/Diagnostics/FxCop/VisualBasic/Performance/RemoveEmptyFinalizers.vb

+' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+Imports Microsoft.CodeAnalysis.Diagnostics
+Imports Microsoft.CodeAnalysis.FxCopAnalyzers.Performance
+Imports Microsoft.CodeAnalysis.FxCopAnalyzers.Utilities
+Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
+
+Namespace Microsoft.CodeAnalysis.VisualBasic.FxCopAnalyzers.Performance
+    <DiagnosticAnalyzer(LanguageNames.VisualBasic)>
+    Public Class BasicRemoveEmptyFinalizers
+        Inherits RemoveEmptyFinalizers(Of SyntaxKind)
+
+        Protected Overrides Function IsEmptyFinalizer(node As SyntaxNode, model As SemanticModel) As Boolean
+
+            For Each exp In node.DescendantNodes(descendIntoTrivia:=False).OfType(Of StatementSyntax)() _
+                        .Where(Function(n) Not n.IsKind(SyntaxKind.SubStatement) AndAlso Not n.IsKind(SyntaxKind.EndSubStatement))
+
+                ' NOTE: FxCop only checks if there Is any method call within a given destructor to decide an empty finalizer.
+                ' Here in order to minimize false negatives, we conservatively treat it as non-empty finalizer if its body contains any statements.
+                ' But, still conditional methods Like Debug.Fail() will be considered as being empty as FxCop currently does.
+
+                ' If a method has conditional attributes (e.g., Debug.Assert), Continue.
+                Dim stmt As ExpressionStatementSyntax = TryCast(exp, ExpressionStatementSyntax)
+                If stmt IsNot Nothing AndAlso HasConditionalAttribute(stmt.Expression, model) Then
+                    Continue For
+                End If
+
+                ' TODO: check if a method is reachable.
+                Return False
+            Next
+
+            Return True
+
+        End Function
+
+        Protected Function HasConditionalAttribute(root As SyntaxNode, model As SemanticModel) As Boolean
+            Dim node = TryCast(root, InvocationExpressionSyntax)
+            If node IsNot Nothing Then
+                Dim exp = TryCast(node.Expression, MemberAccessExpressionSyntax)
+                If exp IsNot Nothing Then
+                    Dim symbol = model.GetSymbolInfo(exp.Name).Symbol
+                    If symbol IsNot Nothing AndAlso symbol.GetAttributes().Any(Function(n) n.AttributeClass.Equals(WellKnownTypes.ConditionalAttribute(model.Compilation))) Then
+                        Return True
+                    End If
+                End If
+            End If
+
+            Return False
+        End Function
+    End Class
+End Namespace