From 43c59ab10fffd70704d4c61cb5590e12861d94f7 Mon Sep 17 00:00:00 2001 From: "Christoph Bergmeister [MVP]" Date: Fri, 8 Mar 2019 05:33:33 +0000 Subject: [PATCH] Add 'UseCorrectCasing' formatting rule for cmdlet/function name (#1117) * first working prototype: Invoke-Formatter get-childitem * tweak for fully qualified cmdlets and add tests * add rule documentation and fix 3 tests as part of that * add resource strings * fix 1 test (severity) and docs test due to rule name * fix failing test using a workaround * fix tests * remove workaround in test that has been fixed upstream * cleanup and use helper method for getting cached cmdlet data --- Engine/Formatter.cs | 3 +- Engine/Settings/CodeFormatting.psd1 | 7 +- Engine/Settings/CodeFormattingAllman.psd1 | 7 +- Engine/Settings/CodeFormattingOTBS.psd1 | 7 +- Engine/Settings/CodeFormattingStroustrup.psd1 | 7 +- RuleDocumentation/README.md | 1 + RuleDocumentation/UseCorrectCasing.md | 27 +++ Rules/Strings.Designer.cs | 36 ++++ Rules/Strings.resx | 12 ++ Rules/UseCorrectCasing.cs | 161 ++++++++++++++++++ Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 4 +- Tests/Rules/UseCorrectCasing.tests.ps1 | 21 +++ 12 files changed, 286 insertions(+), 7 deletions(-) create mode 100644 RuleDocumentation/UseCorrectCasing.md create mode 100644 Rules/UseCorrectCasing.cs create mode 100644 Tests/Rules/UseCorrectCasing.tests.ps1 diff --git a/Engine/Formatter.cs b/Engine/Formatter.cs index c07d7c5ef..d86127f2f 100644 --- a/Engine/Formatter.cs +++ b/Engine/Formatter.cs @@ -40,7 +40,8 @@ public static string Format( "PSPlaceOpenBrace", "PSUseConsistentWhitespace", "PSUseConsistentIndentation", - "PSAlignAssignmentStatement" + "PSAlignAssignmentStatement", + "PSUseCorrectCasing" }; var text = new EditableText(scriptDefinition); diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index 8e9e6c3d6..f95164ba2 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -4,7 +4,8 @@ 'PSPlaceCloseBrace', 'PSUseConsistentWhitespace', 'PSUseConsistentIndentation', - 'PSAlignAssignmentStatement' + 'PSAlignAssignmentStatement', + 'PSUseCorrectCasing' ) Rules = @{ @@ -43,5 +44,9 @@ Enable = $true CheckHashtable = $true } + + PSUseCorrectCasing = @{ + Enable = $true + } } } diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index 7756e87ea..75829097e 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -4,7 +4,8 @@ 'PSPlaceCloseBrace', 'PSUseConsistentWhitespace', 'PSUseConsistentIndentation', - 'PSAlignAssignmentStatement' + 'PSAlignAssignmentStatement', + 'PSUseCorrectCasing' ) Rules = @{ @@ -43,5 +44,9 @@ Enable = $true CheckHashtable = $true } + + PSUseCorrectCasing = @{ + Enable = $true + } } } diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index 5e033b011..0bf5a25ff 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -4,7 +4,8 @@ 'PSPlaceCloseBrace', 'PSUseConsistentWhitespace', 'PSUseConsistentIndentation', - 'PSAlignAssignmentStatement' + 'PSAlignAssignmentStatement', + 'PSUseCorrectCasing' ) Rules = @{ @@ -43,5 +44,9 @@ Enable = $true CheckHashtable = $true } + + PSUseCorrectCasing = @{ + Enable = $true + } } } diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index 9d622d55c..dd53075d2 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -5,7 +5,8 @@ 'PSPlaceCloseBrace', 'PSUseConsistentWhitespace', 'PSUseConsistentIndentation', - 'PSAlignAssignmentStatement' + 'PSAlignAssignmentStatement', + 'PSUseCorrectCasing' ) Rules = @{ @@ -44,5 +45,9 @@ Enable = $true CheckHashtable = $true } + + PSUseCorrectCasing = @{ + Enable = $true + } } } diff --git a/RuleDocumentation/README.md b/RuleDocumentation/README.md index c667d8f7d..02f56a394 100644 --- a/RuleDocumentation/README.md +++ b/RuleDocumentation/README.md @@ -45,6 +45,7 @@ |[UseApprovedVerbs](./UseApprovedVerbs.md) | Warning | | |[UseBOMForUnicodeEncodedFile](./UseBOMForUnicodeEncodedFile.md) | Warning | | |[UseCmdletCorrectly](./UseCmdletCorrectly.md) | Warning | | +|[UseCorrectCasing](./UseCorrectCasing.md) | Information | | |[UseDeclaredVarsMoreThanAssignments](./UseDeclaredVarsMoreThanAssignments.md) | Warning | | |[UseLiteralInitializerForHashtable](./UseLiteralInitializerForHashtable.md) | Warning | | |[UseOutputTypeCorrectly](./UseOutputTypeCorrectly.md) | Information | | diff --git a/RuleDocumentation/UseCorrectCasing.md b/RuleDocumentation/UseCorrectCasing.md new file mode 100644 index 000000000..c0d7a68f7 --- /dev/null +++ b/RuleDocumentation/UseCorrectCasing.md @@ -0,0 +1,27 @@ +# UseCorrectCasing + +**Severity Level: Information** + +## Description + +This is a style/formatting rule. PowerShell is case insensitive where applicable. The casing of cmdlet names does not matter but this rule ensures that the casing matches for consistency and also because most cmdlets start with an upper case and using that improves readability to the human eye. + +## How + +Use exact casing of the cmdlet, e.g. `Invoke-Command`. + +## Example + +### Wrong + +``` PowerShell +invoke-command { 'foo' } +} +``` + +### Correct + +``` PowerShell +Invoke-Command { 'foo' } +} +``` diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 77fb2bb22..c33d6a435 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -2346,6 +2346,42 @@ internal static string UseConsistentWhitespaceName { } } + /// + /// Looks up a localized string similar to Use exact casing of cmdlet/function name.. + /// + internal static string UseCorrectCasingCommonName { + get { + return ResourceManager.GetString("UseCorrectCasingCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to For better readability and consistency, use the exact casing of the cmdlet/function.. + /// + internal static string UseCorrectCasingDescription { + get { + return ResourceManager.GetString("UseCorrectCasingDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Cmdlet/Function does not match its exact casing '{0}'.. + /// + internal static string UseCorrectCasingError { + get { + return ResourceManager.GetString("UseCorrectCasingError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to UseCorrectCasing. + /// + internal static string UseCorrectCasingName { + get { + return ResourceManager.GetString("UseCorrectCasingName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Extra Variables. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 66e1b086b..8d7307b9b 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1056,4 +1056,16 @@ Use space before pipe. + + Use exact casing of cmdlet/function name. + + + For better readability and consistency, use the exact casing of the cmdlet/function. + + + Cmdlet/Function does not match its exact casing '{0}'. + + + UseCorrectCasing + \ No newline at end of file diff --git a/Rules/UseCorrectCasing.cs b/Rules/UseCorrectCasing.cs new file mode 100644 index 000000000..490008b1d --- /dev/null +++ b/Rules/UseCorrectCasing.cs @@ -0,0 +1,161 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +#if !CORECLR +using System.ComponentModel.Composition; +#endif +using System.Globalization; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// UseCorrectCasing: Check if cmdlet is cased correctly. + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class UseCorrectCasing : ConfigurableRule + { + /// + /// AnalyzeScript: Analyze the script to check if cmdlet alias is used. + /// + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); + + // Iterates all CommandAsts and check the command name. + foreach (CommandAst commandAst in commandAsts) + { + string commandName = commandAst.GetCommandName(); + + // Handles the exception caused by commands like, {& $PLINK $args 2> $TempErrorFile}. + // You can also review the remark section in following document, + // MSDN: CommandAst.GetCommandName Method + if (commandName == null) + { + continue; + } + + var commandInfo = Helper.Instance.GetCommandInfo(commandName); + if (commandInfo == null) + { + continue; + } + + var shortName = commandInfo.Name; + var fullyqualifiedName = $"{commandInfo.ModuleName}\\{shortName}"; + var isFullyQualified = commandName.Equals(fullyqualifiedName, StringComparison.OrdinalIgnoreCase); + var correctlyCasedCommandName = isFullyQualified ? fullyqualifiedName : shortName; + + if (!commandName.Equals(correctlyCasedCommandName, StringComparison.Ordinal)) + { + yield return new DiagnosticRecord( + string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectCasingError, commandName, shortName), + GetCommandExtent(commandAst), + GetName(), + DiagnosticSeverity.Warning, + fileName, + commandName, + suggestedCorrections: GetCorrectionExtent(commandAst, correctlyCasedCommandName)); + } + } + } + + /// + /// For a command like "gci -path c:", returns the extent of "gci" in the command + /// + private IScriptExtent GetCommandExtent(CommandAst commandAst) + { + var cmdName = commandAst.GetCommandName(); + foreach (var cmdElement in commandAst.CommandElements) + { + var stringConstExpressinAst = cmdElement as StringConstantExpressionAst; + if (stringConstExpressinAst != null) + { + if (stringConstExpressinAst.Value.Equals(cmdName)) + { + return stringConstExpressinAst.Extent; + } + } + } + return commandAst.Extent; + } + + private IEnumerable GetCorrectionExtent(CommandAst commandAst, string correctlyCaseName) + { + var description = string.Format( + CultureInfo.CurrentCulture, + Strings.UseCorrectCasingDescription, + correctlyCaseName, + correctlyCaseName); + var cmdExtent = GetCommandExtent(commandAst); + var correction = new CorrectionExtent( + cmdExtent.StartLineNumber, + cmdExtent.EndLineNumber, + cmdExtent.StartColumnNumber, + cmdExtent.EndColumnNumber, + correctlyCaseName, + commandAst.Extent.File, + description); + yield return correction; + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public override string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseCorrectCasingName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectCasingCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectCasingDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Information; + } + + /// + /// GetSourceName: Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} \ No newline at end of file diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index d8d8bc91a..dffd4e75c 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -59,7 +59,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 58 + $expectedNumRules = 59 if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4)) { # for PSv3 PSAvoidGlobalAliases is not shipped because @@ -157,7 +157,7 @@ Describe "TestSeverity" { It "filters rules based on multiple severity inputs"{ $rules = Get-ScriptAnalyzerRule -Severity Error,Information - $rules.Count | Should -Be 15 + $rules.Count | Should -Be 16 } It "takes lower case inputs" { diff --git a/Tests/Rules/UseCorrectCasing.tests.ps1 b/Tests/Rules/UseCorrectCasing.tests.ps1 new file mode 100644 index 000000000..396413b38 --- /dev/null +++ b/Tests/Rules/UseCorrectCasing.tests.ps1 @@ -0,0 +1,21 @@ +Describe "UseCorrectCasing" { + It "corrects case of simple cmdlet" { + Invoke-Formatter 'get-childitem' | Should -Be 'Get-ChildItem' + } + + It "corrects case of fully qualified cmdlet" { + Invoke-Formatter 'Microsoft.PowerShell.management\get-childitem' | Should -Be 'Microsoft.PowerShell.Management\Get-ChildItem' + } + + It "corrects case of of cmdlet inside interpolated string" { + Invoke-Formatter '"$(get-childitem)"' | Should -Be '"$(get-childitem)"' + } + + It "corrects case of script function" { + function Invoke-DummyFunction + { + + } + Invoke-Formatter 'invoke-dummyFunction' | Should -Be 'Invoke-DummyFunction' + } +} \ No newline at end of file