Skip to content

Commit

Permalink
Revert RequiredAttribute.DisallowAllDefaultValues (#86378)
Browse files Browse the repository at this point in the history
  • Loading branch information
@eiriktsarpalis
eiriktsarpalis authored May 17, 2023
1 parent 1a598b7 commit bda1f6c
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 159 deletions.
1 change: 0 additions & 1 deletion src/libraries/System.ComponentModel.Annotations/ref/System.ComponentModel.Annotations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ public partial class RequiredAttribute : System.ComponentModel.DataAnnotations.V
{
public RequiredAttribute() { }
public bool AllowEmptyStrings { get { throw null; } set { } }
public bool DisallowAllDefaultValues { get { throw null; } set { } }
public override bool IsValid(object? value) { throw null; }
}
[System.AttributeUsageAttribute(System.AttributeTargets.Field | System.AttributeTargets.Property, AllowMultiple=false)]
Expand Down
58 changes: 0 additions & 58 deletions ...ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/RequiredAttribute.cs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

namespace System.ComponentModel.DataAnnotations
{
/// <summary>
Expand All @@ -31,77 +27,23 @@ public RequiredAttribute()
/// </summary>
public bool AllowEmptyStrings { get; set; }

/// <summary>
/// Gets or sets a flag indicating whether the attribute should also disallow non-null default values.
/// </summary>
public bool DisallowAllDefaultValues { get; set; }

/// <summary>
/// Override of <see cref="ValidationAttribute.IsValid(object)" />
/// </summary>
/// <param name="value">The value to test</param>
/// <returns>
/// Returns <see langword="false" /> if the <paramref name="value" /> is null or an empty string.
/// If <see cref="AllowEmptyStrings" /> then <see langword="true" /> is returned for empty strings.
/// If <see cref="DisallowAllDefaultValues"/> then <see langword="false" /> is returned for values
/// that are equal to the <see langword="default" /> of the declared type.
/// </returns>
public override bool IsValid(object? value)
=> IsValidCore(value, validationContext: null);

/// <inheritdoc />
protected override ValidationResult? IsValid(object? value, ValidationContext validationContext)
{
return IsValidCore(value, validationContext)
? ValidationResult.Success
: CreateFailedValidationResult(validationContext);
}

private bool IsValidCore(object? value, ValidationContext? validationContext)
{
if (value is null)
{
return false;
}

if (DisallowAllDefaultValues)
{
// To determine the default value of non-nullable types we need the declaring type of the value.
// This is the property type in a validation context falling back to the runtime type for root values.
Type declaringType = validationContext?.MemberType ?? value.GetType();
if (GetDefaultValueForNonNullableValueType(declaringType) is object defaultValue)
{
return !defaultValue.Equals(value);
}
}

// only check string length if empty strings are not allowed
return AllowEmptyStrings || value is not string stringValue || !string.IsNullOrWhiteSpace(stringValue);
}


[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2067:UnrecognizedReflectionPattern",
Justification = "GetUninitializedObject is only called struct types. You can always create an instance of a struct.")]
private object? GetDefaultValueForNonNullableValueType(Type type)
{
object? defaultValue = _defaultValueCache;

if (defaultValue != null && defaultValue.GetType() == type)
{
Debug.Assert(type.IsValueType && Nullable.GetUnderlyingType(type) is null);
}
else if (type.IsValueType && Nullable.GetUnderlyingType(type) is null)
{
_defaultValueCache = defaultValue = RuntimeHelpers.GetUninitializedObject(type);
}
else
{
defaultValue = null;
}

return defaultValue;
}

private object? _defaultValueCache;
}
}
108 changes: 8 additions & 100 deletions ...ntModel.Annotations/tests/System/ComponentModel/DataAnnotations/RequiredAttributeTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ protected override IEnumerable<TestCase> ValidValues()
yield return new TestCase(new RequiredAttribute() { AllowEmptyStrings = true }, " \t \r \n ");
yield return new TestCase(new RequiredAttribute(), new object());

// default value types with DisallowAllDefaultValues turned off
// default value types are always valid
var requiredAttribute = new RequiredAttribute();
yield return new TestCase(requiredAttribute, false);
yield return new TestCase(requiredAttribute, 0);
Expand All @@ -25,88 +25,27 @@ protected override IEnumerable<TestCase> ValidValues()
yield return new TestCase(requiredAttribute, default(DateTime));
yield return new TestCase(requiredAttribute, default(Guid));

// non-default value types with DisallowAllDefaultValues turned on
requiredAttribute = new RequiredAttribute { DisallowAllDefaultValues = true };
// non-default value types are always valid
yield return new TestCase(requiredAttribute, true);
yield return new TestCase(requiredAttribute, 1);
yield return new TestCase(requiredAttribute, 0.1);
yield return new TestCase(requiredAttribute, TimeSpan.MaxValue);
yield return new TestCase(requiredAttribute, DateTime.MaxValue);
yield return new TestCase(requiredAttribute, Guid.Parse("c3436566-4083-4bbe-8b56-f9c278162c4b"));

// reference types with DisallowAllDefaultValues turned on
requiredAttribute = new RequiredAttribute { DisallowAllDefaultValues = true };
yield return new TestCase(requiredAttribute, "SomeString");
yield return new TestCase(requiredAttribute, new object());

// reference types with DisallowAllDefaultValues and AllowEmptyStrings turned on
requiredAttribute = new RequiredAttribute { DisallowAllDefaultValues = true, AllowEmptyStrings = true };
yield return new TestCase(requiredAttribute, "SomeString");
yield return new TestCase(requiredAttribute, string.Empty);
yield return new TestCase(requiredAttribute, new object());
// Populated System.Nullable values are always valid
yield return new TestCase(new RequiredAttribute(), (bool?)false);
yield return new TestCase(new RequiredAttribute(), (int?)0);
yield return new TestCase(new RequiredAttribute(), (Guid?)Guid.Empty);
yield return new TestCase(new RequiredAttribute(), (DateTime?)default(DateTime));
yield return new TestCase(new RequiredAttribute(), (TimeSpan?)default(TimeSpan));
}

protected override IEnumerable<TestCase> InvalidValues()
{
yield return new TestCase(new RequiredAttribute(), null);
yield return new TestCase(new RequiredAttribute() { AllowEmptyStrings = false }, string.Empty);
yield return new TestCase(new RequiredAttribute() { AllowEmptyStrings = false }, " \t \r \n ");

// default values with DisallowAllDefaultValues turned on
var requiredAttribute = new RequiredAttribute { DisallowAllDefaultValues = true };
yield return new TestCase(requiredAttribute, null);
yield return new TestCase(requiredAttribute, false);
yield return new TestCase(requiredAttribute, 0);
yield return new TestCase(requiredAttribute, 0d);
yield return new TestCase(requiredAttribute, default(TimeSpan));
yield return new TestCase(requiredAttribute, default(DateTime));
yield return new TestCase(requiredAttribute, default(Guid));
yield return new TestCase(requiredAttribute, default(StructWithTrivialEquality));
// Structs that are not default but *equal* default should also fail validation.
yield return new TestCase(requiredAttribute, new StructWithTrivialEquality { Value = 42 });

// default value properties with DisallowDefaultValues turned on
requiredAttribute = new RequiredAttribute { DisallowAllDefaultValues = true };
yield return new TestCase(requiredAttribute, null, CreatePropertyContext<object?>());
yield return new TestCase(requiredAttribute, null, CreatePropertyContext<int?>());
yield return new TestCase(requiredAttribute, false, CreatePropertyContext<bool>());
yield return new TestCase(requiredAttribute, 0, CreatePropertyContext<int>());
yield return new TestCase(requiredAttribute, 0d, CreatePropertyContext<double>());
yield return new TestCase(requiredAttribute, default(TimeSpan), CreatePropertyContext<TimeSpan>());
yield return new TestCase(requiredAttribute, default(DateTime), CreatePropertyContext<DateTime>());
yield return new TestCase(requiredAttribute, default(Guid), CreatePropertyContext<Guid>());
yield return new TestCase(requiredAttribute, default(ImmutableArray<int>), CreatePropertyContext<ImmutableArray<int>>());
yield return new TestCase(requiredAttribute, default(StructWithTrivialEquality), CreatePropertyContext<StructWithTrivialEquality>());
// Structs that are not default but *equal* default should also fail validation.
yield return new TestCase(requiredAttribute, new StructWithTrivialEquality { Value = 42 }, CreatePropertyContext<StructWithTrivialEquality>());
}

[Theory]
[MemberData(nameof(GetNonNullDefaultValues))]
public void DefaultValueTypes_OnPolymorphicProperties_SucceedValidation(object defaultValue)
{
var attribute = new RequiredAttribute { DisallowAllDefaultValues = true };
Assert.False(attribute.IsValid(defaultValue)); // Fails validation when no contexts present

// Polymorphic contexts should succeed validation
var polymorphicContext = CreatePropertyContext<object>();
attribute.Validate(defaultValue, polymorphicContext);
Assert.Equal(ValidationResult.Success, attribute.GetValidationResult(defaultValue, polymorphicContext));
}

public static IEnumerable<object[]> GetNonNullDefaultValues()
{
// default value types on polymorphic properties with DisallowDefaultValues turned on

yield return new object[] { false };
yield return new object[] { 0 };
yield return new object[] { 0d };
yield return new object[] { default(TimeSpan) };
yield return new object[] { default(DateTime) };
yield return new object[] { default(Guid) };
yield return new object[] { default(ImmutableArray<int>) };
yield return new object[] { default(StructWithTrivialEquality) };
yield return new object[] { new StructWithTrivialEquality { Value = 42 } };
}

[Fact]
Expand All @@ -119,36 +58,5 @@ public void AllowEmptyStrings_GetSet_ReturnsExpectected()
attribute.AllowEmptyStrings = false;
Assert.False(attribute.AllowEmptyStrings);
}

[Fact]
public void DisallowAllowAllDefaultValues_GetSet_ReturnsExpectected()
{
var attribute = new RequiredAttribute();
Assert.False(attribute.DisallowAllDefaultValues);
attribute.DisallowAllDefaultValues = true;
Assert.True(attribute.DisallowAllDefaultValues);
attribute.DisallowAllDefaultValues = false;
Assert.False(attribute.DisallowAllDefaultValues);
}

private static ValidationContext CreatePropertyContext<T>()
=> new ValidationContext(new GenericPoco<T>()) { MemberName = nameof(GenericPoco<T>.Value) };

public class GenericPoco<T>
{
public T Value { get; set; }
}

/// <summary>
/// Defines a struct where all values are equal.
/// </summary>
public readonly struct StructWithTrivialEquality : IEquatable<StructWithTrivialEquality>
{
public int Value { get; init; }

public bool Equals(StructWithTrivialEquality _) => true;
public override bool Equals(object other) => other is StructWithTrivialEquality;
public override int GetHashCode() => 0;
}
}
}

0 comments on commit bda1f6c

Please sign in to comment.