Skip to content

Commit 2ca8aa8

Browse files
Sergio0694Sergio0694
committed
Add new diagnostics for default values
1 parent ddce264 commit 2ca8aa8

File tree

5 files changed

+320
-20
lines changed

5 files changed

+320
-20
lines changed

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/AnalyzerReleases.Shipped.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,5 @@ WCTDP0027 | DependencyPropertyGenerator | Warning |
3737
WCTDP0028 | DependencyPropertyGenerator | Warning |
3838
WCTDP0029 | DependencyPropertyGenerator | Warning |
3939
WCTDP0030 | DependencyPropertyGenerator | Warning |
40+
WCTDP0031 | DependencyPropertyGenerator | Warning |
41+
WCTDP0032 | DependencyPropertyGenerator | Warning |

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

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ public sealed class UseGeneratedDependencyPropertyOnManualPropertyAnalyzer : Dia
7676
InvalidPropertyNameOnDependencyPropertyField,
7777
MismatchedPropertyNameOnDependencyPropertyField,
7878
InvalidOwningTypeOnDependencyPropertyField,
79-
InvalidPropertyTypeOnDependencyPropertyField
79+
InvalidPropertyTypeOnDependencyPropertyField,
80+
InvalidDefaultValueNullOnDependencyPropertyField,
81+
InvalidDefaultValueTypeOnDependencyPropertyField
8082
];
8183

8284
/// <inheritdoc/>
@@ -459,20 +461,16 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla
459461
}
460462
}
461463

462-
// At this point we've exhausted all possible diagnostics that are also valid on orphaned fields.
463-
// We can now validate that we did manage to retrieve the field flags, and stop if we didn't.
464-
if (fieldFlags is null)
465-
{
466-
return;
467-
}
468-
469464
// Extract the property type, we can validate it later
470465
if (propertyTypeArgument.Value is not ITypeOfOperation { TypeOperand: { } propertyTypeSymbol })
471466
{
472467
return;
473468
}
474469

475-
fieldFlags.PropertyType = propertyTypeSymbol;
470+
if (fieldFlags is not null)
471+
{
472+
fieldFlags.PropertyType = propertyTypeSymbol;
473+
}
476474

477475
// First, check if the metadata is 'null' (simplest case)
478476
if (propertyMetadataArgument.Value.ConstantValue is { HasValue: true, Value: null })
@@ -483,7 +481,10 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla
483481
if (!propertyTypeSymbol.IsDefaultValueNull() &&
484482
!propertyTypeSymbol.IsWellKnownWinRTProjectedValueType(useWindowsUIXaml))
485483
{
486-
fieldFlags.DefaultValue = TypedConstantInfo.Null.Instance;
484+
if (fieldFlags is not null)
485+
{
486+
fieldFlags.DefaultValue = TypedConstantInfo.Null.Instance;
487+
}
487488
}
488489
}
489490
else
@@ -506,14 +507,72 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla
506507
return;
507508
}
508509

510+
// Emit diagnostics for invalid default values (equivalent logic to 'InvalidPropertyDefaultValueTypeAnalyzer').
511+
// Note: we don't flag the property if we emit diagnostics here, as we can still apply the code fixer either way.
512+
// If we do that, then the other analyzer will simply start producing an error on the attribute, rather than here.
513+
if (defaultValueArgument.Value is { ConstantValue: { HasValue: true, Value: null } })
514+
{
515+
// Default value diagnostic #1: the value is 'null' but the property type is not nullable
516+
if (!propertyTypeSymbol.IsDefaultValueNull())
517+
{
518+
context.ReportDiagnostic(Diagnostic.Create(
519+
InvalidDefaultValueNullOnDependencyPropertyField,
520+
defaultValueArgument.Syntax.GetLocation(),
521+
fieldSymbol,
522+
propertyTypeSymbol));
523+
}
524+
}
525+
else
526+
{
527+
bool isDependencyPropertyUnsetValue =
528+
defaultValueArgument.Value is IPropertyReferenceOperation { Property: { IsStatic: true, Name: "UnsetValue" } unsetValuePropertySymbol } &&
529+
SymbolEqualityComparer.Default.Equals(unsetValuePropertySymbol.ContainingType, dependencyPropertySymbol);
530+
531+
// We never emit diagnostics when assigning 'UnsetValue': this value is special, so let's just ignore it
532+
if (!isDependencyPropertyUnsetValue)
533+
{
534+
// Get the type of the conversion operation (same as the one below, but we get it here too just to opportunistically emit a diagnostic)
535+
if (defaultValueArgument.Value is IConversionOperation { IsTryCast: false, Type.SpecialType: SpecialType.System_Object, Operand.Type: { } operandTypeSymbol })
536+
{
537+
// Get the target type with a special case for 'Nullable<T>' (same as in the other analyzer)
538+
ITypeSymbol unwrappedPropertyTypeSymbol = propertyTypeSymbol.IsNullableValueType()
539+
? ((INamedTypeSymbol)propertyTypeSymbol).TypeArguments[0]
540+
: propertyTypeSymbol;
541+
542+
// Warn if the type of the default value is not compatible
543+
if (!SymbolEqualityComparer.Default.Equals(unwrappedPropertyTypeSymbol, operandTypeSymbol) &&
544+
!SymbolEqualityComparer.Default.Equals(propertyTypeSymbol, operandTypeSymbol) &&
545+
context.Compilation.ClassifyConversion(operandTypeSymbol, propertyTypeSymbol) is not ({ IsBoxing: true } or { IsReference: true }))
546+
{
547+
context.ReportDiagnostic(Diagnostic.Create(
548+
InvalidDefaultValueTypeOnDependencyPropertyField,
549+
defaultValueArgument.Syntax.GetLocation(),
550+
fieldSymbol,
551+
operandTypeSymbol,
552+
propertyTypeSymbol));
553+
}
554+
}
555+
}
556+
}
557+
509558
// The argument should be a conversion operation (boxing)
510559
if (defaultValueArgument.Value is not IConversionOperation { IsTryCast: false, Type.SpecialType: SpecialType.System_Object } conversionOperation)
511560
{
512561
return;
513562
}
514563

515-
// Store the operation for later, as we need to wait to also get the metadata type to do the full validation
516-
fieldFlags.DefaultValueOperation = conversionOperation.Operand;
564+
if (fieldFlags is not null)
565+
{
566+
// Store the operation for later, as we need to wait to also get the metadata type to do the full validation
567+
fieldFlags.DefaultValueOperation = conversionOperation.Operand;
568+
}
569+
}
570+
571+
// At this point we've exhausted all possible diagnostics that are also valid on orphaned fields.
572+
// We can now validate that we did manage to retrieve the field flags, and stop if we didn't.
573+
if (fieldFlags is null)
574+
{
575+
return;
517576
}
518577

519578
// Find the parent field for the operation (we're guaranteed to only fine one)

‎components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,4 +420,30 @@ internal static class DiagnosticDescriptors
420420
isEnabledByDefault: true,
421421
description: "All dependency property fields should declare property types in metadata matching the type of their wrapping properties, or with a valid type conversion between the two.",
422422
helpLinkUri: "https://aka.ms/toolkit/labs/windows");
423+
424+
/// <summary>
425+
/// <c>The field '{0}' is registering a dependency property, but its default value is set to 'null', which is not compatible with the property type '{1}' declared in metadata (consider changing the default value, implementing the 'Get(ref object)' partial method to handle the type mismatch, or suppressing the diagnostic if this is the intended behavior)</c>.
426+
/// </summary>
427+
public static readonly DiagnosticDescriptor InvalidDefaultValueNullOnDependencyPropertyField = new(
428+
id: "WCTDP0031",
429+
title: "Invalid 'null' default value in dependency property field metadata",
430+
messageFormat: "The field '{0}' is registering a dependency property, but its default value is set to 'null', which is not compatible with the property type '{1}' declared in metadata (consider changing the default value, implementing the 'Get(ref object)' partial method to handle the type mismatch, or suppressing the diagnostic if this is the intended behavior)",
431+
category: DiagnosticCategory,
432+
defaultSeverity: DiagnosticSeverity.Warning,
433+
isEnabledByDefault: true,
434+
description: "All dependency property fields setting an explicit default value in metadata should do so with an expression of a type comparible with the property type. Alternatively, the 'Get(ref object)' method should be implemented to handle the type mismatch.",
435+
helpLinkUri: "https://aka.ms/toolkit/labs/windows");
436+
437+
/// <summary>
438+
/// <c>The field '{0}' is registering a dependency property, but its default value has type '{1}', which is not compatible with the property type '{2}' declared in metadata (consider fixing the default value, or implementing the 'Get(ref object)' partial method to handle the type mismatch)</c>.
439+
/// </summary>
440+
public static readonly DiagnosticDescriptor InvalidDefaultValueTypeOnDependencyPropertyField = new(
441+
id: "WCTDP0032",
442+
title: "Invalid default value type in dependency property field metadata",
443+
messageFormat: "The field '{0}' is registering a dependency property, but its default value has type '{1}', which is not compatible with the property type '{2}' declared in metadata (consider fixing the default value, or implementing the 'Get(ref object)' partial method to handle the type mismatch)",
444+
category: DiagnosticCategory,
445+
defaultSeverity: DiagnosticSeverity.Warning,
446+
isEnabledByDefault: true,
447+
description: "All dependency property fields setting an explicit default value in metadata should do so with an expression of a type comparible with the property type. Alternatively, the 'Get(ref object)' method should be implemented to handle the type mismatch.",
448+
helpLinkUri: "https://aka.ms/toolkit/labs/windows");
423449
}

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

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2246,13 +2246,13 @@ public sealed partial class PlayerControl : UserControl
22462246
{|WCTDP0027:nameof(PlayButtonVisible)|},
22472247
{|WCTDP0030:typeof(bool)|},
22482248
typeof(PlayerControl),
2249-
new PropertyMetadata(Visibility.Visible));
2249+
new PropertyMetadata({|WCTDP0032:Visibility.Visible|}));
22502250
22512251
public static readonly DependencyProperty VolumeVisibleProperty = DependencyProperty.Register(
22522252
nameof(VolumeVisible),
22532253
{|WCTDP0030:typeof(bool)|},
22542254
typeof(PlayerControl),
2255-
new PropertyMetadata(Visibility.Visible));
2255+
new PropertyMetadata({|WCTDP0032:Visibility.Visible|}));
22562256
22572257
public Visibility PlayButtonVisible
22582258
{
@@ -2927,6 +2927,96 @@ public class MyOtherObject : DependencyObject;
29272927
await CSharpAnalyzerTest<UseGeneratedDependencyPropertyOnManualPropertyAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
29282928
}
29292929

2930+
[TestMethod]
2931+
[DataRow("string", "null")]
2932+
[DataRow("string", "\"Bob\"")]
2933+
[DataRow("object", "null")]
2934+
[DataRow("object", "\"Bob\"")]
2935+
[DataRow("object", "42")]
2936+
[DataRow("int?", "null")]
2937+
[DataRow("Visibility?", "null")]
2938+
[DataRow("string", "DependencyProperty.UnsetValue")]
2939+
[DataRow("object", "DependencyProperty.UnsetValue")]
2940+
[DataRow("int", "DependencyProperty.UnsetValue")]
2941+
[DataRow("int?", "DependencyProperty.UnsetValue")]
2942+
[DataRow("Visibility", "DependencyProperty.UnsetValue")]
2943+
[DataRow("Visibility?", "DependencyProperty.UnsetValue")]
2944+
public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_OrphanedPropertyField_WithExplicitDefaultValue_DoesNotWarn(
2945+
string propertyType,
2946+
string defaultValueExpression)
2947+
{
2948+
string source = $$"""
2949+
using Windows.UI.Xaml;
2950+
2951+
namespace MyApp;
2952+
2953+
public class MyObject : DependencyObject
2954+
{
2955+
public static readonly DependencyProperty NameProperty = DependencyProperty.Register(
2956+
name: "Name",
2957+
propertyType: typeof({{propertyType}}),
2958+
typeof(MyObject),
2959+
typeMetadata: new PropertyMetadata({{defaultValueExpression}}));
2960+
}
2961+
""";
2962+
2963+
await CSharpAnalyzerTest<UseGeneratedDependencyPropertyOnManualPropertyAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
2964+
}
2965+
2966+
[TestMethod]
2967+
[DataRow("int")]
2968+
[DataRow("global::System.TimeSpan")]
2969+
[DataRow("global::Windows.Foundation.Rect")]
2970+
[DataRow("Visibility")]
2971+
public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_OrphanedPropertyField_NullDefaultValue_Warns(string propertyType)
2972+
{
2973+
string source = $$"""
2974+
using Windows.UI.Xaml;
2975+
2976+
namespace MyApp;
2977+
2978+
public class MyObject : DependencyObject
2979+
{
2980+
public static readonly DependencyProperty NameProperty = DependencyProperty.Register(
2981+
name: "Name",
2982+
propertyType: typeof({{propertyType}}),
2983+
typeof(MyObject),
2984+
typeMetadata: new PropertyMetadata({|WCTDP0031:null|}));
2985+
}
2986+
""";
2987+
2988+
await CSharpAnalyzerTest<UseGeneratedDependencyPropertyOnManualPropertyAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
2989+
}
2990+
2991+
[TestMethod]
2992+
[DataRow("int", "3.0")]
2993+
[DataRow("int", "3.0F")]
2994+
[DataRow("int", "3L")]
2995+
[DataRow("int", "\"Bob\"")]
2996+
[DataRow("int", "Visibility.Visible")]
2997+
[DataRow("int", "default(Visibility)")]
2998+
[DataRow("bool", "Visibility.Visible")]
2999+
[DataRow("string", "42")]
3000+
public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_OrphanedPropertyField_InvalidDefaultValue_Warns(string propertyType, string defaultValue)
3001+
{
3002+
string source = $$"""
3003+
using Windows.UI.Xaml;
3004+
3005+
namespace MyApp;
3006+
3007+
public class MyObject : DependencyObject
3008+
{
3009+
public static readonly DependencyProperty NameProperty = DependencyProperty.Register(
3010+
name: "Name",
3011+
propertyType: typeof({{propertyType}}),
3012+
typeof(MyObject),
3013+
typeMetadata: new PropertyMetadata({|WCTDP0032:{{defaultValue}}|}));
3014+
}
3015+
""";
3016+
3017+
await CSharpAnalyzerTest<UseGeneratedDependencyPropertyOnManualPropertyAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
3018+
}
3019+
29303020
[TestMethod]
29313021
public async Task InvalidPropertyForwardedAttributeDeclarationAnalyzer_NoDependencyPropertyAttribute_DoesNotWarn()
29323022
{

0 commit comments

Comments
 (0)