From 6fb3193c945a2b5f121657c4cf64f48be2b07c9a Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Mon, 1 Sep 2025 14:29:25 +0100 Subject: [PATCH 1/4] fix: Update helper GetScriptExtentForFunctionName to return correct extent when function name is 'function' --- Engine/Helper.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 82948a4fc..7312c685c 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -761,8 +761,15 @@ public IScriptExtent GetScriptExtentForFunctionName(FunctionDefinitionAst functi token => ContainsExtent(functionDefinitionAst.Extent, token.Extent) && token.Text.Equals(functionDefinitionAst.Name)); - var funcNameToken = funcNameTokens.FirstOrDefault(); - return funcNameToken == null ? null : funcNameToken.Extent; + + // If the functions name is 'function' then the first token in the + // list is the function keyword itself, so we need to skip it + if (functionDefinitionAst.Name.Equals("function")) + { + var funcNameToken = funcNameTokens.Skip(1).FirstOrDefault() ?? funcNameTokens.FirstOrDefault(); + return funcNameToken?.Extent; + } + return funcNameTokens.FirstOrDefault()?.Extent; } /// From 382278a0cca6dc3d792843c9ef508dc91e621217 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Mon, 1 Sep 2025 15:16:32 +0100 Subject: [PATCH 2/4] Add AvoidReservedWordsAsFunctionNames rule --- Rules/AvoidReservedWordsAsFunctionNames.cs | 100 ++++++++++++++++++ Rules/Strings.resx | 12 +++ Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- ...voidReservedWordsAsFunctionNames.tests.ps1 | 99 +++++++++++++++++ .../AvoidReservedWordsAsFunctionNames.md | 42 ++++++++ docs/Rules/README.md | 1 + 6 files changed, 255 insertions(+), 1 deletion(-) create mode 100644 Rules/AvoidReservedWordsAsFunctionNames.cs create mode 100644 Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 create mode 100644 docs/Rules/AvoidReservedWordsAsFunctionNames.md diff --git a/Rules/AvoidReservedWordsAsFunctionNames.cs b/Rules/AvoidReservedWordsAsFunctionNames.cs new file mode 100644 index 000000000..399901e11 --- /dev/null +++ b/Rules/AvoidReservedWordsAsFunctionNames.cs @@ -0,0 +1,100 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Management.Automation.Language; +using System.Linq; + +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + + /// + /// Rule that warns when reserved words are used as function names + /// + public class AvoidReservedWordsAsFunctionNames : IScriptRule + { + + // The list of PowerShell reserved words. + // https://learn.microsoft.com/en-gb/powershell/module/microsoft.powershell.core/about/about_reserved_words + static readonly HashSet reservedWords = new HashSet( + new[] { + "assembly", "base", "begin", "break", + "catch", "class", "command", "configuration", + "continue", "data", "define", "do", + "dynamicparam", "else", "elseif", "end", + "enum", "exit", "filter", "finally", + "for", "foreach", "from", "function", + "hidden", "if", "in", "inlinescript", + "interface", "module", "namespace", "parallel", + "param", "private", "process", "public", + "return", "sequence", "static", "switch", + "throw", "trap", "try", "type", + "until", "using","var", "while", "workflow" + }, + StringComparer.OrdinalIgnoreCase + ); + + /// + /// Analyzes the PowerShell AST for uses of reserved words as function names. + /// + /// The PowerShell Abstract Syntax Tree to analyze. + /// The name of the file being analyzed (for diagnostic reporting). + /// A collection of diagnostic records for each violation. + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) + { + throw new ArgumentNullException(Strings.NullAstErrorMessage); + } + + // Find all FunctionDefinitionAst in the Ast + var functionDefinitions = ast.FindAll( + astNode => astNode is FunctionDefinitionAst, + true + ).Cast(); + + foreach (var function in functionDefinitions) + { + if (reservedWords.Contains(function.Name)) + { + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.AvoidReservedWordsAsFunctionNamesError, + function.Name), + Helper.Instance.GetScriptExtentForFunctionName(function) ?? function.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName + ); + } + } + } + + public string GetCommonName() => Strings.AvoidReservedWordsAsFunctionNamesCommonName; + + public string GetDescription() => Strings.AvoidReservedWordsAsFunctionNamesDescription; + + public string GetName() => string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidReservedWordsAsFunctionNamesName); + + public RuleSeverity GetSeverity() => RuleSeverity.Warning; + + public string GetSourceName() => Strings.SourceName; + + public SourceType GetSourceType() => SourceType.Builtin; + } +} \ No newline at end of file diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 260214967..ddb81be0f 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1224,4 +1224,16 @@ AvoidUsingAllowUnencryptedAuthentication + + Avoid Reserved Words as function names + + + Avoid using reserved words as function names. Using reserved words as function names can cause errors or unexpected behavior in scripts. + + + AvoidReservedWordsAsFunctionNames + + + The reserved word '{0}' was used as a function name. This should be avoided. + \ No newline at end of file diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 8d61c1c7f..c3b744803 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 = 70 + $expectedNumRules = 71 if ($PSVersionTable.PSVersion.Major -le 4) { # for PSv3 PSAvoidGlobalAliases is not shipped because diff --git a/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 b/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 new file mode 100644 index 000000000..550405031 --- /dev/null +++ b/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 @@ -0,0 +1,99 @@ +# Keep in sync with the rule's reserved words list in +# Rules/AvoidReservedWordsAsFunctionNames.cs +$reservedWords = @( + 'assembly','base','begin','break', + 'catch','class','command','configuration', + 'continue','data','define','do', + 'dynamicparam','else','elseif','end', + 'enum','exit','filter','finally', + 'for','foreach','from','function', + 'hidden','if','in','inlinescript', + 'interface','module','namespace','parallel', + 'param','private','process','public', + 'return','sequence','static','switch', + 'throw','trap','try','type', + 'until','using','var','while','workflow' +) + +$randomCasedReservedWords = @( + 'aSSeMbLy','bASe','bEgIN','bReAk', + 'cAtCh','CLasS','cOMmAnD','cONfiGuRaTioN', + 'cONtiNuE','dAtA','dEFInE','Do', + 'DyNaMiCpArAm','eLsE','eLsEiF','EnD', + 'EnUm','eXiT','fIlTeR','fINaLLy', + 'FoR','fOrEaCh','fROm','fUnCtIoN', + 'hIdDeN','iF','IN','iNlInEsCrIpT', + 'InTeRfAcE','mOdUlE','nAmEsPaCe','pArAlLeL', + 'PaRaM','pRiVaTe','pRoCeSs','pUbLiC', + 'ReTuRn','sEqUeNcE','StAtIc','SwItCh', + 'tHrOw','TrAp','tRy','TyPe', + 'uNtIl','UsInG','VaR','wHiLe','wOrKfLoW' +) + +$substringReservedWords = $reservedWords | ForEach-Object { + "$($_)Func" +} + +$safeFunctionNames = @( + 'Get-Something','Do-Work','Classify-Data','Begin-Process' +) + +BeforeAll { + $ruleName = 'PSAvoidReservedWordsAsFunctionNames' +} + +Describe 'AvoidReservedWordsAsFunctionNames' { + Context 'When function names are reserved words' { + It 'flags reserved word "<_>" as a violation' -TestCases $reservedWords { + + $scriptDefinition = "function $_ { 'test' }" + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + + $violations.Count | Should -Be 1 + $violations[0].Severity | Should -Be 'Warning' + $violations[0].RuleName | Should -Be $ruleName + # Message text should include the function name as used + $violations[0].Message | Should -Be "The reserved word '$_' was used as a function name. This should be avoided." + # Extent should ideally capture only the function name + $violations[0].Extent.Text | Should -Be $_ + } + + It 'detects case-insensitively for "<_>"' -TestCases $randomCasedReservedWords { + $scriptDefinition = "function $_ { }" + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations[0].Message | Should -Be "The reserved word '$_' was used as a function name. This should be avoided." + } + + It 'reports one finding per offending function' { + $scriptDefinition = 'function class { };function For { };function Safe-Name { };function TRy { }' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + + $violations.Count | Should -Be 3 + $violations | ForEach-Object { $_.Severity | Should -Be 'Warning' } + ($violations | Select-Object -ExpandProperty Extent | Select-Object -ExpandProperty Text) | + Sort-Object | + Should -Be @('class','For','TRy') + } + } + + Context 'When there are no violations' { + It 'does not flag safe function name "<_>"' -TestCases $safeFunctionNames { + $scriptDefinition = "function $_ { }" + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + + It 'does not flag when script has no functions' { + $scriptDefinition = '"hello";$x = 1..3 | ForEach-Object { $_ }' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + + It 'does not flag substring-like name "<_>"' -TestCases $substringReservedWords { + $scriptDefinition = "function $_ { }" + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + } +} diff --git a/docs/Rules/AvoidReservedWordsAsFunctionNames.md b/docs/Rules/AvoidReservedWordsAsFunctionNames.md new file mode 100644 index 000000000..cfe6b92ca --- /dev/null +++ b/docs/Rules/AvoidReservedWordsAsFunctionNames.md @@ -0,0 +1,42 @@ +--- +description: Avoid reserved words as function names +ms.date: 08/31/2025 +ms.topic: reference +title: AvoidReservedWordsAsFunctionNames +--- +# AvoidReservedWordsAsFunctionNames + +**Severity Level: Warning** + +## Description + +Avoid using reserved words as function names. Using reserved words as function +names can cause errors or unexpected behavior in scripts. + +## How to Fix + +Avoid using any of the reserved words as function names. Instead, choose a +different name that is not reserved. + +See [`about_Reserved_Words`](https://learn.microsoft.com/en-gb/powershell/module/microsoft.powershell.core/about/about_reserved_words) for a list of reserved +words in PowerShell. + +## Example + +### Wrong + +```powershell +# Function is a reserved word +function function { + Write-Host "Hello, World!" +} +``` + +### Correct + +```powershell +# myFunction is not a reserved word +function myFunction { + Write-Host "Hello, World!" +} +``` \ No newline at end of file diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 06f27d2da..da1058bc2 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -23,6 +23,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [AvoidMultipleTypeAttributes1](./AvoidMultipleTypeAttributes.md) | Warning | Yes | | | [AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | Yes | | | [AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | Yes | Yes | +| [AvoidReservedWordsAsFunctionNames](./AvoidReservedWordsAsFunctionNames.md) | Warning | Yes | | | [AvoidSemicolonsAsLineTerminators](./AvoidSemicolonsAsLineTerminators.md) | Warning | No | | | [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | Yes | | | [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md) | Warning | Yes | | From 59b69c8a379b0cf5e857620fe9e51fb93dcbbd52 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Mon, 1 Sep 2025 17:49:19 +0100 Subject: [PATCH 3/4] fix: Handle functions with scopes --- Rules/AvoidReservedWordsAsFunctionNames.cs | 6 ++-- ...voidReservedWordsAsFunctionNames.tests.ps1 | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/Rules/AvoidReservedWordsAsFunctionNames.cs b/Rules/AvoidReservedWordsAsFunctionNames.cs index 399901e11..f5b526099 100644 --- a/Rules/AvoidReservedWordsAsFunctionNames.cs +++ b/Rules/AvoidReservedWordsAsFunctionNames.cs @@ -65,13 +65,15 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) foreach (var function in functionDefinitions) { - if (reservedWords.Contains(function.Name)) + if (reservedWords.Contains( + Helper.Instance.FunctionNameWithoutScope(function.Name) + )) { yield return new DiagnosticRecord( string.Format( CultureInfo.CurrentCulture, Strings.AvoidReservedWordsAsFunctionNamesError, - function.Name), + Helper.Instance.FunctionNameWithoutScope(function.Name)), Helper.Instance.GetScriptExtentForFunctionName(function) ?? function.Extent, GetName(), DiagnosticSeverity.Warning, diff --git a/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 b/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 index 550405031..6484d0232 100644 --- a/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 +++ b/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 @@ -30,6 +30,20 @@ $randomCasedReservedWords = @( 'uNtIl','UsInG','VaR','wHiLe','wOrKfLoW' ) +$functionScopes = @( + "global", "local", "script", "private" +) + +# Generate all combinations of reserved words and function scopes +$scopedReservedWordCases = foreach ($scope in $functionScopes) { + foreach ($word in $reservedWords) { + @{ + Scope = $scope + Name = $word + } + } +} + $substringReservedWords = $reservedWords | ForEach-Object { "$($_)Func" } @@ -58,6 +72,22 @@ Describe 'AvoidReservedWordsAsFunctionNames' { $violations[0].Extent.Text | Should -Be $_ } + # Functions can have scopes. So function global:function {} should still + # alert. + It 'flags reserved word "" with scope "" as a violation' -TestCases $scopedReservedWordCases { + param($Scope, $Name) + + $scriptDefinition = "function $($Scope):$($Name) { 'test' }" + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + + $violations.Count | Should -Be 1 + $violations[0].Severity | Should -Be 'Warning' + $violations[0].RuleName | Should -Be $ruleName + $violations[0].Message | Should -Be "The reserved word '$Name' was used as a function name. This should be avoided." + $violations[0].Extent.Text | Should -Be "$($Scope):$($Name)" + } + + It 'detects case-insensitively for "<_>"' -TestCases $randomCasedReservedWords { $scriptDefinition = "function $_ { }" $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) From 6672621d5bbb445655523fbe16cd886f9536c180 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Wed, 3 Sep 2025 21:06:47 +0100 Subject: [PATCH 4/4] Copyright header --- Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 b/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 index 6484d0232..9f34e09f7 100644 --- a/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 +++ b/Tests/Rules/AvoidReservedWordsAsFunctionNames.tests.ps1 @@ -1,3 +1,6 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + # Keep in sync with the rule's reserved words list in # Rules/AvoidReservedWordsAsFunctionNames.cs $reservedWords = @(