feat: avoid enums in parameters

Documentation/Diagnostics/PH2138.md

@@ -27,7 +27,7 @@ Ultimately we will need to execute changes onto the code's environment. For thes
 
 ## Example
 
-Code that triggers 2 diagnostics:
+Code that triggers the diagnostic:
 ``` cs
         class MyClass
         {

Documentation/Diagnostics/PH2140.md

@@ -0,0 +1,95 @@
+# PH2138: Avoid enum parameters
+
+| Property | Value  |
+|--|--|
+| Package | [Philips.CodeAnalysis.MaintainabilityAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MaintainabilityAnalyzers) |
+| Diagnostic ID | PH2140 |
+| Category  | [Functional Programming](../FunctionalProgramming.md) |
+| Analyzer | [AvoidEnumParametersAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Cardinality/AvoidEnumParametersAnalyzer.cs)
+| CodeFix  | No |
+| Severity | Error |
+| Enabled By Default | No |
+
+## Introduction
+
+This analyzer warns about methods or functions that have enums as parameters. Whenever a method takes in a known subset of values there is an opportunity to invert the control flow and reduce the complexity of the call site.
+
+Often times the method does not apply to all values in the enum, but only a subset of them. In these cases it is better to define the method only for those values.
+
+Otherwise, when all values are addressable it is often the case each individual value has its own particular set of rules it follows. In these cases it is better to isolate each different ruleset to its relevant enum value.
+
+## How to solve
+
+Upgrade the enum to a record and have each value implement its own particular version.
+If that is not possible, create an extension method that performs the relevant pattern matching.
+See this [blog post](https://spencerfarley.com/2021/03/26/unions-in-csharp/) for some more details.
+
+## Example
+
+Code that triggeres the diagnostic:
+```cs
+    class MyClass
+    {
+        public ResultCode processFilename(String filename)
+        {
+            var envConfig = Config.FromEnvironment();
+            return processSysConfig(filename, envConfig);
+        }
+        public ResultCode processSysConfig(String filename, ConfigEnum config)
+        {
+            if (config == ReadOnUpdate)
+            {
+            ...
+            }
+            else if (config == ReadWeekly || config == ReadMonthly)
+            {
+            ...
+            }
+            else
+            {
+            ...
+            }
+        }
+    }
+
+    enum ConfigEnum {
+        ReadOnUpdate, ReadMonthly, ReadWeekly, ReadNever
+    }
+```
+
+And the corrected code:
+```cs
+    class MyClass
+    {
+        public ResultCode processFilename(String filename)
+        {
+            var envConfig = Config.FromEnvironment();
+            return envConfig.process(filename);
+        }
+
+    }
+
+    public abstract record ConfigRecord
+	{
+		public abstract long ProcessFile(string filename);
+
+		public record ReadOnUpdate() : ConfigRecord()
+		{
+			public override long ProcessFile(string filename) { ... }
+		};
+
+		public record ReadWeekly() : ConfigRecord()
+		{
+			public override long ProcessFile(string filename) { ... }
+		}
+
+		public record ReadMonthly() : ConfigRecord()
+		{
+			public override long ProcessFile(string filename) { ... }
+		}
+
+		public record ReadNever() : ConfigRecord() {
+			public override long ProcessFile(string filename) { ... }
+        }
+	}
+```

Philips.CodeAnalysis.Common/DiagnosticId.cs

@@ -133,5 +133,6 @@ public enum DiagnosticId
 		RegexNeedsTimeout = 2137,
 		AvoidVoidReturn = 2138,
 		EnableDocumentationCreation = 2139,
+		AvoidEnumParameters = 2140,
 	}
 }

Philips.CodeAnalysis.Common/ParameterPredicates.cs

@@ -0,0 +1,21 @@
+// © 2023 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Philips.CodeAnalysis.Common
+{
+	public static class ParameterPredicates
+	{
+		public static bool IsEnum(this ParameterSyntax p, SyntaxNodeAnalysisContext context)
+		{
+			if (context.SemanticModel.GetDeclaredSymbol(p).Type is ITypeSymbol typ)
+			{
+				return typ.TypeKind == TypeKind.Enum;
+			}
+			return false;
+		}
+	}
+}

Philips.CodeAnalysis.MaintainabilityAnalyzers/Cardinality/AvoidEnumParametersAnalyzer.cs

@@ -0,0 +1,45 @@
+// © 2023 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Philips.CodeAnalysis.Common;
+using static LanguageExt.Prelude;
+
+namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Cardinality
+{
+	[DiagnosticAnalyzer(LanguageNames.CSharp)]
+	public class AvoidEnumParametersAnalyzer : SingleDiagnosticAnalyzer<MethodDeclarationSyntax, AvoidEnumParametersSyntaxNodeAction>
+	{
+
+		private static readonly string Title = @"Methods should not accept enums";
+		private static readonly string MessageFormat = @"Method {1} should be refactored away. Illegal Enum Parameter {0}";
+		private static readonly string Description = @"Methods taking an enum as a parameter should instead live inside said enum.";
+
+		public AvoidEnumParametersAnalyzer()
+			: base(DiagnosticId.AvoidEnumParameters, Title, MessageFormat, Description, Categories.FunctionalProgramming, isEnabled: false)
+		{ }
+	}
+
+	public class AvoidEnumParametersSyntaxNodeAction : SyntaxNodeAction<MethodDeclarationSyntax>
+	{
+		public override void Analyze()
+		{
+			_ = List(Node)
+				.Filter((m) => !m.IsOverridden())
+				.SelectMany(AnalyzeMethodParameters)
+				.Iter(Context.ReportDiagnostic);
+		}
+
+		private IEnumerable<Diagnostic> AnalyzeMethodParameters(MethodDeclarationSyntax m)
+		{
+			return m.ParameterList.Parameters
+				.Filter((p) => p.IsEnum(Context))
+				.Select((p) => Diagnostic.Create(Rule, p.GetLocation(), p.Identifier.Text, m.Identifier.Text));
+		}
+	}
+
+}

Philips.CodeAnalysis.MaintainabilityAnalyzers/Philips.CodeAnalysis.MaintainabilityAnalyzers.csproj

@@ -97,31 +97,33 @@
       * Introduce PH2114: Avoid empty statement.
       * Introduce PH2115: Every Lambda expression on separate line.
       * Introduce PH2116: Avoid ArrayList, use List<T> instead.
-      * Introduce PH2117: Avoid unnecessary Where.
-      * Introduce PH2118: Avoid magic numbers.
-      * PH2006 now supports folders in namespace
-      * Introduce PH2119: Cast complete object.
-      * Introduce PH2120: Document thrown exceptions.
-      * Introduce PH2121: Throw informational exceptions.
-      * Introduce PH2122: Avoid throwing exceptions from unexpected locations.
-      * Introduce PH2123: Pass sender to EventHandler.
-      * Introduce PH2124: Document unhandled exceptions.
-      * Introduce PH2125: Align number of + and == operators.
-      * Introduce PH2126: Avoid using Parameters as temporary variables.
-      * Introduce PH2127: Avoid changing loop variables.
-      * Introduce PH2128: Split multi-line conditions on logical operators.
-      * Introduce PH2129: Return immutable collections.
-      * Increased the test coverage.
-      * Introduce PH2130: Avoid implementing finalizers.
-      * Introduce PH2131: Align filename and class name.
-      * Introduce PH2131: Remove commented code.
-      * Introduce PH2133: Unmanaged objects need disposing.
-      * Introduce PH2134: Set properties in any order.
-      * PH2090 now also uses an AdditionalFile on top of the existing .editorconfig option to configure allowed methods.
-      * Introduce PH2135: Match namespace and Assembly Name. This extends the functionality that was previously claimed by PH2006 but never enforced.
-	    * Introduce PH2136: Avoid duplicate strings.
-      * Introduce PH2139: Enable documentation creation.
-    </PackageReleaseNotes>
+		* Introduce PH2117: Avoid unnecessary Where.
+		* Introduce PH2118: Avoid magic numbers.
+		* PH2006 now supports folders in namespace
+		* Introduce PH2119: Cast complete object.
+		* Introduce PH2120: Document thrown exceptions.
+		* Introduce PH2121: Throw informational exceptions.
+		* Introduce PH2122: Avoid throwing exceptions from unexpected locations.
+		* Introduce PH2123: Pass sender to EventHandler.
+		* Introduce PH2124: Document unhandled exceptions.
+		* Introduce PH2125: Align number of + and == operators.
+		* Introduce PH2126: Avoid using Parameters as temporary variables.
+		* Introduce PH2127: Avoid changing loop variables.
+		* Introduce PH2128: Split multi-line conditions on logical operators.
+		* Introduce PH2129: Return immutable collections.
+		* Increased the test coverage.
+		* Introduce PH2130: Avoid implementing finalizers.
+		* Introduce PH2131: Align filename and class name.
+		* Introduce PH2131: Remove commented code.
+		* Introduce PH2133: Unmanaged objects need disposing.
+		* Introduce PH2134: Set properties in any order.
+		* PH2090 now also uses an AdditionalFile on top of the existing .editorconfig option to configure allowed methods.
+		* Introduce PH2135: Match namespace and Assembly Name. This extends the functionality that was previously claimed by PH2006 but never enforced.
+		* Introduce PH2136: Avoid duplicate strings.
+		* Introduce PH2138: Avoid void return.
+		* Introduce PH2139: Enable documentation creation.
+		* Introduce PH2140: Avoid enum parameters.
+	</PackageReleaseNotes>
     <Copyright>© 2019-2023 Koninklijke Philips N.V.</Copyright>
     <PackageTags>CSharp Maintainability Roslyn CodeAnalysis analyzers Philips</PackageTags>
     <NoPackageAnalysis>true</NoPackageAnalysis>

Philips.CodeAnalysis.MaintainabilityAnalyzers/Philips.CodeAnalysis.MaintainabilityAnalyzers.md

@@ -89,3 +89,4 @@
 | [PH2136](../Documentation/Diagnostics/PH2136.md)  | Avoid duplicate strings                      | Duplicate strings are less maintainable. |
 | [PH2138](../Documentation/Diagnostics/PH2138.md)  | Avoid void returns                           | Methods that return void are mysterious. |
 | [PH2139](../Documentation/Diagnostics/PH2139.md)  | Enable documentation creation                | Add XML documentation generation to the project file, to be able to see Diagnostics for XML documentation. |
+| [PH2140](../Documentation/Diagnostics/PH2140.md)  | Avoid enum parameters | Methods that take enums as parameters should be factored out the enum parameter it receives as either an extension method or in the relevant enum instances. |

Philips.CodeAnalysis.Test/Cardinality/AvoidEnumParametersTest.cs

@@ -0,0 +1,139 @@
+// © 2023 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Philips.CodeAnalysis.MaintainabilityAnalyzers.Cardinality;
+using Philips.CodeAnalysis.Test.Helpers;
+using Philips.CodeAnalysis.Test.Verifiers;
+
+namespace Philips.CodeAnalysis.Test.Cardinality
+{
+	[TestClass]
+	public class AvoidEnumParametersTest : DiagnosticVerifier
+	{
+		//No diagnostics expected to show up
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task NotFireForEmptyFiles()
+		{
+			var test = "";
+
+			await VerifySuccessfulCompilation(test);
+		}
+
+		//No diagnostics expected to show up
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task NotFireForNonVoidMethods()
+		{
+			var test = @"
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Text;
+    using System.Threading.Tasks;
+    using System.Diagnostics;
+
+    namespace ConsoleApplication1
+    {
+        class MyClass
+        {   
+            public bool Foo(bool b) { return true; }
+        }
+    }";
+
+			await VerifySuccessfulCompilation(test);
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task FireForEnumParam()
+		{
+			var test = @"
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Text;
+    using System.Threading.Tasks;
+    using System.Diagnostics;
+
+    namespace ConsoleApplication1
+    {
+        enum Day { Sunday, Monday, Tuesday, Wednesday, Thursday, Friday, Saturday };  
+
+        class MyClass
+        {   
+            public void Foo(Day d) {}
+        }
+    }";
+
+			await VerifyDiagnostic(test, regex: "Method Foo.*Parameter d");
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task NotFireForInterfaceParam()
+		{
+			var test = @"
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Text;
+    using System.Threading.Tasks;
+    using System.Diagnostics;
+
+    namespace ConsoleApplication1
+    {
+
+
+        class MyClass
+        {   
+            public void Foo(IEnumerable<string> d) {}
+        }
+    }";
+
+			await VerifySuccessfulCompilation(test);
+		}
+
+		/**
+         * The reasoning behind this condition is that its impossible to avoid void methods
+         * when they override those of base class
+         */
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task NotFireForProductParam()
+		{
+			var test = @"
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Text;
+    using System.Threading.Tasks;
+    using System.Diagnostics;
+
+    namespace ConsoleApplication1
+    {
+        public class ProductOf
+	{
+		public bool Red;
+        public int Blue;
+	}
+
+    public class ProductUse
+    {
+        public bool Foo(ProductOf param)
+        {
+            return true;
+        }
+    }
+    }";
+			await VerifySuccessfulCompilation(test);
+		}
+
+		protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
+		{
+			return new AvoidEnumParametersAnalyzer();
+		}
+	}
+}