Skip to content

Commit d0b8661

Browse files
author
Quoc Truong
committed
Merge pull request #426 from PowerShell/FixReservedParamBug
Only raises ReservedParam rule if function is cmdlet and is exported
2 parents c07412b + 697a60c commit d0b8661

File tree

5 files changed

+205
-132
lines changed

5 files changed

+205
-132
lines changed

‎Engine/Helper.cs

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,141 @@ public bool IsDscResourceModule(string filePath)
229229
return false;
230230
}
231231

232+
/// <summary>
233+
/// Get the list of exported function by analyzing the ast
234+
/// </summary>
235+
/// <param name="ast"></param>
236+
/// <returns></returns>
237+
public HashSet<string> GetExportedFunction(Ast ast)
238+
{
239+
HashSet<string> exportedFunctions = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
240+
List<string> exportFunctionsCmdlet = Helper.Instance.CmdletNameAndAliases("export-modulemember");
241+
242+
// find functions exported
243+
IEnumerable<Ast> cmdAsts = ast.FindAll(item => item is CommandAst
244+
&& exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true);
245+
246+
CommandInfo exportMM = Helper.Instance.GetCommandInfo("export-modulemember", CommandTypes.Cmdlet);
247+
248+
// switch parameters
249+
IEnumerable<ParameterMetadata> switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter) : Enumerable.Empty<ParameterMetadata>();
250+
251+
if (exportMM == null)
252+
{
253+
return exportedFunctions;
254+
}
255+
256+
foreach (CommandAst cmdAst in cmdAsts)
257+
{
258+
if (cmdAst.CommandElements == null || cmdAst.CommandElements.Count < 2)
259+
{
260+
continue;
261+
}
262+
263+
int i = 1;
264+
265+
while (i < cmdAst.CommandElements.Count)
266+
{
267+
CommandElementAst ceAst = cmdAst.CommandElements[i];
268+
ExpressionAst exprAst = null;
269+
270+
if (ceAst is CommandParameterAst)
271+
{
272+
var paramAst = ceAst as CommandParameterAst;
273+
var param = exportMM.ResolveParameter(paramAst.ParameterName);
274+
275+
if (param == null)
276+
{
277+
i += 1;
278+
continue;
279+
}
280+
281+
if (string.Equals(param.Name, "function", StringComparison.OrdinalIgnoreCase))
282+
{
283+
// checks for the case of -Function:"verb-nouns"
284+
if (paramAst.Argument != null)
285+
{
286+
exprAst = paramAst.Argument;
287+
}
288+
// checks for the case of -Function "verb-nouns"
289+
else if (i < cmdAst.CommandElements.Count - 1)
290+
{
291+
i += 1;
292+
exprAst = cmdAst.CommandElements[i] as ExpressionAst;
293+
}
294+
}
295+
// some other parameter. we just checks whether the one after this is positional
296+
else if (i < cmdAst.CommandElements.Count - 1)
297+
{
298+
// the next element is a parameter like -module so just move to that one
299+
if (cmdAst.CommandElements[i + 1] is CommandParameterAst)
300+
{
301+
i += 1;
302+
continue;
303+
}
304+
305+
// not a switch parameter so the next element is definitely the argument to this parameter
306+
if (paramAst.Argument == null && !switchParams.Contains(param))
307+
{
308+
// skips the next element
309+
i += 1;
310+
}
311+
312+
i += 1;
313+
continue;
314+
}
315+
}
316+
else if (ceAst is ExpressionAst)
317+
{
318+
exprAst = ceAst as ExpressionAst;
319+
}
320+
321+
if (exprAst != null)
322+
{
323+
// One string so just add this to the list
324+
if (exprAst is StringConstantExpressionAst)
325+
{
326+
exportedFunctions.Add((exprAst as StringConstantExpressionAst).Value);
327+
}
328+
// Array of the form "v-n", "v-n1"
329+
else if (exprAst is ArrayLiteralAst)
330+
{
331+
exportedFunctions.UnionWith(Helper.Instance.GetStringsFromArrayLiteral(exprAst as ArrayLiteralAst));
332+
}
333+
// Array of the form @("v-n", "v-n1")
334+
else if (exprAst is ArrayExpressionAst)
335+
{
336+
ArrayExpressionAst arrExAst = exprAst as ArrayExpressionAst;
337+
if (arrExAst.SubExpression != null && arrExAst.SubExpression.Statements != null)
338+
{
339+
foreach (StatementAst stAst in arrExAst.SubExpression.Statements)
340+
{
341+
if (stAst is PipelineAst)
342+
{
343+
PipelineAst pipeAst = stAst as PipelineAst;
344+
if (pipeAst.PipelineElements != null)
345+
{
346+
foreach (CommandBaseAst cmdBaseAst in pipeAst.PipelineElements)
347+
{
348+
if (cmdBaseAst is CommandExpressionAst)
349+
{
350+
exportedFunctions.UnionWith(Helper.Instance.GetStringsFromArrayLiteral((cmdBaseAst as CommandExpressionAst).Expression as ArrayLiteralAst));
351+
}
352+
}
353+
}
354+
}
355+
}
356+
}
357+
}
358+
}
359+
360+
i += 1;
361+
}
362+
}
363+
364+
return exportedFunctions;
365+
}
366+
232367
/// <summary>
233368
/// Given a filePath. Returns true if it is a powershell help file
234369
/// </summary>

‎Rules/AvoidReservedCharInCmdlet.cs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
using Microsoft.Windows.PowerShell.ScriptAnalyzer;
1919
using System.ComponentModel.Composition;
2020
using System.Globalization;
21+
using System.Management.Automation;
2122

2223
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
2324
{
@@ -41,15 +42,26 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
4142
IEnumerable<Ast> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true);
4243

4344
string reservedChars = Strings.ReserverCmdletChars;
45+
HashSet<string> exportedFunction = Helper.Instance.GetExportedFunction(ast);
4446

4547
foreach (FunctionDefinitionAst funcAst in funcAsts)
4648
{
47-
string funcName = Helper.Instance.FunctionNameWithoutScope(funcAst.Name);
48-
49-
if (funcName != null && funcName.Intersect(reservedChars).Count() > 0)
49+
if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Attributes != null)
5050
{
51-
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ReservedCmdletCharError, funcAst.Name),
52-
funcAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
51+
// only raise this rule for function with cmdletbinding
52+
if (!funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute)))
53+
{
54+
continue;
55+
}
56+
57+
string funcName = Helper.Instance.FunctionNameWithoutScope(funcAst.Name);
58+
59+
// only raise if the function is exported
60+
if (funcName != null && funcName.Intersect(reservedChars).Count() > 0 && (exportedFunction.Contains(funcAst.Name) || exportedFunction.Contains(funcName)))
61+
{
62+
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ReservedCmdletCharError, funcAst.Name),
63+
funcAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
64+
}
5365
}
5466
}
5567
}

‎Rules/ProvideCommentHelp.cs

Lines changed: 1 addition & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -40,130 +40,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName) {
4040

4141
DiagnosticRecords.Clear();
4242
this.fileName = fileName;
43-
exportedFunctions = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
44-
List<string> exportFunctionsCmdlet = Helper.Instance.CmdletNameAndAliases("export-modulemember");
45-
46-
// find functions exported
47-
IEnumerable<Ast> cmdAsts = ast.FindAll(item => item is CommandAst
48-
&& exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true);
49-
50-
CommandInfo exportMM = Helper.Instance.GetCommandInfo("export-modulemember", CommandTypes.Cmdlet);
51-
52-
// switch parameters
53-
IEnumerable<ParameterMetadata> switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter) : Enumerable.Empty<ParameterMetadata>();
54-
55-
if (exportMM == null)
56-
{
57-
return DiagnosticRecords;
58-
}
59-
60-
foreach (CommandAst cmdAst in cmdAsts)
61-
{
62-
if (cmdAst.CommandElements == null || cmdAst.CommandElements.Count < 2)
63-
{
64-
continue;
65-
}
66-
67-
int i = 1;
68-
69-
while (i < cmdAst.CommandElements.Count)
70-
{
71-
CommandElementAst ceAst = cmdAst.CommandElements[i];
72-
ExpressionAst exprAst = null;
73-
74-
if (ceAst is CommandParameterAst)
75-
{
76-
var paramAst = ceAst as CommandParameterAst;
77-
var param = exportMM.ResolveParameter(paramAst.ParameterName);
78-
79-
if (param == null)
80-
{
81-
i += 1;
82-
continue;
83-
}
84-
85-
if (string.Equals(param.Name, "function", StringComparison.OrdinalIgnoreCase))
86-
{
87-
// checks for the case of -Function:"verb-nouns"
88-
if (paramAst.Argument != null)
89-
{
90-
exprAst = paramAst.Argument;
91-
}
92-
// checks for the case of -Function "verb-nouns"
93-
else if (i < cmdAst.CommandElements.Count - 1)
94-
{
95-
i += 1;
96-
exprAst = cmdAst.CommandElements[i] as ExpressionAst;
97-
}
98-
}
99-
// some other parameter. we just checks whether the one after this is positional
100-
else if (i < cmdAst.CommandElements.Count - 1)
101-
{
102-
// the next element is a parameter like -module so just move to that one
103-
if (cmdAst.CommandElements[i + 1] is CommandParameterAst)
104-
{
105-
i += 1;
106-
continue;
107-
}
108-
109-
// not a switch parameter so the next element is definitely the argument to this parameter
110-
if (paramAst.Argument == null && !switchParams.Contains(param))
111-
{
112-
// skips the next element
113-
i += 1;
114-
}
115-
116-
i += 1;
117-
continue;
118-
}
119-
}
120-
else if (ceAst is ExpressionAst)
121-
{
122-
exprAst = ceAst as ExpressionAst;
123-
}
124-
125-
if (exprAst != null)
126-
{
127-
// One string so just add this to the list
128-
if (exprAst is StringConstantExpressionAst)
129-
{
130-
exportedFunctions.Add((exprAst as StringConstantExpressionAst).Value);
131-
}
132-
// Array of the form "v-n", "v-n1"
133-
else if (exprAst is ArrayLiteralAst)
134-
{
135-
exportedFunctions.UnionWith(Helper.Instance.GetStringsFromArrayLiteral(exprAst as ArrayLiteralAst));
136-
}
137-
// Array of the form @("v-n", "v-n1")
138-
else if (exprAst is ArrayExpressionAst)
139-
{
140-
ArrayExpressionAst arrExAst = exprAst as ArrayExpressionAst;
141-
if (arrExAst.SubExpression != null && arrExAst.SubExpression.Statements != null)
142-
{
143-
foreach (StatementAst stAst in arrExAst.SubExpression.Statements)
144-
{
145-
if (stAst is PipelineAst)
146-
{
147-
PipelineAst pipeAst = stAst as PipelineAst;
148-
if (pipeAst.PipelineElements != null)
149-
{
150-
foreach (CommandBaseAst cmdBaseAst in pipeAst.PipelineElements)
151-
{
152-
if (cmdBaseAst is CommandExpressionAst)
153-
{
154-
exportedFunctions.UnionWith(Helper.Instance.GetStringsFromArrayLiteral((cmdBaseAst as CommandExpressionAst).Expression as ArrayLiteralAst));
155-
}
156-
}
157-
}
158-
}
159-
}
160-
}
161-
}
162-
}
163-
164-
i += 1;
165-
}
166-
}
43+
exportedFunctions = Helper.Instance.GetExportedFunction(ast);
16744

16845
ast.Visit(this);
16946

‎Tests/Rules/AvoidUsingReservedCharNames.ps1

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@
2727
}
2828
}
2929

30-
function O {
30+
function O
31+
{
32+
[CmdletBinding()]
33+
[Alias()]
34+
[OutputType([int])]
35+
Param()
36+
3137
Write-Output "I use one char"
32-
}
38+
}
39+
40+
Export-ModuleMember Use-#Reserved
41+
Export-ModuleMember O

‎Tests/Rules/GoodCmdlet.ps1

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,47 @@ function Get-Folder
217217
}
218218

219219
#Write-Verbose should not be required because this is not an advanced function
220-
function global:Get-SimpleFunc
220+
# use reserved param here
221+
function global:Get-SimpleFunc*
221222
{
222223

224+
}
225+
226+
<#
227+
.Synopsis
228+
Short description
229+
.DESCRIPTION
230+
Long description
231+
.EXAMPLE
232+
Example of how to use this cmdlet
233+
.EXAMPLE
234+
Another example of how to use this cmdlet
235+
#>
236+
function Get-Reserved*
237+
{
238+
[CmdletBinding()]
239+
[Alias()]
240+
[OutputType([int])]
241+
Param
242+
(
243+
# Param1 help description
244+
[Parameter(Mandatory=$true,
245+
ValueFromPipelineByPropertyName=$true,
246+
Position=0)]
247+
$Param1,
248+
249+
# Param2 help description
250+
[int]
251+
$Param2
252+
)
253+
254+
Begin
255+
{
256+
}
257+
Process
258+
{
259+
}
260+
End
261+
{
262+
}
223263
}

0 commit comments

Comments
 (0)