From d6743ba2dee62ec42b702f968cf0fee8aa58e507 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 5 Jan 2025 02:05:27 +0100 Subject: [PATCH] Emit diagnostics even if default value is not constant --- ...endencyPropertyOnManualPropertyAnalyzer.cs | 20 +++---- .../Test_Analyzers.cs | 57 +++++++++++++++++++ 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs index 1678545c..a66f3e7d 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs @@ -736,19 +736,19 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla // pretending this were the empty string literal instead. This way we can still support the property and convert to an attribute. fieldFlags.DefaultValue = TypedConstantInfo.Primitive.String.Empty; } - else + else if (fieldFlags.DefaultValueOperation is IDefaultValueOperation { Type: { } defaultValueExpressionType }) { - // If we don't have a constant, check if it's some constant value we can forward. In this case, we - // did not retrieve it. As a last resort, check if this is explicitly a 'default(T)' expression. - if (fieldFlags.DefaultValueOperation is not IDefaultValueOperation { Type: { } defaultValueExpressionType }) - { - continue; - } - - // Store the expression type for later, so we can validate it. We cannot validate it from here, as we - // only see the declared property type for metadata. This isn't guaranteed to match the property type. + // We don't have a constant. As a last resort, check if this is explicitly a 'default(T)' expression. + // If so, store the expression type for later, so we can validate it. We cannot validate it here, as + // we still want to execute the rest of the checks below to potentially emit more diagnostics first. fieldFlags.DefaultValueExpressionType = defaultValueExpressionType; } + else + { + // At this point we know the property cannot possibly be converted, so mark it as invalid. We do this + // rather than returning immediately, to still allow more diagnostics to be produced in the steps below. + fieldFlags.HasAnyDiagnostics = true; + } } } diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs index d7512aa1..afc365c6 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs @@ -2234,6 +2234,63 @@ public partial class MyControl : Control await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13); } + // Regression test for a case found in the Microsoft Store + [TestMethod] + public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_InvalidRegisterArguments_WCTDP0030_WithObjectInitialization_DoesNotWarn() + { + const string source = """ + using Windows.UI.Xaml; + using Windows.UI.Xaml.Controls; + + namespace MyApp; + + public partial class MyControl : Control + { + public static readonly DependencyProperty MarginProperty = DependencyProperty.Register( + nameof(Margin), + {|WCTDP0030:typeof(double)|}, + typeof(MyControl), + new PropertyMetadata({|WCTDP0032:new Thickness(0)|})); + + private Thickness Margin + { + get => (Thickness)GetValue(MarginProperty); + set => SetValue(MarginProperty, value); + } + } + """; + + await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13); + } + + [TestMethod] + public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_InvalidRegisterArguments_WCTDP0030_WithExplicitDefaultValueNull_DoesNotWarn() + { + const string source = """ + using Windows.UI.Xaml; + using Windows.UI.Xaml.Controls; + + namespace MyApp; + + public partial class MyControl : Control + { + public static readonly DependencyProperty MarginProperty = DependencyProperty.Register( + nameof(Margin), + {|WCTDP0030:typeof(double)|}, + typeof(MyControl), + new PropertyMetadata({|WCTDP0031:null|})); + + private Thickness Margin + { + get => (Thickness)GetValue(MarginProperty); + set => SetValue(MarginProperty, value); + } + } + """; + + await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13); + } + // Regression test for a case found in https://github.com/jenius-apps/ambie [TestMethod] public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_InvalidRegisterArguments_WCTDP0030_WithInvalidPropertyName_DoesNotWarn()