diff --git a/RuleDocumentation/README.md b/RuleDocumentation/README.md index fbe0f5cbd..551ad55f5 100644 --- 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*](./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 index 157f4467d..b59837c52 100644 --- a/Rules/Rules.csproj +++ b/Rules/Rules.csproj @@ -7,6 +7,7 @@ 1.20.0 Rules Microsoft.Windows.PowerShell.ScriptAnalyzer + true @@ -18,7 +19,7 @@ - + diff --git a/Rules/UseSingularNouns.cs b/Rules/UseSingularNouns.cs index 2a6fdd9d5..de9264d35 100644 --- a/Rules/UseSingularNouns.cs +++ b/Rules/UseSingularNouns.cs @@ -15,9 +15,13 @@ 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 { @@ -25,8 +29,11 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// CmdletSingularNoun: Analyzes scripts to check that all defined cmdlets use singular nouns. /// /// - [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 { /// /// /// - public IEnumerable AnalyzeScript(Ast ast, string fileName) { + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { if (ast == null) throw new ArgumentNullException(Strings.NullCommandInfoError); IEnumerable 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. /// /// The description of this rule - 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}'"); + } + /// - /// 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. /// - 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 index 6b0d93a51..f08d9689e 100644 --- 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 index 664be838a..e5e2159bd 100644 --- 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 index 3a94699bd..a1b8b7380 100644 --- 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 index b712bc6c0..534e6df4d 100644 --- 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