diff --git a/src/System.Management.Automation/engine/Attributes.cs b/src/System.Management.Automation/engine/Attributes.cs index 876f428bde..d96a428451 100644 --- a/src/System.Management.Automation/engine/Attributes.cs +++ b/src/System.Management.Automation/engine/Attributes.cs @@ -1865,11 +1865,40 @@ namespace System.Management.Automation #endregion #region NULL validation attributes + + /// + /// Base type of Null Validation attributes. + /// + public abstract class NullValidationAttributeBase : ValidateArgumentsAttribute + { + /// + /// Check if the argument type is a collection. + /// + protected bool IsArgumentCollection(Type argumentType, out bool isElementValueType) + { + isElementValueType = false; + var information = new ParameterCollectionTypeInformation(argumentType); + switch (information.ParameterCollectionType) + { + // If 'arguments' is an array, or implement 'IList', or implement 'ICollection<>' + // then we continue to check each element of the collection. + case ParameterCollectionType.Array: + case ParameterCollectionType.IList: + case ParameterCollectionType.ICollectionGeneric: + Type elementType = information.ElementType; + isElementValueType = elementType != null && elementType.IsValueType; + return true; + default: + return false; + } + } + } + /// /// Validates that the parameters's argument is not null /// [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)] - public sealed class ValidateNotNullAttribute : ValidateArgumentsAttribute + public sealed class ValidateNotNullAttribute : NullValidationAttributeBase { /// /// Verifies the argument is not null and if it is a collection, that each @@ -1891,9 +1920,6 @@ namespace System.Management.Automation /// protected override void Validate(object arguments, EngineIntrinsics engineIntrinsics) { - IEnumerable ienum = null; - IEnumerator itor = null; - if (arguments == null || arguments == AutomationNull.Value) { throw new ValidationMetadataException( @@ -1901,10 +1927,16 @@ namespace System.Management.Automation null, Metadata.ValidateNotNullFailure); } - else if ((ienum = arguments as IEnumerable) != null) + else if (IsArgumentCollection(arguments.GetType(), out bool isElementValueType)) { - foreach (object element in ienum) + // If the element of the collection is of value type, then no need to check for null + // because a value-type value cannot be null. + if (isElementValueType) { return; } + + IEnumerator ienum = LanguagePrimitives.GetEnumerator(arguments); + while (ienum.MoveNext()) { + object element = ienum.Current; if (element == null || element == AutomationNull.Value) { throw new ValidationMetadataException( @@ -1914,19 +1946,6 @@ namespace System.Management.Automation } } } - else if ((itor = arguments as IEnumerator) != null) - { - for (; itor.MoveNext() == true;) - { - if (itor.Current == null || itor.Current == AutomationNull.Value) - { - throw new ValidationMetadataException( - "ArgumentIsNull", - null, - Metadata.ValidateNotNullCollectionFailure); - } - } - } } } @@ -1935,7 +1954,7 @@ namespace System.Management.Automation /// an empty string, and is not an empty collection. /// [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)] - public sealed class ValidateNotNullOrEmptyAttribute : ValidateArgumentsAttribute + public sealed class ValidateNotNullOrEmptyAttribute : NullValidationAttributeBase { /// /// Validates that the parameters's argument is not null, is not @@ -1954,10 +1973,6 @@ namespace System.Management.Automation /// protected override void Validate(object arguments, EngineIntrinsics engineIntrinsics) { - IEnumerable ienum = null; - IEnumerator itor = null; - string str = null; - if (arguments == null || arguments == AutomationNull.Value) { throw new ValidationMetadataException( @@ -1965,7 +1980,7 @@ namespace System.Management.Automation null, Metadata.ValidateNotNullOrEmptyFailure); } - else if ((str = arguments as String) != null) + else if (arguments is string str) { if (String.IsNullOrEmpty(str)) { @@ -1975,34 +1990,40 @@ namespace System.Management.Automation Metadata.ValidateNotNullOrEmptyFailure); } } - else if ((ienum = arguments as IEnumerable) != null) + else if (IsArgumentCollection(arguments.GetType(), out bool isElementValueType)) { - int validElements = 0; - foreach (object element in ienum) - { - validElements++; - if (element == null || element == AutomationNull.Value) - { - throw new ValidationMetadataException( - "ArgumentIsNull", - null, - Metadata.ValidateNotNullOrEmptyCollectionFailure); - } + bool isEmpty = true; + IEnumerator ienum = LanguagePrimitives.GetEnumerator(arguments); + if (ienum.MoveNext()) { isEmpty = false; } - string elementAsString = element as String; - if (elementAsString != null) - { - if (String.IsNullOrEmpty(elementAsString)) + // If the element of the collection is of value type, then no need to check for null + // because a value-type value cannot be null. + if (!isEmpty && !isElementValueType) + { + do { + object element = ienum.Current; + if (element == null || element == AutomationNull.Value) { throw new ValidationMetadataException( - "ArgumentCollectionContainsEmpty", + "ArgumentIsNull", null, - Metadata.ValidateNotNullOrEmptyFailure); + Metadata.ValidateNotNullOrEmptyCollectionFailure); } - } + + if (element is string elementAsString) + { + if (String.IsNullOrEmpty(elementAsString)) + { + throw new ValidationMetadataException( + "ArgumentCollectionContainsEmpty", + null, + Metadata.ValidateNotNullOrEmptyCollectionFailure); + } + } + } while (ienum.MoveNext()); } - if (validElements == 0) + if (isEmpty) { throw new ValidationMetadataException( "ArgumentIsEmpty", @@ -2010,21 +2031,9 @@ namespace System.Management.Automation Metadata.ValidateNotNullOrEmptyCollectionFailure); } } - else if ((itor = arguments as IEnumerator) != null) + else if (arguments is IDictionary dict) { - int validElements = 0; - for (; itor.MoveNext() == true;) - { - validElements++; - if (itor.Current == null || itor.Current == AutomationNull.Value) - { - throw new ValidationMetadataException( - "ArgumentIsNull", - null, - Metadata.ValidateNotNullOrEmptyCollectionFailure); - } - } - if (validElements == 0) + if (dict.Count == 0) { throw new ValidationMetadataException( "ArgumentIsEmpty", diff --git a/src/System.Management.Automation/engine/CompiledCommandParameter.cs b/src/System.Management.Automation/engine/CompiledCommandParameter.cs index a8a4eb097d..4a90b0849c 100644 --- a/src/System.Management.Automation/engine/CompiledCommandParameter.cs +++ b/src/System.Management.Automation/engine/CompiledCommandParameter.cs @@ -698,7 +698,7 @@ namespace System.Management.Automation /// /// The type of the elements in the collection /// - internal Type ElementType { get; set; } + internal Type ElementType { get; private set; } } } diff --git a/src/System.Management.Automation/engine/ParameterBinderBase.cs b/src/System.Management.Automation/engine/ParameterBinderBase.cs index 4f3dc90e81..94b15552f6 100644 --- a/src/System.Management.Automation/engine/ParameterBinderBase.cs +++ b/src/System.Management.Automation/engine/ParameterBinderBase.cs @@ -787,18 +787,26 @@ namespace System.Management.Automation // Ensure that each element abides by the metadata bool isEmpty = true; + Type elementType = parameterMetadata.CollectionTypeInformation.ElementType; + bool isElementValueType = elementType != null && elementType.IsValueType; + // Note - we explicitly don't pass the context here because we don't want // the overhead of the calls that check for stopping. - while (ParserOps.MoveNext(null, null, ienum)) + if (ParserOps.MoveNext(null, null, ienum)) { isEmpty = false; } + + // If the element of the collection is of value type, then no need to check for null + // because a value-type value cannot be null. + if (!isEmpty && !isElementValueType) { - object element = ParserOps.Current(null, ienum); - isEmpty = false; - ValidateNullOrEmptyArgument( - parameter, - parameterMetadata, - parameterMetadata.CollectionTypeInformation.ElementType, - element, - false); + do { + object element = ParserOps.Current(null, ienum); + ValidateNullOrEmptyArgument( + parameter, + parameterMetadata, + parameterMetadata.CollectionTypeInformation.ElementType, + element, + false); + } while (ParserOps.MoveNext(null, null, ienum)); } if (isEmpty && !parameterMetadata.AllowsEmptyCollectionArgument) diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index 72934c3348..4f3c5ab80f 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -594,7 +594,6 @@ namespace System.Management.Automation.Language return (errorSuggestion ?? NullResult(target)).WriteToDebugLog(this); } - // In CORECLR System.Data.DataTable does not have the DataRowCollection IEnumerable, so disabling code. if (targetValue is DataTable) { // Generate: diff --git a/test/powershell/engine/Basic/ValidateAttributes.Tests.ps1 b/test/powershell/engine/Basic/ValidateAttributes.Tests.ps1 index 92a04c1497..d95af1cd2b 100644 --- a/test/powershell/engine/Basic/ValidateAttributes.Tests.ps1 +++ b/test/powershell/engine/Basic/ValidateAttributes.Tests.ps1 @@ -260,4 +260,124 @@ Describe 'Validate Attributes Tests' -Tags 'CI' { $ScriptBlock | Should Not Throw } } + + Context "ValidateNotNull, ValidateNotNullOrEmpty and Not-Null-Or-Empty check for Mandatory parameter" { + + BeforeAll { + function MandatoryFunc { + param( + [Parameter(Mandatory, ParameterSetName = "ByteArray")] + [byte[]] $ByteArray, + + [Parameter(Mandatory, ParameterSetName = "ByteList")] + [System.Collections.Generic.List[byte]] $ByteList, + + [Parameter(Mandatory, ParameterSetName = "ByteCollection")] + [System.Collections.ObjectModel.Collection[byte]] $ByteCollection, + + [Parameter(ParameterSetName = "Default")] + $Value + ) + } + + function NotNullFunc { + param( + [ValidateNotNull()] + $Value, + [string] $TestType + ) + + switch ($TestType) { + "COM-Enumerable" { $Value | ForEach-Object Name } + "Enumerator" { + $items = foreach ($i in $Value) { $i } + $items -join "," + } + } + } + + function NotNullOrEmptyFunc { + param( + [ValidateNotNullOrEmpty()] + $Value, + [string] $TestType + ) + + switch ($TestType) { + "COM-Enumerable" { $Value | ForEach-Object Name } + "Enumerator" { + $items = foreach ($i in $Value) { $i } + $items -join "," + } + } + } + + $filePath = Join-Path -Path $PSHOME -ChildPath System.Management.Automation.dll + $byteArray = [System.IO.File]::ReadAllBytes($filePath) + $byteList = [System.Collections.Generic.List[byte]] $byteArray + $byteCollection = [System.Collections.ObjectModel.Collection[byte]] $byteArray + ## Use the running time of 'MandatoryFunc -Value $byteArray' as the baseline time + $baseline = (Measure-Command { MandatoryFunc -Value $byteArray }).Milliseconds + ## Running time should be less than 'expected' + $expected = $baseline + 20 + + if ($IsWindows) { + $null = New-Item -Path $TESTDRIVE/file1 + } + + $testCases = @( + @{ ScriptBlock = { MandatoryFunc -ByteArray $byteArray } } + @{ ScriptBlock = { MandatoryFunc -ByteList $byteList } } + @{ ScriptBlock = { MandatoryFunc -ByteCollection $byteCollection } } + @{ ScriptBlock = { NotNullFunc -Value $byteArray } } + @{ ScriptBlock = { NotNullFunc -Value $byteList } } + @{ ScriptBlock = { NotNullFunc -Value $byteCollection } } + @{ ScriptBlock = { NotNullOrEmptyFunc -Value $byteArray } } + @{ ScriptBlock = { NotNullOrEmptyFunc -Value $byteList } } + @{ ScriptBlock = { NotNullOrEmptyFunc -Value $byteCollection } } + ) + } + + It "Validate running time ''" -TestCases $testCases { + param ($ScriptBlock) + (Measure-Command $ScriptBlock).Milliseconds | Should BeLessThan $expected + } + + It "COM enumerable argument should work with 'ValidateNotNull' and 'ValidateNotNullOrEmpty'" -Skip:(!$IsWindows) { + $shell = New-Object -ComObject "Shell.Application" + $folder = $shell.Namespace("$TESTDRIVE") + $items = $folder.Items() + + NotNullFunc -Value $items -TestType "COM-Enumerable" | Should Be "file1" + NotNullOrEmptyFunc -Value $items -TestType "COM-Enumerable" | Should Be "file1" + } + + It "Enumerator argument should work with 'ValidateNotNull' and 'ValidateNotNullOrEmpty'" { + $data = @(1,2,3) + NotNullFunc -Value $data.GetEnumerator() -TestType "Enumerator" | Should Be "1,2,3" + NotNullOrEmptyFunc -Value $data.GetEnumerator() -TestType "Enumerator" | Should Be "1,2,3" + } + + It "'ValidateNotNull' should throw on null element of a collection argument" { + ## Should throw on null element + { NotNullFunc -Value @("string", $null, 2) } | Should Throw + ## Should not throw on empty string element + { NotNullFunc -Value @("string", "", 2) } | Should Not Throw + ## Should not throw on an empty collection + { NotNullFunc -Value @() } | Should Not Throw + } + + It "'ValidateNotNullOrEmpty' should throw on null element of a collection argument or empty collection/dictionary" { + { NotNullOrEmptyFunc -Value @("string", $null, 2) } | Should Throw + { NotNullOrEmptyFunc -Value @("string", "", 2) } | Should Throw + { NotNullOrEmptyFunc -Value @() } | Should Throw + { NotNullOrEmptyFunc -Value @{} } | Should Throw + } + + It "Mandatory parameter should throw on empty collection" { + { MandatoryFunc -ByteArray ([byte[]]@()) } | Should Throw + { MandatoryFunc -ByteList ([System.Collections.Generic.List[byte]]@()) } | Should Throw + { MandatoryFunc -ByteList ([System.Collections.ObjectModel.Collection[byte]]@()) } | Should Throw + } + } }