diff --git a/Engine/Helper.cs b/Engine/Helper.cs
index ae5858587..cbd8d0cd3 100644
--- a/Engine/Helper.cs
+++ b/Engine/Helper.cs
@@ -1853,6 +1853,188 @@ public Version GetPSVersion()
return psVersionTable == null ? null : psVersionTable.PSVersion;
}
+ ///
+ /// Evaluates all statically evaluable, side-effect-free expressions under an
+ /// expression AST to return a value.
+ /// Throws if an expression cannot be safely evaluated.
+ /// Attempts to replicate the GetSafeValue() method on PowerShell AST methods from PSv5.
+ ///
+ /// The .NET value represented by the PowerShell expression.
+ public static object GetSafeValueFromExpressionAst(ExpressionAst exprAst)
+ {
+ switch (exprAst)
+ {
+ case ConstantExpressionAst constExprAst:
+ // Note, this parses top-level command invocations as bareword strings
+ // However, forbidding this causes hashtable parsing to fail
+ // It is probably not worth the complexity to isolate this case
+ return constExprAst.Value;
+
+ case VariableExpressionAst varExprAst:
+ // $true and $false are VariableExpressionAsts, so look for them here
+ switch (varExprAst.VariablePath.UserPath.ToLowerInvariant())
+ {
+ case "true":
+ return true;
+
+ case "false":
+ return false;
+
+ case "null":
+ return null;
+
+ default:
+ throw CreateInvalidDataExceptionFromAst(varExprAst);
+ }
+
+ case ArrayExpressionAst arrExprAst:
+
+ // Most cases are handled by the inner array handling,
+ // but we may have an empty array
+ if (arrExprAst.SubExpression?.Statements == null)
+ {
+ throw CreateInvalidDataExceptionFromAst(arrExprAst);
+ }
+
+ if (arrExprAst.SubExpression.Statements.Count == 0)
+ {
+ return new object[0];
+ }
+
+ var listComponents = new List();
+ // Arrays can either be array expressions (1, 2, 3) or array literals with statements @(1 `n 2 `n 3)
+ // Or they can be a combination of these
+ // We go through each statement (line) in an array and read the whole subarray
+ // This will also mean that @(1; 2) is parsed as an array of two elements, but there's not much point defending against this
+ foreach (StatementAst statement in arrExprAst.SubExpression.Statements)
+ {
+ if (!(statement is PipelineAst pipelineAst))
+ {
+ throw CreateInvalidDataExceptionFromAst(arrExprAst);
+ }
+
+ ExpressionAst pipelineExpressionAst = pipelineAst.GetPureExpression();
+ if (pipelineExpressionAst == null)
+ {
+ throw CreateInvalidDataExceptionFromAst(arrExprAst);
+ }
+
+ object arrayValue = GetSafeValueFromExpressionAst(pipelineExpressionAst);
+ // We might hit arrays like @(\n1,2,3\n4,5,6), which the parser sees as two statements containing array expressions
+ if (arrayValue is object[] subArray)
+ {
+ listComponents.AddRange(subArray);
+ continue;
+ }
+
+ listComponents.Add(arrayValue);
+ }
+ return listComponents.ToArray();
+
+
+ case ArrayLiteralAst arrLiteralAst:
+ return GetSafeValuesFromArrayAst(arrLiteralAst);
+
+ case HashtableAst hashtableAst:
+ return GetSafeValueFromHashtableAst(hashtableAst);
+
+ default:
+ // Other expression types are too complicated or fundamentally unsafe
+ throw CreateInvalidDataExceptionFromAst(exprAst);
+ }
+ }
+
+ ///
+ /// Create a hashtable value from a PowerShell AST representing one,
+ /// provided that the PowerShell expression is statically evaluable and safe.
+ ///
+ /// The Hashtable as a hydrated .NET value.
+ internal static Hashtable GetSafeValueFromHashtableAst(HashtableAst hashtableAst)
+ {
+ if (hashtableAst == null)
+ {
+ throw new ArgumentNullException(nameof(hashtableAst));
+ }
+
+ if (hashtableAst.KeyValuePairs == null)
+ {
+ throw CreateInvalidDataExceptionFromAst(hashtableAst);
+ }
+
+ var hashtable = new Hashtable();
+ foreach (Tuple entry in hashtableAst.KeyValuePairs)
+ {
+ // Get the key
+ object key = GetSafeValueFromExpressionAst(entry.Item1);
+ if (key == null)
+ {
+ throw CreateInvalidDataExceptionFromAst(entry.Item1);
+ }
+
+ // Get the value
+ ExpressionAst valueExprAst = (entry.Item2 as PipelineAst)?.GetPureExpression();
+ if (valueExprAst == null)
+ {
+ throw CreateInvalidDataExceptionFromAst(entry.Item2);
+ }
+
+ // Add the key/value entry into the hydrated hashtable
+ hashtable[key] = GetSafeValueFromExpressionAst(valueExprAst);
+ }
+
+ return hashtable;
+ }
+
+ ///
+ /// Process a PowerShell array literal with statically evaluable/safe contents
+ /// into a .NET value.
+ ///
+ /// The .NET value represented by PowerShell syntax.
+ private static object[] GetSafeValuesFromArrayAst(ArrayLiteralAst arrLiteralAst)
+ {
+ if (arrLiteralAst == null)
+ {
+ throw new ArgumentNullException(nameof(arrLiteralAst));
+ }
+
+ if (arrLiteralAst.Elements == null)
+ {
+ throw CreateInvalidDataExceptionFromAst(arrLiteralAst);
+ }
+
+ var elements = new List();
+ foreach (ExpressionAst exprAst in arrLiteralAst.Elements)
+ {
+ elements.Add(GetSafeValueFromExpressionAst(exprAst));
+ }
+
+ return elements.ToArray();
+ }
+
+ private static InvalidDataException CreateInvalidDataExceptionFromAst(Ast ast)
+ {
+ if (ast == null)
+ {
+ throw new ArgumentNullException(nameof(ast));
+ }
+
+ return CreateInvalidDataException(ast.Extent);
+ }
+
+ private static InvalidDataException CreateInvalidDataException(IScriptExtent extent)
+ {
+ return new InvalidDataException(string.Format(
+ CultureInfo.CurrentCulture,
+ Strings.WrongValueFormat,
+ extent.StartLineNumber,
+ extent.StartColumnNumber,
+ extent.File ?? ""));
+ }
+
+
#endregion Methods
}
diff --git a/Engine/Settings.cs b/Engine/Settings.cs
index 506733cc2..b13f9405b 100644
--- a/Engine/Settings.cs
+++ b/Engine/Settings.cs
@@ -475,7 +475,7 @@ private void parseSettingsFile(string settingsFilePath)
{
// ideally we should use HashtableAst.SafeGetValue() but since
// it is not available on PSv3, we resort to our own narrow implementation.
- hashtable = GetSafeValueFromHashtableAst(hashTableAst);
+ hashtable = Helper.GetSafeValueFromHashtableAst(hashTableAst);
}
catch (InvalidOperationException e)
{
@@ -494,187 +494,6 @@ private void parseSettingsFile(string settingsFilePath)
parseSettingsHashtable(hashtable);
}
- ///
- /// Evaluates all statically evaluable, side-effect-free expressions under an
- /// expression AST to return a value.
- /// Throws if an expression cannot be safely evaluated.
- /// Attempts to replicate the GetSafeValue() method on PowerShell AST methods from PSv5.
- ///
- /// The .NET value represented by the PowerShell expression.
- private static object GetSafeValueFromExpressionAst(ExpressionAst exprAst)
- {
- switch (exprAst)
- {
- case ConstantExpressionAst constExprAst:
- // Note, this parses top-level command invocations as bareword strings
- // However, forbidding this causes hashtable parsing to fail
- // It is probably not worth the complexity to isolate this case
- return constExprAst.Value;
-
- case VariableExpressionAst varExprAst:
- // $true and $false are VariableExpressionAsts, so look for them here
- switch (varExprAst.VariablePath.UserPath.ToLowerInvariant())
- {
- case "true":
- return true;
-
- case "false":
- return false;
-
- case "null":
- return null;
-
- default:
- throw CreateInvalidDataExceptionFromAst(varExprAst);
- }
-
- case ArrayExpressionAst arrExprAst:
-
- // Most cases are handled by the inner array handling,
- // but we may have an empty array
- if (arrExprAst.SubExpression?.Statements == null)
- {
- throw CreateInvalidDataExceptionFromAst(arrExprAst);
- }
-
- if (arrExprAst.SubExpression.Statements.Count == 0)
- {
- return new object[0];
- }
-
- var listComponents = new List();
- // Arrays can either be array expressions (1, 2, 3) or array literals with statements @(1 `n 2 `n 3)
- // Or they can be a combination of these
- // We go through each statement (line) in an array and read the whole subarray
- // This will also mean that @(1; 2) is parsed as an array of two elements, but there's not much point defending against this
- foreach (StatementAst statement in arrExprAst.SubExpression.Statements)
- {
- if (!(statement is PipelineAst pipelineAst))
- {
- throw CreateInvalidDataExceptionFromAst(arrExprAst);
- }
-
- ExpressionAst pipelineExpressionAst = pipelineAst.GetPureExpression();
- if (pipelineExpressionAst == null)
- {
- throw CreateInvalidDataExceptionFromAst(arrExprAst);
- }
-
- object arrayValue = GetSafeValueFromExpressionAst(pipelineExpressionAst);
- // We might hit arrays like @(\n1,2,3\n4,5,6), which the parser sees as two statements containing array expressions
- if (arrayValue is object[] subArray)
- {
- listComponents.AddRange(subArray);
- continue;
- }
-
- listComponents.Add(arrayValue);
- }
- return listComponents.ToArray();
-
-
- case ArrayLiteralAst arrLiteralAst:
- return GetSafeValuesFromArrayAst(arrLiteralAst);
-
- case HashtableAst hashtableAst:
- return GetSafeValueFromHashtableAst(hashtableAst);
-
- default:
- // Other expression types are too complicated or fundamentally unsafe
- throw CreateInvalidDataExceptionFromAst(exprAst);
- }
- }
-
- ///
- /// Process a PowerShell array literal with statically evaluable/safe contents
- /// into a .NET value.
- ///
- /// The .NET value represented by PowerShell syntax.
- private static object[] GetSafeValuesFromArrayAst(ArrayLiteralAst arrLiteralAst)
- {
- if (arrLiteralAst == null)
- {
- throw new ArgumentNullException(nameof(arrLiteralAst));
- }
-
- if (arrLiteralAst.Elements == null)
- {
- throw CreateInvalidDataExceptionFromAst(arrLiteralAst);
- }
-
- var elements = new List();
- foreach (ExpressionAst exprAst in arrLiteralAst.Elements)
- {
- elements.Add(GetSafeValueFromExpressionAst(exprAst));
- }
-
- return elements.ToArray();
- }
-
- ///
- /// Create a hashtable value from a PowerShell AST representing one,
- /// provided that the PowerShell expression is statically evaluable and safe.
- ///
- /// The Hashtable as a hydrated .NET value.
- private static Hashtable GetSafeValueFromHashtableAst(HashtableAst hashtableAst)
- {
- if (hashtableAst == null)
- {
- throw new ArgumentNullException(nameof(hashtableAst));
- }
-
- if (hashtableAst.KeyValuePairs == null)
- {
- throw CreateInvalidDataExceptionFromAst(hashtableAst);
- }
-
- var hashtable = new Hashtable();
- foreach (Tuple entry in hashtableAst.KeyValuePairs)
- {
- // Get the key
- object key = GetSafeValueFromExpressionAst(entry.Item1);
- if (key == null)
- {
- throw CreateInvalidDataExceptionFromAst(entry.Item1);
- }
-
- // Get the value
- ExpressionAst valueExprAst = (entry.Item2 as PipelineAst)?.GetPureExpression();
- if (valueExprAst == null)
- {
- throw CreateInvalidDataExceptionFromAst(entry.Item2);
- }
-
- // Add the key/value entry into the hydrated hashtable
- hashtable[key] = GetSafeValueFromExpressionAst(valueExprAst);
- }
-
- return hashtable;
- }
-
- private static InvalidDataException CreateInvalidDataExceptionFromAst(Ast ast)
- {
- if (ast == null)
- {
- throw new ArgumentNullException(nameof(ast));
- }
-
- return CreateInvalidDataException(ast.Extent);
- }
-
- private static InvalidDataException CreateInvalidDataException(IScriptExtent extent)
- {
- return new InvalidDataException(string.Format(
- CultureInfo.CurrentCulture,
- Strings.WrongValueFormat,
- extent.StartLineNumber,
- extent.StartColumnNumber,
- extent.File ?? ""));
- }
-
private static bool IsBuiltinSettingPreset(object settingPreset)
{
var preset = settingPreset as string;
diff --git a/Rules/UseShouldProcessCorrectly.cs b/Rules/UseShouldProcessCorrectly.cs
index e08ca098a..74b330a60 100644
--- a/Rules/UseShouldProcessCorrectly.cs
+++ b/Rules/UseShouldProcessCorrectly.cs
@@ -299,48 +299,101 @@ private bool SupportsShouldProcess(string cmdName)
return false;
}
- var cmdInfo = Helper.Instance.GetCommandInfo(cmdName);
+ CommandInfo cmdInfo = Helper.Instance.GetCommandInfo(cmdName);
if (cmdInfo == null)
{
return false;
}
- var cmdletInfo = cmdInfo as CmdletInfo;
- if (cmdletInfo == null)
+ switch (cmdInfo)
{
- // check if it is of functioninfo type
- var funcInfo = cmdInfo as FunctionInfo;
- if (funcInfo != null
- && funcInfo.CmdletBinding
- && funcInfo.ScriptBlock != null
- && funcInfo.ScriptBlock.Attributes != null)
- {
- foreach (var attr in funcInfo.ScriptBlock.Attributes)
+ case CmdletInfo cmdletInfo:
+ return cmdletInfo.ImplementingType.GetCustomAttribute(inherit: true).SupportsShouldProcess;
+
+ case FunctionInfo functionInfo:
+ try
{
- var cmdletBindingAttr = attr as CmdletBindingAttribute;
- if (cmdletBindingAttr != null)
+ if (!functionInfo.CmdletBinding
+ || functionInfo.ScriptBlock?.Attributes == null)
+ {
+ break;
+ }
+
+ foreach (CmdletBindingAttribute cmdletBindingAttribute in functionInfo.ScriptBlock.Attributes.OfType())
{
- return cmdletBindingAttr.SupportsShouldProcess;
+ return cmdletBindingAttribute.SupportsShouldProcess;
+ }
+ }
+ catch
+ {
+ // functionInfo.ScriptBlock.Attributes may throw if it cannot resolve an attribute type
+ // Instead we fall back to AST analysis
+ // See: https://github.com/PowerShell/PSScriptAnalyzer/issues/1217
+ if (TryGetShouldProcessValueFromAst(functionInfo, out bool hasShouldProcessSet))
+ {
+ return hasShouldProcessSet;
}
}
- }
- return false;
+ break;
}
- var attributes = cmdletInfo.ImplementingType.GetTypeInfo().GetCustomAttributes(
- typeof(System.Management.Automation.CmdletCommonMetadataAttribute),
- true);
+ return false;
+ }
+
+ ///
+ /// Attempt to find whether a function has SupportsShouldProcess set based on its AST.
+ ///
+ /// True if a value for SupportsShouldProcess was found, false otherwise.
+ private bool TryGetShouldProcessValueFromAst(FunctionInfo functionInfo, out bool hasShouldProcessSet)
+ {
+ // Get the body of the function
+ ScriptBlockAst functionBodyAst = (ScriptBlockAst)functionInfo.ScriptBlock.Ast.Find(ast => ast is ScriptBlockAst, searchNestedScriptBlocks: false);
- foreach (var attr in attributes)
+ // Go through attributes on the parameter block, since this is where [CmdletBinding()] will be
+ foreach (AttributeAst attributeAst in functionBodyAst.ParamBlock.Attributes)
{
- var cmdletAttribute = attr as System.Management.Automation.CmdletAttribute;
- if (cmdletAttribute != null)
+ // We're looking for [CmdletBinding()]
+ if (!attributeAst.TypeName.FullName.Equals("CmdletBinding", StringComparison.OrdinalIgnoreCase))
{
- return cmdletAttribute.SupportsShouldProcess;
+ continue;
+ }
+
+ foreach (NamedAttributeArgumentAst namedArgumentAst in attributeAst.NamedArguments)
+ {
+ // We want [CmdletBinding(SupportsShouldProcess)]
+ if (!namedArgumentAst.ArgumentName.Equals("SupportsShouldProcess", StringComparison.OrdinalIgnoreCase))
+
+ {
+ continue;
+ }
+
+ // [CmdletBinding(SupportsShouldProcess)] is the same as [CmdletBinding(SupportsShouldProcess = $true)]
+ if (namedArgumentAst.ExpressionOmitted)
+ {
+ hasShouldProcessSet = true;
+ return true;
+ }
+
+ // Otherwise try to get the value assigned to the parameter, and assume false if value cannot be determined
+ try
+ {
+ hasShouldProcessSet = LanguagePrimitives.IsTrue(
+ Helper.GetSafeValueFromExpressionAst(
+ namedArgumentAst.Argument));
+ }
+ catch
+ {
+ hasShouldProcessSet = false;
+ }
+
+ return true;
}
}
+ hasShouldProcessSet = false;
return false;
}
diff --git a/Tests/Engine/Settings.tests.ps1 b/Tests/Engine/Settings.tests.ps1
index 4a5969cb1..cfd814274 100644
--- a/Tests/Engine/Settings.tests.ps1
+++ b/Tests/Engine/Settings.tests.ps1
@@ -251,8 +251,6 @@ Describe "Settings Class" {
@{ Expr = ';)' }
)
- $gsvMethod = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Settings].GetMethod('GetSafeValueFromExpressionAst', [System.Reflection.BindingFlags]'nonpublic,static')
-
function ShouldBeDeeplyEqual
{
param(
@@ -294,7 +292,7 @@ Describe "Settings Class" {
{
@(,$inputVal) | ShouldBeDeeplyEqual -To $To[$i]
continue
- }
+ }
$inputVal | ShouldBeDeeplyEqual -To $To[$i]
}
return
@@ -311,7 +309,7 @@ Describe "Settings Class" {
$exprAst = [System.Management.Automation.Language.Parser]::ParseInput($Expr, [ref]$null, [ref]$null)
$exprAst = $exprAst.Find({$args[0] -is [System.Management.Automation.Language.ExpressionAst]}, $true)
- $gsvVal = $gsvMethod.Invoke($null, @($exprAst))
+ $gsvVal = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper]::GetSafeValueFromExpressionAst($exprAst)
$gsvVal | Should -Be $pwshVal
}
@@ -323,7 +321,7 @@ Describe "Settings Class" {
$exprAst = [System.Management.Automation.Language.Parser]::ParseInput($Expr, [ref]$null, [ref]$null)
$exprAst = $exprAst.Find({$args[0] -is [System.Management.Automation.Language.ExpressionAst]}, $true)
- $gsvVal = $gsvMethod.Invoke($null, @($exprAst))
+ $gsvVal = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper]::GetSafeValueFromExpressionAst($exprAst)
# Need to test the type like this so that the pipeline doesn't unwrap the type,
@@ -340,7 +338,7 @@ Describe "Settings Class" {
$exprAst = [System.Management.Automation.Language.Parser]::ParseInput($Expr, [ref]$null, [ref]$null)
$exprAst = $exprAst.Find({$args[0] -is [System.Management.Automation.Language.ExpressionAst]}, $true)
- $gsvVal = $gsvMethod.Invoke($null, @($exprAst))
+ $gsvVal = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper]::GetSafeValueFromExpressionAst($exprAst)
$gsvVal | Should -BeOfType [hashtable]
$gsvVal | ShouldBeDeeplyEqual -To $pwshVal
@@ -358,7 +356,7 @@ Describe "Settings Class" {
$expectedError = 'ArgumentNullException'
}
- { $gsvVal = $gsvMethod.Invoke($null, @($exprAst)) } | Should -Throw -ErrorId $expectedError
+ { $gsvVal = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper]::GetSafeValueFromExpressionAst($exprAst) } | Should -Throw -ErrorId $expectedError
}
}
}