Make UseSingularNouns rule work on PowerShell Core (#1627)

bergmeister · web-flow · commit 1eb56713df71 · 2021-04-28T09:32:38.000-07:00
diff --git a/RuleDocumentation/README.md b/RuleDocumentation/README.md
@@ -57,7 +57,7 @@
 |[UseProcessBlockForPipelineCommand](./UseProcessBlockForPipelineCommand.md) | Warning | |
 |[UsePSCredentialType](./UsePSCredentialType.md) | Warning | |
 |[UseShouldProcessForStateChangingFunctions](./UseShouldProcessForStateChangingFunctions.md) | Warning | |
-|[UseSingularNouns<sup>*</sup>](./UseSingularNouns.md) | Warning | |
+|[UseSingularNouns](./UseSingularNouns.md) | Warning | |
 |[UseSupportsShouldProcess](./UseSupportsShouldProcess.md) | Warning | |
 |[UseToExportFieldsInManifest](./UseToExportFieldsInManifest.md) | Warning | |
 |[UseCompatibleCmdlets](./UseCompatibleCmdlets.md) | Warning | Yes |
diff --git a/Rules/Rules.csproj b/Rules/Rules.csproj
@@ -7,6 +7,7 @@
     <AssemblyVersion>1.20.0</AssemblyVersion>
     <PackageId>Rules</PackageId>
     <RootNamespace>Microsoft.Windows.PowerShell.ScriptAnalyzer</RootNamespace> <!-- Namespace needs to match Assembly name for ressource binding -->
+    <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies> <!-- Needed in order for Pluralize.NET DLL to appear in bin folder - https://github.com/NuGet/Home/issues/4488 -->
   </PropertyGroup>
 
   <ItemGroup>
@@ -18,7 +19,7 @@
   <ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.1' ">
     <PackageReference Include="System.Reflection.TypeExtensions" Version="4.7.0" />
     <PackageReference Include="Microsoft.Management.Infrastructure" Version="2.0.0" />
-    <Compile Remove="UseSingularNouns.cs" />
+    <PackageReference Include="Pluralize.NET" Version="1.0.2" />
   </ItemGroup>
 
   <ItemGroup Condition=" '$(TargetFramework)' == 'net452' ">
diff --git a/Rules/UseSingularNouns.cs b/Rules/UseSingularNouns.cs
@@ -15,18 +15,25 @@
 using System.Linq;
 using System.Management.Automation.Language;
 using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+#if CORECLR
+using Pluralize.NET;
+#else
 using System.ComponentModel.Composition;
+using System.Data.Entity.Design.PluralizationServices;
+#endif
 using System.Globalization;
-using System.Text.RegularExpressions;
 
 namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 {
     /// <summary>
     /// CmdletSingularNoun: Analyzes scripts to check that all defined cmdlets use singular nouns.
     /// 
     /// </summary>
-    [Export(typeof(IScriptRule))]
-    public class CmdletSingularNoun : IScriptRule {
+#if !CORECLR
+[Export(typeof(IScriptRule))]
+#endif
+    public class CmdletSingularNoun : IScriptRule
+    {
 
         private readonly string[] nounAllowList =
         {
@@ -39,46 +46,49 @@ public class CmdletSingularNoun : IScriptRule {
         /// <param name="ast"></param>
         /// <param name="fileName"></param>
         /// <returns></returns>
-        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName) {
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
             if (ast == null) throw new ArgumentNullException(Strings.NullCommandInfoError);
 
             IEnumerable<Ast> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true);
 
-            char[] funcSeperator = { '-' };
-            string[] funcNamePieces = new string[2];
+            var pluralizer = new PluralizerProxy();
 
             foreach (FunctionDefinitionAst funcAst in funcAsts)
             {
-                if (funcAst.Name != null && funcAst.Name.Contains('-'))
+                if (funcAst.Name == null || !funcAst.Name.Contains('-'))
+                {
+                    continue;
+                }
+
+                string noun = GetLastWordInCmdlet(funcAst.Name);
+
+                if (noun is null)
                 {
-                    funcNamePieces = funcAst.Name.Split(funcSeperator);
-                    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
-                    nounPart = SplitCamelCaseString(nounPart);
-                    var words = nounPart.Split(new char [] { ' ' });
-                    var noun = words.LastOrDefault();
-                    if (noun == null)
+                    continue;
+                }
+
+                if (pluralizer.CanOnlyBePlural(noun))
+                {
+                    if (nounAllowList.Contains(noun, StringComparer.OrdinalIgnoreCase))
                     {
                         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 (extent is null)
                     {
-                        IScriptExtent extent = Helper.Instance.GetScriptExtentForFunctionName(funcAst);
-                        if (nounAllowList.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);
+                        extent = funcAst.Extent;
                     }
+
+                    yield return new DiagnosticRecord(
+                        string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsError, funcAst.Name),
+                        extent,
+                        GetName(),
+                        DiagnosticSeverity.Warning,
+                        fileName,
+                        suggestedCorrections: new CorrectionExtent[] { GetCorrection(pluralizer, extent, funcAst.Name, noun) });
                 }
             }
 
@@ -106,7 +116,8 @@ public string GetCommonName()
         /// GetDescription: Retrieves the description of this rule.
         /// </summary>
         /// <returns>The description of this rule</returns>
-        public string GetDescription() {
+        public string GetDescription()
+        {
             return string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsDescription);
         }
 
@@ -135,18 +146,77 @@ public string GetSourceName()
             return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
         }
 
+        private CorrectionExtent GetCorrection(PluralizerProxy pluralizer, IScriptExtent extent, string commandName, string noun)
+        {
+            string singularNoun = pluralizer.Singularize(noun);
+            string newCommandName = commandName.Substring(0, commandName.Length - noun.Length) + singularNoun;
+            return new CorrectionExtent(extent, newCommandName, extent.File, $"Singularized correction of '{extent.Text}'");
+        }
+
         /// <summary>
-        /// SplitCamelCaseString: Splits a Camel Case'd string into individual words with space delimited
+        /// Gets the last word in a standard syntax, CamelCase cmdlet.
+        /// If the cmdlet name is non-standard, returns null.
         /// </summary>
-        private string SplitCamelCaseString(string input)
+        private string GetLastWordInCmdlet(string cmdletName)
         {
-            if (String.IsNullOrEmpty(input))
+            if (string.IsNullOrEmpty(cmdletName))
             {
-                return String.Empty;
+                return null;
             }
 
-            return Regex.Replace(input, "([A-Z])", " $1", RegexOptions.Compiled).Trim();
+            // Cmdlet doesn't use CamelCase, so assume it's something like an initialism that shouldn't be singularized
+            if (!char.IsLower(cmdletName[cmdletName.Length - 1]))
+            {
+                return null;
+            }
+
+            for (int i = cmdletName.Length - 1; i >= 0; i--)
+            {
+                if (cmdletName[i] == '-')
+                {
+                    // We got to the dash without seeing a CamelCase word, so nothing to singularize
+                    return null;
+                }
+
+                // We just changed from lower case to upper, so we have the end word
+                if (char.IsUpper(cmdletName[i]))
+                {
+                    return cmdletName.Substring(i);
+                }
+            }
+
+            // We shouldn't ever get here since we should always eventually hit a '-'
+            // But if we do, assume this isn't supported cmdlet name
+            return null;
+        }
+
+#if CORECLR
+        private class PluralizerProxy
+        {
+            private readonly Pluralizer _pluralizer;
+
+            public PluralizerProxy()
+            {
+                _pluralizer = new Pluralizer();
+            }
+
+            public bool CanOnlyBePlural(string noun) =>
+                !_pluralizer.IsSingular(noun) && _pluralizer.IsPlural(noun);
+
+            public string Singularize(string noun) => _pluralizer.Singularize(noun);
+        }
+#else
+        private class PluralizerProxy
+        {
+            private static readonly PluralizationService s_pluralizationService = PluralizationService.CreateService(
+                CultureInfo.GetCultureInfo("en-us"));
+
+            public bool CanOnlyBePlural(string noun) =>
+                !s_pluralizationService.IsSingular(noun) && s_pluralizationService.IsPlural(noun);
+
+            public string Singularize(string noun) => s_pluralizationService.Singularize(noun);
         }
+#endif
     }
 
 }
diff --git a/Tests/Documentation/RuleDocumentation.tests.ps1 b/Tests/Documentation/RuleDocumentation.tests.ps1
@@ -15,14 +15,8 @@ Describe "Validate rule documentation files" {
             }} |
             Sort-Object
 
-        # Remove rules from the diff list that aren't supported on PSCore
-        if (($PSVersionTable.PSVersion.Major -ge 6) -and ($PSVersionTable.PSEdition -eq "Core"))
-        {
-            $RulesNotSupportedInNetstandard2 = @("PSUseSingularNouns")
-            $docs = $docs | Where-Object {$RulesNotSupportedInNetstandard2 -notcontains $_}
-            $readmeRules = $readmeRules | Where-Object { $RulesNotSupportedInNetstandard2 -notcontains $_ }
-        }
-        elseif ($PSVersionTable.PSVersion.Major -eq 4) {
+        # Remove rules from the diff list that aren't supported on old PS version
+        if ($PSVersionTable.PSVersion.Major -eq 4) {
             $docs = $docs | Where-Object {$_ -notmatch '^PSAvoidGlobalAliases$'}
             $readmeRules = $readmeRules | Where-Object { $_ -notmatch '^PSAvoidGlobalAliases$' }
         }
diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1
@@ -64,14 +64,11 @@ Describe "Test Name parameters" {
         It "get Rules with no parameters supplied" {
             $defaultRules = Get-ScriptAnalyzerRule
             $expectedNumRules = 65
-            if ($IsCoreCLR -or ($PSVersionTable.PSVersion.Major -eq 3) -or ($PSVersionTable.PSVersion.Major -eq 4))
+            if ($PSVersionTable.PSVersion.Major -le 4)
             {
                 # for PSv3 PSAvoidGlobalAliases is not shipped because
                 # it uses StaticParameterBinder.BindCommand which is
                 # available only on PSv4 and above
-                # for PowerShell Core, PSUseSingularNouns is not
-                # shipped because it uses APIs that are not present
-                # in dotnet core.
 
                 $expectedNumRules--
             }
diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1
@@ -270,12 +270,7 @@ Describe "Test IncludeRule" {
         }
 
         It "includes the given rules" {
-            # CoreCLR version of PSScriptAnalyzer does not contain PSUseSingularNouns rule
             $expectedNumViolations = 2
-            if ($IsCoreCLR)
-            {
-                $expectedNumViolations = 1
-            }
             $violations = Invoke-ScriptAnalyzer $PSScriptRoot\..\Rules\BadCmdlet.ps1 -IncludeRule $rules
             $violations.Count | Should -Be $expectedNumViolations
         }
@@ -295,12 +290,7 @@ Describe "Test IncludeRule" {
         }
 
         It "includes 2 wildcardrules" {
-            # CoreCLR version of PSScriptAnalyzer does not contain PSUseSingularNouns rule
             $expectedNumViolations = 4
-            if ($IsCoreCLR)
-            {
-                $expectedNumViolations = 3
-            }
             $includeWildcard = Invoke-ScriptAnalyzer $PSScriptRoot\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules
             $includeWildcard += Invoke-ScriptAnalyzer $PSScriptRoot\..\Rules\BadCmdlet.ps1 -IncludeRule $useRules
             $includeWildcard.Count | Should -Be $expectedNumViolations
diff --git a/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 b/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1
@@ -1,4 +1,4 @@
-# Copyright (c) Microsoft Corporation. All rights reserved.
+﻿# Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the MIT License.
 
 BeforeAll {
@@ -15,7 +15,7 @@ BeforeAll {
 }
 
 # UseSingularNouns rule doesn't exist in the non desktop version of PSScriptAnalyzer due to missing .Net Pluralization API
-Describe "UseSingularNouns" -Skip:$IsCoreCLR {
+Describe "UseSingularNouns" {
     Context "When there are violations" {
         It "has a cmdlet singular noun violation" {
             $nounViolations.Count | Should -Be 1
@@ -51,6 +51,41 @@ Write-Output "Adding some data"
             $nounNoViolations.Count | Should -Be 0
         }
     }
+
+    Context "Inline tests" {
+        It 'Correctly diagnoses and corrects <Script>' -TestCases @(
+            @{ Script = 'function Get-Bananas { "bananas" }'; Extent = @{ StartCol = 10; EndCol = 21 }; Correction = 'Get-Banana' }
+            @{ Script = 'function ConvertTo-StartingCriteria { "criteria" }'; Extent = @{ StartCol = 10; EndCol = 36 }; Correction = 'ConvertTo-StartingCriterion' }
+            @{ Script = 'function Invoke-Data { "data" }' }
+            @{ Script = 'function Get-Banana { "bananas" }' }
+            @{ Script = 'function get-banana { "bananas" }' }
+            @{ Script = 'function banana { "bananas" }' }
+        ) {
+            param([string]$Script, $Extent, $Correction)
+
+            $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $Script
+
+            if (-not $Extent)
+            {
+                $diagnostics | Should -BeNullOrEmpty
+                return
+            }
+
+            $expectedStartLine = if ($Extent.StartLine) { $Extent.StartLine } else { 1 }
+            $expectedEndLine = if ($Extent.EndLine) { $Extent.EndLine } else { 1 }
+
+            $diagnostics.Extent.StartLineNumber | Should -BeExactly $expectedStartLine
+            $diagnostics.Extent.EndLineNumber | Should -BeExactly $expectedEndLine
+            $diagnostics.Extent.StartColumnNumber | Should -BeExactly $Extent.StartCol
+            $diagnostics.Extent.EndColumnNumber | Should -BeExactly $Extent.EndCol
+
+            $diagnostics.SuggestedCorrections.StartLineNumber | Should -BeExactly $expectedStartLine
+            $diagnostics.SuggestedCorrections.EndLineNumber | Should -BeExactly $expectedEndLine
+            $diagnostics.SuggestedCorrections.StartColumnNumber | Should -BeExactly $Extent.StartCol
+            $diagnostics.SuggestedCorrections.EndColumnNumber | Should -BeExactly $Extent.EndCol
+            $diagnostics.SuggestedCorrections.Text | Should -BeExactly $Correction
+        }
+    }
 }
 
 Describe "UseApprovedVerbs" {
diff --git a/build.psm1 b/build.psm1
@@ -311,15 +311,18 @@ function Start-ScriptAnalyzerBuild
         $settingsFiles = Get-Childitem "$projectRoot\Engine\Settings" | ForEach-Object -MemberName FullName
         Publish-File $settingsFiles (Join-Path -Path $script:destinationDir -ChildPath Settings)
 
+        $rulesProjectOutputDir = if ($env:TF_BUILD) {
+            "$projectRoot\bin\${buildConfiguration}\${framework}" }
+        else {
+            "$projectRoot\Rules\bin\${buildConfiguration}\${framework}"
+        }
         if ($framework -eq 'net452') {
-            if ( $env:TF_BUILD ) {
-                $nsoft =  "$projectRoot\bin\${buildConfiguration}\${framework}\Newtonsoft.Json.dll"
-            }
-            else {
-                $nsoft =  "$projectRoot\Rules\bin\${buildConfiguration}\${framework}\Newtonsoft.Json.dll"
-            }
+            $nsoft = Join-Path $rulesProjectOutputDir 'Newtonsoft.Json.dll'
             Copy-Item -path $nsoft -Destination $destinationDirBinaries
         }
+        else {
+            Copy-Item -Path (Join-Path $rulesProjectOutputDir 'Pluralize.NET.dll') -Destination $destinationDirBinaries
+        }
 
         Pop-Location
     }
diff --git a/tools/releaseBuild/signing.xml b/tools/releaseBuild/signing.xml
@@ -24,6 +24,7 @@
     <file src="__INPATHROOT__\out\PSScriptAnalyzer\1.19.1\PSv7\Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.dll" signType="Authenticode" dest="__OUTPATHROOT__\PSScriptAnalyzer\1.19.1\PSv7\Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.dll" />
     <file src="__INPATHROOT__\out\PSScriptAnalyzer\1.19.1\PSv7\Microsoft.Windows.PowerShell.ScriptAnalyzer.dll" signType="Authenticode" dest="__OUTPATHROOT__\PSScriptAnalyzer\1.19.1\PSv7\Microsoft.Windows.PowerShell.ScriptAnalyzer.dll" />
     <file src="__INPATHROOT__\out\PSScriptAnalyzer\1.19.1\PSv7\Microsoft.PowerShell.CrossCompatibility.dll" signType="Authenticode" dest="__OUTPATHROOT__\PSScriptAnalyzer\1.19.1\PSv7\Microsoft.PowerShell.CrossCompatibility.dll" />
+    <file src="__INPATHROOT__\out\PSScriptAnalyzer\1.19.1\PSv7\Pluralize.NET.dll" signType="Authenticode" dest="__OUTPATHROOT__\PSScriptAnalyzer\1.19.1\PSv7\Pluralize.NET.dll" />
  </job>
  <job platform="" configuration="" dest="__OUTPATHROOT__\signed" jobname="PowerShell Script Analyzer PSv3" approvers="vigarg;gstolt">
     <file src="__INPATHROOT__\out\PSScriptAnalyzer\1.19.1\PSv3\Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.dll" signType="Authenticode" dest="__OUTPATHROOT__\PSScriptAnalyzer\1.19.1\PSv3\Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.dll" />
