Skip to content

Commit 7d65ea1

Browse files
Sergio0694Sergio0694
committed
Fix handling of defaulted custom structs in analyzer
1 parent 3241ea5 commit 7d65ea1

File tree

7 files changed

+189
-71
lines changed

7 files changed

+189
-71
lines changed

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/DependencyPropertyGenerator.Execute.cs‎

Lines changed: 7 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,6 @@ partial class DependencyPropertyGenerator
2727
/// </summary>
2828
private static partial class Execute
2929
{
30-
/// <summary>
31-
/// Placeholder for <see langword="null"/>.
32-
/// </summary>
33-
private static readonly DependencyPropertyDefaultValue.Null NullInfo = new();
34-
3530
/// <summary>
3631
/// Generates the sources for the embedded types, for <c>PrivateAssets="all"</c> scenarios.
3732
/// </summary>
@@ -274,7 +269,7 @@ public static DependencyPropertyDefaultValue GetDefaultValue(
274269
}
275270

276271
// Invalid callback, the analyzer will emit an error
277-
return NullInfo;
272+
return DependencyPropertyDefaultValue.Null.Instance;
278273
}
279274

280275
token.ThrowIfCancellationRequested();
@@ -313,7 +308,7 @@ public static DependencyPropertyDefaultValue GetDefaultValue(
313308
}
314309

315310
// Otherwise, the value has been explicitly set to 'null', so let's respect that
316-
return NullInfo;
311+
return DependencyPropertyDefaultValue.Null.Instance;
317312
}
318313

319314
token.ThrowIfCancellationRequested();
@@ -322,54 +317,14 @@ public static DependencyPropertyDefaultValue GetDefaultValue(
322317
// First we need to special case non nullable values, as for those we need 'default'.
323318
if (!propertySymbol.Type.IsDefaultValueNull())
324319
{
325-
string fullyQualifiedTypeName = propertySymbol.Type.GetFullyQualifiedName();
326-
327-
// There is a special case for this: if the type of the property is a built-in WinRT
328-
// projected enum type or struct type (ie. some projected value type in general, except
329-
// for 'Nullable<T>' values), then we can just use 'null' and bypass creating the property
330-
// metadata. The WinRT runtime will automatically instantiate a default value for us.
331-
if (propertySymbol.Type.IsContainedInNamespace(WellKnownTypeNames.XamlNamespace(useWindowsUIXaml)) ||
332-
propertySymbol.Type.IsContainedInNamespace("System.Numerics"))
333-
{
334-
return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: true);
335-
}
336-
337-
// Special case a few more well known value types that are mapped for WinRT
338-
if (propertySymbol.Type.Name is "Point" or "Rect" or "Size" &&
339-
propertySymbol.Type.IsContainedInNamespace("Windows.Foundation"))
340-
{
341-
return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: true);
342-
}
343-
344-
// Special case two more system types
345-
if (propertySymbol.Type is INamedTypeSymbol { MetadataName: "TimeSpan" or "DateTimeOffset", ContainingNamespace.MetadataName: "System" })
346-
{
347-
return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: true);
348-
}
349-
350-
// Lastly, special case the well known primitive types
351-
if (propertySymbol.Type.SpecialType is
352-
SpecialType.System_Int32 or
353-
SpecialType.System_Byte or
354-
SpecialType.System_SByte or
355-
SpecialType.System_Int16 or
356-
SpecialType.System_UInt16 or
357-
SpecialType.System_UInt32 or
358-
SpecialType.System_Int64 or
359-
SpecialType.System_UInt64 or
360-
SpecialType.System_Char or
361-
SpecialType.System_Single or
362-
SpecialType.System_Double)
363-
{
364-
return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: true);
365-
}
366-
367-
// In all other cases, just use 'default(T)' here
368-
return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: false);
320+
// For non nullable types, we return 'default(T)', unless we can optimize for projected types
321+
return new DependencyPropertyDefaultValue.Default(
322+
TypeName: propertySymbol.Type.GetFullyQualifiedName(),
323+
IsProjectedType: propertySymbol.Type.IsWellKnownWinRTProjectedValueType(useWindowsUIXaml));
369324
}
370325

371326
// For all other ones, we can just use the 'null' placeholder again
372-
return NullInfo;
327+
return DependencyPropertyDefaultValue.Null.Instance;
373328
}
374329

375330
/// <summary>

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public override void Initialize(AnalysisContext context)
6565
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
6666
context.EnableConcurrentExecution();
6767

68+
6869
context.RegisterCompilationStartAction(static context =>
6970
{
7071
// Get the XAML mode to use
@@ -393,7 +394,18 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla
393394
}
394395

395396
// First, check if the metadata is 'null' (simplest case)
396-
if (propertyMetadataArgument.Value.ConstantValue is not { HasValue: true, Value: null })
397+
if (propertyMetadataArgument.Value.ConstantValue is { HasValue: true, Value: null })
398+
{
399+
// Here we need to special case non nullable value types that are not well known WinRT projected types.
400+
// In this case, we cannot rely on XAML calling their default constructor. Rather, we need to preserve
401+
// the explicit 'null' value that users had in their code. The analyzer will then warn on these cases
402+
if (!propertyTypeSymbol.IsDefaultValueNull() &&
403+
!propertyTypeSymbol.IsWellKnownWinRTProjectedValueType(useWindowsUIXaml))
404+
{
405+
fieldFlags.DefaultValue = TypedConstantInfo.Null.Instance;
406+
}
407+
}
408+
else
397409
{
398410
// Next, check if the argument is 'new PropertyMetadata(...)' with the default value for the property type
399411
if (propertyMetadataArgument.Value is not IObjectCreationOperation { Arguments: [{ } defaultValueArgument] } objectCreationOperation)

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/WinRTExtensions.cs‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using CommunityToolkit.GeneratedDependencyProperty.Constants;
6+
using Microsoft.CodeAnalysis;
7+
8+
namespace CommunityToolkit.GeneratedDependencyProperty.Extensions;
9+
10+
/// <summary>
11+
/// Extension methods for WinRT scenarios.
12+
/// </summary>
13+
internal static class WinRTExtensions
14+
{
15+
/// <summary>
16+
/// Checks whether a given type is a well known WinRT projected value type (ie. a type that XAML can default).
17+
/// </summary>
18+
/// <param name="symbol">The input <see cref="ITypeSymbol"/> instance to check.</param>
19+
/// <param name="useWindowsUIXaml">Whether to use the UWP XAML or WinUI 3 XAML namespaces.</param>
20+
/// <returns>Whether <paramref name="symbol"/> is a well known WinRT projected value type..</returns>
21+
public static bool IsWellKnownWinRTProjectedValueType(this ITypeSymbol symbol, bool useWindowsUIXaml)
22+
{
23+
// This method only cares about non nullable value types
24+
if (symbol.IsDefaultValueNull())
25+
{
26+
return false;
27+
}
28+
29+
// There is a special case for this: if the type of the property is a built-in WinRT
30+
// projected enum type or struct type (ie. some projected value type in general, except
31+
// for 'Nullable<T>' values), then we can just use 'null' and bypass creating the property
32+
// metadata. The WinRT runtime will automatically instantiate a default value for us.
33+
if (symbol.IsContainedInNamespace(WellKnownTypeNames.XamlNamespace(useWindowsUIXaml)) ||
34+
symbol.IsContainedInNamespace("System.Numerics"))
35+
{
36+
return true;
37+
}
38+
39+
// Special case a few more well known value types that are mapped for WinRT
40+
if (symbol.Name is "Point" or "Rect" or "Size" &&
41+
symbol.IsContainedInNamespace("Windows.Foundation"))
42+
{
43+
return true;
44+
}
45+
46+
// Special case two more system types
47+
if (symbol is INamedTypeSymbol { MetadataName: "TimeSpan" or "DateTimeOffset", ContainingNamespace.MetadataName: "System" })
48+
{
49+
return true;
50+
}
51+
52+
// Lastly, special case the well known primitive types
53+
if (symbol.SpecialType is
54+
SpecialType.System_Int32 or
55+
SpecialType.System_Byte or
56+
SpecialType.System_SByte or
57+
SpecialType.System_Int16 or
58+
SpecialType.System_UInt16 or
59+
SpecialType.System_UInt32 or
60+
SpecialType.System_Int64 or
61+
SpecialType.System_UInt64 or
62+
SpecialType.System_Char or
63+
SpecialType.System_Single or
64+
SpecialType.System_Double)
65+
{
66+
return true;
67+
}
68+
69+
return false;
70+
}
71+
}

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/DependencyPropertyDefaultValue.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ internal abstract partial record DependencyPropertyDefaultValue
1616
/// </summary>
1717
public sealed record Null : DependencyPropertyDefaultValue
1818
{
19+
/// <summary>
20+
/// The shared <see cref="Null"/> instance (the type is stateless).
21+
/// </summary>
22+
public static Null Instance { get; } = new();
23+
1924
/// <inheritdoc/>
2025
public override string ToString()
2126
{

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/TypedConstantInfo.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ internal abstract partial record TypedConstantInfo
2323
/// </summary>
2424
public sealed record Null : TypedConstantInfo
2525
{
26+
/// <summary>
27+
/// The shared <see cref="Null"/> instance (the type is stateless).
28+
/// </summary>
29+
public static Null Instance { get; } = new();
30+
2631
/// <inheritdoc/>
2732
public override string ToString()
2833
{

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,7 +1382,6 @@ public string? Name
13821382
[TestMethod]
13831383
[DataRow("global::System.TimeSpan", "global::System.TimeSpan", "global::System.TimeSpan.FromSeconds(1)")]
13841384
[DataRow("global::System.TimeSpan?", "global::System.TimeSpan?", "global::System.TimeSpan.FromSeconds(1)")]
1385-
[DataRow("global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility.Collapsed")]
13861385
public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_ValidProperty_ExplicitDefaultValue_DoesNotWarn(
13871386
string dependencyPropertyType,
13881387
string propertyType,
@@ -1425,21 +1424,21 @@ public enum MyEnum { A, B, C }
14251424
[DataRow("object", "object?")]
14261425
[DataRow("int", "int")]
14271426
[DataRow("int?", "int?")]
1428-
[DataRow("global::System.TimeSpan", "global::System.TimeSpan", "null")]
1429-
[DataRow("global::System.TimeSpan?", "global::System.TimeSpan?", "default(global::System.TimeSpan?)")]
1430-
[DataRow("global::System.DateTimeOffset", "global::System.DateTimeOffset", "null")]
1431-
[DataRow("global::System.DateTimeOffset?", "global::System.DateTimeOffset?", "default(global::System.DateTimeOffset?)")]
1432-
[DataRow("global::System.Guid?", "global::System.Guid?", "default(global::System.Guid?)")]
1433-
[DataRow("global::System.Collections.Generic.KeyValuePair<int, float>?", "global::System.Collections.Generic.KeyValuePair<int, float>?", "default(global::System.Collections.Generic.KeyValuePair<int, float>?)")]
1434-
[DataRow("global::System.Collections.Generic.KeyValuePair<int, float>?", "global::System.Collections.Generic.KeyValuePair<int, float>?", "null")]
1435-
[DataRow("global::MyApp.MyStruct", "global::MyApp.MyStruct", "default(global::MyApp.MyStruct)")]
1436-
[DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?", "null")]
1437-
[DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?", "default(global::MyApp.MyStruct?)")]
1438-
[DataRow("global::MyApp.MyEnum", "global::MyApp.MyEnum", "default(global::MyApp.MyEnum)")]
1439-
[DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?", "null")]
1440-
[DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?", "default(global::MyApp.MyEnum?)")]
1441-
[DataRow("global::MyApp.MyClass", "global::MyApp.MyClass", "null")]
1442-
[DataRow("global::MyApp.MyClass", "global::MyApp.MyClass", "default(global::MyApp.MyClass)")]
1427+
[DataRow("global::System.TimeSpan", "global::System.TimeSpan")]
1428+
[DataRow("global::System.TimeSpan?", "global::System.TimeSpan?")]
1429+
[DataRow("global::System.DateTimeOffset", "global::System.DateTimeOffset")]
1430+
[DataRow("global::System.DateTimeOffset?", "global::System.DateTimeOffset?")]
1431+
[DataRow("global::System.Guid?", "global::System.Guid?")]
1432+
[DataRow("global::System.Collections.Generic.KeyValuePair<int, float>?", "global::System.Collections.Generic.KeyValuePair<int, float>?")]
1433+
[DataRow("global::System.Collections.Generic.KeyValuePair<int, float>?", "global::System.Collections.Generic.KeyValuePair<int, float>?" )]
1434+
[DataRow("global::MyApp.MyStruct", "global::MyApp.MyStruct")]
1435+
[DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?")]
1436+
[DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?")]
1437+
[DataRow("global::MyApp.MyEnum", "global::MyApp.MyEnum")]
1438+
[DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?")]
1439+
[DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?")]
1440+
[DataRow("global::MyApp.MyClass", "global::MyApp.MyClass")]
1441+
[DataRow("global::MyApp.MyClass", "global::MyApp.MyClass")]
14431442
public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_ValidProperty_Warns(
14441443
string dependencyPropertyType,
14451444
string propertyType)
@@ -1504,6 +1503,7 @@ public class MyClass { }
15041503
[DataRow("global::Windows.Foundation.Size", "global::Windows.Foundation.Size", "default(global::Windows.Foundation.Size)")]
15051504
[DataRow("global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility", "default(global::Windows.UI.Xaml.Visibility)")]
15061505
[DataRow("global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility.Visible")]
1506+
[DataRow("global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility.Collapsed")]
15071507
[DataRow("global::System.TimeSpan", "global::System.TimeSpan", "default(System.TimeSpan)")]
15081508
[DataRow("global::System.DateTimeOffset", "global::System.DateTimeOffset", "default(global::System.DateTimeOffset)")]
15091509
[DataRow("global::System.DateTimeOffset?", "global::System.DateTimeOffset?", "null")]

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_UseGeneratedDependencyPropertyOnManualPropertyCodeFixer.cs‎

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@ public class Test_UseGeneratedDependencyPropertyOnManualPropertyCodeFixer
5757
[DataRow("global::System.TimeSpan?", "global::System.TimeSpan?")]
5858
[DataRow("global::System.Guid?", "global::System.Guid?")]
5959
[DataRow("global::System.Collections.Generic.KeyValuePair<int, float>?", "global::System.Collections.Generic.KeyValuePair<int, float>?")]
60-
[DataRow("global::MyApp.MyStruct", "global::MyApp.MyStruct")]
6160
[DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?")]
62-
[DataRow("global::MyApp.MyEnum", "global::MyApp.MyEnum")]
6361
[DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?")]
62+
[DataRow("global::MyApp.MyClass", "global::MyApp.MyClass")]
63+
[DataRow("global::MyApp.MyClass", "global::MyApp.MyClass?")]
6464
public async Task SimpleProperty(string dependencyPropertyType, string propertyType)
6565
{
6666
string original = $$"""
@@ -86,6 +86,7 @@ public class MyControl : Control
8686
8787
public struct MyStruct { public string X { get; set; } }
8888
public enum MyEnum { A, B, C }
89+
public class MyClass { }
8990
""";
9091

9192
string @fixed = $$"""
@@ -101,6 +102,75 @@ public partial class MyControl : Control
101102
public partial {{propertyType}} {|CS9248:Name|} { get; set; }
102103
}
103104
105+
public struct MyStruct { public string X { get; set; } }
106+
public enum MyEnum { A, B, C }
107+
public class MyClass { }
108+
""";
109+
110+
CSharpCodeFixTest test = new(LanguageVersion.Preview)
111+
{
112+
TestCode = original,
113+
FixedCode = @fixed,
114+
ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
115+
TestState = { AdditionalReferences =
116+
{
117+
MetadataReference.CreateFromFile(typeof(Point).Assembly.Location),
118+
MetadataReference.CreateFromFile(typeof(ApplicationView).Assembly.Location),
119+
MetadataReference.CreateFromFile(typeof(DependencyProperty).Assembly.Location),
120+
MetadataReference.CreateFromFile(typeof(GeneratedDependencyPropertyAttribute).Assembly.Location)
121+
}}
122+
};
123+
124+
await test.RunAsync();
125+
}
126+
127+
// These are custom value types, on properties where the metadata was set to 'null'. In this case, the
128+
// default value would just be 'null', as XAML can't default initialize them. To preserve behavior,
129+
// we must include an explicit default value. This will warn when the code is recompiled, but that
130+
// is expected, because this specific scenario was (1) niche, and (2) kinda busted already anyway.
131+
[TestMethod]
132+
[DataRow("global::MyApp.MyStruct", "global::MyApp.MyStruct")]
133+
[DataRow("global::MyApp.MyEnum", "global::MyApp.MyEnum")]
134+
public async Task SimpleProperty_ExplicitNull(string dependencyPropertyType, string propertyType)
135+
{
136+
string original = $$"""
137+
using Windows.UI.Xaml;
138+
using Windows.UI.Xaml.Controls;
139+
140+
namespace MyApp;
141+
142+
public class MyControl : Control
143+
{
144+
public static readonly DependencyProperty NameProperty = DependencyProperty.Register(
145+
name: nameof(Name),
146+
propertyType: typeof({{dependencyPropertyType}}),
147+
ownerType: typeof(MyControl),
148+
typeMetadata: null);
149+
150+
public {{propertyType}} [|Name|]
151+
{
152+
get => ({{propertyType}})GetValue(NameProperty);
153+
set => SetValue(NameProperty, value);
154+
}
155+
}
156+
157+
public struct MyStruct { public string X { get; set; } }
158+
public enum MyEnum { A, B, C }
159+
""";
160+
161+
string @fixed = $$"""
162+
using CommunityToolkit.WinUI;
163+
using Windows.UI.Xaml;
164+
using Windows.UI.Xaml.Controls;
165+
166+
namespace MyApp;
167+
168+
public partial class MyControl : Control
169+
{
170+
[GeneratedDependencyProperty(DefaultValue = null)]
171+
public partial {{propertyType}} {|CS9248:Name|} { get; set; }
172+
}
173+
104174
public struct MyStruct { public string X { get; set; } }
105175
public enum MyEnum { A, B, C }
106176
""";

0 commit comments

Comments
 (0)