From dc8936605433db92ccd4ac00ee7716f74ceea601 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 4 Aug 2016 15:45:01 -0700 Subject: [PATCH 1/4] Add whitelist to UseSingularNoun rule --- Rules/UseSingularNouns.cs | 12 ++++++++++-- Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 | 13 +++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/Rules/UseSingularNouns.cs b/Rules/UseSingularNouns.cs index 64c5965bb..81d3fc392 100644 --- a/Rules/UseSingularNouns.cs +++ b/Rules/UseSingularNouns.cs @@ -27,6 +27,12 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// [Export(typeof(IScriptRule))] public class CmdletSingularNoun : IScriptRule { + + private readonly string[] nounWhiteList = + { + "Data" + }; + /// /// Checks that all defined cmdlet use singular noun /// @@ -57,12 +63,14 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { 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); } diff --git a/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 b/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 index 6685bd0d4..10f5686fd 100644 --- a/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 +++ b/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 @@ -24,6 +24,19 @@ Describe "UseSingularNouns" { It "has the correct extent" { $nounViolations[0].Extent.Text | Should be "Verb-Files" } + + It "excludes items from noun whitelist" { + $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" { From 9d53d8d9d4849e32a40575334f4928c653692eb6 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 4 Aug 2016 17:06:28 -0700 Subject: [PATCH 2/4] Examine only the last word in noun part of function name --- Rules/UseSingularNouns.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Rules/UseSingularNouns.cs b/Rules/UseSingularNouns.cs index 81d3fc392..9f64cc46c 100644 --- a/Rules/UseSingularNouns.cs +++ b/Rules/UseSingularNouns.cs @@ -52,12 +52,17 @@ public IEnumerable 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)) From 9af76ec463bdade60f53c94c25c1d7da81aadb4e Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 4 Aug 2016 17:07:35 -0700 Subject: [PATCH 3/4] Add test to verify whitelist in UseSingularNoun rule --- .../UseSingularNounsReservedVerbs.tests.ps1 | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 b/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 index 10f5686fd..7cdb6b23e 100644 --- a/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 +++ b/Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1 @@ -21,22 +21,25 @@ Describe "UseSingularNouns" { $nounViolations[0].Message | Should Match $nounViolationMessage } - It "has the correct extent" { - $nounViolations[0].Extent.Text | Should be "Verb-Files" - } - - It "excludes items from noun whitelist" { - $nounViolationScript = @' - Function Add-SomeData - { - Write-Output "Adding some data" + It "has the correct extent" { + $nounViolations[0].Extent.Text | Should be "Verb-Files" } -'@ - Invoke-ScriptAnalyzer -ScriptDefinition $nounViolationScript ` - -IncludeRule "PSUseSingularNouns" ` - -OutVariable violations - $violations.Count | Should Be 0 } + + 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" { From 7a8ab5e6f6847f3e085fc741d0012709d470db96 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 4 Aug 2016 17:46:22 -0700 Subject: [PATCH 4/4] Sort items for correction text in UseToExportFieldsInManifest rule --- Rules/UseToExportFieldsInManifest.cs | 3 ++- Tests/Rules/UseToExportFieldsInManifest.tests.ps1 | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Rules/UseToExportFieldsInManifest.cs b/Rules/UseToExportFieldsInManifest.cs index 5521642bc..1d09a3477 100644 --- a/Rules/UseToExportFieldsInManifest.cs +++ b/Rules/UseToExportFieldsInManifest.cs @@ -93,9 +93,10 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } - private string GetListLiteral(Dictionary exportedItems) + private string GetListLiteral(Dictionary exportedItemsDict) { const int lineWidth = 64; + var exportedItems = new SortedDictionary(exportedItemsDict); if (exportedItems == null || exportedItems.Keys == null) { return null; diff --git a/Tests/Rules/UseToExportFieldsInManifest.tests.ps1 b/Tests/Rules/UseToExportFieldsInManifest.tests.ps1 index a70a70931..baaf726fd 100644 --- a/Tests/Rules/UseToExportFieldsInManifest.tests.ps1 +++ b/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" {