IDISP024 don't call GC.SuppressFinalize if sealed and no finalizer.

JohanLarsson · JohanLarsson · commit 74a5eae1e091 · 2019-11-26T14:15:11.000+01:00
Close #182
diff --git a/IDisposableAnalyzers.Test/IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizerTests/CodeFix.cs b/IDisposableAnalyzers.Test/IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizerTests/CodeFix.cs
@@ -0,0 +1,45 @@
+﻿namespace IDisposableAnalyzers.Test.IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizerTests
+{
+    using Gu.Roslyn.Asserts;
+    using Microsoft.CodeAnalysis.CodeFixes;
+    using Microsoft.CodeAnalysis.Diagnostics;
+    using NUnit.Framework;
+
+    public static class CodeFix
+    {
+        private static readonly DiagnosticAnalyzer Analyzer = new SuppressFinalizeAnalyzer();
+        private static readonly CodeFixProvider Fix = new RemoveCallFix();
+
+        [Test]
+        public static void SealedSimple()
+        {
+            var before = @"
+namespace N
+{
+    using System;
+
+    public sealed class C : IDisposable
+    {
+        public void Dispose()
+        {
+            ↓GC.SuppressFinalize(this);
+        }
+    }
+}";
+
+            var after = @"
+namespace N
+{
+    using System;
+
+    public sealed class C : IDisposable
+    {
+        public void Dispose()
+        {
+        }
+    }
+}";
+            RoslynAssert.CodeFix(Analyzer, Fix, before, after);
+        }
+    }
+}
diff --git a/IDisposableAnalyzers.Test/IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizerTests/Valid.cs b/IDisposableAnalyzers.Test/IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizerTests/Valid.cs
@@ -0,0 +1,93 @@
+﻿namespace IDisposableAnalyzers.Test.IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizerTests
+{
+    using Gu.Roslyn.Asserts;
+    using Microsoft.CodeAnalysis.Diagnostics;
+    using NUnit.Framework;
+
+    public static class Valid
+    {
+        private static readonly DiagnosticAnalyzer Analyzer = new SuppressFinalizeAnalyzer();
+
+        [Test]
+        public static void SealedSimple()
+        {
+            var code = @"
+namespace N
+{
+    using System;
+
+    public sealed class C : IDisposable
+    {
+        public void Dispose()
+        {
+        }
+    }
+}";
+            RoslynAssert.Valid(Analyzer, code);
+        }
+
+        [Test]
+        public static void SealedNoFinalizer()
+        {
+            var code = @"
+namespace N
+{
+    using System;
+
+    public sealed class C : IDisposable
+    {
+        private bool isDisposed = false;
+
+        public void Dispose()
+        {
+            this.Dispose(true);
+        }
+
+        private void Dispose(bool disposing)
+        {
+            if (!this.isDisposed)
+            {
+                this.isDisposed = true;
+            }
+        }
+    }
+}";
+            RoslynAssert.Valid(Analyzer, code);
+        }
+
+        [Test]
+        public static void SealedWithFinalizer()
+        {
+            var code = @"
+namespace N
+{
+    using System;
+
+    public sealed class C : IDisposable
+    {
+        private bool isDisposed = false;
+
+        ~C()
+        {
+            this.Dispose(false);
+        }
+
+        public void Dispose()
+        {
+            this.Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
+        void Dispose(bool disposing)
+        {
+            if (!this.isDisposed)
+            {
+                this.isDisposed = true;
+            }
+        }
+    }
+}";
+            RoslynAssert.Valid(Analyzer, code);
+        }
+    }
+}
diff --git a/IDisposableAnalyzers.sln b/IDisposableAnalyzers.sln
@@ -37,6 +37,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "docs", "docs", "{BAD2A4EE-6
 		documentation\IDISP021.md = documentation\IDISP021.md
 		documentation\IDISP022.md = documentation\IDISP022.md
 		documentation\IDISP023.md = documentation\IDISP023.md
+		documentation\IDISP024.md = documentation\IDISP024.md
 		README.md = README.md
 		RELEASE_NOTES.md = RELEASE_NOTES.md
 		documentation\SyntaxTreeCacheAnalyzer.md = documentation\SyntaxTreeCacheAnalyzer.md
diff --git a/IDisposableAnalyzers/Analyzers/ReturnValueAnalyzer.cs b/IDisposableAnalyzers/Analyzers/ReturnValueAnalyzer.cs
@@ -223,7 +223,7 @@ private static bool IsIgnored(ISymbol symbol)
 
         private static ITypeSymbol? ReturnType(SyntaxNodeAnalysisContext context)
         {
-            if (context.Node.FirstAncestorOrSelf<AnonymousFunctionExpressionSyntax>() is {} lambda)
+            if (context.Node.FirstAncestorOrSelf<AnonymousFunctionExpressionSyntax>() is { } lambda)
             {
                 var method = context.SemanticModel.GetSymbolSafe(lambda, context.CancellationToken) as IMethodSymbol;
                 return method?.ReturnType;
diff --git a/IDisposableAnalyzers/Analyzers/SuppressFinalizeAnalyzer.cs b/IDisposableAnalyzers/Analyzers/SuppressFinalizeAnalyzer.cs
@@ -0,0 +1,38 @@
+﻿namespace IDisposableAnalyzers
+{
+    using System.Collections.Immutable;
+    using Gu.Roslyn.AnalyzerExtensions;
+    using Microsoft.CodeAnalysis;
+    using Microsoft.CodeAnalysis.CSharp;
+    using Microsoft.CodeAnalysis.CSharp.Syntax;
+    using Microsoft.CodeAnalysis.Diagnostics;
+
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    internal class SuppressFinalizeAnalyzer : DiagnosticAnalyzer
+    {
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(
+            Descriptors.IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizer);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+            context.EnableConcurrentExecution();
+            context.RegisterSyntaxNodeAction(c => Handle(c), SyntaxKind.InvocationExpression);
+        }
+
+        private static void Handle(SyntaxNodeAnalysisContext context)
+        {
+            if (context.Node is InvocationExpressionSyntax { ArgumentList: { Arguments: { Count: 1 } arguments } } invocation &&
+                invocation.IsSymbol(KnownSymbol.GC.SuppressFinalize, context.SemanticModel, context.CancellationToken) &&
+                context.SemanticModel.TryGetNamedType(arguments[0].Expression, context.CancellationToken, out var type) &&
+                type.IsSealed &&
+                !type.TryFindFirstMethod(x => x.MethodKind == MethodKind.Destructor, out _))
+            {
+                context.ReportDiagnostic(
+                    Diagnostic.Create(
+                        Descriptors.IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizer,
+                        invocation.GetLocation()));
+            }
+        }
+    }
+}
diff --git a/IDisposableAnalyzers/CodeFixes/RemoveCallFix.cs b/IDisposableAnalyzers/CodeFixes/RemoveCallFix.cs
@@ -0,0 +1,39 @@
+﻿namespace IDisposableAnalyzers
+{
+    using System.Collections.Immutable;
+    using System.Composition;
+    using System.Threading.Tasks;
+    using Gu.Roslyn.CodeFixExtensions;
+    using Microsoft.CodeAnalysis;
+    using Microsoft.CodeAnalysis.CodeFixes;
+    using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+    [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(RemoveCallFix))]
+    [Shared]
+    internal class RemoveCallFix : DocumentEditorCodeFixProvider
+    {
+        public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(
+            Descriptors.IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizer.Id);
+
+        protected override DocumentEditorFixAllProvider? FixAllProvider() => null;
+
+        protected override async Task RegisterCodeFixesAsync(DocumentEditorCodeFixContext context)
+        {
+            var syntaxRoot = await context.Document.GetSyntaxRootAsync(context.CancellationToken)
+                                          .ConfigureAwait(false);
+
+            foreach (var diagnostic in context.Diagnostics)
+            {
+                if (syntaxRoot.TryFindNodeOrAncestor(diagnostic, out ExpressionStatementSyntax? statement))
+                {
+                    context.RegisterCodeFix(
+                        "Remove",
+                        (e, _) => e.RemoveNode(statement),
+                        equivalenceKey: nameof(SuppressFinalizeFix),
+                        diagnostic);
+                    break;
+                }
+            }
+        }
+    }
+}
diff --git a/IDisposableAnalyzers/Descriptors.cs b/IDisposableAnalyzers/Descriptors.cs
@@ -211,6 +211,15 @@ internal static class Descriptors
             isEnabledByDefault: true,
             description: "Don't use reference types in finalizer context.");
 
+        internal static readonly DiagnosticDescriptor IDISP024DoNotCallSuppressFinalizeIfSealedAndNoFinalizer = Descriptors.Create(
+            id: "IDISP024",
+            title: "Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.",
+            messageFormat: "Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.",
+            category: AnalyzerCategory.Correctness,
+            defaultSeverity: DiagnosticSeverity.Warning,
+            isEnabledByDefault: true,
+            description: "Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.");
+
         /// <summary>
         /// Create a DiagnosticDescriptor, which provides description about a <see cref="Diagnostic" />.
         /// </summary>
diff --git a/README.md b/README.md
@@ -38,6 +38,7 @@ Roslyn analyzers for IDisposable
 | [IDISP021](https://github.com/DotNetAnalyzers/IDisposableAnalyzers/blob/master/documentation/IDISP021.md)| Call this.Dispose(true).
 | [IDISP022](https://github.com/DotNetAnalyzers/IDisposableAnalyzers/blob/master/documentation/IDISP022.md)| Call this.Dispose(false).
 | [IDISP023](https://github.com/DotNetAnalyzers/IDisposableAnalyzers/blob/master/documentation/IDISP023.md)| Don't use reference types in finalizer context.
+| [IDISP024](https://github.com/DotNetAnalyzers/IDisposableAnalyzers/blob/master/documentation/IDISP024.md)| Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.
 | [SyntaxTreeCacheAnalyzer]()| Controls caching of for example semantic models for syntax trees.
 
 ## Using IDisposableAnalyzers
diff --git a/documentation/IDISP024.md b/documentation/IDISP024.md
@@ -0,0 +1,51 @@
+# IDISP024
+## Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.
+
+| Topic    | Value
+| :--      | :--
+| Id       | IDISP024
+| Severity | Warning
+| Enabled  | True
+| Category | IDisposableAnalyzers.Correctness
+| Code     | [SuppressFinalizeAnalyzer](https://github.com/DotNetAnalyzers/IDisposableAnalyzers/blob/master/IDisposableAnalyzers/Analyzers/SuppressFinalizeAnalyzer.cs)
+
+
+## Description
+
+Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.
+
+## Motivation
+
+ADD MOTIVATION HERE
+
+## How to fix violations
+
+ADD HOW TO FIX VIOLATIONS HERE
+
+<!-- start generated config severity -->
+## Configure severity
+
+### Via ruleset file.
+
+Configure the severity per project, for more info see [MSDN](https://msdn.microsoft.com/en-us/library/dd264949.aspx).
+
+### Via #pragma directive.
+```C#
+#pragma warning disable IDISP024 // Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.
+Code violating the rule here
+#pragma warning restore IDISP024 // Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.
+```
+
+Or put this at the top of the file to disable all instances.
+```C#
+#pragma warning disable IDISP024 // Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.
+```
+
+### Via attribute `[SuppressMessage]`.
+
+```C#
+[System.Diagnostics.CodeAnalysis.SuppressMessage("IDisposableAnalyzers.Correctness", 
+    "IDISP024:Don't call GC.SuppressFinalize(this) when the type is sealed and has no finalizer.", 
+    Justification = "Reason...")]
+```
+<!-- end generated config severity -->

Original file line number	Diff line number	Diff line change
`@@ -223,7 +223,7 @@ private static bool IsIgnored(ISymbol symbol)`
`223`	`223`
`224`	`224`	`private static ITypeSymbol? ReturnType(SyntaxNodeAnalysisContext context)`
`225`	`225`	`{`
`226`		`- if (context.Node.FirstAncestorOrSelf<AnonymousFunctionExpressionSyntax>() is {} lambda)`
	`226`	`+ if (context.Node.FirstAncestorOrSelf<AnonymousFunctionExpressionSyntax>() is { } lambda)`
`227`	`227`	`{`
`228`	`228`	`var method = context.SemanticModel.GetSymbolSafe(lambda, context.CancellationToken) as IMethodSymbol;`
`229`	`229`	`return method?.ReturnType;`