admin/PowerShell-PowerShell
1
0
Fork 0
mirror of https://github.com/PowerShell/PowerShell synced 2026-06-08 12:12:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Skip null-element check for collections with a value-type element type (#5432)

Browse Source 
* Fix NotNullOrEmpty check logic

* Fix a test issue on Unix
This commit is contained in:
Dongbo Wang
2017-11-30 14:46:09 -08:00
committed by Aditya Patwardhan
parent 52df947080
commit b5f84c2b39
5 changed files with 206 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/System.Management.Automation/engine/Attributes.cs
+68 -59
View File
@@ -1865,11 +1865,40 @@ namespace System.Management.Automation
#endregion
#region NULL validation attributes
/// <summary>
/// Base type of Null Validation attributes.
/// </summary>
public abstract class NullValidationAttributeBase : ValidateArgumentsAttribute
{
/// <summary>
/// Check if the argument type is a collection.
/// </summary>
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;
}
}
}
/// <summary>
/// Validates that the parameters's argument is not null
/// </summary>
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
public sealed class ValidateNotNullAttribute : ValidateArgumentsAttribute
public sealed class ValidateNotNullAttribute : NullValidationAttributeBase
{
/// <summary>
/// Verifies the argument is not null and if it is a collection, that each
@@ -1891,9 +1920,6 @@ namespace System.Management.Automation
/// </exception>
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.
/// </summary>
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
public sealed class ValidateNotNullOrEmptyAttribute : ValidateArgumentsAttribute
public sealed class ValidateNotNullOrEmptyAttribute : NullValidationAttributeBase
{
/// <summary>
/// Validates that the parameters's argument is not null, is not
@@ -1954,10 +1973,6 @@ namespace System.Management.Automation
/// </exception>
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",
src/System.Management.Automation/engine/CompiledCommandParameter.cs
+1 -1
View File
@@ -698,7 +698,7 @@ namespace System.Management.Automation
/// <summary>
/// The type of the elements in the collection
/// </summary>
internal Type ElementType { get; set; }
internal Type ElementType { get; private set; }
}
}
src/System.Management.Automation/engine/ParameterBinderBase.cs
+17 -9
View File
@@ -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)
src/System.Management.Automation/engine/runtime/Binding/Binders.cs
-1
View File
@@ -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:
test/powershell/engine/Basic/ValidateAttributes.Tests.ps1
+120
View File
@@ -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 '<ScriptBlock>'" -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
}
}
}