diff --git a/Rules/Strings.resx b/Rules/Strings.resx index c7645c9cf..d7b072d66 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1236,4 +1236,19 @@ The reserved word '{0}' was used as a function name. This should be avoided. + + Use correct function parameters definition kind. + + + Use consistent parameters definition kind to prevent potential unexpected behavior with inline functions parameters or param() block. + + + UseCorrectParametersKind + + + Use param() block in function body instead of inline parameters. + + + Use inline parameters definition instead of param() block in function body. + \ No newline at end of file diff --git a/Rules/UseCorrectParametersKind.cs b/Rules/UseCorrectParametersKind.cs new file mode 100644 index 000000000..d56176be5 --- /dev/null +++ b/Rules/UseCorrectParametersKind.cs @@ -0,0 +1,171 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +#if !CORECLR +using System.ComponentModel.Composition; +#endif +using System.Globalization; +using System.Linq; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// UseCorrectParametersKind: Checks if function parameters definition kind is same as preferred. + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class UseCorrectParametersKind : ConfigurableRule + { + private enum ParametersDefinitionKind + { + Inline, + ParamBlock + } + + private ParametersDefinitionKind parametersKind; + + /// + /// Construct an object of UseCorrectParametersKind type. + /// + public UseCorrectParametersKind() : base() + { + Enable = false; // Disable rule by default + } + + /// + /// The type of preferred parameters definition for functions. + /// + /// Default value is "ParamBlock". + /// + [ConfigurableRuleProperty(defaultValue: "ParamBlock")] + public string ParametersKind + { + get + { + return parametersKind.ToString(); + } + set + { + if (String.IsNullOrWhiteSpace(value) || + !Enum.TryParse(value, true, out parametersKind)) + { + parametersKind = ParametersDefinitionKind.ParamBlock; + } + } + } + + /// + /// AnalyzeScript: Analyze the script to check if any function is using not preferred parameters kind. + /// + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) { throw new ArgumentNullException(Strings.NullAstErrorMessage); } + + IEnumerable functionAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true); + if (parametersKind == ParametersDefinitionKind.ParamBlock) + { + return checkInlineParameters(functionAsts, fileName); + } + else + { + return checkParamBlockParameters(functionAsts, fileName); + } + } + + private IEnumerable checkInlineParameters(IEnumerable functionAsts, string fileName) + { + foreach (FunctionDefinitionAst functionAst in functionAsts) + { + if (functionAst.Parameters != null) + { + yield return new DiagnosticRecord( + string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectParametersKindInlineError, functionAst.Name), + functionAst.Extent, + GetName(), + GetDiagnosticSeverity(), + fileName + ); + } + } + } + + private IEnumerable checkParamBlockParameters(IEnumerable functionAsts, string fileName) + { + foreach (FunctionDefinitionAst functionAst in functionAsts) + { + if (functionAst.Body.ParamBlock != null) + { + yield return new DiagnosticRecord( + string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectParametersKindParamBlockError, functionAst.Name), + functionAst.Extent, + GetName(), + GetDiagnosticSeverity(), + fileName + ); + } + } + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectParametersKindCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseCorrectParametersKindDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseCorrectParametersKindName); + } + + /// + /// Retrieves the severity of the rule: error, warning or information. + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// Gets the severity of the returned diagnostic record: error, warning, or information. + /// + /// + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + + /// + /// Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + } +} diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index c3b744803..fbd076af5 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -63,7 +63,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 71 + $expectedNumRules = 72 if ($PSVersionTable.PSVersion.Major -le 4) { # for PSv3 PSAvoidGlobalAliases is not shipped because diff --git a/Tests/Rules/UseCorrectParametersKind.Tests.ps1 b/Tests/Rules/UseCorrectParametersKind.Tests.ps1 new file mode 100644 index 000000000..3b4a9e3ac --- /dev/null +++ b/Tests/Rules/UseCorrectParametersKind.Tests.ps1 @@ -0,0 +1,428 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + + +Describe 'UseCorrectParametersKind' { + Context 'When preferred parameters kind is set to "ParamBlock" explicitly' { + + BeforeAll { + $ruleConfiguration = @{ + Enable = $true + ParametersKind = "ParamBlock" + } + $settings = @{ + IncludeRules = @("PSUseCorrectParametersKind") + Rules = @{ + PSUseCorrectParametersKind = $ruleConfiguration + } + } + } + + It "Returns no violations for parameters outside function" { + $scriptDefinition = @' +[Parameter()]$FirstParam +[Parameter()]$SecondParam + +$FirstParam | Out-Null +$SecondParam | Out-Null +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for param() block outside function" { + $scriptDefinition = @' +param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam +) + +$FirstParam | Out-Null +$SecondParam | Out-Null +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function without parameters" { + $scriptDefinition = @' +function Test-Function { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function with empty param() block" { + $scriptDefinition = @' +function Test-Function { + param() + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function with non-empty param() block" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam + ) + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function with empty inline parameters" { + $scriptDefinition = @' +function Test-Function() { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function with empty inline parameters and non-empty param() block" { + $scriptDefinition = @' +function Test-Function() { + param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam + ) + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns violations for function with non-empty inline parameters" { + $scriptDefinition = @' +function Test-Function( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam +) { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 1 + } + } + + Context 'When preferred parameters kind is set to "ParamBlock" via default value' { + + BeforeAll { + $ruleConfiguration = @{ + Enable = $true + } + $settings = @{ + IncludeRules = @("PSUseCorrectParametersKind") + Rules = @{ + PSUseCorrectParametersKind = $ruleConfiguration + } + } + } + + It "Returns no violations for parameters outside function" { + $scriptDefinition = @' +[Parameter()]$FirstParam +[Parameter()]$SecondParam + +$FirstParam | Out-Null +$SecondParam | Out-Null +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for param() block outside function" { + $scriptDefinition = @' +param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam +) + +$FirstParam | Out-Null +$SecondParam | Out-Null +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function without parameters" { + $scriptDefinition = @' +function Test-Function { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function with empty param() block" { + $scriptDefinition = @' +function Test-Function { + param() + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function with non-empty param() block" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam + ) + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function with empty inline parameters" { + $scriptDefinition = @' +function Test-Function() { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function with empty inline parameters and non-empty param() block" { + $scriptDefinition = @' +function Test-Function() { + param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam + ) + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns violations for function with non-empty inline parameters" { + $scriptDefinition = @' +function Test-Function( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam +) { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 1 + } + } + + Context 'When preferred parameters kind is set to "Inline" explicitly' { + + BeforeAll { + $ruleConfiguration = @{ + Enable = $true + ParametersKind = "Inline" + } + + $settings = @{ + IncludeRules = @("PSUseCorrectParametersKind") + Rules = @{ + PSUseCorrectParametersKind = $ruleConfiguration + } + } + } + + It "Returns no violations for parameters outside function" { + $scriptDefinition = @' +[Parameter()]$FirstParam +[Parameter()]$SecondParam + +$FirstParam | Out-Null +$SecondParam | Out-Null +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for param() block outside function" { + $scriptDefinition = @' +param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam +) + +$FirstParam | Out-Null +$SecondParam | Out-Null +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns no violations for function without parameters" { + $scriptDefinition = @' +function Test-Function { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns violations for function with empty param() block" { + $scriptDefinition = @' +function Test-Function { + param() + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 1 + } + + It "Returns violations for function with non-empty param() block" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam + ) + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 1 + } + + It "Returns no violations for function with empty inline parameters" { + $scriptDefinition = @' +function Test-Function() { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + + It "Returns violations for function with empty inline parameters and non-empty param() block" { + $scriptDefinition = @' +function Test-Function() { + param( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam + ) + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations.Count | Should -Be 1 + } + + It "Returns no violations for function with non-empty inline parameters" { + $scriptDefinition = @' +function Test-Function( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam +) { + return +} +'@ + + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + } + + Context 'When rule is disabled explicitly' { + + BeforeAll { + $ruleConfiguration = @{ + Enable = $false + ParametersKind = "ParamBlock" + } + $settings = @{ + IncludeRules = @("PSUseCorrectParametersKind") + Rules = @{ + PSUseCorrectParametersKind = $ruleConfiguration + } + } + } + + It "Returns no violations for function with non-empty inline parameters" { + $scriptDefinition = @' +function Test-Function( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam +) { + return +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + } + + Context 'When rule is disabled via default "Enable" value' { + + BeforeAll { + $ruleConfiguration = @{ + ParametersKind = "ParamBlock" + } + $settings = @{ + IncludeRules = @("PSUseCorrectParametersKind") + Rules = @{ + PSUseCorrectParametersKind = $ruleConfiguration + } + } + } + + It "Returns no violations for function with non-empty inline parameters" { + $scriptDefinition = @' +function Test-Function( + [Parameter()]$FirstParam, + [Parameter()]$SecondParam +) { + return +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings + $violations | Should -BeNullOrEmpty + } + } +} \ No newline at end of file diff --git a/docs/Rules/README.md b/docs/Rules/README.md index da1058bc2..d975477f2 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -70,6 +70,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [UseConsistentIndentation](./UseConsistentIndentation.md) | Warning | No | Yes | | [UseConsistentWhitespace](./UseConsistentWhitespace.md) | Warning | No | Yes | | [UseCorrectCasing](./UseCorrectCasing.md) | Information | No | Yes | +| [UseCorrectParametersKind](./UseCorrectParametersKind.md) | Warning | No | Yes | | [UseDeclaredVarsMoreThanAssignments](./UseDeclaredVarsMoreThanAssignments.md) | Warning | Yes | | | [UseLiteralInitializerForHashtable](./UseLiteralInitializerForHashtable.md) | Warning | Yes | | | [UseOutputTypeCorrectly](./UseOutputTypeCorrectly.md) | Information | Yes | | diff --git a/docs/Rules/UseCorrectParametersKind.md b/docs/Rules/UseCorrectParametersKind.md new file mode 100644 index 000000000..eb6d0af88 --- /dev/null +++ b/docs/Rules/UseCorrectParametersKind.md @@ -0,0 +1,28 @@ +# UseCorrectParametersKind + +**Severity Level: Warning** + +## Description + +All functions should have same parameters definition kind specified in the rule. Either using inline parameters in function definition or using param() block inside function body. + +## How to Fix + +Rewrite function so it defines parameters as specified in the rule + +## Example + +### Correct for parameters definition kind set to 'Inline': +``````PowerShell +function f([Parameter()]$FirstParam) { + return +} +`````` + +### Correct for parameters definition kind set to 'ParamBlock': +``````PowerShell +function f { + param([Parameter()]$FirstParam) + return +} +`````` \ No newline at end of file