From ebd9efa35eccab1937b00ed5d184df5468c2c994 Mon Sep 17 00:00:00 2001 From: Haimasker Date: Fri, 2 Jan 2026 16:37:13 +0300 Subject: [PATCH] Add UseCorrectParametersKind rule * Consistent function parameters definition * Rule is disabled by default * Possible types of preferred function parameters are: "Inline", "ParamBlock" --- Rules/Strings.resx | 15 + Rules/UseConsistentParametersKind.cs | 171 +++++++ Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- .../UseConsistentParametersKind.Tests.ps1 | 428 ++++++++++++++++++ docs/Rules/README.md | 1 + docs/Rules/UseConsistentParametersKind.md | 57 +++ 6 files changed, 673 insertions(+), 1 deletion(-) create mode 100644 Rules/UseConsistentParametersKind.cs create mode 100644 Tests/Rules/UseConsistentParametersKind.Tests.ps1 create mode 100644 docs/Rules/UseConsistentParametersKind.md diff --git a/Rules/Strings.resx b/Rules/Strings.resx index c7645c9cf..f94c9cceb 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. + + + UseConsistentParametersKind + + + 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/UseConsistentParametersKind.cs b/Rules/UseConsistentParametersKind.cs new file mode 100644 index 000000000..fd2dfa732 --- /dev/null +++ b/Rules/UseConsistentParametersKind.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 +{ + /// + /// UseConsistentParametersKind: Checks if function parameters definition kind is same as preferred. + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class UseConsistentParametersKind : ConfigurableRule + { + private enum ParametersDefinitionKind + { + Inline, + ParamBlock + } + + private ParametersDefinitionKind parametersKind; + + /// + /// Construct an object of UseConsistentParametersKind type. + /// + public UseConsistentParametersKind() : 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.UseConsistentParametersKindInlineError, 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.UseConsistentParametersKindParamBlockError, functionAst.Name), + functionAst.Extent, + GetName(), + GetDiagnosticSeverity(), + fileName + ); + } + } + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentParametersKindCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentParametersKindDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseConsistentParametersKindName); + } + + /// + /// 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/UseConsistentParametersKind.Tests.ps1 b/Tests/Rules/UseConsistentParametersKind.Tests.ps1 new file mode 100644 index 000000000..1dfae19f2 --- /dev/null +++ b/Tests/Rules/UseConsistentParametersKind.Tests.ps1 @@ -0,0 +1,428 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + + +Describe 'UseConsistentParametersKind' { + Context 'When preferred parameters kind is set to "ParamBlock" explicitly' { + + BeforeAll { + $ruleConfiguration = @{ + Enable = $true + ParametersKind = "ParamBlock" + } + $settings = @{ + IncludeRules = @("PSUseConsistentParametersKind") + Rules = @{ + PSUseConsistentParametersKind = $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 = @("PSUseConsistentParametersKind") + Rules = @{ + PSUseConsistentParametersKind = $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 = @("PSUseConsistentParametersKind") + Rules = @{ + PSUseConsistentParametersKind = $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 = @("PSUseConsistentParametersKind") + Rules = @{ + PSUseConsistentParametersKind = $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 = @("PSUseConsistentParametersKind") + Rules = @{ + PSUseConsistentParametersKind = $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..51858d0de 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -68,6 +68,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [UseCompatibleSyntax](./UseCompatibleSyntax.md) | Warning | No | Yes | | [UseCompatibleTypes](./UseCompatibleTypes.md) | Warning | No | Yes | | [UseConsistentIndentation](./UseConsistentIndentation.md) | Warning | No | Yes | +| [UseConsistentParametersKind](./UseConsistentParametersKind.md) | Warning | No | Yes | | [UseConsistentWhitespace](./UseConsistentWhitespace.md) | Warning | No | Yes | | [UseCorrectCasing](./UseCorrectCasing.md) | Information | No | Yes | | [UseDeclaredVarsMoreThanAssignments](./UseDeclaredVarsMoreThanAssignments.md) | Warning | Yes | | diff --git a/docs/Rules/UseConsistentParametersKind.md b/docs/Rules/UseConsistentParametersKind.md new file mode 100644 index 000000000..04a323b3d --- /dev/null +++ b/docs/Rules/UseConsistentParametersKind.md @@ -0,0 +1,57 @@ +# UseConsistentParametersKind + +**Severity Level: Warning** + +## Description + +All functions should have same parameters definition kind specified in the rule. +Possible kinds are: +1. `Inline`, i.e.: +```PowerShell +function f([Parameter()]$FirstParam) { + return +} +``` +2. `ParamBlock`, i.e.: +```PowerShell +function f { + param([Parameter()]$FirstParam) + return +} +``` + +* For information: in simple scenarios both function definitions above may be considered as equal. Using this rule as-is is more for consistent code-style than functional, but it can be useful in combination with other rules. + +## How to Fix + +Rewrite function so it defines parameters as specified in the rule + +## Example + +### When the rule sets parameters definition kind to 'Inline': +```PowerShell +# Correct +function f([Parameter()]$FirstParam) { + return +} + +# Incorrect +function g { + param([Parameter()]$FirstParam) + return +} +``` + +### When the rule sets parameters definition kind to 'ParamBlock': +```PowerShell +# Inorrect +function f([Parameter()]$FirstParam) { + return +} + +# Correct +function g { + param([Parameter()]$FirstParam) + return +} +``` \ No newline at end of file