Skip to content

Commit

Permalink
Add 'UseCorrectCasing' formatting rule for cmdlet/function name (#1117)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
bergmeister authored Mar 8, 2019
1 parent 012fd5e commit 55d98f2
Show file tree
Hide file tree
Showing 12 changed files with 286 additions and 7 deletions.
3 changes: 2 additions & 1 deletion Engine/Formatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public static string Format<TCmdlet>(
"PSPlaceOpenBrace",
"PSUseConsistentWhitespace",
"PSUseConsistentIndentation",
"PSAlignAssignmentStatement"
"PSAlignAssignmentStatement",
"PSUseCorrectCasing"
};

var text = new EditableText(scriptDefinition);
Expand Down
7 changes: 6 additions & 1 deletion Engine/Settings/CodeFormatting.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
'PSPlaceCloseBrace',
'PSUseConsistentWhitespace',
'PSUseConsistentIndentation',
'PSAlignAssignmentStatement'
'PSAlignAssignmentStatement',
'PSUseCorrectCasing'
)

Rules = @{
Expand Down Expand Up @@ -43,5 +44,9 @@
Enable = $true
CheckHashtable = $true
}

PSUseCorrectCasing = @{
Enable = $true
}
}
}
7 changes: 6 additions & 1 deletion Engine/Settings/CodeFormattingAllman.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
'PSPlaceCloseBrace',
'PSUseConsistentWhitespace',
'PSUseConsistentIndentation',
'PSAlignAssignmentStatement'
'PSAlignAssignmentStatement',
'PSUseCorrectCasing'
)

Rules = @{
Expand Down Expand Up @@ -43,5 +44,9 @@
Enable = $true
CheckHashtable = $true
}

PSUseCorrectCasing = @{
Enable = $true
}
}
}
7 changes: 6 additions & 1 deletion Engine/Settings/CodeFormattingOTBS.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
'PSPlaceCloseBrace',
'PSUseConsistentWhitespace',
'PSUseConsistentIndentation',
'PSAlignAssignmentStatement'
'PSAlignAssignmentStatement',
'PSUseCorrectCasing'
)

Rules = @{
Expand Down Expand Up @@ -43,5 +44,9 @@
Enable = $true
CheckHashtable = $true
}

PSUseCorrectCasing = @{
Enable = $true
}
}
}
7 changes: 6 additions & 1 deletion Engine/Settings/CodeFormattingStroustrup.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
'PSPlaceCloseBrace',
'PSUseConsistentWhitespace',
'PSUseConsistentIndentation',
'PSAlignAssignmentStatement'
'PSAlignAssignmentStatement',
'PSUseCorrectCasing'
)

Rules = @{
Expand Down Expand Up @@ -44,5 +45,9 @@
Enable = $true
CheckHashtable = $true
}

PSUseCorrectCasing = @{
Enable = $true
}
}
}
1 change: 1 addition & 0 deletions RuleDocumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
Expand Down
27 changes: 27 additions & 0 deletions RuleDocumentation/UseCorrectCasing.md
Original file line number Diff line number Diff line change
@@ -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' }
}
```
36 changes: 36 additions & 0 deletions Rules/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions Rules/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1056,4 +1056,16 @@
<data name="UseConsistentWhitespaceErrorSpaceBeforePipe" xml:space="preserve">
<value>Use space before pipe.</value>
</data>
<data name="UseCorrectCasingCommonName" xml:space="preserve">
<value>Use exact casing of cmdlet/function name.</value>
</data>
<data name="UseCorrectCasingDescription" xml:space="preserve">
<value>For better readability and consistency, use the exact casing of the cmdlet/function.</value>
</data>
<data name="UseCorrectCasingError" xml:space="preserve">
<value>Cmdlet/Function does not match its exact casing '{0}'.</value>
</data>
<data name="UseCorrectCasingName" xml:space="preserve">
<value>UseCorrectCasing</value>
</data>
</root>
161 changes: 161 additions & 0 deletions Rules/UseCorrectCasing.cs
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// UseCorrectCasing: Check if cmdlet is cased correctly.
/// </summary>
#if !CORECLR
[Export(typeof(IScriptRule))]
#endif
public class UseCorrectCasing : ConfigurableRule
{
/// <summary>
/// AnalyzeScript: Analyze the script to check if cmdlet alias is used.
/// </summary>
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);

IEnumerable<Ast> 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));
}
}
}

/// <summary>
/// For a command like "gci -path c:", returns the extent of "gci" in the command
/// </summary>
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<CorrectionExtent> 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;
}

/// <summary>
/// GetName: Retrieves the name of this rule.
/// </summary>
/// <returns>The name of this rule</returns>
public override string GetName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseCorrectCasingName);
}

/// <summary>
/// GetCommonName: Retrieves the common name of this rule.
/// </summary>
/// <returns>The common name of this rule</returns>
public override string GetCommonName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectCasingCommonName);
}

/// <summary>
/// GetDescription: Retrieves the description of this rule.
/// </summary>
/// <returns>The description of this rule</returns>
public override string GetDescription()
{
return string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectCasingDescription);
}

/// <summary>
/// GetSourceType: Retrieves the type of the rule, Builtin, Managed or Module.
/// </summary>
public override SourceType GetSourceType()
{
return SourceType.Builtin;
}

/// <summary>
/// GetSeverity: Retrieves the severity of the rule: error, warning of information.
/// </summary>
/// <returns></returns>
public override RuleSeverity GetSeverity()
{
return RuleSeverity.Information;
}

/// <summary>
/// GetSourceName: Retrieves the name of the module/assembly the rule is from.
/// </summary>
public override string GetSourceName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
}
}
}
4 changes: 2 additions & 2 deletions Tests/Engine/GetScriptAnalyzerRule.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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" {
Expand Down
21 changes: 21 additions & 0 deletions Tests/Rules/UseCorrectCasing.tests.ps1
Original file line number Diff line number Diff line change
@@ -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'
}
}

0 comments on commit 55d98f2

Please sign in to comment.