Skip to content

Commit 25dcf51

Browse files
David ReynoldsDavid Reynolds
David Reynolds
authored and
David Reynolds
committed
Added new rule AvoidGlobalFunctions
New rule AvoidGlobalFunctions checks that global functions and aliases are not used.
1 parent 67a6c4e commit 25dcf51

9 files changed

+352
-3
lines changed

‎RuleDocumentation/AvoidGlobalFunctions.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#AvoidGlobalFunctions
2+
**Severity Level: Warning**
3+
4+
##Description
5+
Globally scoped functions and alias override existing functions and aliases within the sessions with matching names. This name collision can cause difficult to debug issues for consumers of modules and scripts.
6+
7+
8+
To understand more about scoping, see ```Get-Help about_Scopes```.
9+
10+
##How to Fix
11+
Use other scope modifiers for functions and aliases.
12+
13+
##Example
14+
###Wrong:
15+
``` PowerShell
16+
function global:functionName {}
17+
18+
New-Alias -Name CommandName -Value NewCommandAlias -scope:global
19+
```
20+
21+
###Correct:
22+
``` PowerShell
23+
function functionName {}
24+
25+
New-Alias -Name CommandName -Value NewCommandAlias
26+
```

‎Rules/AvoidGlobalFunctions.cs‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
2+
using System;
3+
using System.Collections.Generic;
4+
using System.Linq;
5+
using System.Text;
6+
using System.Threading.Tasks;
7+
using System.Management.Automation.Language;
8+
using System.Globalization;
9+
using System.ComponentModel.Composition;
10+
11+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
12+
{
13+
[Export(typeof(IScriptRule))]
14+
public class AvoidGlobalFunctions : AstVisitor, IScriptRule
15+
{
16+
List<DiagnosticRecord> records;
17+
string fileName;
18+
19+
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
20+
{
21+
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
22+
23+
records = new List<DiagnosticRecord>();
24+
this.fileName = fileName;
25+
26+
ast.Visit(this);
27+
28+
return records;
29+
}
30+
31+
#region VisitComman functions
32+
public override AstVisitAction VisitCommand(CommandAst commandAst)
33+
{
34+
if (!IsNewAliasCmdlet(commandAst))
35+
{
36+
return AstVisitAction.SkipChildren;
37+
}
38+
39+
return AstVisitAction.Continue;
40+
}
41+
42+
public override AstVisitAction VisitCommandParameter(CommandParameterAst commandParameterAst)
43+
{
44+
if(isScopeParameterForNewAliasCmdlet(commandParameterAst))
45+
{
46+
if ((commandParameterAst.Argument != null) // if the cmdlet looks like -Scope:Global check Parameter.Argument
47+
&& (commandParameterAst.Argument.ToString().Equals("Global", StringComparison.OrdinalIgnoreCase)))
48+
{
49+
records.Add(new DiagnosticRecord(
50+
string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalFunctionsAliasError),
51+
commandParameterAst.Extent,
52+
GetName(),
53+
DiagnosticSeverity.Warning,
54+
fileName,
55+
commandParameterAst.ParameterName));
56+
}
57+
else
58+
{
59+
var nextAst = FindNextAst(commandParameterAst);
60+
61+
if((nextAst is StringConstantExpressionAst) // if the cmdlet looks like -Scope Global
62+
&& (((StringConstantExpressionAst)nextAst).Value.ToString().Equals("Global", StringComparison.OrdinalIgnoreCase)))
63+
{
64+
records.Add(new DiagnosticRecord(
65+
string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalFunctionsAliasError),
66+
((StringConstantExpressionAst)nextAst).Extent,
67+
GetName(),
68+
DiagnosticSeverity.Warning,
69+
fileName,
70+
((StringConstantExpressionAst)nextAst).Value));
71+
}
72+
}
73+
}
74+
75+
return AstVisitAction.SkipChildren;
76+
}
77+
78+
public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst)
79+
{
80+
if (functionDefinitionAst.Name.StartsWith("Global:", StringComparison.OrdinalIgnoreCase))
81+
{
82+
records.Add(new DiagnosticRecord(
83+
string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalFunctionsError),
84+
functionDefinitionAst.Extent,
85+
GetName(),
86+
DiagnosticSeverity.Warning,
87+
fileName,
88+
functionDefinitionAst.Name));
89+
}
90+
91+
return AstVisitAction.Continue;
92+
}
93+
#endregion
94+
95+
private Ast FindNextAst(Ast ast)
96+
{
97+
IEnumerable<Ast> matchingLevelAsts = ast.Parent.FindAll(item => item is Ast, true );
98+
99+
Ast currentClosest = null;
100+
foreach(var matchingLevelAst in matchingLevelAsts)
101+
{
102+
if (currentClosest == null)
103+
{
104+
if (isAstAfter(ast, matchingLevelAst))
105+
{
106+
currentClosest = matchingLevelAst;
107+
}
108+
}
109+
else
110+
{
111+
if((isAstAfter(ast, matchingLevelAst)) && (isAstAfter(matchingLevelAst, currentClosest)))
112+
{
113+
currentClosest = matchingLevelAst;
114+
}
115+
}
116+
}
117+
118+
return currentClosest;
119+
}
120+
121+
private bool isAstAfter(Ast ast1, Ast ast2)
122+
{
123+
if(ast1.Extent.EndLineNumber > ast2.Extent.StartLineNumber) // ast1 ends on a line after ast2 starts
124+
{
125+
return false;
126+
}
127+
else if(ast1.Extent.EndLineNumber == ast2.Extent.StartLineNumber)
128+
{
129+
if(ast2.Extent.StartColumnNumber > ast1.Extent.EndColumnNumber)
130+
{
131+
return true;
132+
}
133+
else
134+
{
135+
return false;
136+
}
137+
}
138+
else // ast2 starts on a line after ast 1 ends
139+
{
140+
return true;
141+
}
142+
}
143+
144+
private bool isScopeParameterForNewAliasCmdlet(CommandParameterAst commandParameterAst)
145+
{
146+
if (commandParameterAst == null || commandParameterAst.ParameterName == null)
147+
return false;
148+
149+
if(commandParameterAst.ParameterName.Equals("Scope", StringComparison.OrdinalIgnoreCase)
150+
&& (commandParameterAst.Parent is CommandAst)
151+
&& IsNewAliasCmdlet((CommandAst)commandParameterAst.Parent))
152+
{
153+
return true;
154+
}
155+
156+
return false;
157+
}
158+
159+
private bool IsNewAliasCmdlet(CommandAst commandAst)
160+
{
161+
if (commandAst == null || commandAst.GetCommandName() == null)
162+
return false;
163+
164+
var AliasList = Helper.Instance.CmdletNameAndAliases("New-Alias");
165+
if (AliasList.Contains(commandAst.GetCommandName()))
166+
{
167+
return true;
168+
}
169+
170+
return false;
171+
}
172+
173+
public string GetCommonName()
174+
{
175+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalFunctionsCommonName);
176+
}
177+
178+
public string GetDescription()
179+
{
180+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalFunctionsDescription);
181+
}
182+
183+
public string GetName()
184+
{
185+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalFunctionsName);
186+
}
187+
188+
public RuleSeverity GetSeverity()
189+
{
190+
return RuleSeverity.Warning;
191+
}
192+
193+
public string GetSourceName()
194+
{
195+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
196+
}
197+
198+
public SourceType GetSourceType()
199+
{
200+
return SourceType.Builtin;
201+
}
202+
}
203+
}

‎Rules/ScriptAnalyzerBuiltinRules.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
<Compile Include="AvoidAlias.cs" />
7171
<Compile Include="AvoidDefaultTrueValueSwitchParameter.cs" />
7272
<Compile Include="AvoidEmptyCatchBlock.cs" />
73+
<Compile Include="AvoidGlobalFunctions.cs" />
7374
<Compile Include="AvoidGlobalVars.cs" />
7475
<Compile Include="AvoidInvokingEmptyMembers.cs" />
7576
<Compile Include="AvoidNullOrEmptyHelpMessageAttribute.cs" />
@@ -95,6 +96,7 @@
9596
<DependentUpon>Strings.resx</DependentUpon>
9697
</Compile>
9798
<Compile Include="UseBOMForUnicodeEncodedFile.cs" />
99+
<Compile Include="UseCompatibleCmdlets.cs" />
98100
<Compile Include="UseLiteralInitializerForHashtable.cs" />
99101
<Compile Include="UseToExportFieldsInManifest.cs" />
100102
<Compile Include="UseOutputTypeCorrectly.cs" />
@@ -114,7 +116,6 @@
114116
<Compile Include="UseStandardDSCFunctionsInResource.cs" />
115117
<Compile Include="UseUTF8EncodingForHelpFile.cs" />
116118
<Compile Include="ReturnCorrectTypesForDSCFunctions.cs" />
117-
<Compile Include="UseCompatibleCmdlets.cs" />
118119
</ItemGroup>
119120
<ItemGroup>
120121
<ProjectReference Include="..\Engine\ScriptAnalyzerEngine.csproj">

‎Rules/Strings.Designer.cs‎

Lines changed: 45 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rules/Strings.resx‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,4 +846,19 @@
846846
<data name="UseCompatibleCmdletsError" xml:space="preserve">
847847
<value>'{0}' is not compatible with PowerShell edition '{1}', version '{2}' and OS '{3}'</value>
848848
</data>
849+
<data name="AvoidGlobalFunctionsAliasError" xml:space="preserve">
850+
<value>Avoid creating aliases with a Global scope.</value>
851+
</data>
852+
<data name="AvoidGlobalFunctionsCommonName" xml:space="preserve">
853+
<value>Avoid global functiosn and aliases</value>
854+
</data>
855+
<data name="AvoidGlobalFunctionsDescription" xml:space="preserve">
856+
<value>Checks that global functions and aliases are not used. Global functions are strongly discouraged as they can cause errors across different systems.</value>
857+
</data>
858+
<data name="AvoidGlobalFunctionsError" xml:space="preserve">
859+
<value>Avoid creating functions with a Global scope.</value>
860+
</data>
861+
<data name="AvoidGlobalFunctionsName" xml:space="preserve">
862+
<value>AvoidGlobalFunctions</value>
863+
</data>
849864
</root>

‎Tests/Engine/GetScriptAnalyzerRule.tests.ps1‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ Describe "Test Name parameters" {
6161

6262
It "get Rules with no parameters supplied" {
6363
$defaultRules = Get-ScriptAnalyzerRule
64-
$expectedNumRules = 43
64+
$expectedNumRules = 44
6565
if ((Test-PSEditionCoreClr))
6666
{
67-
$expectedNumRules = 42
67+
$expectedNumRules = 43
6868
}
6969
$defaultRules.Count | Should be $expectedNumRules
7070
}

‎Tests/Rules/AvoidGlobalFunctions.ps1‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
function global:functionName {}
3+
{
4+
New-Alias -Name Name -Scope `
5+
Global -Value Value
6+
7+
nal -Name Name -Value Value -Scope "Global"
8+
}
9+
10+
11+
New-Alias -Name Name -scope:global -Value Value
12+
nal -Name Name -scope:global -Value Value
13+
14+
Get-Content -Narrly "hi" -Scope "Global"

‎Tests/Rules/AvoidGlobalFunctions.tests.ps1‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
Import-Module PSScriptAnalyzer
2+
3+
$functionErroMessage = "Avoid creating functions with a Global scope."
4+
$aliasErrorMessage = "Avoid creating aliases with a Global scope."
5+
$violationName = "AvoidGlobalFunctions"
6+
7+
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
8+
$violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalFunctions.ps1 | Where-Object {$_.RuleName -eq $violationName}
9+
$noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalFunctionsNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
10+
11+
12+
Describe "$violationName " {
13+
Context "When there are violations" {
14+
It "has 5 avoid using empty Catch block violations" {
15+
$violations.Count | Should Be 5
16+
}
17+
18+
It "has the correct description message" {
19+
$violations[0].Message | Should Match $functionErroMessage
20+
$violations[1].Message | Should Match $aliasErrorMessage
21+
$violations[2].Message | Should Match $aliasErrorMessage
22+
$violations[3].Message | Should Match $aliasErrorMessage
23+
$violations[4].Message | Should Match $aliasErrorMessage
24+
}
25+
26+
}
27+
28+
Context "When there are no violations" {
29+
It "returns no violations" {
30+
$noViolations.Count | Should Be 0
31+
}
32+
}
33+
}

0 commit comments

Comments
 (0)