Skip to content

Commit da64672

Browse files
MJVLbergmeister
andauthored
Convert UseSingularNouns to configurable rule and add Windows to allowlist (#1858)
* Add Windows to the UseSingularNouns allow list * Add test case for Windows verb * Refactor UseSingularNouns to configurable rule and add tests * Update UseSingularNouns docs with configuration information * Remove extra test code --------- Co-authored-by: Christoph Bergmeister <c.bergmeister@gmail.com>
1 parent 9314e69 commit da64672

File tree

4 files changed

+64
-14
lines changed

4 files changed

+64
-14
lines changed

Diff for: Rules/UseSingularNouns.cs

+14-12
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,23 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
3232
#if !CORECLR
3333
[Export(typeof(IScriptRule))]
3434
#endif
35-
public class CmdletSingularNoun : IScriptRule
35+
public class CmdletSingularNoun : ConfigurableRule
3636
{
37+
[ConfigurableRuleProperty(defaultValue: new string[] { "Data", "Windows" })]
38+
public string[] NounAllowList { get; set; }
3739

38-
private readonly string[] nounAllowList =
40+
public CmdletSingularNoun()
3941
{
40-
"Data"
41-
};
42+
Enable = true;
43+
}
4244

4345
/// <summary>
4446
/// Checks that all defined cmdlet use singular noun
4547
/// </summary>
4648
/// <param name="ast"></param>
4749
/// <param name="fileName"></param>
4850
/// <returns></returns>
49-
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
51+
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
5052
{
5153
if (ast == null) throw new ArgumentNullException(Strings.NullCommandInfoError);
5254

@@ -70,7 +72,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
7072

7173
if (pluralizer.CanOnlyBePlural(noun))
7274
{
73-
if (nounAllowList.Contains(noun, StringComparer.OrdinalIgnoreCase))
75+
if (NounAllowList.Contains(noun, StringComparer.OrdinalIgnoreCase))
7476
{
7577
continue;
7678
}
@@ -99,7 +101,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
99101
/// GetName: Retrieves the name of this rule.
100102
/// </summary>
101103
/// <returns>The name of this rule</returns>
102-
public string GetName()
104+
public override string GetName()
103105
{
104106
return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseSingularNounsName);
105107
}
@@ -108,7 +110,7 @@ public string GetName()
108110
/// GetName: Retrieves the common name of this rule.
109111
/// </summary>
110112
/// <returns>The common name of this rule</returns>
111-
public string GetCommonName()
113+
public override string GetCommonName()
112114
{
113115
return string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsCommonName);
114116
}
@@ -117,15 +119,15 @@ public string GetCommonName()
117119
/// GetDescription: Retrieves the description of this rule.
118120
/// </summary>
119121
/// <returns>The description of this rule</returns>
120-
public string GetDescription()
122+
public override string GetDescription()
121123
{
122124
return string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsDescription);
123125
}
124126

125127
/// <summary>
126128
/// GetSourceType: Retrieves the type of the rule: builtin, managed or module.
127129
/// </summary>
128-
public SourceType GetSourceType()
130+
public override SourceType GetSourceType()
129131
{
130132
return SourceType.Builtin;
131133
}
@@ -134,15 +136,15 @@ public SourceType GetSourceType()
134136
/// GetSeverity: Retrieves the severity of the rule: error, warning of information.
135137
/// </summary>
136138
/// <returns></returns>
137-
public RuleSeverity GetSeverity()
139+
public override RuleSeverity GetSeverity()
138140
{
139141
return RuleSeverity.Warning;
140142
}
141143

142144
/// <summary>
143145
/// GetSourceName: Retrieves the module/assembly name the rule is from.
144146
/// </summary>
145-
public string GetSourceName()
147+
public override string GetSourceName()
146148
{
147149
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
148150
}

Diff for: Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1

+28-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Describe "UseSingularNouns" {
3232

3333
Context "When function names have nouns from allowlist" {
3434

35-
It "ignores function name ending with Data" {
35+
It "ignores function name ending with Data by default" {
3636
$nounViolationScript = @'
3737
Function Add-SomeData
3838
{
@@ -44,6 +44,33 @@ Write-Output "Adding some data"
4444
-OutVariable violations
4545
$violations.Count | Should -Be 0
4646
}
47+
48+
It "ignores function name ending with Windows by default" {
49+
$nounViolationScript = @'
50+
Function Test-Windows
51+
{
52+
Write-Output "Testing Microsoft Windows"
53+
}
54+
'@
55+
Invoke-ScriptAnalyzer -ScriptDefinition $nounViolationScript `
56+
-IncludeRule "PSUseSingularNouns" `
57+
-OutVariable violations
58+
$violations.Count | Should -Be 0
59+
}
60+
61+
It "ignores function names defined in settings" {
62+
$nounViolationScript = @'
63+
Function Get-Bananas
64+
{
65+
Write-Output "Bananas"
66+
}
67+
'@
68+
Invoke-ScriptAnalyzer -ScriptDefinition $nounViolationScript -Settings @{
69+
IncludeRules = @("PSUseSingularNouns")
70+
Rules = @{ PSUseSingularNouns = @{ NounAllowList = "Bananas" } }
71+
} | Should -BeNullOrEmpty
72+
}
73+
4774
}
4875

4976
Context "When there are no violations" {

Diff for: docs/Rules/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ The PSScriptAnalyzer contains the following rule definitions.
7575
| [UseProcessBlockForPipelineCommand](./UseProcessBlockForPipelineCommand.md) | Warning | Yes | |
7676
| [UsePSCredentialType](./UsePSCredentialType.md) | Warning | Yes | |
7777
| [UseShouldProcessForStateChangingFunctions](./UseShouldProcessForStateChangingFunctions.md) | Warning | Yes | |
78-
| [UseSingularNouns](./UseSingularNouns.md) | Warning | Yes | |
78+
| [UseSingularNouns](./UseSingularNouns.md) | Warning | Yes | Yes |
7979
| [UseSupportsShouldProcess](./UseSupportsShouldProcess.md) | Warning | Yes | |
8080
| [UseToExportFieldsInManifest](./UseToExportFieldsInManifest.md) | Warning | Yes | |
8181
| [UseUsingScopeModifierInNewRunspaces](./UseUsingScopeModifierInNewRunspaces.md) | Warning | Yes | |

Diff for: docs/Rules/UseSingularNouns.md

+21
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,27 @@ function Get-Elements {
2222
}
2323
```
2424

25+
## Configuration
26+
27+
```powershell
28+
Rules = @{
29+
UseSingularNouns = @{
30+
NounAllowList = 'Data', 'Windows', 'Foos'
31+
Enable = $true
32+
}
33+
}
34+
```
35+
36+
### Parameters
37+
38+
#### `UseSingularNouns: string[]` (Default value is `{'Data', 'Windows'}`)
39+
40+
Commands to be excluded from this rule. `Data` and `Windows` are common false positives and are excluded by default
41+
42+
#### Enable: `bool` (Default value is `$true`)
43+
44+
Enable or disable the rule during ScriptAnalyzer invocation.
45+
2546
## How
2647

2748
Change plurals to singular.

0 commit comments

Comments
 (0)