Skip to content
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

Make UseSingularNouns rule work on PowerShell Core #1627

Merged
merged 46 commits into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
cdcea4e
Make UseSingularNouns rule work for PowerShell Core versions
bergmeister Jan 23, 2021
8b75e06
fix test
bergmeister Jan 23, 2021
fb51900
fix build
bergmeister Jan 23, 2021
e0132d2
try remove NuGet.Config that led to build failure and use same old ge…
bergmeister Jan 23, 2021
842b866
fix more tests
bergmeister Jan 23, 2021
240f056
fix build
bergmeister Jan 23, 2021
532053c
fix last 2 test failures
bergmeister Jan 23, 2021
930997b
use Pluralize.NET library, which has MIT licence
bergmeister Feb 7, 2021
52ac58a
cleanup diff
bergmeister Feb 7, 2021
f4c19d8
whitespace
bergmeister Feb 7, 2021
9c99b9d
Update SingularNouns rule approach
Apr 23, 2021
7389618
Fix dumb string characters
Apr 23, 2021
9327e6f
Add NuGet.config back
Apr 23, 2021
05ccc2b
Fix namespacing
Apr 23, 2021
ef21d48
Fix pragma casing
Apr 23, 2021
d013b61
Use old index syntax
Apr 23, 2021
a6f08d2
Change singularization for dash cases
rjmholt Apr 23, 2021
1402a5e
Remove dead variables
rjmholt Apr 23, 2021
c40f6c6
Make UseSingularNouns rule work for PowerShell Core versions
bergmeister Jan 23, 2021
d9f2065
fix test
bergmeister Jan 23, 2021
7c3862d
fix build
bergmeister Jan 23, 2021
c6f90fb
try remove NuGet.Config that led to build failure and use same old ge…
bergmeister Jan 23, 2021
ae74050
fix more tests
bergmeister Jan 23, 2021
12e634f
fix build
bergmeister Jan 23, 2021
c474c90
fix last 2 test failures
bergmeister Jan 23, 2021
e8467d7
use Pluralize.NET library, which has MIT licence
bergmeister Feb 7, 2021
ade612b
cleanup diff
bergmeister Feb 7, 2021
b070fda
whitespace
bergmeister Feb 7, 2021
62de267
Update SingularNouns rule approach
Apr 23, 2021
a4eac45
Fix dumb string characters
Apr 23, 2021
6832617
Add NuGet.config back
Apr 23, 2021
adb8081
Fix namespacing
Apr 23, 2021
2372c9e
Fix pragma casing
Apr 23, 2021
c465363
Use old index syntax
Apr 23, 2021
4e1045b
Change singularization for dash cases
rjmholt Apr 23, 2021
d09fcad
Remove dead variables
rjmholt Apr 23, 2021
025d9a4
Add corrections
rjmholt Apr 27, 2021
5e71bf3
Fix correction
rjmholt Apr 27, 2021
5832be4
Add more tests
rjmholt Apr 27, 2021
515a8fe
Put test cases directly in block
rjmholt Apr 27, 2021
2230c9c
Place test cases inline
rjmholt Apr 27, 2021
5d91969
Remove Write-Host
rjmholt Apr 27, 2021
f83f72f
Use an example that isn't broken in .NET Framework
rjmholt Apr 28, 2021
b9b7cb9
Use the right number
rjmholt Apr 28, 2021
ec0ab4b
Fix other tests
rjmholt Apr 28, 2021
8969546
Merge branch 'master' into UseSingularNoun_PSCore
rjmholt Apr 28, 2021
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
2 changes: 1 addition & 1 deletion RuleDocumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
3 changes: 2 additions & 1 deletion Rules/Rules.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<AssemblyVersion>1.19.1</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>
Expand All @@ -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' ">
Expand Down
127 changes: 93 additions & 34 deletions Rules/UseSingularNouns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@
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;

Expand All @@ -25,8 +30,11 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
/// 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 =
{
Expand All @@ -39,46 +47,48 @@ 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)
{
continue;
}

if (pluralizer.CanOnlyBePlural(noun))
{
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)
IScriptExtent extent = Helper.Instance.GetScriptExtentForFunctionName(funcAst);

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))
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(
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsError, funcAst.Name),
extent,
GetName(),
DiagnosticSeverity.Warning,
fileName);
}
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -136,17 +147,65 @@ public string GetSourceName()
}

/// <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)
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
{
if (string.IsNullOrEmpty(cmdletName))
{
return null;
}

// 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
{
if (String.IsNullOrEmpty(input))
private readonly Pluralizer _pluralizer;

public PluralizerProxy()
{
return String.Empty;
_pluralizer = new Pluralizer();
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
}

return Regex.Replace(input, "([A-Z])", " $1", RegexOptions.Compiled).Trim();
public bool CanOnlyBePlural(string noun) =>
!_pluralizer.IsSingular(noun) && _pluralizer.IsPlural(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);
}
#endif
}

}
10 changes: 2 additions & 8 deletions Tests/Documentation/RuleDocumentation.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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$' }
}
Expand Down
5 changes: 1 addition & 4 deletions Tests/Engine/GetScriptAnalyzerRule.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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--
}
Expand Down
10 changes: 0 additions & 10 deletions Tests/Engine/InvokeScriptAnalyzer.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Tests/Rules/UseSingularNounsReservedVerbs.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
Context "When there are violations" {
It "has a cmdlet singular noun violation" {
$nounViolations.Count | Should -Be 1
Expand Down
15 changes: 9 additions & 6 deletions build.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions tools/releaseBuild/signing.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down