diff --git a/src/Controls/src/BindingSourceGen/BindingCodeWriter.cs b/src/Controls/src/BindingSourceGen/BindingCodeWriter.cs index 15ec92e17a82..5a994b926117 100644 --- a/src/Controls/src/BindingSourceGen/BindingCodeWriter.cs +++ b/src/Controls/src/BindingSourceGen/BindingCodeWriter.cs @@ -154,7 +154,7 @@ public void AppendSetBindingInterceptor(int id, CodeWriterBinding binding) Indent(); Indent(); - if (binding.GenerateSetter) + if (binding.SetterOptions.IsWritable) { AppendLines(""" setter = static (source, value) => @@ -162,13 +162,14 @@ public void AppendSetBindingInterceptor(int id, CodeWriterBinding binding) """); Indent(); - AppendSetterAction(binding.SourceType, binding.Path); + AppendSetterAction(binding); Unindent(); AppendLine("};"); } else { + // TODO is this too strict? I believe today when the Binding can't write to the property, it just silently ignores the value AppendLine("throw new InvalidOperationException(\"Cannot set value on the source object.\");"); // TODO improve exception wording } @@ -215,9 +216,30 @@ private void AppendInterceptorAttribute(SourceCodeLocation location) AppendLine($"[InterceptsLocationAttribute(@\"{location.FilePath}\", {location.Line}, {location.Column})]"); } - private void AppendSetterAction(TypeDescription sourceType, IPathPart[] path) + private void AppendSetterAction(CodeWriterBinding binding, string sourceVariableName = "source", string valueVariableName = "value") { - var setter = Setter.From(sourceType, path); + var assignedValueExpression = valueVariableName; + + // early return for nullable values if the setter doesn't accept them + if (binding.PropertyType.IsNullable && !binding.SetterOptions.AcceptsNullValue) + { + if (binding.PropertyType.IsValueType) + { + AppendLine($"if (!{valueVariableName}.HasValue)"); + assignedValueExpression = $"{valueVariableName}.Value"; + } + else + { + AppendLine($"if ({valueVariableName} is null)"); + } + AppendLine('{'); + Indent(); + AppendLine("return;"); + Unindent(); + AppendLine('}'); + } + + var setter = Setter.From(binding.SourceType, binding.Path, sourceVariableName, assignedValueExpression); if (setter.PatternMatchingExpressions.Length > 0) { Append("if ("); diff --git a/src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs b/src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs index 9a3db814169a..d84419e64915 100644 --- a/src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs +++ b/src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs @@ -102,8 +102,7 @@ static BindingDiagnosticsWrapper GetBindingForGeneration(GeneratorSyntaxContext SourceType: BindingGenerationUtilities.CreateTypeNameFromITypeSymbol(lambdaSymbol.Parameters[0].Type, enabledNullable), PropertyType: BindingGenerationUtilities.CreateTypeNameFromITypeSymbol(lambdaTypeInfo.Type, enabledNullable), Path: parts.ToArray(), - GenerateSetter: false //TODO: Implement - ); + SetterOptions: DeriveSetterOptions(lambdaBody, context.SemanticModel, enabledNullable)); return new BindingDiagnosticsWrapper(codeWriterBinding, diagnostics.ToArray()); } @@ -147,6 +146,48 @@ private static (ExpressionSyntax? lambdaBodyExpression, IMethodSymbol? lambdaSym return (lambdaBody, lambdaSymbol, Array.Empty()); } + private static SetterOptions DeriveSetterOptions(ExpressionSyntax? lambdaBodyExpression, SemanticModel semanticModel, bool enabledNullable) + { + if (lambdaBodyExpression is null) + { + return new SetterOptions(IsWritable: false, AcceptsNullValue: false); + } + else if (lambdaBodyExpression is IdentifierNameSyntax identifier) + { + var symbol = semanticModel.GetSymbolInfo(identifier).Symbol; + return new SetterOptions(IsWritable(symbol), AcceptsNullValue(symbol, enabledNullable)); + } + + var nestedExpression = lambdaBodyExpression switch + { + MemberAccessExpressionSyntax memberAccess => memberAccess.Name, + ConditionalAccessExpressionSyntax conditionalAccess => conditionalAccess.WhenNotNull, + MemberBindingExpressionSyntax memberBinding => memberBinding.Name, + BinaryExpressionSyntax binary when binary.Kind() == SyntaxKind.AsExpression => binary.Left, + ElementAccessExpressionSyntax elementAccess => elementAccess.Expression, // TODO indexers don't work correctlly yet + ParenthesizedExpressionSyntax parenthesized => parenthesized.Expression, + _ => null, + }; + + return DeriveSetterOptions(nestedExpression, semanticModel, enabledNullable); + + static bool IsWritable(ISymbol? symbol) + => symbol switch + { + IPropertySymbol propertySymbol => propertySymbol.SetMethod != null, + IFieldSymbol fieldSymbol => !fieldSymbol.IsReadOnly, + _ => false, + }; + + static bool AcceptsNullValue(ISymbol? symbol, bool enabledNullable) + => symbol switch + { + IPropertySymbol propertySymbol => BindingGenerationUtilities.IsTypeNullable(propertySymbol.Type, enabledNullable), + IFieldSymbol fieldSymbol => BindingGenerationUtilities.IsTypeNullable(fieldSymbol.Type, enabledNullable), + _ => false, + }; + } + private static BindingDiagnosticsWrapper ReportDiagnostics(Diagnostic[] diagnostics) => new(null, diagnostics); } @@ -159,7 +200,7 @@ public sealed record CodeWriterBinding( TypeDescription SourceType, TypeDescription PropertyType, IPathPart[] Path, - bool GenerateSetter); + SetterOptions SetterOptions); public sealed record SourceCodeLocation(string FilePath, int Line, int Column); @@ -175,6 +216,8 @@ public override string ToString() : GlobalName; } +public sealed record SetterOptions(bool IsWritable, bool AcceptsNullValue = false); + public sealed record MemberAccess(string MemberName) : IPathPart { public string? PropertyName => MemberName; diff --git a/src/Controls/src/BindingSourceGen/SetterBuilder.cs b/src/Controls/src/BindingSourceGen/SetterBuilder.cs index e3a8b103746f..54c6bcb5fe6d 100644 --- a/src/Controls/src/BindingSourceGen/SetterBuilder.cs +++ b/src/Controls/src/BindingSourceGen/SetterBuilder.cs @@ -9,9 +9,9 @@ public static Setter From( TypeDescription sourceTypeDescription, IPathPart[] path, string sourceVariableName = "source", - string valueVariableName = "value") + string assignedValueExpression = "value") { - var builder = new SetterBuilder(sourceVariableName, valueVariableName); + var builder = new SetterBuilder(sourceVariableName, assignedValueExpression); if (path.Length > 0) { @@ -32,17 +32,17 @@ public static Setter From( private sealed class SetterBuilder { private readonly string _sourceVariableName; - private readonly string _valueVariableName; + private readonly string _assignedValueExpression; private string _expression; private int _variableCounter = 0; private List? _patternMatching; private IPathPart? _previousPart; - public SetterBuilder(string sourceVariableName, string valueVariableName) + public SetterBuilder(string sourceVariableName, string assignedValueExpression) { _sourceVariableName = sourceVariableName; - _valueVariableName = valueVariableName; + _assignedValueExpression = assignedValueExpression; _expression = sourceVariableName; } @@ -98,7 +98,7 @@ private string CreateNextUniqueVariableName() private string CreateAssignmentStatement() { HandlePreviousPart(nextPart: null); - return $"{_expression} = {_valueVariableName};"; + return $"{_expression} = {_assignedValueExpression};"; } public Setter Build() diff --git a/src/Controls/tests/BindingSourceGen.UnitTests/BindingCodeWriterTests.cs b/src/Controls/tests/BindingSourceGen.UnitTests/BindingCodeWriterTests.cs index 878999139f8d..50096be6c3e7 100644 --- a/src/Controls/tests/BindingSourceGen.UnitTests/BindingCodeWriterTests.cs +++ b/src/Controls/tests/BindingSourceGen.UnitTests/BindingCodeWriterTests.cs @@ -19,7 +19,7 @@ public void BuildsWholeDocument() new ConditionalAccess(new MemberAccess("B")), new ConditionalAccess(new MemberAccess("C")), ], - GenerateSetter: true)); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false))); var code = codeWriter.GenerateCode(); AssertExtensions.CodeIsEqual( @@ -140,7 +140,7 @@ public void CorrectlyFormatsSimpleBinding() new ConditionalAccess(new MemberAccess("B")), new ConditionalAccess(new MemberAccess("C")), ], - GenerateSetter: true)); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false))); var code = codeBuilder.ToString(); AssertExtensions.CodeIsEqual( @@ -210,7 +210,7 @@ public void CorrectlyFormatsBindingWithoutAnyNullablesInPath() new MemberAccess("B"), new MemberAccess("C"), ], - GenerateSetter: true)); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false))); var code = codeBuilder.ToString(); AssertExtensions.CodeIsEqual( @@ -276,7 +276,7 @@ public void CorrectlyFormatsBindingWithoutSetter() new MemberAccess("B"), new MemberAccess("C"), ], - GenerateSetter: false)); + SetterOptions: new(IsWritable: false))); var code = codeBuilder.ToString(); AssertExtensions.CodeIsEqual( @@ -339,7 +339,7 @@ public void CorrectlyFormatsBindingWithIndexers() new ConditionalAccess(new IndexAccess("Indexer", new StringIndex("Abc"))), new IndexAccess("Item", new NumericIndex(0)), ], - GenerateSetter: true)); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false))); var code = codeBuilder.ToString(); AssertExtensions.CodeIsEqual( @@ -412,7 +412,7 @@ public void CorrectlyFormatsBindingWithCasts() new Cast(new TypeDescription("Z", IsValueType: true, IsNullable: true, IsGenericParameter: false)), new ConditionalAccess(new MemberAccess("D")), ], - GenerateSetter: true)); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false))); var code = codeBuilder.ToString(); diff --git a/src/Controls/tests/BindingSourceGen.UnitTests/BindingRepresentationGenTests.cs b/src/Controls/tests/BindingSourceGen.UnitTests/BindingRepresentationGenTests.cs index 14490e7bd113..12a2b746ebb1 100644 --- a/src/Controls/tests/BindingSourceGen.UnitTests/BindingRepresentationGenTests.cs +++ b/src/Controls/tests/BindingSourceGen.UnitTests/BindingRepresentationGenTests.cs @@ -24,7 +24,7 @@ public void GenerateSimpleBinding() [ new MemberAccess("Length"), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -47,7 +47,7 @@ public void GenerateBindingWithNestedProperties() new MemberAccess("Text"), new ConditionalAccess(new MemberAccess("Length")), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -76,7 +76,7 @@ class Foo new ConditionalAccess(new MemberAccess("Text")), new ConditionalAccess(new MemberAccess("Length")), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); @@ -100,7 +100,7 @@ public void GenerateBindingWithNullableReferenceSourceWhenNullableEnabled() new ConditionalAccess(new MemberAccess("Text")), new ConditionalAccess(new MemberAccess("Length")), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -127,7 +127,7 @@ class Foo [ new MemberAccess("Value"), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: true, AcceptsNullValue: true)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -150,7 +150,7 @@ public void GenerateBindingWithNullableSourceReferenceAndNullableReferenceElemen new ConditionalAccess(new MemberAccess("Text")), new ConditionalAccess(new MemberAccess("Length")), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -177,8 +177,7 @@ class Foo [ new MemberAccess("Value"), ], - GenerateSetter: false - ); + SetterOptions: new(IsWritable: true, AcceptsNullValue: true)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -207,7 +206,7 @@ class Foo new ConditionalAccess(new MemberAccess("Bar")), new ConditionalAccess(new MemberAccess("Length")), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -235,7 +234,7 @@ class Foo [ new MemberAccess("Value"), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: true, AcceptsNullValue: true)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -264,7 +263,7 @@ class Foo new IndexAccess("Item", new NumericIndex(0)), new MemberAccess("Length"), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -294,7 +293,7 @@ class Foo new IndexAccess("Item", new StringIndex("key")), new MemberAccess("Length"), ], - GenerateSetter: false); + SetterOptions: new(IsWritable: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -322,8 +321,7 @@ class Foo new MemberAccess("Value"), new Cast(new TypeDescription("string")), ], - GenerateSetter: false - ); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -357,8 +355,7 @@ class C new Cast(new TypeDescription("global::C")), new ConditionalAccess(new MemberAccess("X")), ], - GenerateSetter: false - ); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -392,8 +389,7 @@ class C new Cast(new TypeDescription("global::C")), new ConditionalAccess(new MemberAccess("X")), ], - GenerateSetter: false - ); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } @@ -421,8 +417,7 @@ class Foo new MemberAccess("Value"), new Cast(new TypeDescription("int", IsNullable: true, IsValueType: true)), ], - GenerateSetter: false - ); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); @@ -457,8 +452,7 @@ struct C new Cast(new TypeDescription("global::C", IsNullable: true, IsValueType: true)), new ConditionalAccess(new MemberAccess("X")), ], - GenerateSetter: false - ); + SetterOptions: new(IsWritable: true, AcceptsNullValue: false)); AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult); } diff --git a/src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs b/src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs index f2f52b469cff..9e534b3b2a49 100644 --- a/src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs +++ b/src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs @@ -213,7 +213,19 @@ public static void SetBinding1( Action? setter = null; if (ShouldUseSetter(mode, bindableProperty)) { - throw new InvalidOperationException("Cannot set value on the source object."); + setter = static (source, value) => + { + if (value is null) + { + return; + } + if (source.A is global::MyNamespace.X p0 + && p0.B is global::MyNamespace.Y p1 + && p1.C is global::MyNamespace.Z p2) + { + p2.D = value; + } + }; } var binding = new TypedBinding(