Skip to content

Commit 6973dfb

Browse files
bergmeisterbergmeister
authored
Merge branch 'main' into avoid-new-object
2 parents 26c779c + a7b06bb commit 6973dfb

14 files changed

+563
-80
lines changed

‎.github/CODEOWNERS‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Default owners
2-
* @andyleejordan @bergmeister
2+
* @PowerShell/extension @bergmeister
33

44
# Version bumps and documentation updates
55
Directory.Build.props @sdwheeler @michaeltlombardi
6-
/docs/ @sdwheeler @michaeltlombardi
6+
/docs/ @PowerShell/extension @sdwheeler @michaeltlombardi

‎Directory.Packages.props‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@
66
<PackageVersion Include="Microsoft.PowerShell.5.ReferenceAssemblies" Version="1.1.0" />
77
<PackageVersion Include="Microsoft.Win32.Registry" Version="5.0.0" />
88
<!-- The version of Newtonsoft.Json needs to be newer than the version in the oldest supported version of PowerShell 7: https://github.com/PowerShell/PowerShell/blob/v7.2.17/src/System.Management.Automation/System.Management.Automation.csproj#L15 -->
9-
<PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
9+
<PackageVersion Include="Newtonsoft.Json" Version="13.0.4" />
1010
<PackageVersion Include="Pluralize.NET" Version="1.0.2" />
1111
<PackageVersion Include="PowerShellStandard.Library" Version="5.1.1" />
1212
<!-- Please update minimumPowerShellCoreVersion in PSScriptAnalyzer.psm1 when updating below SMA version for better user-facing error message -->
13-
<PackageVersion Include="System.Management.Automation" Version="7.4.7" />
13+
<PackageVersion Include="System.Management.Automation" Version="7.4.13" />
1414
<PackageVersion Include="System.Reflection.TypeExtensions" Version="4.7.0" />
1515
</ItemGroup>
1616
</Project>

‎Engine/CommandInfoCache.cs‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,23 +80,43 @@ public CommandInfo GetCommandInfo(string commandName, CommandTypes? commandTypes
8080
/// <returns>Returns null if command does not exists</returns>
8181
private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType)
8282
{
83+
string moduleName = null;
84+
string actualCmdName = cmdName;
85+
86+
// Check if cmdName is in the format "moduleName\CmdletName" (exactly one backslash)
87+
int backslashIndex = cmdName.IndexOf('\\');
88+
if (
89+
backslashIndex > 0 &&
90+
backslashIndex == cmdName.LastIndexOf('\\') &&
91+
backslashIndex != cmdName.Length - 1 &&
92+
backslashIndex != 0
93+
)
94+
{
95+
moduleName = cmdName.Substring(0, backslashIndex);
96+
actualCmdName = cmdName.Substring(backslashIndex + 1);
97+
}
8398
// 'Get-Command ?' would return % for example due to PowerShell interpreting is a single-character-wildcard search and not just the ? alias.
8499
// For more details see https://github.com/PowerShell/PowerShell/issues/9308
85-
cmdName = WildcardPattern.Escape(cmdName);
100+
actualCmdName = WildcardPattern.Escape(actualCmdName);
86101

87102
using (var ps = System.Management.Automation.PowerShell.Create())
88103
{
89104
ps.RunspacePool = _runspacePool;
90105

91106
ps.AddCommand("Get-Command")
92-
.AddParameter("Name", cmdName)
107+
.AddParameter("Name", actualCmdName)
93108
.AddParameter("ErrorAction", "SilentlyContinue");
94109

95110
if (commandType != null)
96111
{
97112
ps.AddParameter("CommandType", commandType);
98113
}
99114

115+
if (!string.IsNullOrEmpty(moduleName))
116+
{
117+
ps.AddParameter("Module", moduleName);
118+
}
119+
100120
return ps.Invoke<CommandInfo>()
101121
.FirstOrDefault();
102122
}

‎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>

‎Engine/PSScriptAnalyzer.psm1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ $PSModuleRoot = $PSModule.ModuleBase
99

1010
# Import the appropriate nested binary module based on the current PowerShell version
1111
$binaryModuleRoot = $PSModuleRoot
12-
[Version] $minimumPowerShellCoreVersion = '7.4.7'
12+
[Version] $minimumPowerShellCoreVersion = '7.4.13'
1313
if ($PSVersionTable.PSVersion.Major -ge 6) {
1414
$binaryModuleRoot = Join-Path -Path $PSModuleRoot -ChildPath "PSv$($PSVersionTable.PSVersion.Major)"
1515
# Minimum PowerShell Core version given by PowerShell Core support itself and

‎Rules/AvoidDefaultValueForMandatoryParameter.cs‎

Lines changed: 64 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Linq;
67
using System.Management.Automation.Language;
78
#if !CORECLR
89
using System.ComponentModel.Composition;
@@ -27,59 +28,73 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
2728
{
2829
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
2930

30-
// Finds all functionAst
31-
IEnumerable<Ast> functionAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true);
32-
33-
foreach (FunctionDefinitionAst funcAst in functionAsts)
31+
// Find all ParameterAst which are children of a ParamBlockAst. This
32+
// doesn't pick up where they appear as children of a
33+
// FunctionDefinitionAst. i.e.
34+
//
35+
// function foo ($a,$b){} -> $a and $b are `ParameterAst`
36+
//
37+
// Include only parameters which have a default value (as without
38+
// one this rule would never alert)
39+
// Include only parameters where ALL parameter attributes have the
40+
// mandatory named argument set to true (implicitly or explicitly)
41+
42+
var mandatoryParametersWithDefaultValues =
43+
ast.FindAll(testAst => testAst is ParamBlockAst, true)
44+
.Cast<ParamBlockAst>()
45+
.Where(pb => pb.Parameters?.Count > 0)
46+
.SelectMany(pb => pb.Parameters)
47+
.Where(paramAst =>
48+
paramAst.DefaultValue != null &&
49+
HasMandatoryInAllParameterAttributes(paramAst)
50+
);
51+
52+
// Report diagnostics for each parameter that violates the rule
53+
foreach (var parameter in mandatoryParametersWithDefaultValues)
3454
{
35-
if (funcAst.Body != null && funcAst.Body.ParamBlock != null
36-
&& funcAst.Body.ParamBlock.Attributes != null && funcAst.Body.ParamBlock.Parameters != null)
37-
{
38-
foreach (var paramAst in funcAst.Body.ParamBlock.Parameters)
39-
{
40-
bool mandatory = false;
41-
42-
// check that param is mandatory
43-
foreach (var paramAstAttribute in paramAst.Attributes)
44-
{
45-
if (paramAstAttribute is AttributeAst)
46-
{
47-
var namedArguments = (paramAstAttribute as AttributeAst).NamedArguments;
48-
if (namedArguments != null)
49-
{
50-
foreach (NamedAttributeArgumentAst namedArgument in namedArguments)
51-
{
52-
if (String.Equals(namedArgument.ArgumentName, "mandatory", StringComparison.OrdinalIgnoreCase))
53-
{
54-
// 3 cases: [Parameter(Mandatory)], [Parameter(Mandatory=$true)] and [Parameter(Mandatory=value)] where value is not equal to 0.
55-
if (namedArgument.ExpressionOmitted
56-
|| (String.Equals(namedArgument.Argument.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase))
57-
|| (int.TryParse(namedArgument.Argument.Extent.Text, out int mandatoryValue) && mandatoryValue != 0))
58-
{
59-
mandatory = true;
60-
break;
61-
}
62-
}
63-
}
64-
}
65-
}
66-
}
67-
68-
if (!mandatory)
69-
{
70-
break;
71-
}
72-
73-
if (paramAst.DefaultValue != null)
74-
{
75-
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidDefaultValueForMandatoryParameterError, paramAst.Name.VariablePath.UserPath),
76-
paramAst.Name.Extent, GetName(), DiagnosticSeverity.Warning, fileName, paramAst.Name.VariablePath.UserPath);
77-
}
78-
}
79-
}
55+
yield return new DiagnosticRecord(
56+
string.Format(
57+
CultureInfo.CurrentCulture,
58+
Strings.AvoidDefaultValueForMandatoryParameterError,
59+
parameter.Name.VariablePath.UserPath
60+
),
61+
parameter.Name.Extent,
62+
GetName(),
63+
DiagnosticSeverity.Warning,
64+
fileName,
65+
parameter.Name.VariablePath.UserPath
66+
);
8067
}
8168
}
8269

70+
/// <summary>
71+
/// Determines if a parameter is mandatory in all of its Parameter attributes.
72+
/// A parameter may have multiple [Parameter] attributes for different parameter sets.
73+
/// This method returns true only if ALL [Parameter] attributes have Mandatory=true.
74+
/// </summary>
75+
/// <param name="paramAst">The parameter AST to examine</param>
76+
/// <param name="comparer">String comparer for case-insensitive attribute name matching</param>
77+
/// <returns>
78+
/// True if the parameter has at least one [Parameter] attribute and ALL of them
79+
/// have the Mandatory named argument set to true (explicitly or implicitly).
80+
/// False if the parameter has no [Parameter] attributes or if any [Parameter]
81+
/// attribute does not have Mandatory=true.
82+
/// </returns>
83+
private static bool HasMandatoryInAllParameterAttributes(ParameterAst paramAst)
84+
{
85+
var parameterAttributes = paramAst.Attributes.OfType<AttributeAst>()
86+
.Where(attr => string.Equals(attr.TypeName?.Name, "parameter", StringComparison.OrdinalIgnoreCase));
87+
88+
return parameterAttributes.Any() &&
89+
parameterAttributes.All(attr =>
90+
attr.NamedArguments.OfType<NamedAttributeArgumentAst>()
91+
.Any(namedArg =>
92+
string.Equals(namedArg.ArgumentName, "mandatory", StringComparison.OrdinalIgnoreCase) &&
93+
Helper.Instance.GetNamedArgumentAttributeValue(namedArg)
94+
)
95+
);
96+
}
97+
8398
/// <summary>
8499
/// GetName: Retrieves the name of this rule.
85100
/// </summary>
@@ -134,6 +149,3 @@ public string GetSourceName()
134149
}
135150
}
136151

137-
138-
139-

‎Rules/AvoidReservedWordsAsFunctionNames.cs‎

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
@@ -1236,4 +1236,16 @@
12361236
<data name="AvoidUsingAllowUnencryptedAuthenticationName" xml:space="preserve">
12371237
<value>AvoidUsingAllowUnencryptedAuthentication</value>
12381238
</data>
1239+
<data name="AvoidReservedWordsAsFunctionNamesCommonName" xml:space="preserve">
1240+
<value>Avoid reserved words as function names</value>
1241+
</data>
1242+
<data name="AvoidReservedWordsAsFunctionNamesDescription" xml:space="preserve">
1243+
<value>Avoid using reserved words as function names. Using reserved words as function names can cause errors or unexpected behavior in scripts.</value>
1244+
</data>
1245+
<data name="AvoidReservedWordsAsFunctionNamesName" xml:space="preserve">
1246+
<value>AvoidReservedWordsAsFunctionNames</value>
1247+
</data>
1248+
<data name="AvoidReservedWordsAsFunctionNamesError" xml:space="preserve">
1249+
<value>The reserved word '{0}' was used as a function name. This should be avoided.</value>
1250+
</data>
12391251
</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

0 commit comments

Comments
 (0)