diff --git a/Rules/AvoidValidateRangeTypeMismatch.cs b/Rules/AvoidValidateRangeTypeMismatch.cs index 319a865ba..aa48db75f 100644 --- a/Rules/AvoidValidateRangeTypeMismatch.cs +++ b/Rules/AvoidValidateRangeTypeMismatch.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Linq; using System.Management.Automation.Language; +using System.Reflection; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; #if !CORECLR using System.ComponentModel.Composition; @@ -109,16 +110,22 @@ private static bool IsIntegralBoundary(ExpressionAst expressionAst) return IsIntegralBoundary(unaryExpressionAst.Child); } + if (expressionAst is MemberExpressionAst memberExpressionAst + && memberExpressionAst.Static + && memberExpressionAst.Expression is TypeExpressionAst typeExpressionAst + && memberExpressionAst.Member is StringConstantExpressionAst memberName) + { + Type type = typeExpressionAst.TypeName.GetReflectionType(); + FieldInfo field = type?.GetField(memberName.Value, BindingFlags.Public | BindingFlags.Static | BindingFlags.IgnoreCase | BindingFlags.FlattenHierarchy); + return field != null && field.IsLiteral && IntegralTypes.Contains(field.FieldType); + } + return false; } private static bool IsValidateRangeAttribute(AttributeAst attributeAst) { - string attributeName = attributeAst.TypeName.FullName; - int lastDot = attributeName.LastIndexOf('.'); - string simpleName = lastDot < 0 ? attributeName : attributeName.Substring(lastDot + 1); - return String.Equals(simpleName, "ValidateRange", StringComparison.OrdinalIgnoreCase) - || String.Equals(simpleName, "ValidateRangeAttribute", StringComparison.OrdinalIgnoreCase); + return attributeAst.TypeName.GetReflectionAttributeType() == typeof(System.Management.Automation.ValidateRangeAttribute); } public string GetName() diff --git a/Tests/Rules/AvoidValidateRangeTypeMismatch.tests.ps1 b/Tests/Rules/AvoidValidateRangeTypeMismatch.tests.ps1 index bc3915240..24e6ad129 100644 --- a/Tests/Rules/AvoidValidateRangeTypeMismatch.tests.ps1 +++ b/Tests/Rules/AvoidValidateRangeTypeMismatch.tests.ps1 @@ -4,6 +4,39 @@ Describe 'AvoidValidateRangeTypeMismatch' { BeforeAll { $ruleName = 'PSAvoidValidateRangeTypeMismatch' + Add-Type -TypeDefinition @' +public class ValidateRangeBoundsBaseForScriptAnalyzer +{ + public const int Max = 100; +} + +public class ValidateRangeBoundsForScriptAnalyzer : ValidateRangeBoundsBaseForScriptAnalyzer +{ + public static readonly int ReadonlyIntegral = 100; + public const double FloatingPoint = 100.0; + public static int ThrowingGetterInvocationCount; + + public static int ThrowingProperty + { + get + { + ThrowingGetterInvocationCount++; + throw new System.InvalidOperationException(); + } + } +} + +namespace Contoso +{ + [System.AttributeUsage(System.AttributeTargets.All)] + public class ValidateRangeAttribute : System.Attribute + { + public ValidateRangeAttribute(int minRange, int maxRange) + { + } + } +} +'@ } It 'reports integral boundaries for floating-point and decimal parameter types' { @@ -15,6 +48,9 @@ Describe 'AvoidValidateRangeTypeMismatch' { 'function Test { param([double][System.Management.Automation.ValidateRangeAttribute(1, 2)]$Value) }' 'function Test { param([double][ValidateRange([int]0.0, [int]100.0)]$Value) }' 'function Test { param([double][ValidateRange([int32]0.0, [int32]100.0)]$Value) }' + 'function Test { param([double][ValidateRange(0, [int]::MaxValue)]$Value) }' + 'function Test { param([double][ValidateRange(0, [ValidateRangeBoundsForScriptAnalyzer]::Max)]$Value) }' + 'function Test { param([double][System.Management.Automation.ValidateRange(0, 100)]$Value) }' ) foreach ($scriptDefinition in $scripts) { @@ -30,6 +66,7 @@ Describe 'AvoidValidateRangeTypeMismatch' { 'function Test { param([single][ValidateRange(1.0, 2.0)]$Value) }' 'function Test { param([decimal][ValidateRange(1.0, 2.0)]$Value) }' 'function Test { param([double][ValidateRange(1, 2.0)]$Value) }' + 'function Test { param([double][Contoso.ValidateRange(0, 100)]$Value) }' 'function Test { param([int][ValidateRange(1, 2)]$Value) }' ) @@ -37,4 +74,19 @@ Describe 'AvoidValidateRangeTypeMismatch' { Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName | Should -BeNullOrEmpty } } + + It 'ignores non-constant static member expressions without invoking getters' { + $scripts = @( + 'function Test { param([double][ValidateRange(0, [ValidateRangeBoundsForScriptAnalyzer]::ThrowingProperty)]$Value) }' + 'function Test { param([double][ValidateRange(0, [ValidateRangeBoundsForScriptAnalyzer]::ReadonlyIntegral)]$Value) }' + 'function Test { param([double][ValidateRange(0, [ValidateRangeBoundsForScriptAnalyzer]::FloatingPoint)]$Value) }' + ) + + foreach ($scriptDefinition in $scripts) { + Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName | + Where-Object RuleName -eq $ruleName | Should -BeNullOrEmpty + } + + [ValidateRangeBoundsForScriptAnalyzer]::ThrowingGetterInvocationCount | Should -Be 0 + } }