Skip to content

Commit e03e765

Browse files
committed
Added compiler warning and code fix for complex value objects having strings
1 parent 5397088 commit e03e765

File tree

16 files changed

+230
-19
lines changed

16 files changed

+230
-19
lines changed

samples/Thinktecture.Runtime.Extensions.Samples/ValueObjects/ComplexValueObjectWithCustomEqualityComparison.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
using System;
2+
13
namespace Thinktecture.ValueObjects;
24

3-
[ComplexValueObject(SkipToString = true)]
5+
[ComplexValueObject(SkipToString = true, DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
46
public partial class ComplexValueObjectWithCustomEqualityComparison
57
{
68
[ValueObjectMemberEqualityComparer<ComparerAccessors.StringOrdinal, string>]

src/Thinktecture.Runtime.Extensions.SourceGenerator/CodeAnalysis/CodeFixes/ThinktectureRuntimeExtensionsCodeFixProvider.cs

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ public sealed class ThinktectureRuntimeExtensionsCodeFixProvider : CodeFixProvid
3131
DiagnosticsDescriptors.InnerEnumOnNonFirstLevelMustBePublic.Id,
3232
DiagnosticsDescriptors.EnumWithoutDerivedTypesMustBeSealed.Id,
3333
DiagnosticsDescriptors.InitAccessorMustBePrivate.Id,
34-
DiagnosticsDescriptors.StringBaseValueObjectNeedsEqualityComparer.Id
34+
DiagnosticsDescriptors.StringBaseValueObjectNeedsEqualityComparer.Id,
35+
DiagnosticsDescriptors.ComplexValueObjectWithStringMembersNeedsDefaultEqualityComparer.Id
3536
];
3637

3738
/// <inheritdoc />
@@ -91,7 +92,11 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
9192
}
9293
else if (diagnostic.Id == DiagnosticsDescriptors.StringBaseValueObjectNeedsEqualityComparer.Id)
9394
{
94-
context.RegisterCodeFix(CodeAction.Create(_DEFINE_VALUE_OBJECT_EQUALITY_COMPARER, t => AddValueObjectKeyMemberEqualityComparerAttribute(context.Document, root, GetCodeFixesContext().TypeDeclaration, t), _DEFINE_VALUE_OBJECT_EQUALITY_COMPARER), diagnostic);
95+
context.RegisterCodeFix(CodeAction.Create(_DEFINE_VALUE_OBJECT_EQUALITY_COMPARER, t => AddValueObjectKeyMemberEqualityComparerAttributeAsync(context.Document, root, GetCodeFixesContext().TypeDeclaration, t), _DEFINE_VALUE_OBJECT_EQUALITY_COMPARER), diagnostic);
96+
}
97+
else if (diagnostic.Id == DiagnosticsDescriptors.ComplexValueObjectWithStringMembersNeedsDefaultEqualityComparer.Id)
98+
{
99+
context.RegisterCodeFix(CodeAction.Create(_DEFINE_VALUE_OBJECT_EQUALITY_COMPARER, t => AddDefaultStringComparisonAsync(context.Document, root, GetCodeFixesContext().TypeDeclaration, t), _DEFINE_VALUE_OBJECT_EQUALITY_COMPARER), diagnostic);
95100
}
96101
}
97102
}
@@ -252,7 +257,7 @@ private static async Task<Document> AddCreateInvalidItemAsync(
252257
return newDoc;
253258
}
254259

255-
private static async Task<Document> AddValueObjectKeyMemberEqualityComparerAttribute(
260+
private static async Task<Document> AddValueObjectKeyMemberEqualityComparerAttributeAsync(
256261
Document document,
257262
SyntaxNode root,
258263
TypeDeclarationSyntax? declaration,
@@ -269,7 +274,7 @@ private static async Task<Document> AddValueObjectKeyMemberEqualityComparerAttri
269274
var valueObjectType = model.GetDeclaredSymbol(declaration);
270275

271276
if (!valueObjectType.IsValueObjectType(out var valueObjectAttribute)
272-
|| valueObjectAttribute.AttributeClass?.TypeArguments.Length != 1)
277+
|| valueObjectAttribute.AttributeClass?.IsKeyedValueObjectAttribute() != true)
273278
{
274279
return document;
275280
}
@@ -297,6 +302,47 @@ private static async Task<Document> AddValueObjectKeyMemberEqualityComparerAttri
297302
return newDoc;
298303
}
299304

305+
private static async Task<Document> AddDefaultStringComparisonAsync(
306+
Document document,
307+
SyntaxNode root,
308+
TypeDeclarationSyntax? declaration,
309+
CancellationToken cancellationToken)
310+
{
311+
if (declaration is null)
312+
return document;
313+
314+
var model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
315+
316+
if (model is null)
317+
return document;
318+
319+
var valueObjectType = model.GetDeclaredSymbol(declaration);
320+
321+
if (!valueObjectType.IsValueObjectType(out var valueObjectAttribute)
322+
|| valueObjectAttribute.ApplicationSyntaxReference is null
323+
|| valueObjectAttribute.AttributeClass?.IsComplexValueObjectAttribute() != true)
324+
{
325+
return document;
326+
}
327+
328+
var attributeSyntax = await valueObjectAttribute.ApplicationSyntaxReference.GetSyntaxAsync(cancellationToken) as AttributeSyntax;
329+
330+
if (attributeSyntax is null)
331+
{
332+
return document;
333+
}
334+
335+
var newAttribute = attributeSyntax.AddArgumentListArguments(
336+
SyntaxFactory.AttributeArgument(SyntaxFactory.ParseExpression("StringComparison.OrdinalIgnoreCase"))
337+
.WithNameEquals(SyntaxFactory.NameEquals("DefaultStringComparison")));
338+
339+
var newDeclaration = declaration.ReplaceNode(attributeSyntax, newAttribute);
340+
var newRoot = root.ReplaceNode(declaration, newDeclaration);
341+
var newDoc = document.WithSyntaxRoot(newRoot);
342+
343+
return newDoc;
344+
}
345+
300346
private static ThrowStatementSyntax BuildThrowNotImplementedException()
301347
{
302348
var notImplementedExceptionType = SyntaxFactory.ParseTypeName($"global::System.{nameof(NotImplementedException)}");

src/Thinktecture.Runtime.Extensions.SourceGenerator/CodeAnalysis/Constants.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public static class Properties
5252
public const string KEY_MEMBER_ACCESS_MODIFIER = "KeyMemberAccessModifier";
5353
public const string KEY_MEMBER_KIND = "KeyMemberKind";
5454
public const string CONSTRUCTOR_ACCESS_MODIFIER = "ConstructorAccessModifier";
55+
public const string DEFAULT_STRING_COMPARISON = "DefaultStringComparison";
5556
}
5657

5758
public static class ValueObject

src/Thinktecture.Runtime.Extensions.SourceGenerator/CodeAnalysis/Diagnostics/ThinktectureRuntimeExtensionsAnalyzer.cs

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ public sealed class ThinktectureRuntimeExtensionsAnalyzer : DiagnosticAnalyzer
4444
DiagnosticsDescriptors.CustomKeyMemberImplementationTypeMismatch,
4545
DiagnosticsDescriptors.IndexBasedSwitchAndMapMustUseNamedParameters,
4646
DiagnosticsDescriptors.VariableMustBeInitializedWithNonDefaultValue,
47-
DiagnosticsDescriptors.StringBaseValueObjectNeedsEqualityComparer
47+
DiagnosticsDescriptors.StringBaseValueObjectNeedsEqualityComparer,
48+
DiagnosticsDescriptors.ComplexValueObjectWithStringMembersNeedsDefaultEqualityComparer
4849
];
4950

5051
/// <inheritdoc />
@@ -276,7 +277,7 @@ private static void AnalyzeValueObject(OperationAnalysisContext context)
276277
ValidateKeyedValueObject(context, assignableMembers, type, attrCreation, locationOfFirstDeclaration);
277278

278279
if (isComplex && assignableMembers is not null)
279-
ValidateComplexValueObject(context, assignableMembers);
280+
ValidateComplexValueObject(context, assignableMembers, attrCreation, locationOfFirstDeclaration);
280281
}
281282
catch (Exception ex)
282283
{
@@ -523,22 +524,45 @@ private static void ValidateCustomKeyMemberImplementation(
523524
return assignableMembers;
524525
}
525526

526-
private static void ValidateComplexValueObject(OperationAnalysisContext context, IReadOnlyList<InstanceMemberInfo> assignableMembers)
527+
private static void ValidateComplexValueObject(
528+
OperationAnalysisContext context,
529+
IReadOnlyList<InstanceMemberInfo> assignableMembers,
530+
IObjectCreationOperation attribute,
531+
Location locationOfFirstDeclaration)
527532
{
528-
CheckAssignableMembers(context, assignableMembers);
533+
CheckAssignableMembers(context, assignableMembers, attribute, locationOfFirstDeclaration);
529534
}
530535

531-
private static void CheckAssignableMembers(OperationAnalysisContext context, IReadOnlyList<InstanceMemberInfo> assignableMembers)
536+
private static void CheckAssignableMembers(
537+
OperationAnalysisContext context,
538+
IReadOnlyList<InstanceMemberInfo> assignableMembers,
539+
IObjectCreationOperation attribute,
540+
Location locationOfFirstDeclaration)
532541
{
542+
var hasStringMembersWithoutComparer = false;
543+
533544
for (var i = 0; i < assignableMembers.Count; i++)
534545
{
535546
var assignableMember = assignableMembers[i];
547+
var isString = assignableMember.SpecialType == SpecialType.System_String;
536548

537549
if (!assignableMember.ValueObjectMemberSettings.IsExplicitlyDeclared)
550+
{
551+
hasStringMembersWithoutComparer |= isString;
538552
continue;
553+
}
554+
555+
hasStringMembersWithoutComparer |= isString && assignableMember.ValueObjectMemberSettings.EqualityComparerAccessor is null;
539556

540557
CheckComparerTypes(context, assignableMember);
541558
}
559+
560+
if (hasStringMembersWithoutComparer && !attribute.HasDefaultStringComparison())
561+
{
562+
ReportDiagnostic(context,
563+
DiagnosticsDescriptors.ComplexValueObjectWithStringMembersNeedsDefaultEqualityComparer,
564+
locationOfFirstDeclaration);
565+
}
542566
}
543567

544568
private static void CheckComparerTypes(OperationAnalysisContext context, InstanceMemberInfo member)

src/Thinktecture.Runtime.Extensions.SourceGenerator/CodeAnalysis/DiagnosticsDescriptors.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ internal static class DiagnosticsDescriptors
3030
public static readonly DiagnosticDescriptor IndexBasedSwitchAndMapMustUseNamedParameters = new("TTRESG046", "The arguments of \"Switch\" and \"Map\" must named", "Not all arguments of \"Switch/Map\" on type '{0}' are named", nameof(ThinktectureRuntimeExtensionsAnalyzer), DiagnosticSeverity.Error, true);
3131
public static readonly DiagnosticDescriptor VariableMustBeInitializedWithNonDefaultValue = new("TTRESG047", "Variable must be initialed with non-default value", "Variable of type '{0}' must be initialized with non default value", nameof(ThinktectureRuntimeExtensionsAnalyzer), DiagnosticSeverity.Error, true);
3232
public static readonly DiagnosticDescriptor StringBaseValueObjectNeedsEqualityComparer = new("TTRESG048", "String-based Value Object needs equality comparer", "String-based Value Object needs equality comparer. Use ValueObjectKeyMemberEqualityComparerAttribute<TAccessor, TKey> to defined the equality comparer. Example: [ValueObjectKeyMemberEqualityComparer<ComparerAccessors.StringOrdinalIgnoreCase, string>].", nameof(ThinktectureRuntimeExtensionsAnalyzer), DiagnosticSeverity.Warning, true);
33+
public static readonly DiagnosticDescriptor ComplexValueObjectWithStringMembersNeedsDefaultEqualityComparer = new("TTRESG049", "Complex Value Object with string members needs equality comparer", "Complex Value Object with string members needs equality comparer. Use ValueObjectKeyMemberEqualityComparerAttribute<TAccessor, TKey> to defined the equality comparer. Example: [ValueObjectKeyMemberEqualityComparer<ComparerAccessors.StringOrdinalIgnoreCase, string>].", nameof(ThinktectureRuntimeExtensionsAnalyzer), DiagnosticSeverity.Warning, true);
3334

3435
public static readonly DiagnosticDescriptor ErrorDuringCodeAnalysis = new("TTRESG098", "Error during code analysis", "Error during code analysis of '{0}': '{1}'", nameof(ThinktectureRuntimeExtensionsAnalyzer), DiagnosticSeverity.Warning, true);
3536
public static readonly DiagnosticDescriptor ErrorDuringGeneration = new("TTRESG099", "Error during code generation", "Error during code generation for '{0}': '{1}'", nameof(ThinktectureRuntimeExtensionsAnalyzer), DiagnosticSeverity.Error, true);

src/Thinktecture.Runtime.Extensions.SourceGenerator/Extensions/ObjectCreationOperationExtensions.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ public static class ObjectCreationOperationExtensions
4040
return kind.Value;
4141
}
4242

43+
public static bool HasDefaultStringComparison(this IObjectCreationOperation operation)
44+
{
45+
return operation.Initializer is not null
46+
&& HasInitialization(operation.Initializer.Initializers, Constants.Attributes.Properties.DEFAULT_STRING_COMPARISON);
47+
}
48+
4349
private static bool? GetBooleanParameterValue(IObjectOrCollectionInitializerOperation? initializer, string name)
4450
{
4551
return (bool?)initializer?.Initializers.FindInitialization(name);
@@ -77,4 +83,25 @@ public static class ObjectCreationOperationExtensions
7783

7884
return null;
7985
}
86+
87+
private static bool HasInitialization(this ImmutableArray<IOperation> initializations, string name)
88+
{
89+
for (var i = 0; i < initializations.Length; i++)
90+
{
91+
var init = initializations[i];
92+
93+
if (init.Kind != OperationKind.SimpleAssignment || init is not ISimpleAssignmentOperation simpleAssignment)
94+
continue;
95+
96+
if (simpleAssignment.Target.Kind != OperationKind.PropertyReference || simpleAssignment.Target is not IPropertyReferenceOperation propRef)
97+
continue;
98+
99+
if (propRef.Property.Name != name)
100+
continue;
101+
102+
return true;
103+
}
104+
105+
return false;
106+
}
80107
}

test/Thinktecture.Runtime.Extensions.Json.Tests/Text/Json/Serialization/ValueObjectJsonConverterFactoryTests/TestClasses/ValueObjectWithMultipleProperties.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
using System;
2+
13
namespace Thinktecture.Runtime.Tests.Text.Json.Serialization.ValueObjectJsonConverterFactoryTests.TestClasses;
24

3-
[ComplexValueObject]
5+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
46
public partial class ValueObjectWithMultipleProperties
57
{
68
public decimal StructProperty { get; }

test/Thinktecture.Runtime.Extensions.MessagePack.Tests/Formatters/EnumMessagePackFormatterTests/TestClasses/TestValueObject_Complex_Class_WithFormatter.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
using System;
2+
13
namespace Thinktecture.Runtime.Tests.Formatters.EnumMessagePackFormatterTests.TestClasses;
24

35
// ReSharper disable InconsistentNaming
4-
[ComplexValueObject]
6+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
57
public partial class TestValueObject_Complex_Class_WithFormatter
68
{
79
public string Property1 { get; }

test/Thinktecture.Runtime.Extensions.MessagePack.Tests/Formatters/EnumMessagePackFormatterTests/TestClasses/TestValueObject_Complex_Struct_WithFormatter.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1+
using System;
2+
13
namespace Thinktecture.Runtime.Tests.Formatters.EnumMessagePackFormatterTests.TestClasses;
24

35
// ReSharper disable InconsistentNaming
4-
[ComplexValueObject(AllowDefaultStructs = true)]
6+
[ComplexValueObject(
7+
AllowDefaultStructs = true,
8+
DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
59
public partial struct TestValueObject_Complex_Struct_WithFormatter
610
{
711
public string Property1 { get; }

test/Thinktecture.Runtime.Extensions.MessagePack.Tests/Formatters/EnumMessagePackFormatterTests/TestClasses/ValueObjectWithMultipleProperties.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
using System;
2+
13
namespace Thinktecture.Runtime.Tests.Formatters.EnumMessagePackFormatterTests.TestClasses;
24

3-
[ComplexValueObject]
5+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
46
public partial class ValueObjectWithMultipleProperties
57
{
68
public decimal StructProperty { get; }

test/Thinktecture.Runtime.Extensions.Newtonsoft.Json.Tests/Json/ValueObjectNewtonsoftJsonConverterTests/TestClasses/ValueObjectWithMultipleProperties.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
using System;
2+
13
namespace Thinktecture.Runtime.Tests.Json.ValueObjectNewtonsoftJsonConverterTests.TestClasses;
24

3-
[ComplexValueObject]
5+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
46
public partial class ValueObjectWithMultipleProperties
57
{
68
public decimal StructProperty { get; }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
using System.Threading.Tasks;
2+
using Verifier = Thinktecture.Runtime.Tests.Verifiers.CodeFixVerifier<Thinktecture.CodeAnalysis.Diagnostics.ThinktectureRuntimeExtensionsAnalyzer, Thinktecture.CodeAnalysis.CodeFixes.ThinktectureRuntimeExtensionsCodeFixProvider>;
3+
4+
namespace Thinktecture.Runtime.Tests.AnalyzerAndCodeFixTests;
5+
6+
// ReSharper disable InconsistentNaming
7+
public class TTRESG049_ComplexValueObjectWithStringMembersNeedsDefaultEqualityComparer
8+
{
9+
private const string _DIAGNOSTIC_ID = "TTRESG049";
10+
11+
[Fact]
12+
public async Task Should_trigger_on_string_members_without_comparer()
13+
{
14+
var code = """
15+
16+
using System;
17+
using Thinktecture;
18+
19+
namespace TestNamespace
20+
{
21+
[ComplexValueObject]
22+
public partial class {|#0:TestValueObject|}
23+
{
24+
public string Property { get; }
25+
}
26+
}
27+
""";
28+
29+
var expectedCode = """
30+
31+
using System;
32+
using Thinktecture;
33+
34+
namespace TestNamespace
35+
{
36+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
37+
public partial class TestValueObject
38+
{
39+
public string Property { get; }
40+
}
41+
}
42+
""";
43+
44+
var expected = Verifier.Diagnostic(_DIAGNOSTIC_ID).WithLocation(0);
45+
await Verifier.VerifyCodeFixAsync(code, expectedCode, [typeof(ComplexValueObjectAttribute).Assembly], expected);
46+
}
47+
48+
[Fact]
49+
public async Task Should_not_trigger_when_DefaultStringComparison_present()
50+
{
51+
var code = """
52+
53+
using System;
54+
using Thinktecture;
55+
56+
namespace TestNamespace
57+
{
58+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
59+
public partial class {|#0:TestValueObject|}
60+
{
61+
public string Property { get; }
62+
}
63+
}
64+
""";
65+
66+
await Verifier.VerifyAnalyzerAsync(code, [typeof(ComplexValueObjectAttribute).Assembly]);
67+
}
68+
69+
[Fact]
70+
public async Task Should_not_trigger_on_string_members_with_comparer()
71+
{
72+
var code = """
73+
74+
using System;
75+
using Thinktecture;
76+
77+
namespace TestNamespace
78+
{
79+
[ComplexValueObject]
80+
public partial class {|#0:TestValueObject|}
81+
{
82+
[ValueObjectMemberEqualityComparer<ComparerAccessors.StringOrdinal, string>]
83+
public string Property { get; }
84+
}
85+
}
86+
""";
87+
88+
await Verifier.VerifyAnalyzerAsync(code, [typeof(ComplexValueObjectAttribute).Assembly]);
89+
}
90+
}

test/Thinktecture.Runtime.Extensions.Tests.Shared/TestValueObjects/BoundaryWithNullableMembers.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
namespace Thinktecture.Runtime.Tests.TestValueObjects;
55

6-
[ComplexValueObject]
6+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
77
public sealed partial class BoundaryWithNullableMembers
88
{
99
public string? Prop1 { get; }

test/Thinktecture.Runtime.Extensions.Tests.Shared/TestValueObjects/BoundaryWithStrings.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
using System;
2+
13
namespace Thinktecture.Runtime.Tests.TestValueObjects;
24

3-
[ComplexValueObject]
5+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
46
public partial class BoundaryWithStrings
57
{
68
public string Lower { get; }

test/Thinktecture.Runtime.Extensions.Tests.Shared/TestValueObjects/TestValueObject_Complex_Class.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
using System;
2+
13
namespace Thinktecture.Runtime.Tests.TestValueObjects;
24

35
// ReSharper disable once InconsistentNaming
4-
[ComplexValueObject]
6+
[ComplexValueObject(DefaultStringComparison = StringComparison.OrdinalIgnoreCase)]
57
public partial class TestValueObject_Complex_Class
68
{
79
public string Property1 { get; }

0 commit comments

Comments
 (0)