Convert UseSingularNouns to configurable rule and add Windows to allowlist (#1858)

* Add Windows to the UseSingularNouns allow list

* Add test case for Windows verb

* Refactor UseSingularNouns to configurable rule and add tests

* Update UseSingularNouns docs with configuration information

* Remove extra test code

---------

Co-authored-by: Christoph Bergmeister <c.bergmeister@gmail.com>

Author: MJVL

Rules/UseSingularNouns.cs

@@ -32,21 +32,23 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 #if !CORECLR
 [Export(typeof(IScriptRule))]
 #endif
-    public class CmdletSingularNoun : IScriptRule
+    public class CmdletSingularNoun : ConfigurableRule 
     {
+        [ConfigurableRuleProperty(defaultValue: new string[] { "Data", "Windows" })]
+        public string[] NounAllowList { get; set; }
 
-        private readonly string[] nounAllowList =
+        public CmdletSingularNoun()
         {
-            "Data"
-        };
+            Enable = true;
+        }
 
         /// <summary>
         /// Checks that all defined cmdlet use singular noun
         /// </summary>
         /// <param name="ast"></param>
         /// <param name="fileName"></param>
         /// <returns></returns>
-        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
         {
             if (ast == null) throw new ArgumentNullException(Strings.NullCommandInfoError);
 
@@ -70,7 +72,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
 
                 if (pluralizer.CanOnlyBePlural(noun))
                 {
-                    if (nounAllowList.Contains(noun, StringComparer.OrdinalIgnoreCase))
+                    if (NounAllowList.Contains(noun, StringComparer.OrdinalIgnoreCase))
                     {
                         continue;
                     }
@@ -99,7 +101,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
         /// GetName: Retrieves the name of this rule.
         /// </summary>
         /// <returns>The name of this rule</returns>
-        public string GetName()
+        public override string GetName()
         {
             return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseSingularNounsName);
         }
@@ -108,7 +110,7 @@ public string GetName()
         /// GetName: Retrieves the common name of this rule.
         /// </summary>
         /// <returns>The common name of this rule</returns>
-        public string GetCommonName()
+        public override string GetCommonName()
         {
             return string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsCommonName);
         }
@@ -117,15 +119,15 @@ public string GetCommonName()
         /// GetDescription: Retrieves the description of this rule.
         /// </summary>
         /// <returns>The description of this rule</returns>
-        public string GetDescription()
+        public override string GetDescription()
         {
             return string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsDescription);
         }
 
         /// <summary>
         /// GetSourceType: Retrieves the type of the rule: builtin, managed or module.
         /// </summary>
-        public SourceType GetSourceType()
+        public override SourceType GetSourceType()
         {
             return SourceType.Builtin;
         }
@@ -134,15 +136,15 @@ public SourceType GetSourceType()
         /// GetSeverity: Retrieves the severity of the rule: error, warning of information.
         /// </summary>
         /// <returns></returns>
-        public RuleSeverity GetSeverity()
+        public override RuleSeverity GetSeverity()
         {
             return RuleSeverity.Warning;
         }
 
         /// <summary>
         /// GetSourceName: Retrieves the module/assembly name the rule is from.
         /// </summary>
-        public string GetSourceName()
+        public override string GetSourceName()
         {
             return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
         }

Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1

@@ -32,7 +32,7 @@ Describe "UseSingularNouns" {
 
     Context "When function names have nouns from allowlist" {
 
-        It "ignores function name ending with Data" {
+        It "ignores function name ending with Data by default" {
             $nounViolationScript = @'
 Function Add-SomeData
 {
@@ -44,6 +44,33 @@ Write-Output "Adding some data"
                 -OutVariable violations
             $violations.Count | Should -Be 0
         }
+
+        It "ignores function name ending with Windows by default" {
+            $nounViolationScript = @'
+Function Test-Windows
+{
+Write-Output "Testing Microsoft Windows"
+}
+'@
+            Invoke-ScriptAnalyzer -ScriptDefinition $nounViolationScript `
+                -IncludeRule "PSUseSingularNouns" `
+                -OutVariable violations
+            $violations.Count | Should -Be 0
+        }
+
+        It "ignores function names defined in settings" {
+            $nounViolationScript = @'
+Function Get-Bananas
+{
+Write-Output "Bananas"
+}
+'@
+            Invoke-ScriptAnalyzer -ScriptDefinition $nounViolationScript -Settings @{
+                IncludeRules = @("PSUseSingularNouns")
+                Rules        = @{ PSUseSingularNouns = @{ NounAllowList = "Bananas" } }
+            } | Should -BeNullOrEmpty
+        }
+
     }
 
     Context "When there are no violations" {

docs/Rules/README.md

@@ -75,7 +75,7 @@ The PSScriptAnalyzer contains the following rule definitions.
 | [UseProcessBlockForPipelineCommand](./UseProcessBlockForPipelineCommand.md)                       | Warning     |        Yes         |                 |
 | [UsePSCredentialType](./UsePSCredentialType.md)                                                   | Warning     |        Yes         |                 |
 | [UseShouldProcessForStateChangingFunctions](./UseShouldProcessForStateChangingFunctions.md)       | Warning     |        Yes         |                 |
-| [UseSingularNouns](./UseSingularNouns.md)                                                         | Warning     |        Yes         |                 |
+| [UseSingularNouns](./UseSingularNouns.md)                                                         | Warning     |        Yes         |       Yes       |
 | [UseSupportsShouldProcess](./UseSupportsShouldProcess.md)                                         | Warning     |        Yes         |                 |
 | [UseToExportFieldsInManifest](./UseToExportFieldsInManifest.md)                                   | Warning     |        Yes         |                 |
 | [UseUsingScopeModifierInNewRunspaces](./UseUsingScopeModifierInNewRunspaces.md)                   | Warning     |        Yes         |                 |

docs/Rules/UseSingularNouns.md

@@ -22,6 +22,27 @@ function Get-Elements {
 }
 ```
 
+## Configuration
+
+```powershell
+Rules = @{
+    UseSingularNouns = @{
+        NounAllowList    = 'Data', 'Windows', 'Foos'
+        Enable           = $true
+    }
+}
+```
+
+### Parameters
+
+#### `UseSingularNouns: string[]` (Default value is `{'Data', 'Windows'}`)
+
+Commands to be excluded from this rule. `Data` and `Windows` are common false positives and are excluded by default
+
+#### Enable: `bool` (Default value is `$true`)
+
+Enable or disable the rule during ScriptAnalyzer invocation.
+
 ## How
 
 Change plurals to singular.