Merge pull request #595 from PowerShell/kapilmb/AddWhitelistToSingularNounRule

Add noun whitelist to singular noun rule

Author: Kapil Borle

Rules/UseSingularNouns.cs

@@ -27,6 +27,12 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
     /// </summary>
     [Export(typeof(IScriptRule))]
     public class CmdletSingularNoun : IScriptRule {
+
+        private readonly string[] nounWhiteList =
+        {
+            "Data"
+        };
+
         /// <summary>
         /// Checks that all defined cmdlet use singular noun
         /// </summary>
@@ -46,23 +52,30 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName) {
                 if (funcAst.Name != null && funcAst.Name.Contains('-'))
                 {
                     funcNamePieces = funcAst.Name.Split(funcSeperator);
-                    String noun = funcNamePieces[1];
+                    String nounPart = funcNamePieces[1];
 
                     // Convert the noun part of the function into a series of space delimited words
                     // This helps the PluralizationService to provide an accurate determination about the plurality of the string
-                    noun = SplitCamelCaseString(noun);
-
+                    nounPart = SplitCamelCaseString(nounPart);
+                    var words = nounPart.Split(new char [] { ' ' });
+                    var noun = words.LastOrDefault();
+                    if (noun == null)
+                    {
+                        continue;
+                    }
                     var ps = System.Data.Entity.Design.PluralizationServices.PluralizationService.CreateService(CultureInfo.GetCultureInfo("en-us"));
 
                     if (!ps.IsSingular(noun) && ps.IsPlural(noun))
                     {
                         IScriptExtent extent = Helper.Instance.GetScriptExtentForFunctionName(funcAst);
-
+                        if (nounWhiteList.Contains(noun, StringComparer.OrdinalIgnoreCase))
+                        {
+                            continue;
+                        }
                         if (null == extent)
                         {
                             extent = funcAst.Extent;
                         }
-
                         yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsError, funcAst.Name),
                             extent, GetName(), DiagnosticSeverity.Warning, fileName);
                     }

Rules/UseToExportFieldsInManifest.cs

@@ -93,9 +93,10 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
 
         }
 
-        private string GetListLiteral<T>(Dictionary<string, T> exportedItems)
+        private string GetListLiteral<T>(Dictionary<string, T> exportedItemsDict)
         {
             const int lineWidth = 64;
+            var exportedItems = new SortedDictionary<string, T>(exportedItemsDict);
             if (exportedItems == null || exportedItems.Keys == null)
             {
                 return null;

Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1

@@ -21,9 +21,25 @@ Describe "UseSingularNouns" {
             $nounViolations[0].Message | Should Match $nounViolationMessage
         }
 
-	It "has the correct extent" {
-	   $nounViolations[0].Extent.Text | Should be "Verb-Files"
-	}
+        It "has the correct extent" {
+        $nounViolations[0].Extent.Text | Should be "Verb-Files"
+        }
+    }
+
+    Context "When function names have nouns from whitelist" {
+
+        It "ignores function name ending with Data" {
+            $nounViolationScript = @'
+Function Add-SomeData
+{
+    Write-Output "Adding some data"
+}
+'@
+            Invoke-ScriptAnalyzer -ScriptDefinition $nounViolationScript `
+                -IncludeRule "PSUseSingularNouns" `
+                -OutVariable violations
+            $violations.Count | Should Be 0
+        }
     }
 
     Context "When there are no violations" {

Tests/Rules/UseToExportFieldsInManifest.tests.ps1

@@ -43,8 +43,8 @@ Describe "UseManifestExportFields" {
 	It "suggests corrections for FunctionsToExport with wildcard" {
 	    $violations = Run-PSScriptAnalyzerRule $testManifestBadFunctionsWildcardPath
 	    $violationFilepath = Join-path $testManifestPath $testManifestBadFunctionsWildcardPath
-	    Test-CorrectionExtent $violationFilepath $violations[0] 1 "'*'" "@('Get-Foo', 'Get-Bar')"
-	    $violations[0].SuggestedCorrections[0].Description | Should Be "Replace '*' with @('Get-Foo', 'Get-Bar')"
+	    Test-CorrectionExtent $violationFilepath $violations[0] 1 "'*'" "@('Get-Bar', 'Get-Foo')"
+	    $violations[0].SuggestedCorrections[0].Description | Should Be "Replace '*' with @('Get-Bar', 'Get-Foo')"
 	}
 
         It "detects FunctionsToExport with null" {
@@ -56,7 +56,8 @@ Describe "UseManifestExportFields" {
 	It "suggests corrections for FunctionsToExport with null and line wrapping" {
 	    $violations = Run-PSScriptAnalyzerRule $testManifestBadFunctionsNullPath
 	    $violationFilepath = Join-path $testManifestPath $testManifestBadFunctionsNullPath
-	    Test-CorrectionExtent $violationFilepath $violations[0] 1  '$null' "@('Get-Foo1', 'Get-Foo2', 'Get-Foo3', 'Get-Foo4', 'Get-Foo5', 'Get-Foo6', `r`n`t`t'Get-Foo7', 'Get-Foo8', 'Get-Foo9', 'Get-Foo10', 'Get-Foo11', `r`n`t`t'Get-Foo12')"
+        $expectedCorrectionExtent = "@('Get-Foo1', 'Get-Foo10', 'Get-Foo11', 'Get-Foo12', 'Get-Foo2', 'Get-Foo3', {0}`t`t'Get-Foo4', 'Get-Foo5', 'Get-Foo6', 'Get-Foo7', 'Get-Foo8', {0}`t`t'Get-Foo9')" -f [System.Environment]::NewLine
+	    Test-CorrectionExtent $violationFilepath $violations[0] 1  '$null' $expectedCorrectionExtent
 	}
 
         It "detects array element containing wildcard" {
@@ -84,7 +85,7 @@ Describe "UseManifestExportFields" {
 	It "suggests corrections for AliasesToExport with wildcard" {
 	    $violations = Run-PSScriptAnalyzerRule $testManifestBadAliasesWildcardPath
 	    $violationFilepath = Join-path $testManifestPath $testManifestBadAliasesWildcardPath
-	    Test-CorrectionExtent $violationFilepath $violations[0] 1  "'*'" "@('gfoo', 'gbar')"
+	    Test-CorrectionExtent $violationFilepath $violations[0] 1  "'*'" "@('gbar', 'gfoo')"
 	}
 
         It "detects all the *ToExport violations" {