Skip to content

Add noun whitelist to singular noun rule #595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions Rules/UseSingularNouns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion Rules/UseToExportFieldsInManifest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
22 changes: 19 additions & 3 deletions Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
9 changes: 5 additions & 4 deletions Tests/Rules/UseToExportFieldsInManifest.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand All @@ -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" {
Expand Down Expand Up @@ -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" {
Expand Down