From 149c9521ac0d0a06948d407a08d3527090594312 Mon Sep 17 00:00:00 2001 From: Ian Good Date: Sat, 26 Jun 2021 12:56:29 -0700 Subject: [PATCH] Code cleanup (#9) * minor cleanup - corrected some comments - updated gen code formatting (added some indentation) - change `ApppendPropertyMetadata` to `GetPropertyMetadataDeclaration` - check cancellation during code gen - added param names for clarity * more unit tests --- Bpz.Test/Bpz.Test.csproj | 1 + Bpz.Test/Widget.cs | 61 ++++++++++ Bpz.Test/WidgetTests.cs | 107 ++++++++++++++++++ .../Wpf/DependencyPropertyGenerator.cs | 71 ++++++------ boilerplatezero/boilerplatezero.csproj | 2 +- 5 files changed, 205 insertions(+), 37 deletions(-) create mode 100644 Bpz.Test/Widget.cs create mode 100644 Bpz.Test/WidgetTests.cs diff --git a/Bpz.Test/Bpz.Test.csproj b/Bpz.Test/Bpz.Test.csproj index 6d18873..ef75b84 100644 --- a/Bpz.Test/Bpz.Test.csproj +++ b/Bpz.Test/Bpz.Test.csproj @@ -25,6 +25,7 @@ + diff --git a/Bpz.Test/Widget.cs b/Bpz.Test/Widget.cs new file mode 100644 index 0000000..0978ee1 --- /dev/null +++ b/Bpz.Test/Widget.cs @@ -0,0 +1,61 @@ +// Copyright © Ian Good + +using System; +using System.Windows; + +namespace Bpz.Test +{ + public partial class Widget : DependencyObject + { + public static readonly DependencyProperty MyBool0Property = Gen.MyBool0(true); + public static readonly DependencyProperty MyBool1Property = Gen.MyBool1(); + public static readonly DependencyProperty MyBool2Property = Gen.MyBool2((bool?)false); + + public static readonly DependencyProperty MyString0Property = Gen.MyString0("asdf"); + public static readonly DependencyProperty MyString1Property = Gen.MyString1(); + public static readonly DependencyProperty MyString2Property = Gen.MyString2(default(string?)); + public static readonly DependencyProperty MyString3Property = Gen.MyString3("qwer"); + + private static void MyString0PropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) + { + ((Widget)d).MyString0Changed?.Invoke(d, EventArgs.Empty); + } + + private static void MyString1PropertyChanged(Widget self, DependencyPropertyChangedEventArgs e) + { + self.MyString1Changed?.Invoke(self, EventArgs.Empty); + } + + private void OnMyString2Changed(string? oldValue, string? newValue) + { + this.MyString2Changed?.Invoke(this, EventArgs.Empty); + } + + private void OnMyString3Changed(DependencyPropertyChangedEventArgs e) + { + this.MyString3Changed?.Invoke(this, EventArgs.Empty); + } + + public event EventHandler? MyString0Changed; + public event EventHandler? MyString1Changed; + public event EventHandler? MyString2Changed; + public event EventHandler? MyString3Changed; + + private static readonly DependencyPropertyKey MyFloat0PropertyKey = Gen.MyFloat0(3.14f); + protected static readonly DependencyPropertyKey MyFloat1PropertyKey = Gen.MyFloat1(); + + private static float CoerceMyFloat1(Widget self, float baseValue) + { + return Math.Abs(baseValue); + } + + private void OnMyFloat1Changed(float old, float @new) + { + this.MyFloat1Changed?.Invoke(this, EventArgs.Empty); + } + + public void SetMyFloat1(float value) => this.MyFloat1 = value; + + public event EventHandler? MyFloat1Changed; + } +} diff --git a/Bpz.Test/WidgetTests.cs b/Bpz.Test/WidgetTests.cs new file mode 100644 index 0000000..1afdd6d --- /dev/null +++ b/Bpz.Test/WidgetTests.cs @@ -0,0 +1,107 @@ +// Copyright © Ian Good + +using NUnit.Framework; +using System; + +namespace Bpz.Test +{ + /// + /// Exercises basic dependency property behavior. + /// This won't compile if the properties we expect weren't generated. + /// + public class WidgetTests + { + [Test(Description = "Checks default values.")] + public void ExpectDefaults() + { + var w = new Widget(); + + Assert.AreEqual(true, w.MyBool0); + Assert.AreEqual(false, w.MyBool1); + Assert.AreEqual(false, w.MyBool2); + + Assert.AreEqual("asdf", w.MyString0); + Assert.AreEqual(null, w.MyString1); + Assert.AreEqual(null, w.MyString2); + Assert.AreEqual("qwer", w.MyString3); + + Assert.AreEqual(3.14f, w.MyFloat0); + Assert.AreEqual(0, w.MyFloat1); + } + + [Test(Description = "Change-handers should raise events.")] + public void ExpectEvents() + { + var w = new Widget(); + + bool eventWasRaised; + EventHandler handler = (s, e) => + { + Assert.AreSame(w, s); + eventWasRaised = true; + }; + + w.MyString0Changed += handler; + w.MyString1Changed += handler; + w.MyString2Changed += handler; + w.MyString3Changed += handler; + w.MyFloat1Changed += handler; + + { + eventWasRaised = false; + w.MyString0 = "foo"; + Assert.IsTrue(eventWasRaised); + } + { + eventWasRaised = false; + w.MyString1 = "foo"; + Assert.IsTrue(eventWasRaised); + } + { + eventWasRaised = false; + w.MyString2 = "foo"; + Assert.IsTrue(eventWasRaised); + } + { + eventWasRaised = false; + w.MyString3 = "foo"; + Assert.IsTrue(eventWasRaised); + } + { + eventWasRaised = false; + w.SetMyFloat1(123.456f); + Assert.IsTrue(eventWasRaised); + + eventWasRaised = false; + w.SetMyFloat1(123.456f); + Assert.IsFalse(eventWasRaised); + } + } + + [Test(Description = "Coercion should take place.")] + public void ExpectCoercion() + { + var w = new Widget(); + + bool eventWasRaised = false; + w.MyFloat1Changed += (s, e) => + { + Assert.AreSame(w, s); + eventWasRaised = true; + }; + + // `Float1` coerces values using `Math.Abs`. + w.SetMyFloat1(-40); + Assert.AreEqual(40, w.MyFloat1); + Assert.IsTrue(eventWasRaised); + + // The value has been coerced from negative to positive, + // so assigning the positive value should not produce a "changed" event. + eventWasRaised = false; + w.SetMyFloat1(40); + Assert.AreEqual(40, w.MyFloat1); + + Assert.IsFalse(eventWasRaised); + } + } +} diff --git a/boilerplatezero/Wpf/DependencyPropertyGenerator.cs b/boilerplatezero/Wpf/DependencyPropertyGenerator.cs index d772e92..12c0bf8 100644 --- a/boilerplatezero/Wpf/DependencyPropertyGenerator.cs +++ b/boilerplatezero/Wpf/DependencyPropertyGenerator.cs @@ -32,7 +32,7 @@ public class DependencyPropertyGenerator : ISourceGenerator /// private bool useNullableContext; - // These will be initialized before first. + // These will be initialized before first use. private INamedTypeSymbol objTypeSymbol = null!; // System.Object private INamedTypeSymbol doTypeSymbol = null!; // System.Windows.DependencyObject private INamedTypeSymbol argsTypeSymbol = null!; // System.Windows.DependencyPropertyChangedEventArgs @@ -78,6 +78,8 @@ public void Execute(GeneratorExecutionContext context) foreach (var generateThis in classGroup) { + context.CancellationToken.ThrowIfCancellationRequested(); + this.ApppendSource(context, sourceBuilder, generateThis); } @@ -309,24 +311,25 @@ private void ApppendSource(GeneratorExecutionContext context, StringBuilder sour /// public static {returnType} {propertyName}<__T>({parameters}) {{"); - this.ApppendPropertyMetadata(sourceBuilder, generateThis, hasDefaultValue, hasFlags); + sourceBuilder.Append("\t\t\t\t"); + sourceBuilder.AppendLine(this.GetPropertyMetadataDeclaration(generateThis, hasDefaultValue, hasFlags)); string a = generateThis.IsAttached ? "Attached" : ""; string ro = generateThis.IsDpk ? "ReadOnly" : ""; string ownerTypeName = GetTypeName(generateThis.FieldSymbol.ContainingType); sourceBuilder.AppendLine( -$@"return DependencyProperty.Register{a}{ro}(""{propertyName}"", typeof(__T), typeof({ownerTypeName}), metadata); +$@" return DependencyProperty.Register{a}{ro}(""{propertyName}"", typeof(__T), typeof({ownerTypeName}), metadata); }} }}"); } /// - /// Appends source text that declares the property metadata variable named "metadata". + /// Gets source text that declares the property metadata variable named "metadata". /// Accounts for whether a default value exists. /// Accounts for whether a compatible property-changed handler exists. /// Accounts for whether a compatible coercion handler exists. /// - private void ApppendPropertyMetadata(StringBuilder sourceBuilder, GenerationDetails generateThis, bool hasDefaultValue, bool hasFlags) + private string GetPropertyMetadataDeclaration(GenerationDetails generateThis, bool hasDefaultValue, bool hasFlags) { INamedTypeSymbol ownerType = generateThis.FieldSymbol.ContainingType; string propertyName = generateThis.MethodNameNode.Identifier.ValueText; @@ -554,30 +557,26 @@ bool _TryGetCoercionHandler(IMethodSymbol methodSymbol, out string coercionHandl if (hasFlags) { string defaultValue = hasDefaultValue ? "defaultValue" : "default(__T)"; - sourceBuilder.AppendLine($"FrameworkPropertyMetadata metadata = new FrameworkPropertyMetadata({defaultValue}, flags, {changeHandler}, {coercionHandler});"); - return; + return $"FrameworkPropertyMetadata metadata = new FrameworkPropertyMetadata({defaultValue}, flags, {changeHandler}, {coercionHandler});"; } if (hasDefaultValue) { - sourceBuilder.AppendLine($"PropertyMetadata metadata = new PropertyMetadata(defaultValue, {changeHandler}, {coercionHandler});"); - return; + return $"PropertyMetadata metadata = new PropertyMetadata(defaultValue, {changeHandler}, {coercionHandler});"; } if (changeHandler != "null") { - sourceBuilder.AppendLine($"PropertyMetadata metadata = new PropertyMetadata({changeHandler}) {{ CoerceValueCallback = {coercionHandler} }};"); - return; + return $"PropertyMetadata metadata = new PropertyMetadata({changeHandler}) {{ CoerceValueCallback = {coercionHandler} }};"; } if (coercionHandler != "null") { - sourceBuilder.AppendLine($"PropertyMetadata metadata = new PropertyMetadata() {{ CoerceValueCallback = {coercionHandler} }};"); - return; + return $"PropertyMetadata metadata = new PropertyMetadata() {{ CoerceValueCallback = {coercionHandler} }};"; } string nullLiteral = this.useNullableContext ? "null!" : "null"; - sourceBuilder.AppendLine($"PropertyMetadata metadata = {nullLiteral};"); + return $"PropertyMetadata metadata = {nullLiteral};"; } /// @@ -808,21 +807,21 @@ public GenerationDetails(SimpleNameSyntax methodNameNode, bool isAttached) private static class Diagnostics { private static readonly DiagnosticDescriptor MismatchedIdentifiersError = new( - "BPZ0001", - "Mismatched identifiers", - "Field name '{0}' and method name '{1}' do not match. Expected '{2} = {3}'.", - "Naming", - DiagnosticSeverity.Error, - true, - null, - HelpLinkUri, - WellKnownDiagnosticTags.Compiler); + id: "BPZ0001", + title: "Mismatched identifiers", + messageFormat: "Field name '{0}' and method name '{1}' do not match. Expected '{2} = {3}'.", + category: "Naming", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: null, + helpLinkUri: HelpLinkUri, + customTags: WellKnownDiagnosticTags.Compiler); public static Diagnostic MismatchedIdentifiers(IFieldSymbol fieldSymbol, string methodName, string expectedFieldName, string initializer) { return Diagnostic.Create( - MismatchedIdentifiersError, - fieldSymbol.Locations[0], + descriptor: MismatchedIdentifiersError, + location: fieldSymbol.Locations[0], fieldSymbol.Name, methodName, expectedFieldName, @@ -830,21 +829,21 @@ public static Diagnostic MismatchedIdentifiers(IFieldSymbol fieldSymbol, string } private static readonly DiagnosticDescriptor UnexpectedFieldTypeError = new( - "BPZ1001", - "Unexpected field type", - "'{0}.{1}' has unexpected type '{2}'. Expected 'System.Windows.DependencyProperty' or 'System.Windows.DependencyPropertyKey'.", - "Types", - DiagnosticSeverity.Error, - true, - null, - HelpLinkUri, - WellKnownDiagnosticTags.Compiler); + id: "BPZ1001", + title: "Unexpected field type", + messageFormat: "'{0}.{1}' has unexpected type '{2}'. Expected 'System.Windows.DependencyProperty' or 'System.Windows.DependencyPropertyKey'.", + category: "Types", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: null, + helpLinkUri: HelpLinkUri, + customTags: WellKnownDiagnosticTags.Compiler); public static Diagnostic UnexpectedFieldType(IFieldSymbol fieldSymbol) { return Diagnostic.Create( - UnexpectedFieldTypeError, - fieldSymbol.Locations[0], + descriptor: UnexpectedFieldTypeError, + location: fieldSymbol.Locations[0], fieldSymbol.ContainingType.Name, fieldSymbol.Name, fieldSymbol.Type.ToDisplayString()); diff --git a/boilerplatezero/boilerplatezero.csproj b/boilerplatezero/boilerplatezero.csproj index 676a3f9..d0a397a 100644 --- a/boilerplatezero/boilerplatezero.csproj +++ b/boilerplatezero/boilerplatezero.csproj @@ -11,7 +11,7 @@ true false boilerplatezero - 1.5.0 + 1.5.1 IGood Copyright (c) Ian Good