Skip to content

Commit a9898a6

Browse files
liamjpetersandyleejordanSeeminglyScience
authored
Add AvoidReservedWordsAsFunctionNames Rule (#2128)
* fix: Update helper GetScriptExtentForFunctionName to return correct extent when function name is 'function' * Add AvoidReservedWordsAsFunctionNames rule * fix: Handle functions with scopes * Copyright header * Extend Substring tests to starts with reserved word, ends with reserved word, and reserved word with a letter missing * Check if the function name is 'Function' case insensitively. Add a test for good measure * Fix casing of common name Co-authored-by: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> * AvoidReservedWordsAsFunctionNames: factor our function name for readability Co-authored-by: Patrick Meinecke <SeeminglyScience@users.noreply.github.com> * Remove reserved words which do not pose an issue: assembly, base, command, hidden, in, inlinescript, interface, module, namespace, private, public, static --------- Co-authored-by: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> Co-authored-by: Patrick Meinecke <SeeminglyScience@users.noreply.github.com>
1 parent dc55078 commit a9898a6

File tree

7 files changed

+315
-3
lines changed

7 files changed

+315
-3
lines changed

Engine/Helper.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -761,8 +761,15 @@ public IScriptExtent GetScriptExtentForFunctionName(FunctionDefinitionAst functi
761761
token =>
762762
ContainsExtent(functionDefinitionAst.Extent, token.Extent)
763763
&& token.Text.Equals(functionDefinitionAst.Name));
764-
var funcNameToken = funcNameTokens.FirstOrDefault();
765-
return funcNameToken == null ? null : funcNameToken.Extent;
764+
765+
// If the functions name is 'function' then the first token in the
766+
// list is the function keyword itself, so we need to skip it
767+
if (functionDefinitionAst.Name.Equals("function", StringComparison.OrdinalIgnoreCase))
768+
{
769+
var funcNameToken = funcNameTokens.Skip(1).FirstOrDefault() ?? funcNameTokens.FirstOrDefault();
770+
return funcNameToken?.Extent;
771+
}
772+
return funcNameTokens.FirstOrDefault()?.Extent;
766773
}
767774

768775
/// <summary>
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Globalization;
8+
using System.Management.Automation.Language;
9+
using System.Linq;
10+
11+
#if !CORECLR
12+
using System.ComponentModel.Composition;
13+
#endif
14+
15+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
16+
{
17+
#if !CORECLR
18+
[Export(typeof(IScriptRule))]
19+
#endif
20+
21+
/// <summary>
22+
/// Rule that warns when reserved words are used as function names
23+
/// </summary>
24+
public class AvoidReservedWordsAsFunctionNames : IScriptRule
25+
{
26+
27+
// The list of PowerShell reserved words.
28+
// https://learn.microsoft.com/en-gb/powershell/module/microsoft.powershell.core/about/about_reserved_words
29+
//
30+
// The Below are omitted as they don't pose an issue being a function
31+
// name:
32+
// assembly, base, command, hidden, in, inlinescript, interface, module,
33+
// namespace, private, public, static
34+
static readonly HashSet<string> reservedWords = new HashSet<string>(
35+
new[] {
36+
"begin", "break", "catch", "class", "configuration",
37+
"continue", "data", "define", "do",
38+
"dynamicparam", "else", "elseif", "end",
39+
"enum", "exit", "filter", "finally",
40+
"for", "foreach", "from", "function",
41+
"if", "parallel", "param", "process",
42+
"return", "sequence", "switch",
43+
"throw", "trap", "try", "type",
44+
"until", "using","var", "while", "workflow"
45+
},
46+
StringComparer.OrdinalIgnoreCase
47+
);
48+
49+
/// <summary>
50+
/// Analyzes the PowerShell AST for uses of reserved words as function names.
51+
/// </summary>
52+
/// <param name="ast">The PowerShell Abstract Syntax Tree to analyze.</param>
53+
/// <param name="fileName">The name of the file being analyzed (for diagnostic reporting).</param>
54+
/// <returns>A collection of diagnostic records for each violation.</returns>
55+
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
56+
{
57+
if (ast == null)
58+
{
59+
throw new ArgumentNullException(Strings.NullAstErrorMessage);
60+
}
61+
62+
// Find all FunctionDefinitionAst in the Ast
63+
var functionDefinitions = ast.FindAll(
64+
astNode => astNode is FunctionDefinitionAst,
65+
true
66+
).Cast<FunctionDefinitionAst>();
67+
68+
foreach (var function in functionDefinitions)
69+
{
70+
string functionName = Helper.Instance.FunctionNameWithoutScope(function.Name);
71+
if (reservedWords.Contains(functionName))
72+
{
73+
yield return new DiagnosticRecord(
74+
string.Format(
75+
CultureInfo.CurrentCulture,
76+
Strings.AvoidReservedWordsAsFunctionNamesError,
77+
functionName),
78+
Helper.Instance.GetScriptExtentForFunctionName(function) ?? function.Extent,
79+
GetName(),
80+
DiagnosticSeverity.Warning,
81+
fileName
82+
);
83+
}
84+
}
85+
}
86+
87+
public string GetCommonName() => Strings.AvoidReservedWordsAsFunctionNamesCommonName;
88+
89+
public string GetDescription() => Strings.AvoidReservedWordsAsFunctionNamesDescription;
90+
91+
public string GetName() => string.Format(
92+
CultureInfo.CurrentCulture,
93+
Strings.NameSpaceFormat,
94+
GetSourceName(),
95+
Strings.AvoidReservedWordsAsFunctionNamesName);
96+
97+
public RuleSeverity GetSeverity() => RuleSeverity.Warning;
98+
99+
public string GetSourceName() => Strings.SourceName;
100+
101+
public SourceType GetSourceType() => SourceType.Builtin;
102+
}
103+
}

Rules/Strings.resx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,4 +1224,16 @@
12241224
<data name="AvoidUsingAllowUnencryptedAuthenticationName" xml:space="preserve">
12251225
<value>AvoidUsingAllowUnencryptedAuthentication</value>
12261226
</data>
1227+
<data name="AvoidReservedWordsAsFunctionNamesCommonName" xml:space="preserve">
1228+
<value>Avoid reserved words as function names</value>
1229+
</data>
1230+
<data name="AvoidReservedWordsAsFunctionNamesDescription" xml:space="preserve">
1231+
<value>Avoid using reserved words as function names. Using reserved words as function names can cause errors or unexpected behavior in scripts.</value>
1232+
</data>
1233+
<data name="AvoidReservedWordsAsFunctionNamesName" xml:space="preserve">
1234+
<value>AvoidReservedWordsAsFunctionNames</value>
1235+
</data>
1236+
<data name="AvoidReservedWordsAsFunctionNamesError" xml:space="preserve">
1237+
<value>The reserved word '{0}' was used as a function name. This should be avoided.</value>
1238+
</data>
12271239
</root>

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ Describe "Test Name parameters" {
6363

6464
It "get Rules with no parameters supplied" {
6565
$defaultRules = Get-ScriptAnalyzerRule
66-
$expectedNumRules = 70
66+
$expectedNumRules = 71
6767
if ($PSVersionTable.PSVersion.Major -le 4)
6868
{
6969
# for PSv3 PSAvoidGlobalAliases is not shipped because
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
# Keep in sync with the rule's reserved words list in
5+
# Rules/AvoidReservedWordsAsFunctionNames.cs
6+
$reservedWords = @(
7+
'begin','break','catch','class','configuration',
8+
'continue','data','define','do',
9+
'dynamicparam','else','elseif','end',
10+
'enum','exit','filter','finally',
11+
'for','foreach','from','function',
12+
'if','parallel','param','process',
13+
'return','sequence','switch',
14+
'throw','trap','try','type',
15+
'until','using','var','while','workflow'
16+
)
17+
18+
$randomCasedReservedWords = @(
19+
'bEgIN','bReAk','cAtCh','CLasS','cONfiGuRaTioN',
20+
'cONtiNuE','dAtA','dEFInE','Do',
21+
'DyNaMiCpArAm','eLsE','eLsEiF','EnD',
22+
'EnUm','eXiT','fIlTeR','fINaLLy',
23+
'FoR','fOrEaCh','fROm','fUnCtIoN',
24+
'iF','pArAlLeL','PaRaM','pRoCeSs',
25+
'ReTuRn','sEqUeNcE','SwItCh',
26+
'tHrOw','TrAp','tRy','TyPe',
27+
'uNtIl','UsInG','VaR','wHiLe','wOrKfLoW'
28+
)
29+
30+
$functionScopes = @(
31+
"global", "local", "script", "private"
32+
)
33+
34+
# Generate all combinations of reserved words and function scopes
35+
$scopedReservedWordCases = foreach ($scope in $functionScopes) {
36+
foreach ($word in $reservedWords) {
37+
@{
38+
Scope = $scope
39+
Name = $word
40+
}
41+
}
42+
}
43+
44+
# Build variants of reserved words where the reserverd word:
45+
# appearing at the start and end of a function
46+
# name.
47+
$substringReservedWords = $reservedWords | ForEach-Object {
48+
"$($_)A", "A$($_)", $_.Substring(0, $_.Length - 1)
49+
}
50+
51+
$safeFunctionNames = @(
52+
'Get-Something','Do-Work','Classify-Data','Begin-Process'
53+
)
54+
55+
BeforeAll {
56+
$ruleName = 'PSAvoidReservedWordsAsFunctionNames'
57+
}
58+
59+
Describe 'AvoidReservedWordsAsFunctionNames' {
60+
Context 'When function names are reserved words' {
61+
It 'flags reserved word "<_>" as a violation' -TestCases $reservedWords {
62+
63+
$scriptDefinition = "function $_ { 'test' }"
64+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
65+
66+
$violations.Count | Should -Be 1
67+
$violations[0].Severity | Should -Be 'Warning'
68+
$violations[0].RuleName | Should -Be $ruleName
69+
# Message text should include the function name as used
70+
$violations[0].Message | Should -Be "The reserved word '$_' was used as a function name. This should be avoided."
71+
# Extent should ideally capture only the function name
72+
$violations[0].Extent.Text | Should -Be $_
73+
}
74+
75+
It 'flags the correct extent for a function named Function' {
76+
77+
$scriptDefinition = "Function Function { 'test' }"
78+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
79+
80+
$violations.Count | Should -Be 1
81+
$violations[0].Severity | Should -Be 'Warning'
82+
$violations[0].RuleName | Should -Be $ruleName
83+
# Message text should include the function name as used
84+
$violations[0].Message | Should -Be "The reserved word 'Function' was used as a function name. This should be avoided."
85+
# Extent should ideally capture only the function name
86+
$violations[0].Extent.Text | Should -Be 'Function'
87+
88+
# Make sure the extent is the correct `Function` (not the one at the
89+
# very start)
90+
$violations[0].Extent.StartOffset | Should -not -Be 0
91+
}
92+
93+
# Functions can have scopes. So function global:function {} should still
94+
# alert.
95+
It 'flags reserved word "<Name>" with scope "<Scope>" as a violation' -TestCases $scopedReservedWordCases {
96+
param($Scope, $Name)
97+
98+
$scriptDefinition = "function $($Scope):$($Name) { 'test' }"
99+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
100+
101+
$violations.Count | Should -Be 1
102+
$violations[0].Severity | Should -Be 'Warning'
103+
$violations[0].RuleName | Should -Be $ruleName
104+
$violations[0].Message | Should -Be "The reserved word '$Name' was used as a function name. This should be avoided."
105+
$violations[0].Extent.Text | Should -Be "$($Scope):$($Name)"
106+
}
107+
108+
109+
It 'detects case-insensitively for "<_>"' -TestCases $randomCasedReservedWords {
110+
$scriptDefinition = "function $_ { }"
111+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
112+
$violations.Count | Should -Be 1
113+
$violations[0].Message | Should -Be "The reserved word '$_' was used as a function name. This should be avoided."
114+
}
115+
116+
It 'reports one finding per offending function' {
117+
$scriptDefinition = 'function class { };function For { };function Safe-Name { };function TRy { }'
118+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
119+
120+
$violations.Count | Should -Be 3
121+
$violations | ForEach-Object { $_.Severity | Should -Be 'Warning' }
122+
($violations | Select-Object -ExpandProperty Extent | Select-Object -ExpandProperty Text) |
123+
Sort-Object |
124+
Should -Be @('class','For','TRy')
125+
}
126+
}
127+
128+
Context 'When there are no violations' {
129+
It 'does not flag safe function name "<_>"' -TestCases $safeFunctionNames {
130+
$scriptDefinition = "function $_ { }"
131+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
132+
$violations.Count | Should -Be 0
133+
}
134+
135+
It 'does not flag when script has no functions' {
136+
$scriptDefinition = '"hello";$x = 1..3 | ForEach-Object { $_ }'
137+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
138+
$violations.Count | Should -Be 0
139+
}
140+
141+
It 'does not flag substring-like name "<_>"' -TestCases $substringReservedWords {
142+
$scriptDefinition = "function $_ { }"
143+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
144+
$violations.Count | Should -Be 0
145+
}
146+
}
147+
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
description: Avoid reserved words as function names
3+
ms.date: 08/31/2025
4+
ms.topic: reference
5+
title: AvoidReservedWordsAsFunctionNames
6+
---
7+
# AvoidReservedWordsAsFunctionNames
8+
9+
**Severity Level: Warning**
10+
11+
## Description
12+
13+
Avoid using reserved words as function names. Using reserved words as function
14+
names can cause errors or unexpected behavior in scripts.
15+
16+
## How to Fix
17+
18+
Avoid using any of the reserved words as function names. Instead, choose a
19+
different name that is not reserved.
20+
21+
See [`about_Reserved_Words`](https://learn.microsoft.com/en-gb/powershell/module/microsoft.powershell.core/about/about_reserved_words) for a list of reserved
22+
words in PowerShell.
23+
24+
## Example
25+
26+
### Wrong
27+
28+
```powershell
29+
# Function is a reserved word
30+
function function {
31+
Write-Host "Hello, World!"
32+
}
33+
```
34+
35+
### Correct
36+
37+
```powershell
38+
# myFunction is not a reserved word
39+
function myFunction {
40+
Write-Host "Hello, World!"
41+
}
42+
```

docs/Rules/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ The PSScriptAnalyzer contains the following rule definitions.
2323
| [AvoidMultipleTypeAttributes<sup>1</sup>](./AvoidMultipleTypeAttributes.md) | Warning | Yes | |
2424
| [AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | Yes | |
2525
| [AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | Yes | Yes |
26+
| [AvoidReservedWordsAsFunctionNames](./AvoidReservedWordsAsFunctionNames.md) | Warning | Yes | |
2627
| [AvoidSemicolonsAsLineTerminators](./AvoidSemicolonsAsLineTerminators.md) | Warning | No | |
2728
| [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | Yes | |
2829
| [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md) | Warning | Yes | |

0 commit comments

Comments
 (0)