Skip to content

Commit

Permalink
Merge pull request RehanSaeed#676 from SmithPlatts/fix-empty-values-c…
Browse files Browse the repository at this point in the history 
…ast-hasvaluex

Fix HasValueX incorrectly returning true
  • Loading branch information
@Turnerj
Turnerj authored Dec 17, 2023
2 parents 4cc9d94 + 60dd7f6 commit 7f99d27
Show file tree
Hide file tree
Showing 15 changed files with 278 additions and 97 deletions.
31 changes: 31 additions & 0 deletions .editorconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ spelling_checkable_types = # NEW
# Default Severity for all .NET Code Style rules below
dotnet_analyzer_diagnostic.severity = warning

# VSSpell001: Spell Check
dotnet_diagnostic.VSSpell001.severity = suggestion
dotnet_diagnostic.VSSpell002.severity = suggestion

##########################################
# Language Rules - .NET
# https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/language-rules
Expand Down Expand Up @@ -162,6 +166,7 @@ dotnet_diagnostic.IDE0063.severity = suggestion
csharp_style_namespace_declarations = file_scoped
csharp_style_prefer_method_group_conversion = true # NEW
csharp_style_prefer_top_level_statements = true # NEW
dotnet_diagnostic.IDE0290.severity = suggestion # NEW - Use primary constructor

# Expression-bodied members
csharp_style_expression_bodied_constructors = true
Expand Down Expand Up @@ -189,6 +194,11 @@ csharp_style_implicit_object_creation_when_type_is_apparent = true
csharp_style_prefer_null_check_over_type_check = true
csharp_style_prefer_tuple_swap = true # NEW
csharp_style_prefer_utf8_string_literals = true # NEW
dotnet_diagnostic.IDE0028.severity = suggestion # NEW - Use collection initializers
dotnet_diagnostic.IDE0090.severity = suggestion # NEW - Simplify new expression
dotnet_diagnostic.IDE0300.severity = suggestion # NEW - Collection initialization can be simplified
dotnet_diagnostic.IDE0301.severity = suggestion # NEW - Collection initialization can be simplified
dotnet_diagnostic.IDE0305.severity = suggestion # NEW - Collection initialization can be simplified

# Modifier preferences
csharp_preferred_modifier_order = public,private,protected,internal,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,volatile,async
Expand Down Expand Up @@ -294,6 +304,27 @@ csharp_space_between_square_brackets = false
csharp_preserve_single_line_statements = false
csharp_preserve_single_line_blocks = true

##########################################
# Code Quality Rules - C#
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/
##########################################

[*.{cs,csx,cake}]

# Usage rules
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/usage-warnings
dotnet_diagnostic.CA2249.severity = suggestion # NEW - Consider using String.Contains instead of String.IndexOf

# Performance rules
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/performance-warnings
dotnet_diagnostic.CA1854.severity = suggestion # NEW - Prefer the IDictionary.TryGetValue(TKey, out TValue) method
dotnet_diagnostic.CA1859.severity = suggestion # NEW - Use concrete types when possible for improved performance
dotnet_diagnostic.CA1861.severity = suggestion # NEW - Avoid constant arrays as arguments
dotnet_diagnostic.CA1865.severity = suggestion # NEW - Use 'string.Method(char)' instead of 'string.Method(string)' for string with single char
dotnet_diagnostic.CA1866.severity = suggestion # NEW - Use 'string.Method(char)' instead of 'string.Method(string)' for string with single char
dotnet_diagnostic.CA1867.severity = suggestion # NEW - Use 'string.Method(char)' instead of 'string.Method(string)' for string with single char
dotnet_diagnostic.CA1869.severity = suggestion # NEW - Cache and reuse 'JsonSerializerOptions' instances

##########################################
# .NET Naming Rules
# https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/naming-rules
Expand Down
16 changes: 8 additions & 8 deletions Source/Common/Values{T1,T2,T3,T4,T5,T6,T7}.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,21 +245,21 @@ public Values(IEnumerable<object?> items)
}
}

this.HasValue1 = items1?.Count > 0;
this.HasValue2 = items2?.Count > 0;
this.HasValue3 = items3?.Count > 0;
this.HasValue4 = items4?.Count > 0;
this.HasValue5 = items5?.Count > 0;
this.HasValue6 = items6?.Count > 0;
this.HasValue7 = items7?.Count > 0;

this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
this.Value4 = items4 == null ? default : (OneOrMany<T4>)items4;
this.Value5 = items5 == null ? default : (OneOrMany<T5>)items5;
this.Value6 = items6 == null ? default : (OneOrMany<T6>)items6;
this.Value7 = items7 == null ? default : (OneOrMany<T7>)items7;

this.HasValue1 = this.Value1.Count > 0;
this.HasValue2 = this.Value2.Count > 0;
this.HasValue3 = this.Value3.Count > 0;
this.HasValue4 = this.Value4.Count > 0;
this.HasValue5 = this.Value5.Count > 0;
this.HasValue6 = this.Value6.Count > 0;
this.HasValue7 = this.Value7.Count > 0;
}

/// <summary>
Expand Down
14 changes: 7 additions & 7 deletions Source/Common/Values{T1,T2,T3,T4,T5,T6}.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,19 @@ public Values(IEnumerable<object?> items)
}
}

this.HasValue1 = items1?.Count > 0;
this.HasValue2 = items2?.Count > 0;
this.HasValue3 = items3?.Count > 0;
this.HasValue4 = items4?.Count > 0;
this.HasValue5 = items5?.Count > 0;
this.HasValue6 = items6?.Count > 0;

this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
this.Value4 = items4 == null ? default : (OneOrMany<T4>)items4;
this.Value5 = items5 == null ? default : (OneOrMany<T5>)items5;
this.Value6 = items6 == null ? default : (OneOrMany<T6>)items6;

this.HasValue1 = this.Value1.Count > 0;
this.HasValue2 = this.Value2.Count > 0;
this.HasValue3 = this.Value3.Count > 0;
this.HasValue4 = this.Value4.Count > 0;
this.HasValue5 = this.Value5.Count > 0;
this.HasValue6 = this.Value6.Count > 0;
}

/// <summary>
Expand Down
12 changes: 6 additions & 6 deletions Source/Common/Values{T1,T2,T3,T4,T5}.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,17 +167,17 @@ public Values(IEnumerable<object?> items)
}
}

this.HasValue1 = items1?.Count > 0;
this.HasValue2 = items2?.Count > 0;
this.HasValue3 = items3?.Count > 0;
this.HasValue4 = items4?.Count > 0;
this.HasValue5 = items5?.Count > 0;

this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
this.Value4 = items4 == null ? default : (OneOrMany<T4>)items4;
this.Value5 = items5 == null ? default : (OneOrMany<T5>)items5;

this.HasValue1 = this.Value1.Count > 0;
this.HasValue2 = this.Value2.Count > 0;
this.HasValue3 = this.Value3.Count > 0;
this.HasValue4 = this.Value4.Count > 0;
this.HasValue5 = this.Value5.Count > 0;
}

/// <summary>
Expand Down
10 changes: 5 additions & 5 deletions Source/Common/Values{T1,T2,T3,T4}.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,15 @@ public Values(IEnumerable<object?> items)
}
}

this.HasValue1 = items1?.Count > 0;
this.HasValue2 = items2?.Count > 0;
this.HasValue3 = items3?.Count > 0;
this.HasValue4 = items4?.Count > 0;

this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
this.Value4 = items4 == null ? default : (OneOrMany<T4>)items4;

this.HasValue1 = this.Value1.Count > 0;
this.HasValue2 = this.Value2.Count > 0;
this.HasValue3 = this.Value3.Count > 0;
this.HasValue4 = this.Value4.Count > 0;
}

/// <summary>
Expand Down
8 changes: 4 additions & 4 deletions Source/Common/Values{T1,T2,T3}.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ public Values(IEnumerable<object> items)
}
}

this.HasValue1 = items1?.Count > 0;
this.HasValue2 = items2?.Count > 0;
this.HasValue3 = items3?.Count > 0;

this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;

this.HasValue1 = this.Value1.Count > 0;
this.HasValue2 = this.Value2.Count > 0;
this.HasValue3 = this.Value3.Count > 0;
}

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions Source/Common/Values{T1,T2}.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ public Values(IEnumerable<object?> items)
}
}

this.HasValue1 = items1?.Count > 0;
this.HasValue2 = items2?.Count > 0;

this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;

this.HasValue1 = this.Value1.Count > 0;
this.HasValue2 = this.Value2.Count > 0;
}

/// <summary>
Expand Down
18 changes: 18 additions & 0 deletions Tests/Schema.NET.Test/AssertEx.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
namespace Schema.NET.Test;

using System.Collections;
using Xunit;

/// <summary>
/// Provides extended methods and helpers to the Xunit.Assert class.
/// </summary>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "Name needs to be short to ensure readability of consuming code.")]
public static class AssertEx
{
/// <summary>
/// Verifies that a <see cref="OneOrMany{T}"/> collection is empty.
/// </summary>
/// <typeparam name="T">The type of the values.</typeparam>
/// <inheritdoc cref="Assert.Empty(IEnumerable)"/>
public static void Empty<T>(OneOrMany<T> collection) => Assert.Empty(collection);
}
2 changes: 1 addition & 1 deletion Tests/Schema.NET.Test/OneOrManyTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void Constructor_NullList_ThrowsArgumentNullException() =>
public void Count_DefaultStructConstructor_ReturnsZero() => Assert.Empty(default(OneOrMany<int>));

[Fact]
public void Count_DefaultClassConstructor_ReturnsZero() => Assert.Empty(default(OneOrMany<string>)!);
public void Count_DefaultClassConstructor_ReturnsZero() => AssertEx.Empty(default(OneOrMany<string>));

[Fact]
public void Count_NullItem_ReturnsZero() => Assert.Empty(new OneOrMany<int?>((int?)null));
Expand Down
23 changes: 20 additions & 3 deletions Tests/Schema.NET.Test/Values2Test.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public void Constructor_Value1Passed_OnlyValue1HasValue()
Assert.True(values.HasValue1);
Assert.Single(values.Value1);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.Equal(new List<object>() { 1 }, values.Cast<object>().ToList());
}

Expand Down Expand Up @@ -43,6 +43,23 @@ public void Constructor_Items_HasAllItems()
Assert.Equal(new List<object>() { 1, "Foo" }, values.Cast<object>().ToList());
}

[Fact]
public void Constructor_StringItems_NullOrWhitespaceDoesntHaveValue()
{
object[] nullOrWhitespaceValues = new[]
{
string.Empty,
null!,
"\u2028 \u2029 \u0009 \u000A \u000B \u000C \u000D \u0085",
};
var values = new Values<int, string>(nullOrWhitespaceValues);

Assert.False(values.HasValue1);
Assert.Empty(values.Value1);
Assert.False(values.HasValue2, $"{nameof(values.HasValue2)}: Expected: False, Actual: True");
AssertEx.Empty(values.Value2);
}

[Fact]
public void Constructor_NullList_ThrowsArgumentNullException() =>
Assert.Throws<ArgumentNullException>(() => new Values<int, string>((List<object>)null!));
Expand Down Expand Up @@ -88,7 +105,7 @@ public void ImplicitConversionOperator_Value1Passed_OnlyValue1HasValue()
Assert.True(values.HasValue1);
Assert.Single(values.Value1);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.Equal(new List<object>() { 1 }, values.Cast<object>().ToList());
}

Expand All @@ -112,7 +129,7 @@ public void ImplicitConversionOperator_Value1ListPassed_OnlyValue1HasValue()
Assert.True(values.HasValue1);
Assert.Equal(2, values.Value1.Count);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.Equal(new List<object>() { 1, 2 }, values.Cast<object>().ToList());
}

Expand Down
31 changes: 25 additions & 6 deletions Tests/Schema.NET.Test/Values3Test.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public void Constructor_Value1Passed_OnlyValue1HasValue()
Assert.True(values.HasValue1);
Assert.Single(values.Value1);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.Equal(new List<object>() { 1 }, values.Cast<object>().ToList());
}

Expand All @@ -39,7 +39,7 @@ public void Constructor_Value3Passed_OnlyValue3HasValue()
Assert.False(values.HasValue1);
Assert.Empty(values.Value1);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.True(values.HasValue3);
Assert.Single(values.Value3);
Assert.Equal(new List<object>() { DayOfWeek.Friday }, values.Cast<object>().ToList());
Expand All @@ -59,6 +59,25 @@ public void Constructor_Items_HasAllItems()
Assert.Equal(new List<object>() { 1, "Foo", DayOfWeek.Friday }, values.Cast<object>().ToList());
}

[Fact]
public void Constructor_StringItems_NullOrWhitespaceDoesntHaveValue()
{
object[] nullOrWhitespaceValues = new[]
{
string.Empty,
null!,
"\u2028 \u2029 \u0009 \u000A \u000B \u000C \u000D \u0085",
};
var values = new Values<int, string, DayOfWeek>(nullOrWhitespaceValues);

Assert.False(values.HasValue1);
Assert.Empty(values.Value1);
Assert.False(values.HasValue2, $"{nameof(values.HasValue2)}: Expected: False, Actual: True");
AssertEx.Empty(values.Value2);
Assert.False(values.HasValue3);
Assert.Empty(values.Value3);
}

[Fact]
public void Constructor_NullList_ThrowsArgumentNullException() =>
Assert.Throws<ArgumentNullException>(() => new Values<int, string, DayOfWeek>((List<object>)null!));
Expand Down Expand Up @@ -119,7 +138,7 @@ public void ImplicitConversionOperator_Value1Passed_OnlyValue1HasValue()
Assert.True(values.HasValue1);
Assert.Single(values.Value1);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.Equal(new List<object>() { 1 }, values.Cast<object>().ToList());
}

Expand All @@ -143,7 +162,7 @@ public void ImplicitConversionOperator_Value3Passed_OnlyValue3HasValue()
Assert.False(values.HasValue1);
Assert.Empty(values.Value1);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.True(values.HasValue3);
Assert.Single(values.Value3);
Assert.Equal(new List<object>() { DayOfWeek.Friday }, values.Cast<object>().ToList());
Expand All @@ -157,7 +176,7 @@ public void ImplicitConversionOperator_Value1CollectionPassed_OnlyValue1HasValue
Assert.True(values.HasValue1);
Assert.Equal(2, values.Value1.Count);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.Equal(new List<object>() { 1, 2 }, values.Cast<object>().ToList());
}

Expand All @@ -181,7 +200,7 @@ public void ImplicitConversionOperator_Value3CollectionPassed_OnlyValue3HasValue
Assert.False(values.HasValue1);
Assert.Empty(values.Value1);
Assert.False(values.HasValue2);
Assert.Empty(values.Value2!);
AssertEx.Empty(values.Value2);
Assert.True(values.HasValue3);
Assert.Equal(2, values.Value3.Count);
Assert.Equal(new List<object>() { DayOfWeek.Friday, DayOfWeek.Monday }, values.Cast<object>().ToList());
Expand Down
Loading

0 comments on commit 7f99d27

Please sign in to comment.