Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement detection of writable properties #19

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Implement detection of writable properties #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions src/Controls/src/BindingSourceGen/BindingCodeWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,21 +154,22 @@ public void AppendSetBindingInterceptor(int id, CodeWriterBinding binding)
Indent();
Indent();

if (binding.GenerateSetter)
if (binding.SetterOptions.IsWritable)
{
AppendLines("""
setter = static (source, value) =>
{
""");
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
}

Expand Down Expand Up @@ -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 (");
Expand Down
49 changes: 46 additions & 3 deletions src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down Expand Up @@ -147,6 +146,48 @@ private static (ExpressionSyntax? lambdaBodyExpression, IMethodSymbol? lambdaSym
return (lambdaBody, lambdaSymbol, Array.Empty<Diagnostic>());
}

private static SetterOptions DeriveSetterOptions(ExpressionSyntax? lambdaBodyExpression, SemanticModel semanticModel, bool enabledNullable)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. In the future we probably want to add diagnostics emission here.

{
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);
}

Expand All @@ -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);

Expand All @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions src/Controls/src/BindingSourceGen/SetterBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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<string>? _patternMatching;
private IPathPart? _previousPart;

public SetterBuilder(string sourceVariableName, string valueVariableName)
public SetterBuilder(string sourceVariableName, string assignedValueExpression)
{
_sourceVariableName = sourceVariableName;
_valueVariableName = valueVariableName;
_assignedValueExpression = assignedValueExpression;

_expression = sourceVariableName;
}
Expand Down Expand Up @@ -98,7 +98,7 @@ private string CreateNextUniqueVariableName()
private string CreateAssignmentStatement()
{
HandlePreviousPart(nextPart: null);
return $"{_expression} = {_valueVariableName};";
return $"{_expression} = {_assignedValueExpression};";
}

public Setter Build()
Expand Down
12 changes: 6 additions & 6 deletions src/Controls/tests/BindingSourceGen.UnitTests/BindingCodeWriterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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();

Expand Down
38 changes: 16 additions & 22 deletions src/Controls/tests/BindingSourceGen.UnitTests/BindingRepresentationGenTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public void GenerateSimpleBinding()
[
new MemberAccess("Length"),
],
GenerateSetter: false);
SetterOptions: new(IsWritable: false));

AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}
Expand All @@ -127,7 +127,7 @@ class Foo
[
new MemberAccess("Value"),
],
GenerateSetter: false);
SetterOptions: new(IsWritable: true, AcceptsNullValue: true));

AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult);
}
Expand All @@ -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);
}
Expand All @@ -177,8 +177,7 @@ class Foo
[
new MemberAccess("Value"),
],
GenerateSetter: false
);
SetterOptions: new(IsWritable: true, AcceptsNullValue: true));

AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -235,7 +234,7 @@ class Foo
[
new MemberAccess("Value"),
],
GenerateSetter: false);
SetterOptions: new(IsWritable: true, AcceptsNullValue: true));

AssertExtensions.BindingsAreEqual(expectedBinding, codeGeneratorResult);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
Loading
Loading