Skip to content

Commit 7f99d27

Browse files
TurnerjTurnerj
authored
Merge pull request #676 from SmithPlatts/fix-empty-values-cast-hasvaluex
Fix HasValueX incorrectly returning true
2 parents 4cc9d94 + 60dd7f6 commit 7f99d27

15 files changed

+278
-97
lines changed

‎.editorconfig

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ spelling_checkable_types = # NEW
8484
# Default Severity for all .NET Code Style rules below
8585
dotnet_analyzer_diagnostic.severity = warning
8686

87+
# VSSpell001: Spell Check
88+
dotnet_diagnostic.VSSpell001.severity = suggestion
89+
dotnet_diagnostic.VSSpell002.severity = suggestion
90+
8791
##########################################
8892
# Language Rules - .NET
8993
# https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/language-rules
@@ -162,6 +166,7 @@ dotnet_diagnostic.IDE0063.severity = suggestion
162166
csharp_style_namespace_declarations = file_scoped
163167
csharp_style_prefer_method_group_conversion = true # NEW
164168
csharp_style_prefer_top_level_statements = true # NEW
169+
dotnet_diagnostic.IDE0290.severity = suggestion # NEW - Use primary constructor
165170

166171
# Expression-bodied members
167172
csharp_style_expression_bodied_constructors = true
@@ -189,6 +194,11 @@ csharp_style_implicit_object_creation_when_type_is_apparent = true
189194
csharp_style_prefer_null_check_over_type_check = true
190195
csharp_style_prefer_tuple_swap = true # NEW
191196
csharp_style_prefer_utf8_string_literals = true # NEW
197+
dotnet_diagnostic.IDE0028.severity = suggestion # NEW - Use collection initializers
198+
dotnet_diagnostic.IDE0090.severity = suggestion # NEW - Simplify new expression
199+
dotnet_diagnostic.IDE0300.severity = suggestion # NEW - Collection initialization can be simplified
200+
dotnet_diagnostic.IDE0301.severity = suggestion # NEW - Collection initialization can be simplified
201+
dotnet_diagnostic.IDE0305.severity = suggestion # NEW - Collection initialization can be simplified
192202

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

307+
##########################################
308+
# Code Quality Rules - C#
309+
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/
310+
##########################################
311+
312+
[*.{cs,csx,cake}]
313+
314+
# Usage rules
315+
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/usage-warnings
316+
dotnet_diagnostic.CA2249.severity = suggestion # NEW - Consider using String.Contains instead of String.IndexOf
317+
318+
# Performance rules
319+
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/performance-warnings
320+
dotnet_diagnostic.CA1854.severity = suggestion # NEW - Prefer the IDictionary.TryGetValue(TKey, out TValue) method
321+
dotnet_diagnostic.CA1859.severity = suggestion # NEW - Use concrete types when possible for improved performance
322+
dotnet_diagnostic.CA1861.severity = suggestion # NEW - Avoid constant arrays as arguments
323+
dotnet_diagnostic.CA1865.severity = suggestion # NEW - Use 'string.Method(char)' instead of 'string.Method(string)' for string with single char
324+
dotnet_diagnostic.CA1866.severity = suggestion # NEW - Use 'string.Method(char)' instead of 'string.Method(string)' for string with single char
325+
dotnet_diagnostic.CA1867.severity = suggestion # NEW - Use 'string.Method(char)' instead of 'string.Method(string)' for string with single char
326+
dotnet_diagnostic.CA1869.severity = suggestion # NEW - Cache and reuse 'JsonSerializerOptions' instances
327+
297328
##########################################
298329
# .NET Naming Rules
299330
# https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/naming-rules

‎Source/Common/Values{T1,T2,T3,T4,T5,T6,T7}.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -245,21 +245,21 @@ public Values(IEnumerable<object?> items)
245245
}
246246
}
247247

248-
this.HasValue1 = items1?.Count > 0;
249-
this.HasValue2 = items2?.Count > 0;
250-
this.HasValue3 = items3?.Count > 0;
251-
this.HasValue4 = items4?.Count > 0;
252-
this.HasValue5 = items5?.Count > 0;
253-
this.HasValue6 = items6?.Count > 0;
254-
this.HasValue7 = items7?.Count > 0;
255-
256248
this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
257249
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
258250
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
259251
this.Value4 = items4 == null ? default : (OneOrMany<T4>)items4;
260252
this.Value5 = items5 == null ? default : (OneOrMany<T5>)items5;
261253
this.Value6 = items6 == null ? default : (OneOrMany<T6>)items6;
262254
this.Value7 = items7 == null ? default : (OneOrMany<T7>)items7;
255+
256+
this.HasValue1 = this.Value1.Count > 0;
257+
this.HasValue2 = this.Value2.Count > 0;
258+
this.HasValue3 = this.Value3.Count > 0;
259+
this.HasValue4 = this.Value4.Count > 0;
260+
this.HasValue5 = this.Value5.Count > 0;
261+
this.HasValue6 = this.Value6.Count > 0;
262+
this.HasValue7 = this.Value7.Count > 0;
263263
}
264264

265265
/// <summary>

‎Source/Common/Values{T1,T2,T3,T4,T5,T6}.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -204,19 +204,19 @@ public Values(IEnumerable<object?> items)
204204
}
205205
}
206206

207-
this.HasValue1 = items1?.Count > 0;
208-
this.HasValue2 = items2?.Count > 0;
209-
this.HasValue3 = items3?.Count > 0;
210-
this.HasValue4 = items4?.Count > 0;
211-
this.HasValue5 = items5?.Count > 0;
212-
this.HasValue6 = items6?.Count > 0;
213-
214207
this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
215208
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
216209
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
217210
this.Value4 = items4 == null ? default : (OneOrMany<T4>)items4;
218211
this.Value5 = items5 == null ? default : (OneOrMany<T5>)items5;
219212
this.Value6 = items6 == null ? default : (OneOrMany<T6>)items6;
213+
214+
this.HasValue1 = this.Value1.Count > 0;
215+
this.HasValue2 = this.Value2.Count > 0;
216+
this.HasValue3 = this.Value3.Count > 0;
217+
this.HasValue4 = this.Value4.Count > 0;
218+
this.HasValue5 = this.Value5.Count > 0;
219+
this.HasValue6 = this.Value6.Count > 0;
220220
}
221221

222222
/// <summary>

‎Source/Common/Values{T1,T2,T3,T4,T5}.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,17 +167,17 @@ public Values(IEnumerable<object?> items)
167167
}
168168
}
169169

170-
this.HasValue1 = items1?.Count > 0;
171-
this.HasValue2 = items2?.Count > 0;
172-
this.HasValue3 = items3?.Count > 0;
173-
this.HasValue4 = items4?.Count > 0;
174-
this.HasValue5 = items5?.Count > 0;
175-
176170
this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
177171
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
178172
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
179173
this.Value4 = items4 == null ? default : (OneOrMany<T4>)items4;
180174
this.Value5 = items5 == null ? default : (OneOrMany<T5>)items5;
175+
176+
this.HasValue1 = this.Value1.Count > 0;
177+
this.HasValue2 = this.Value2.Count > 0;
178+
this.HasValue3 = this.Value3.Count > 0;
179+
this.HasValue4 = this.Value4.Count > 0;
180+
this.HasValue5 = this.Value5.Count > 0;
181181
}
182182

183183
/// <summary>

‎Source/Common/Values{T1,T2,T3,T4}.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,15 +134,15 @@ public Values(IEnumerable<object?> items)
134134
}
135135
}
136136

137-
this.HasValue1 = items1?.Count > 0;
138-
this.HasValue2 = items2?.Count > 0;
139-
this.HasValue3 = items3?.Count > 0;
140-
this.HasValue4 = items4?.Count > 0;
141-
142137
this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
143138
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
144139
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
145140
this.Value4 = items4 == null ? default : (OneOrMany<T4>)items4;
141+
142+
this.HasValue1 = this.Value1.Count > 0;
143+
this.HasValue2 = this.Value2.Count > 0;
144+
this.HasValue3 = this.Value3.Count > 0;
145+
this.HasValue4 = this.Value4.Count > 0;
146146
}
147147

148148
/// <summary>

‎Source/Common/Values{T1,T2,T3}.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,13 @@ public Values(IEnumerable<object> items)
105105
}
106106
}
107107

108-
this.HasValue1 = items1?.Count > 0;
109-
this.HasValue2 = items2?.Count > 0;
110-
this.HasValue3 = items3?.Count > 0;
111-
112108
this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
113109
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
114110
this.Value3 = items3 == null ? default : (OneOrMany<T3>)items3;
111+
112+
this.HasValue1 = this.Value1.Count > 0;
113+
this.HasValue2 = this.Value2.Count > 0;
114+
this.HasValue3 = this.Value3.Count > 0;
115115
}
116116

117117
/// <summary>

‎Source/Common/Values{T1,T2}.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ public Values(IEnumerable<object?> items)
8080
}
8181
}
8282

83-
this.HasValue1 = items1?.Count > 0;
84-
this.HasValue2 = items2?.Count > 0;
85-
8683
this.Value1 = items1 == null ? default : (OneOrMany<T1>)items1;
8784
this.Value2 = items2 == null ? default : (OneOrMany<T2>)items2;
85+
86+
this.HasValue1 = this.Value1.Count > 0;
87+
this.HasValue2 = this.Value2.Count > 0;
8888
}
8989

9090
/// <summary>

‎Tests/Schema.NET.Test/AssertEx.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
namespace Schema.NET.Test;
2+
3+
using System.Collections;
4+
using Xunit;
5+
6+
/// <summary>
7+
/// Provides extended methods and helpers to the Xunit.Assert class.
8+
/// </summary>
9+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "Name needs to be short to ensure readability of consuming code.")]
10+
public static class AssertEx
11+
{
12+
/// <summary>
13+
/// Verifies that a <see cref="OneOrMany{T}"/> collection is empty.
14+
/// </summary>
15+
/// <typeparam name="T">The type of the values.</typeparam>
16+
/// <inheritdoc cref="Assert.Empty(IEnumerable)"/>
17+
public static void Empty<T>(OneOrMany<T> collection) => Assert.Empty(collection);
18+
}

‎Tests/Schema.NET.Test/OneOrManyTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public void Constructor_NullList_ThrowsArgumentNullException() =>
6060
public void Count_DefaultStructConstructor_ReturnsZero() => Assert.Empty(default(OneOrMany<int>));
6161

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

6565
[Fact]
6666
public void Count_NullItem_ReturnsZero() => Assert.Empty(new OneOrMany<int?>((int?)null));

‎Tests/Schema.NET.Test/Values2Test.cs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public void Constructor_Value1Passed_OnlyValue1HasValue()
1515
Assert.True(values.HasValue1);
1616
Assert.Single(values.Value1);
1717
Assert.False(values.HasValue2);
18-
Assert.Empty(values.Value2!);
18+
AssertEx.Empty(values.Value2);
1919
Assert.Equal(new List<object>() { 1 }, values.Cast<object>().ToList());
2020
}
2121

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

46+
[Fact]
47+
public void Constructor_StringItems_NullOrWhitespaceDoesntHaveValue()
48+
{
49+
object[] nullOrWhitespaceValues = new[]
50+
{
51+
string.Empty,
52+
null!,
53+
"\u2028 \u2029 \u0009 \u000A \u000B \u000C \u000D \u0085",
54+
};
55+
var values = new Values<int, string>(nullOrWhitespaceValues);
56+
57+
Assert.False(values.HasValue1);
58+
Assert.Empty(values.Value1);
59+
Assert.False(values.HasValue2, $"{nameof(values.HasValue2)}: Expected: False, Actual: True");
60+
AssertEx.Empty(values.Value2);
61+
}
62+
4663
[Fact]
4764
public void Constructor_NullList_ThrowsArgumentNullException() =>
4865
Assert.Throws<ArgumentNullException>(() => new Values<int, string>((List<object>)null!));
@@ -88,7 +105,7 @@ public void ImplicitConversionOperator_Value1Passed_OnlyValue1HasValue()
88105
Assert.True(values.HasValue1);
89106
Assert.Single(values.Value1);
90107
Assert.False(values.HasValue2);
91-
Assert.Empty(values.Value2!);
108+
AssertEx.Empty(values.Value2);
92109
Assert.Equal(new List<object>() { 1 }, values.Cast<object>().ToList());
93110
}
94111

@@ -112,7 +129,7 @@ public void ImplicitConversionOperator_Value1ListPassed_OnlyValue1HasValue()
112129
Assert.True(values.HasValue1);
113130
Assert.Equal(2, values.Value1.Count);
114131
Assert.False(values.HasValue2);
115-
Assert.Empty(values.Value2!);
132+
AssertEx.Empty(values.Value2);
116133
Assert.Equal(new List<object>() { 1, 2 }, values.Cast<object>().ToList());
117134
}
118135

‎Tests/Schema.NET.Test/Values3Test.cs

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public void Constructor_Value1Passed_OnlyValue1HasValue()
1515
Assert.True(values.HasValue1);
1616
Assert.Single(values.Value1);
1717
Assert.False(values.HasValue2);
18-
Assert.Empty(values.Value2!);
18+
AssertEx.Empty(values.Value2);
1919
Assert.Equal(new List<object>() { 1 }, values.Cast<object>().ToList());
2020
}
2121

@@ -39,7 +39,7 @@ public void Constructor_Value3Passed_OnlyValue3HasValue()
3939
Assert.False(values.HasValue1);
4040
Assert.Empty(values.Value1);
4141
Assert.False(values.HasValue2);
42-
Assert.Empty(values.Value2!);
42+
AssertEx.Empty(values.Value2);
4343
Assert.True(values.HasValue3);
4444
Assert.Single(values.Value3);
4545
Assert.Equal(new List<object>() { DayOfWeek.Friday }, values.Cast<object>().ToList());
@@ -59,6 +59,25 @@ public void Constructor_Items_HasAllItems()
5959
Assert.Equal(new List<object>() { 1, "Foo", DayOfWeek.Friday }, values.Cast<object>().ToList());
6060
}
6161

62+
[Fact]
63+
public void Constructor_StringItems_NullOrWhitespaceDoesntHaveValue()
64+
{
65+
object[] nullOrWhitespaceValues = new[]
66+
{
67+
string.Empty,
68+
null!,
69+
"\u2028 \u2029 \u0009 \u000A \u000B \u000C \u000D \u0085",
70+
};
71+
var values = new Values<int, string, DayOfWeek>(nullOrWhitespaceValues);
72+
73+
Assert.False(values.HasValue1);
74+
Assert.Empty(values.Value1);
75+
Assert.False(values.HasValue2, $"{nameof(values.HasValue2)}: Expected: False, Actual: True");
76+
AssertEx.Empty(values.Value2);
77+
Assert.False(values.HasValue3);
78+
Assert.Empty(values.Value3);
79+
}
80+
6281
[Fact]
6382
public void Constructor_NullList_ThrowsArgumentNullException() =>
6483
Assert.Throws<ArgumentNullException>(() => new Values<int, string, DayOfWeek>((List<object>)null!));
@@ -119,7 +138,7 @@ public void ImplicitConversionOperator_Value1Passed_OnlyValue1HasValue()
119138
Assert.True(values.HasValue1);
120139
Assert.Single(values.Value1);
121140
Assert.False(values.HasValue2);
122-
Assert.Empty(values.Value2!);
141+
AssertEx.Empty(values.Value2);
123142
Assert.Equal(new List<object>() { 1 }, values.Cast<object>().ToList());
124143
}
125144

@@ -143,7 +162,7 @@ public void ImplicitConversionOperator_Value3Passed_OnlyValue3HasValue()
143162
Assert.False(values.HasValue1);
144163
Assert.Empty(values.Value1);
145164
Assert.False(values.HasValue2);
146-
Assert.Empty(values.Value2!);
165+
AssertEx.Empty(values.Value2);
147166
Assert.True(values.HasValue3);
148167
Assert.Single(values.Value3);
149168
Assert.Equal(new List<object>() { DayOfWeek.Friday }, values.Cast<object>().ToList());
@@ -157,7 +176,7 @@ public void ImplicitConversionOperator_Value1CollectionPassed_OnlyValue1HasValue
157176
Assert.True(values.HasValue1);
158177
Assert.Equal(2, values.Value1.Count);
159178
Assert.False(values.HasValue2);
160-
Assert.Empty(values.Value2!);
179+
AssertEx.Empty(values.Value2);
161180
Assert.Equal(new List<object>() { 1, 2 }, values.Cast<object>().ToList());
162181
}
163182

@@ -181,7 +200,7 @@ public void ImplicitConversionOperator_Value3CollectionPassed_OnlyValue3HasValue
181200
Assert.False(values.HasValue1);
182201
Assert.Empty(values.Value1);
183202
Assert.False(values.HasValue2);
184-
Assert.Empty(values.Value2!);
203+
AssertEx.Empty(values.Value2);
185204
Assert.True(values.HasValue3);
186205
Assert.Equal(2, values.Value3.Count);
187206
Assert.Equal(new List<object>() { DayOfWeek.Friday, DayOfWeek.Monday }, values.Cast<object>().ToList());

0 commit comments

Comments
 (0)