Skip to content

add AvoidMultipleTypesParameter Rule #1705

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
50 changes: 50 additions & 0 deletions RuleDocumentation/AvoidMultipleTypeAttributes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# AvoidMultipleTypeAttributes

**Severity Level: Warning**

## Description

Parameters should not have more than one type specifier. Multiple type specifiers on parameters will cause a runtime error.

## How

Ensure each parameter has only 1 type specifier.

## Example

### Wrong

``` PowerShell
function Test-Script
{
[CmdletBinding()]
Param
(
[String]
$Param1,

[switch]
[bool]
$Switch
)
...
}
```

### Correct

``` PowerShell
function Test-Script
{
[CmdletBinding()]
Param
(
[String]
$Param1,

[switch]
$Switch
)
...
}
```
1 change: 1 addition & 0 deletions RuleDocumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
|[AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | |
|[AvoidInvokingEmptyMembers](./AvoidInvokingEmptyMembers.md) | Warning | |
|[AvoidLongLines](./AvoidLongLines.md) | Warning | |
|[AvoidMultipleTypeAttributes](./AvoidMultipleTypeAttributes.md) | Warning | |
|[AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | |
|[AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | |
|[AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | |
Expand Down
104 changes: 104 additions & 0 deletions Rules/AvoidMultipleTypeAttributes.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Linq;
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>
/// AvoidMultipleTypeAttributes: Check that parameter does not be assigned to multiple types.
/// </summary>
#if !CORECLR
[Export(typeof(IScriptRule))]
#endif
public sealed class AvoidMultipleTypeAttributes : IScriptRule
{
/// <summary>
/// AvoidMultipleTypeAttributes: Check that parameter does not be assigned to multiple types.
/// </summary>
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
if (ast is null)
{
throw new ArgumentNullException(Strings.NullAstErrorMessage);
}

// Finds all ParamAsts.
IEnumerable<Ast> paramAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true);

// Iterates all ParamAsts and check the number of its types.
foreach (ParameterAst paramAst in paramAsts)
{
if (paramAst.Attributes.Where(typeAst => typeAst is TypeConstraintAst).Count() > 1)
{
yield return new DiagnosticRecord(
String.Format(CultureInfo.CurrentCulture, Strings.AvoidMultipleTypeAttributesError, paramAst.Name),
paramAst.Name.Extent,
GetName(),
DiagnosticSeverity.Warning,
fileName);
}
}
}

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

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

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

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

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

/// <summary>
/// GetSourceName: Retrieves the name of the module/assembly the rule is from.
/// </summary>
public string GetSourceName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
}
}
}
38 changes: 37 additions & 1 deletion 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 @@ -1152,4 +1152,16 @@
<data name="InvalidSyntaxAroundProcessBlockError" xml:space="preserve">
<value>When using an explicit process block, no preceding code is allowed, only begin, end and dynamicparams blocks.</value>
</data>
<data name="AvoidMultipleTypeAttributesCommonName" xml:space="preserve">
<value>Avoid multiple type specifiers on parameters</value>
</data>
<data name="AvoidMultipleTypeAttributesDescription" xml:space="preserve">
<value>Prameter should not have more than one type specifier.</value>
</data>
<data name="AvoidMultipleTypeAttributesError" xml:space="preserve">
<value>Parameter '{0}' has more than one type specifier.</value>
</data>
<data name="AvoidMultipleTypeAttributesName" xml:space="preserve">
<value>AvoidMultipleTypeAttributes</value>
</data>
</root>
2 changes: 1 addition & 1 deletion Tests/Engine/GetScriptAnalyzerRule.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ Describe "Test Name parameters" {

It "get Rules with no parameters supplied" {
$defaultRules = Get-ScriptAnalyzerRule
$expectedNumRules = 65
$expectedNumRules = 66
if ($PSVersionTable.PSVersion.Major -le 4)
{
# for PSv3 PSAvoidGlobalAliases is not shipped because
Expand Down
42 changes: 42 additions & 0 deletions Tests/Rules/AvoidMultipleTypeAttributes.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

BeforeAll {
$ruleName = "AvoidMultipleTypeAttributes"

$settings = @{
IncludeRules = @($ruleName)
}
}

Describe 'AvoidMultipleTypeAttributes' {
It 'Correctly diagnoses and corrects <Script>' -TestCases @(
@{ Script = 'function F1 ($s1, $p1){}' }
@{ Script = 'function F2 ([int] $s2, [int] $p2){}' }
@{ Script = 'function F3 ([int][switch] $s3, [int] $p3){}';Extent = @{ StartCol = 28; EndCol = 31 }; Message = 'Parameter ''$s3'' has more than one type specifier.' }
@{ Script = 'function F4 ([int][ref] $s4, [int] $p4){}';Extent = @{ StartCol = 25; EndCol = 28 }; Message = 'Parameter ''$s4'' has more than one type specifier.' }
@{ Script = 'function F5 ([int][switch][boolean] $s5, [int] $p5){}';Extent = @{ StartCol = 37; EndCol = 40 }; Message = 'Parameter ''$s5'' has more than one type specifier.' }
@{ Script = 'function F6 ([ValidateSet()][int] $s6, [int] $p6){}' }
@{ Script = 'function F7 ([Parameter(Mandatory=$true)][ValidateSet()][int] $s7, [int] $p7){}' }
) {
param([string]$Script, $Extent, $Message)

$diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $Script

if (-not $Extent)
{
$diagnostics | Should -BeNullOrEmpty
return
}

$expectedStartLine = if ($Extent.StartLine) { $Extent.StartLine } else { 1 }
$expectedEndLine = if ($Extent.EndLine) { $Extent.EndLine } else { 1 }

$diagnostics.Extent.StartLineNumber | Should -BeExactly $expectedStartLine
$diagnostics.Extent.EndLineNumber | Should -BeExactly $expectedEndLine
$diagnostics.Extent.StartColumnNumber | Should -BeExactly $Extent.StartCol
$diagnostics.Extent.EndColumnNumber | Should -BeExactly $Extent.EndCol

$diagnostics.Message | Should -BeExactly $Message
}
}