QAction - Unrecommended Finalizer (#60)

* Add check for finalizer * CR remarks * Updated NuGet to released version (yeah, I know, I made the wrong pre-release version) * CR remark * CR remark * Update CSharpCheckUnrecommendedFinalizer.cs * Update ErrorMessages.xml * Update CSharpCheckUnrecommendedFinalizer.cs --------- Co-authored-by: Pedro Debevere <97611683+PedroDebevere@users.noreply.github.com>
SkylineCommunications · Jul 5, 2024 · 6374a4c · 6374a4c
1 parent 2681f61
commit 6374a4c
Show file tree

Hide file tree

Showing 6 changed files with 332 additions and 1 deletion.
diff --git a/Protocol/Error Messages/Protocol/QActions/QAction/CSharpCheckUnrecommendedFinalizer.cs b/Protocol/Error Messages/Protocol/QActions/QAction/CSharpCheckUnrecommendedFinalizer.cs
@@ -0,0 +1,56 @@
+// <auto-generated>This is auto-generated code by Validator Management Tool. Do not modify.</auto-generated>
+namespace Skyline.DataMiner.CICD.Validators.Protocol.Tests.Protocol.QActions.QAction.CSharpCheckUnrecommendedFinalizer
+{
+    using System;
+    using System.Collections.Generic;
+
+    using Skyline.DataMiner.CICD.Models.Protocol.Read;
+    using Skyline.DataMiner.CICD.Validators.Common.Interfaces;
+    using Skyline.DataMiner.CICD.Validators.Common.Model;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Common;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Interfaces;
+
+    internal static class Error
+    {
+        public static IValidationResult UnrecommendedFinalizer(IValidate test, IReadable referenceNode, IReadable positionNode, string finalizerName, string qactionId)
+        {
+            return new ValidationResult
+            {
+                Test = test,
+                CheckId = CheckId.CSharpCheckUnrecommendedFinalizer,
+                ErrorId = ErrorIds.UnrecommendedFinalizer,
+                FullId = "3.41.1",
+                Category = Category.QAction,
+                Severity = Severity.Critical,
+                Certainty = Certainty.Certain,
+                Source = Source.Validator,
+                FixImpact = FixImpact.NonBreaking,
+                GroupDescription = "",
+                Description = String.Format("Finalizer '{0}' is unrecommended. QAction ID '{1}'.", finalizerName, qactionId),
+                HowToFix = "",
+                ExampleCode = "",
+                Details = "Finalizers need careful implementation as any exception thrown in a finalizer will result in a process crash as this code is executed by the finalizer thread. The performance impact arises from the delayed cleanup until the finalizer finalizes the object. Finalizers can clean up unmanaged resources in case the dispose method was not called, but it is preferred to use a SafeHandle to avoid the need for a finalizer. For resource management, it is recommended to use the IDisposable interface and the dispose pattern instead. More information can be found on the Microsoft docs (https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/finalizers).",
+                HasCodeFix = false,
+
+                PositionNode = positionNode,
+                ReferenceNode = referenceNode,
+            };
+        }
+    }
+
+    internal static class ErrorIds
+    {
+        public const uint UnrecommendedFinalizer = 1;
+    }
+
+    /// <summary>
+    /// Contains the identifiers of the checks.
+    /// </summary>
+    public static class CheckId
+    {
+        /// <summary>
+        /// The check identifier.
+        /// </summary>
+        public const uint CSharpCheckUnrecommendedFinalizer = 41;
+    }
+}
diff --git a/Protocol/ErrorMessages.xml b/Protocol/ErrorMessages.xml
@@ -12187,6 +12187,30 @@
 							</ErrorMessage>
 						</ErrorMessages>
 					</Check>
+					<Check id="41">
+						<Name namespace="Protocol.QActions.QAction">CSharpCheckUnrecommendedFinalizer</Name>
+						<ErrorMessages>
+							<ErrorMessage id="1">
+								<Name>UnrecommendedFinalizer</Name>
+								<GroupDescription />
+								<Description>
+									<Format>Finalizer '{0}' is unrecommended. QAction ID '{1}'.</Format>
+									<InputParameters>
+										<InputParameter id="0">finalizerName</InputParameter>
+										<InputParameter id="1">qactionId</InputParameter>
+									</InputParameters>
+								</Description>
+								<Severity>Critical</Severity>
+								<Certainty>Certain</Certainty>
+								<Source>Validator</Source>
+								<FixImpact>NonBreaking</FixImpact>
+								<HasCodeFix>False</HasCodeFix>
+								<HowToFix><![CDATA[]]></HowToFix>
+								<ExampleCode><![CDATA[]]></ExampleCode>
+								<Details><![CDATA[Finalizers need careful implementation as any exception thrown in a finalizer will result in a process crash as this code is executed by the finalizer thread. The performance impact arises from the delayed cleanup until the finalizer finalizes the object. Finalizers can clean up unmanaged resources in case the dispose method was not called, but it is preferred to use a SafeHandle to avoid the need for a finalizer. For resource management, it is recommended to use the IDisposable interface and the dispose pattern instead. More information can be found on the Microsoft docs (https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/finalizers).]]></Details>
+							</ErrorMessage>
+						</ErrorMessages>
+					</Check>
 				</Checks>
 			</Category>
 			<Category id="4">
@@ -20694,4 +20718,4 @@
 			</Category>
 		</Categories>
 	</ValidationChecks>
-</Validator>
+</Validator>
diff --git a/Protocol/Tests/Protocol/QActions/QAction/CSharpCheckUnrecommendedFinalizer.cs b/Protocol/Tests/Protocol/QActions/QAction/CSharpCheckUnrecommendedFinalizer.cs
@@ -0,0 +1,77 @@
+namespace Skyline.DataMiner.CICD.Validators.Protocol.Tests.Protocol.QActions.QAction.CSharpCheckUnrecommendedFinalizer
+{
+    using System.Collections.Generic;
+
+    using Microsoft.CodeAnalysis;
+
+    using Skyline.DataMiner.CICD.CSharpAnalysis;
+    using Skyline.DataMiner.CICD.CSharpAnalysis.Classes;
+    using Skyline.DataMiner.CICD.Models.Protocol;
+    using Skyline.DataMiner.CICD.Models.Protocol.Read;
+    using Skyline.DataMiner.CICD.Validators.Common.Interfaces;
+    using Skyline.DataMiner.CICD.Validators.Common.Model;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Common;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Common.Attributes;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Common.Extensions;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Helpers;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Interfaces;
+
+    [Test(CheckId.CSharpCheckUnrecommendedFinalizer, Category.QAction)]
+    internal class CSharpCheckUnrecommendedFinalizer : IValidate/*, ICodeFix, ICompare*/
+    {
+        public List<IValidationResult> Validate(ValidatorContext context)
+        {
+            List<IValidationResult> results = new List<IValidationResult>();
+
+            foreach ((IQActionsQAction qaction, SyntaxTree syntaxTree, SemanticModel semanticModel, CompiledQActionProject projectData) in context.EachQActionProjectsAndSyntaxTreesAndModelsAndProjectDatas())
+            {
+                Solution solution = projectData.Project.Solution;
+                QActionAnalyzer analyzer = new QActionAnalyzer(this, qaction, results, semanticModel, solution);
+                RoslynVisitor parser = new RoslynVisitor(analyzer);
+
+                parser.Visit(syntaxTree.GetRoot());
+            }
+
+            return results;
+        }
+
+        ////public ICodeFixResult Fix(CodeFixContext context)
+        ////{
+        ////    CodeFixResult result = new CodeFixResult();
+
+        ////    switch (context.Result.ErrorId)
+        ////    {
+
+        ////        default:
+        ////            result.Message = $"This error ({context.Result.ErrorId}) isn't implemented.";
+        ////            break;
+        ////    }
+
+        ////    return result;
+        ////}
+
+        ////public List<IValidationResult> Compare(MajorChangeCheckContext context)
+        ////{
+        ////    List<IValidationResult> results = new List<IValidationResult>();
+
+        ////    return results;
+        ////}
+    }
+
+    internal class QActionAnalyzer : QActionAnalyzerBase
+    {
+        public QActionAnalyzer(IValidate test, IQActionsQAction qAction, List<IValidationResult> results, SemanticModel semanticModel, Solution solution)
+            : base(test, results, qAction, semanticModel, solution)
+        {
+        }
+
+        public override void CheckClass(ClassClass classClass)
+        {
+            if (classClass.Finalizer != null)
+            {
+                results.Add(Error.UnrecommendedFinalizer(test, qAction, qAction, classClass.Finalizer.Name, qAction.Id.RawValue)
+                                 .WithCSharp(classClass.Finalizer.SyntaxNode));
+            }
+        }
+    }
+}
diff --git a/...l/QActions/QAction/CSharpCheckUnrecommendedFinalizer/CSharpCheckUnrecommendedFinalizer.cs b/...l/QActions/QAction/CSharpCheckUnrecommendedFinalizer/CSharpCheckUnrecommendedFinalizer.cs
@@ -0,0 +1,95 @@
+namespace ProtocolTests.Protocol.QActions.QAction.CSharpCheckUnrecommendedFinalizer
+{
+    using System.Collections.Generic;
+
+    using FluentAssertions;
+
+    using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+    using Skyline.DataMiner.CICD.Validators.Common.Interfaces;
+    using Skyline.DataMiner.CICD.Validators.Common.Model;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Common;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Interfaces;
+    using Skyline.DataMiner.CICD.Validators.Protocol.Tests.Protocol.QActions.QAction.CSharpCheckUnrecommendedFinalizer;
+
+    [TestClass]
+    public class Validate
+    {
+        private readonly IValidate check = new CSharpCheckUnrecommendedFinalizer();
+
+        #region Valid Checks
+
+        [TestMethod]
+        public void QAction_CSharpCheckUnrecommendedFinalizer_Valid()
+        {
+            Generic.ValidateData data = new Generic.ValidateData
+            {
+                TestType = Generic.TestType.Valid,
+                FileName = "Valid",
+                ExpectedResults = new List<IValidationResult>()
+            };
+
+            Generic.Validate(check, data);
+        }
+
+        #endregion
+
+        #region Invalid Checks
+
+        [TestMethod]
+        public void QAction_CSharpCheckUnrecommendedFinalizer_UnrecommendedFinalizer()
+        {
+            Generic.ValidateData data = new Generic.ValidateData
+            {
+                TestType = Generic.TestType.Invalid,
+                FileName = "UnrecommendedFinalizer",
+                ExpectedResults = new List<IValidationResult>
+                {
+                    Error.UnrecommendedFinalizer(null, null, null, "QAction", "1"),
+                    Error.UnrecommendedFinalizer(null, null, null, "RandomClass", "2"),
+                }
+            };
+
+            Generic.Validate(check, data);
+        }
+
+        #endregion
+    }
+
+    [TestClass]
+    public class ErrorMessages
+    {
+        [TestMethod]
+        public void QAction_CSharpCheckUnrecommendedFinalizer_UnrecommendedFinalizer()
+        {
+            // Create ErrorMessage
+            var message = Error.UnrecommendedFinalizer(null, null, null, "finalizerName", "qactionId");
+
+            var expected = new ValidationResult
+            {
+                Severity = Severity.Critical,
+                Certainty = Certainty.Certain,
+                FixImpact = FixImpact.NonBreaking,
+                GroupDescription = "",
+                Description = "Finalizer 'finalizerName' is unrecommended. QAction ID 'qactionId'.",
+                Details = "Finalizers need careful implementation as any exception thrown in a finalizer will result in a process crash as this code is executed by the finalizer thread. The performance impact arises from the delayed cleanup until the finalizer finalizes the object. Finalizers can clean up unmanaged resources in case the dispose method was not called, but it is preferred to use a SafeHandle to avoid the need for a finalizer. For resource management, it is recommended to use the IDisposable interface and the dispose pattern instead. More information can be found on the Microsoft docs (https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/finalizers).",
+                HasCodeFix = false,
+            };
+
+            // Assert
+            message.Should().BeEquivalentTo(expected, Generic.ExcludePropertiesForErrorMessages);
+        }
+    }
+
+    [TestClass]
+    public class Attribute
+    {
+        private readonly IRoot check = new CSharpCheckUnrecommendedFinalizer();
+
+        [TestMethod]
+        public void QAction_CSharpCheckUnrecommendedFinalizer_CheckCategory() => Generic.CheckCategory(check, Category.QAction);
+
+        [TestMethod]
+        public void QAction_CSharpCheckUnrecommendedFinalizer_CheckId() => Generic.CheckId(check, CheckId.CSharpCheckUnrecommendedFinalizer);
+    }
+}
diff --git a/...ion/CSharpCheckUnrecommendedFinalizer/Samples/Validate/Invalid/UnrecommendedFinalizer.xml b/...ion/CSharpCheckUnrecommendedFinalizer/Samples/Validate/Invalid/UnrecommendedFinalizer.xml
@@ -0,0 +1,41 @@
+<Protocol xmlns="http://www.skyline.be/validatorProtocolUnitTest">
+	<Name>UnrecommendedFinalizer</Name>
+	<Version>1.0.0.1</Version>
+	<QActions>
+		<QAction id="1" name="QAction - Finalizer" encoding="csharp">
+			<![CDATA[using System;
+using System.Xml.Serialization;
+
+using Skyline.DataMiner.Net.Messages;
+using Skyline.DataMiner.Scripting;
+
+public class QAction
+{
+	public static void Run(SLProtocol protocol)
+	{
+	}
+	
+	~QAction() { }
+}]]>
+		</QAction>
+		<QAction id="2" name="Other Class - Finalizer" encoding="csharp">
+			<![CDATA[using System;
+using System.Xml.Serialization;
+
+using Skyline.DataMiner.Net.Messages;
+using Skyline.DataMiner.Scripting;
+
+public class QAction
+{
+	public static void Run(SLProtocol protocol)
+	{
+	}
+}
+
+public class RandomClass
+{
+	~RandomClass() { }
+}]]>
+		</QAction>
+	</QActions>
+</Protocol>
diff --git a/...tocol/QActions/QAction/CSharpCheckUnrecommendedFinalizer/Samples/Validate/Valid/Valid.xml b/...tocol/QActions/QAction/CSharpCheckUnrecommendedFinalizer/Samples/Validate/Valid/Valid.xml
@@ -0,0 +1,38 @@
+<Protocol xmlns="http://www.skyline.be/validatorProtocolUnitTest">
+    <Name>Valid</Name>
+    <Version>1.0.0.1</Version>
+    <QActions>
+        <QAction id="1" name="QAction - Finalizer" encoding="csharp">
+            <![CDATA[using System;
+using System.Xml.Serialization;
+
+using Skyline.DataMiner.Net.Messages;
+using Skyline.DataMiner.Scripting;
+
+public class QAction
+{
+	public static void Run(SLProtocol protocol)
+	{
+	}
+}]]>
+        </QAction>
+        <QAction id="2" name="Other Class - Finalizer" encoding="csharp">
+            <![CDATA[using System;
+using System.Xml.Serialization;
+
+using Skyline.DataMiner.Net.Messages;
+using Skyline.DataMiner.Scripting;
+
+public class QAction
+{
+	public static void Run(SLProtocol protocol)
+	{
+	}
+}
+
+public class RandomClass
+{
+}]]>
+        </QAction>
+    </QActions>
+</Protocol>