From e6e2ee5f4f2ec1eec1c6f4aeaebe01a75a38d0d8 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Fri, 10 Apr 2015 13:21:43 -0700 Subject: [PATCH 1/2] Fix AvoidReservedParams does not work if function does not have cmdletbinding --- Rules/AvoidReservedParams.cs | 53 ++++++++++------------- Tests/Rules/AvoidReservedParams.tests.ps1 | 2 +- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/Rules/AvoidReservedParams.cs b/Rules/AvoidReservedParams.cs index b8aaa979c..b427cc7ae 100644 --- a/Rules/AvoidReservedParams.cs +++ b/Rules/AvoidReservedParams.cs @@ -30,43 +30,38 @@ public class AvoidReservedParams : IScriptRule public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - IEnumerable paramAsts = ast.FindAll(testAst => testAst is ParameterAst, true); - Ast parentAst; + IEnumerable funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true); string paramName; - PropertyInfo[] commonParams = typeof(CommonParameters).GetProperties(); - List commonParamNames = new List(); + List commonParamNames = typeof(CommonParameters).GetProperties().Select(param => param.Name).ToList(); - if (commonParams != null) { - foreach (PropertyInfo commonParam in commonParams) { - commonParamNames.Add("$" + commonParam.Name); + foreach (FunctionDefinitionAst funcAst in funcAsts) + { + IEnumerable parameters = null; + if (funcAst.Parameters != null) + { + parameters = funcAst.Parameters; + } + // Check param block + else + { + if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null) + { + parameters = funcAst.Body.ParamBlock.Parameters; + } } - } - - if (paramAsts != null) { - foreach (ParameterAst paramAst in paramAsts) { - paramName = paramAst.Name.ToString(); - if (commonParamNames.Contains(paramName, StringComparer.OrdinalIgnoreCase)) { - parentAst = paramAst.Parent; - while (parentAst != null && !(parentAst is FunctionDefinitionAst)) { - parentAst = parentAst.Parent; - } + if (parameters != null) + { + foreach (ParameterAst paramAst in parameters) + { + paramName = paramAst.Name.VariablePath.UserPath; - if (parentAst is FunctionDefinitionAst) + if (commonParamNames.Contains(paramName, StringComparer.OrdinalIgnoreCase)) { - IEnumerable attrs = parentAst.FindAll(testAttr => testAttr is AttributeAst, true); - foreach (AttributeAst attr in attrs) - { - if (string.Equals(attr.Extent.Text, "[CmdletBinding()]", - StringComparison.OrdinalIgnoreCase)) - { - string funcName = string.Format(CultureInfo.CurrentCulture,Strings.ReservedParamsCmdletPrefix, (parentAst as FunctionDefinitionAst).Name); - yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ReservedParamsError, funcName,paramName), paramAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); - - } - } + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ReservedParamsError, funcAst.Name, paramName), + paramAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); } } } diff --git a/Tests/Rules/AvoidReservedParams.tests.ps1 b/Tests/Rules/AvoidReservedParams.tests.ps1 index a2dbc475b..b6b70f42b 100644 --- a/Tests/Rules/AvoidReservedParams.tests.ps1 +++ b/Tests/Rules/AvoidReservedParams.tests.ps1 @@ -1,5 +1,5 @@ Import-Module ScriptAnalyzer -$violationMessage = [regex]::Escape('The cmdlet Verb-Files defines the reserved common parameter $Verbose.') +$violationMessage = [regex]::Escape("Verb-Files' defines the reserved common parameter 'Verbose'.") $violationName = "PSReservedParams" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} From 8d531d9b9aacecd5d85253b2aea0f17f1b83dc1e Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Fri, 10 Apr 2015 13:46:39 -0700 Subject: [PATCH 2/2] Check for cmdletbinding attribute before applying the rule --- Rules/AvoidReservedParams.cs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/Rules/AvoidReservedParams.cs b/Rules/AvoidReservedParams.cs index b427cc7ae..d065fd881 100644 --- a/Rules/AvoidReservedParams.cs +++ b/Rules/AvoidReservedParams.cs @@ -32,31 +32,23 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { IEnumerable funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true); - string paramName; - List commonParamNames = typeof(CommonParameters).GetProperties().Select(param => param.Name).ToList(); foreach (FunctionDefinitionAst funcAst in funcAsts) { - IEnumerable parameters = null; - if (funcAst.Parameters != null) - { - parameters = funcAst.Parameters; - } - // Check param block - else + // this rule only applies to function with param block + if (funcAst.Body != null && funcAst.Body.ParamBlock != null + && funcAst.Body.ParamBlock.Attributes != null && funcAst.Body.ParamBlock.Parameters != null) { - if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null) + // no cmdlet binding + if (!funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute))) { - parameters = funcAst.Body.ParamBlock.Parameters; + continue; } - } - if (parameters != null) - { - foreach (ParameterAst paramAst in parameters) + foreach (ParameterAst paramAst in funcAst.Body.ParamBlock.Parameters) { - paramName = paramAst.Name.VariablePath.UserPath; + string paramName = paramAst.Name.VariablePath.UserPath; if (commonParamNames.Contains(paramName, StringComparer.OrdinalIgnoreCase)) {