Skip to content

Commit 6d9c7e1

Browse files
Tim-MaesTim-Maes
committed
Fix issue
1 parent e5ca98c commit 6d9c7e1

File tree

4 files changed

+257
-4
lines changed

4 files changed

+257
-4
lines changed

‎src/Facet/Generators/FacetGenerators/ProjectionGenerator.cs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ private static void GenerateProjectionDocumentation(StringBuilder sb, FacetTarge
6565
/// <summary>
6666
/// Generates the projection expression body using object initializer syntax for EF Core compatibility.
6767
/// This allows EF Core to automatically include navigation properties without requiring explicit .Include() calls.
68+
/// For positional records without a parameterless constructor, uses constructor invocation syntax instead.
6869
/// </summary>
6970
private static void GenerateProjectionExpression(
7071
StringBuilder sb,
@@ -73,6 +74,61 @@ private static void GenerateProjectionExpression(
7374
Dictionary<string, FacetTargetModel> facetLookup)
7475
{
7576
var indent = baseIndent + " ";
77+
78+
// Check if this is a positional record without a parameterless constructor
79+
// In this case, we need to use constructor syntax instead of object initializer
80+
var isPositionalWithoutParameterless = model.IsRecord &&
81+
!model.HasExistingPrimaryConstructor &&
82+
!model.GenerateParameterlessConstructor;
83+
84+
if (isPositionalWithoutParameterless)
85+
{
86+
// Use constructor invocation syntax for positional records
87+
GeneratePositionalRecordProjection(sb, model, indent, facetLookup);
88+
}
89+
else
90+
{
91+
// Use object initializer syntax (standard approach)
92+
GenerateObjectInitializerProjection(sb, model, indent, facetLookup);
93+
}
94+
}
95+
96+
/// <summary>
97+
/// Generates projection using constructor invocation syntax for positional records.
98+
/// </summary>
99+
private static void GeneratePositionalRecordProjection(
100+
StringBuilder sb,
101+
FacetTargetModel model,
102+
string indent,
103+
Dictionary<string, FacetTargetModel> facetLookup)
104+
{
105+
var visitedTypes = new HashSet<string> { model.Name };
106+
var includedMembers = model.Members.Where(m => m.MapFromIncludeInProjection).ToArray();
107+
108+
sb.Append($"{indent}source => new {model.Name}(");
109+
110+
for (int i = 0; i < includedMembers.Length; i++)
111+
{
112+
var member = includedMembers[i];
113+
var projectionValue = GetProjectionValueExpression(member, "source", indent, facetLookup, visitedTypes, 0, model.MaxDepth);
114+
sb.Append(projectionValue);
115+
116+
if (i < includedMembers.Length - 1)
117+
sb.Append(", ");
118+
}
119+
120+
sb.AppendLine(");");
121+
}
122+
123+
/// <summary>
124+
/// Generates projection using object initializer syntax (standard approach).
125+
/// </summary>
126+
private static void GenerateObjectInitializerProjection(
127+
StringBuilder sb,
128+
FacetTargetModel model,
129+
string indent,
130+
Dictionary<string, FacetTargetModel> facetLookup)
131+
{
76132
sb.AppendLine($"{indent}source => new {model.Name}");
77133
sb.AppendLine($"{indent}{{");
78134

‎src/Facet/Generators/Shared/GeneratorUtilities.cs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,10 @@ _ when IsValueType(cleanTypeName) => $"default({cleanTypeName})",
209209
/// <returns>A C# expression string representing the default value.</returns>
210210
public static string GetDefaultValue(string typeName)
211211
{
212-
// Handle nullable types
212+
// Handle nullable types - use typed null to avoid ambiguity with record copy constructors
213213
if (typeName.EndsWith("?"))
214214
{
215-
return "null";
215+
return $"({typeName})null";
216216
}
217217

218218
// Handle common value types
@@ -236,8 +236,11 @@ var t when t.StartsWith("System.DateTime") => "default",
236236
var t when t.StartsWith("System.DateTimeOffset") => "default",
237237
var t when t.StartsWith("System.TimeSpan") => "default",
238238
var t when t.StartsWith("System.Guid") => "default",
239-
// For other types, use default() expression
240-
_ => $"default({typeName})"
239+
// For value types, use default() expression
240+
var t when IsValueType(t) => $"default({typeName})",
241+
// For reference types (collections, objects, etc.), use typed default to avoid
242+
// ambiguity with record copy constructors when null could match multiple parameter types
243+
_ => $"default({typeName})!"
241244
};
242245
}
243246

‎test/Facet.Tests/TestModels/TestEntities.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,3 +160,44 @@ public class InitOnlyWithInitializers
160160

161161
[Facet(typeof(InitOnlyWithInitializers))]
162162
public partial class InitOnlyWithInitializersDto;
163+
164+
// Test entity for GitHub issue #251 - Nullable issues with object properties
165+
// When a model has a single reference type property like List<string>,
166+
// the generated record positional constructor can become ambiguous with
167+
// the compiler-generated copy constructor
168+
public class ModelWithListProperty
169+
{
170+
public List<string> Tags { get; set; } = [];
171+
}
172+
173+
// These tests verify that the ambiguity is resolved by using typed default values
174+
// in the generated parameterless constructor
175+
176+
[Facet(typeof(ModelWithListProperty))]
177+
public partial record RecordWithListDefault;
178+
179+
[Facet(typeof(ModelWithListProperty), GenerateParameterlessConstructor = false)]
180+
public partial record RecordWithListNoParameterless;
181+
182+
[Facet(typeof(ModelWithListProperty), GenerateProjection = false)]
183+
public partial record RecordWithListNoProjection;
184+
185+
// Test with multiple properties to ensure the fix doesn't break normal cases
186+
public class ModelWithMultipleProperties
187+
{
188+
public string Name { get; set; } = string.Empty;
189+
public List<string> Tags { get; set; } = [];
190+
public int Count { get; set; }
191+
}
192+
193+
[Facet(typeof(ModelWithMultipleProperties))]
194+
public partial record RecordWithMultipleProperties;
195+
196+
// Test with nullable reference type property
197+
public class ModelWithNullableList
198+
{
199+
public List<string>? Tags { get; set; }
200+
}
201+
202+
[Facet(typeof(ModelWithNullableList))]
203+
public partial record RecordWithNullableList;

‎test/Facet.Tests/UnitTests/Core/Facet/RecordListPropertyTests.cs‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
using Facet.Tests.TestModels;
2+
3+
namespace Facet.Tests.UnitTests.Core.Facet;
4+
5+
/// <summary>
6+
/// Tests for GitHub issue #251 - Nullable issues with object properties.
7+
/// When a record facet has a single reference type property like List&lt;string&gt;,
8+
/// the generated constructors should not cause ambiguity with the record's
9+
/// compiler-generated copy constructor.
10+
/// </summary>
11+
public class RecordListPropertyTests
12+
{
13+
[Fact]
14+
public void RecordWithListDefault_ShouldCompileAndConstruct()
15+
{
16+
// Arrange
17+
var source = new ModelWithListProperty
18+
{
19+
Tags = new List<string> { "tag1", "tag2" }
20+
};
21+
22+
// Act - Should not throw ambiguous constructor error
23+
var dto = new RecordWithListDefault(source);
24+
25+
// Assert
26+
dto.Tags.Should().BeEquivalentTo(new[] { "tag1", "tag2" });
27+
}
28+
29+
[Fact]
30+
public void RecordWithListDefault_ParameterlessConstructor_ShouldWork()
31+
{
32+
// Act - Should be able to call parameterless constructor without ambiguity
33+
var dto = new RecordWithListDefault();
34+
35+
// Assert - Tags should be default(List<string>)! which is null
36+
// The null-forgiving operator ensures no null reference issues at compile time
37+
dto.Tags.Should().BeNull();
38+
}
39+
40+
[Fact]
41+
public void RecordWithListNoParameterless_ShouldConstructFromSource()
42+
{
43+
// Arrange
44+
var source = new ModelWithListProperty
45+
{
46+
Tags = new List<string> { "test" }
47+
};
48+
49+
// Act
50+
var dto = new RecordWithListNoParameterless(source);
51+
52+
// Assert
53+
dto.Tags.Should().ContainSingle().Which.Should().Be("test");
54+
}
55+
56+
[Fact]
57+
public void RecordWithListNoProjection_ShouldConstructFromSource()
58+
{
59+
// Arrange
60+
var source = new ModelWithListProperty
61+
{
62+
Tags = new List<string> { "a", "b", "c" }
63+
};
64+
65+
// Act
66+
var dto = new RecordWithListNoProjection(source);
67+
68+
// Assert
69+
dto.Tags.Should().HaveCount(3);
70+
}
71+
72+
[Fact]
73+
public void RecordWithMultipleProperties_ShouldWork()
74+
{
75+
// Arrange
76+
var source = new ModelWithMultipleProperties
77+
{
78+
Name = "Test",
79+
Tags = new List<string> { "tag1", "tag2" },
80+
Count = 42
81+
};
82+
83+
// Act
84+
var dto = new RecordWithMultipleProperties(source);
85+
86+
// Assert
87+
dto.Name.Should().Be("Test");
88+
dto.Tags.Should().BeEquivalentTo(new[] { "tag1", "tag2" });
89+
dto.Count.Should().Be(42);
90+
}
91+
92+
[Fact]
93+
public void RecordWithMultipleProperties_ParameterlessConstructor_ShouldWork()
94+
{
95+
// Act
96+
var dto = new RecordWithMultipleProperties();
97+
98+
// Assert - string gets string.Empty, List gets default, int gets 0
99+
dto.Name.Should().BeEmpty();
100+
dto.Tags.Should().BeNull();
101+
dto.Count.Should().Be(0);
102+
}
103+
104+
[Fact]
105+
public void RecordWithNullableList_ShouldWork()
106+
{
107+
// Arrange
108+
var source = new ModelWithNullableList
109+
{
110+
Tags = new List<string> { "nullable" }
111+
};
112+
113+
// Act
114+
var dto = new RecordWithNullableList(source);
115+
116+
// Assert
117+
dto.Tags.Should().ContainSingle().Which.Should().Be("nullable");
118+
}
119+
120+
[Fact]
121+
public void RecordWithNullableList_NullValue_ShouldWork()
122+
{
123+
// Arrange
124+
var source = new ModelWithNullableList
125+
{
126+
Tags = null
127+
};
128+
129+
// Act
130+
var dto = new RecordWithNullableList(source);
131+
132+
// Assert
133+
dto.Tags.Should().BeNull();
134+
}
135+
136+
[Fact]
137+
public void RecordWithListDefault_WithExpression_ShouldWork()
138+
{
139+
// Arrange
140+
var source = new ModelWithListProperty
141+
{
142+
Tags = new List<string> { "original" }
143+
};
144+
var dto = new RecordWithListDefault(source);
145+
146+
// Act - Records should support 'with' expressions
147+
var modified = dto with { Tags = new List<string> { "modified" } };
148+
149+
// Assert
150+
modified.Tags.Should().ContainSingle().Which.Should().Be("modified");
151+
dto.Tags.Should().ContainSingle().Which.Should().Be("original");
152+
}
153+
}

0 commit comments

Comments
 (0)