Skip to content

Commit

Permalink
Improved nullable support (#30)
Browse files Browse the repository at this point in the history 
* added unit tests for nullable disabled

* added integration tests for nullable disabled

* add nullable disabled support for reference types

* wip: added support for nullable disabled on value types

* improved integration tests

* initialize support for indexers

* add indexers support to SetterBuilder

* added additional tests for setter builder

* cleaned up SetterBuilder

* Fixed nullable access with BindingTransformer

Co-authored-by: Šimon Rozsíval <simon@rozsival.com>

* Removed IsNullableValueType property

Co-authored-by: Šimon Rozsíval <simon@rozsival.com>

* Cleaned up nullability

Co-authored-by: Šimon Rozsíval <simon@rozsival.com>

---------

Co-authored-by: Šimon Rozsíval <simon@rozsival.com>
  • Loading branch information
@jkurdek @simonrozsival
jkurdek and simonrozsival authored May 14, 2024
1 parent b9394d9 commit bff3429
Show file tree
Hide file tree
Showing 12 changed files with 1,080 additions and 125 deletions.
16 changes: 11 additions & 5 deletions src/Controls/src/BindingSourceGen/BindingCodeWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ private static bool ShouldUseSetter(BindingMode mode, BindableProperty bindableP

public void AddBinding(SetBindingInvocationDescription binding)
{
if (!binding.NullableContextEnabled)
{
var referenceTypesConditionalAccessTransformer = new ReferenceTypesConditionalAccessTransformer();
binding = referenceTypesConditionalAccessTransformer.Transform(binding);
}
_bindings.Add(binding);
}

Expand Down Expand Up @@ -184,7 +189,7 @@ public void AppendSetBindingInterceptor(int id, SetBindingInvocationDescription
Indent();

Append("handlers: ");
AppendHandlersArray(binding.SourceType, binding.Path);
AppendHandlersArray(binding);
AppendLine(")");

Unindent();
Expand Down Expand Up @@ -234,7 +239,7 @@ private void AppendSetterAction(SetBindingInvocationDescription binding, string
AppendLine('}');
}

var setter = Setter.From(binding.SourceType, binding.Path, sourceVariableName, assignedValueExpression);
var setter = Setter.From(binding.Path, sourceVariableName, assignedValueExpression);
if (setter.PatternMatchingExpressions.Length > 0)
{
Append("if (");
Expand Down Expand Up @@ -274,19 +279,20 @@ private void AppendSetterAction(SetBindingInvocationDescription binding, string
}
}

private void AppendHandlersArray(TypeDescription sourceType, EquatableArray<IPathPart> path)
private void AppendHandlersArray(SetBindingInvocationDescription binding)
{
AppendLine($"new Tuple<Func<{sourceType}, object?>, string>[]");
AppendLine($"new Tuple<Func<{binding.SourceType}, object?>, string>[]");
AppendLine('{');

Indent();

string nextExpression = "source";
bool forceConditonalAccessToNextPart = false;
foreach (var part in path)
foreach (var part in binding.Path)
{
var previousExpression = nextExpression;
nextExpression = AccessExpressionBuilder.Build(previousExpression, MaybeWrapInConditionalAccess(part, forceConditonalAccessToNextPart));
var isNullableReferenceType = part is MemberAccess memberAccess && !memberAccess.IsValueType;
forceConditonalAccessToNextPart = part is Cast;

// Some parts don't have a property name, so we can't generate a handler for them (for example casts)
Expand Down
3 changes: 2 additions & 1 deletion src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ static BindingDiagnosticsWrapper GetBindingForGeneration(GeneratorSyntaxContext
SourceType: BindingGenerationUtilities.CreateTypeNameFromITypeSymbol(lambdaSymbol.Parameters[0].Type, enabledNullable),
PropertyType: BindingGenerationUtilities.CreateTypeNameFromITypeSymbol(lambdaTypeInfo.Type, enabledNullable),
Path: new EquatableArray<IPathPart>([.. parts]),
SetterOptions: DeriveSetterOptions(lambdaBody, context.SemanticModel, enabledNullable));
SetterOptions: DeriveSetterOptions(lambdaBody, context.SemanticModel, enabledNullable),
NullableContextEnabled: enabledNullable);
return new BindingDiagnosticsWrapper(binding, new EquatableArray<DiagnosticInfo>([.. diagnostics]));
}

Expand Down
10 changes: 7 additions & 3 deletions src/Controls/src/BindingSourceGen/BindingSourceGeneratorUtilities.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ namespace Microsoft.Maui.Controls.BindingSourceGen;

internal static class BindingGenerationUtilities
{

internal static bool IsNullableValueType(ITypeSymbol typeInfo) =>
typeInfo is INamedTypeSymbol namedTypeSymbol
&& namedTypeSymbol.IsGenericType
&& namedTypeSymbol.ConstructedFrom.SpecialType == SpecialType.System_Nullable_T;

internal static bool IsTypeNullable(ITypeSymbol typeInfo, bool enabledNullable)
{
if (!enabledNullable && typeInfo.IsReferenceType)
Expand All @@ -16,9 +22,7 @@ internal static bool IsTypeNullable(ITypeSymbol typeInfo, bool enabledNullable)
return true;
}

return typeInfo is INamedTypeSymbol namedTypeSymbol
&& namedTypeSymbol.IsGenericType
&& namedTypeSymbol.ConstructedFrom.SpecialType == SpecialType.System_Nullable_T;
return IsNullableValueType(typeInfo);
}

internal static TypeDescription CreateTypeNameFromITypeSymbol(ITypeSymbol typeSymbol, bool enabledNullable)
Expand Down
50 changes: 50 additions & 0 deletions src/Controls/src/BindingSourceGen/BindingTransformer.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@

namespace Microsoft.Maui.Controls.BindingSourceGen;

public interface IBindingInvocationTransformer
{
SetBindingInvocationDescription Transform(SetBindingInvocationDescription setBindingInvocationDescription);
}

public class ReferenceTypesConditionalAccessTransformer : IBindingInvocationTransformer
{
public SetBindingInvocationDescription Transform(SetBindingInvocationDescription setBindingInvocationDescription)
{
var path = TransformPath(setBindingInvocationDescription);
return setBindingInvocationDescription with { Path = path };
}

private static EquatableArray<IPathPart> TransformPath(SetBindingInvocationDescription setBindingInvocationDescription)
{
var newPath = new List<IPathPart>();
foreach (var pathPart in setBindingInvocationDescription.Path)
{
var sourceIsReferenceType = newPath.Count == 0 && !setBindingInvocationDescription.SourceType.IsValueType;
var previousPartIsReferenceType = newPath.Count > 0 && PreviousPartIsReferenceType(newPath.Last());

if (pathPart is not MemberAccess && pathPart is not IndexAccess)
{
newPath.Add(pathPart);
}
else if (sourceIsReferenceType || previousPartIsReferenceType)
{
newPath.Add(new ConditionalAccess(pathPart));
}
else
{
newPath.Add(pathPart);
}
}

return new EquatableArray<IPathPart>(newPath.ToArray());

static bool PreviousPartIsReferenceType(IPathPart previousPathPart) =>
previousPathPart switch
{
MemberAccess memberAccess => !memberAccess.IsValueType,
IndexAccess indexAccess => !indexAccess.IsValueType,
ConditionalAccess { Part: var inner } => PreviousPartIsReferenceType(inner),
_ => false,
};
}
}
18 changes: 12 additions & 6 deletions src/Controls/src/BindingSourceGen/GeneratorDataModels.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ public sealed record SetBindingInvocationDescription(
TypeDescription SourceType,
TypeDescription PropertyType,
EquatableArray<IPathPart> Path,
SetterOptions SetterOptions);
SetterOptions SetterOptions,
bool NullableContextEnabled);

public sealed record SourceCodeLocation(string FilePath, TextSpan TextSpan, LinePositionSpan LineSpan)
{
Expand Down Expand Up @@ -54,23 +55,28 @@ public override string ToString()

public sealed record SetterOptions(bool IsWritable, bool AcceptsNullValue = false);

public sealed record MemberAccess(string MemberName) : IPathPart
public sealed record MemberAccess(string MemberName, bool IsValueType = false) : IPathPart
{
public string? PropertyName => MemberName;
public string PropertyName => MemberName;

public bool Equals(IPathPart other)
{
return other is MemberAccess memberAccess && MemberName == memberAccess.MemberName;
return other is MemberAccess memberAccess
&& MemberName == memberAccess.MemberName
&& IsValueType == memberAccess.IsValueType;
}
}

public sealed record IndexAccess(string DefaultMemberName, object Index) : IPathPart
public sealed record IndexAccess(string DefaultMemberName, object Index, bool IsValueType = false) : IPathPart
{
public string? PropertyName => $"{DefaultMemberName}[{Index}]";

public bool Equals(IPathPart other)
{
return other is IndexAccess indexAccess && DefaultMemberName == indexAccess.DefaultMemberName && Index.Equals(indexAccess.Index);
return other is IndexAccess indexAccess
&& DefaultMemberName == indexAccess.DefaultMemberName
&& Index.Equals(indexAccess.Index)
&& IsValueType == indexAccess.IsValueType;
}
}

Expand Down
22 changes: 15 additions & 7 deletions src/Controls/src/BindingSourceGen/PathParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ BinaryExpressionSyntax asExpression when asExpression.Kind() == SyntaxKind.AsExp
}

var member = memberAccess.Name.Identifier.Text;
IPathPart part = new MemberAccess(member);
var typeInfo = Context.SemanticModel.GetTypeInfo(memberAccess).Type;
var isReferenceType = typeInfo?.IsReferenceType ?? false;
IPathPart part = new MemberAccess(member, !isReferenceType);
parts.Add(part);
return (diagnostics, parts);
}
Expand All @@ -55,12 +57,13 @@ BinaryExpressionSyntax asExpression when asExpression.Kind() == SyntaxKind.AsExp
}

var elementAccessSymbol = Context.SemanticModel.GetSymbolInfo(elementAccess).Symbol;
var (elementAccessDiagnostics, elementAccessParts) = HandleElementAccessSymbol(elementAccessSymbol, elementAccess.ArgumentList.Arguments, elementAccess.GetLocation());
var elementType = Context.SemanticModel.GetTypeInfo(elementAccess).Type;

var (elementAccessDiagnostics, elementAccessParts) = CreateIndexAccess(elementAccessSymbol, elementType, elementAccess.ArgumentList.Arguments, elementAccess.GetLocation());
if (elementAccessDiagnostics.Length > 0)
{
return (elementAccessDiagnostics, elementAccessParts);
}

parts.AddRange(elementAccessParts);
return (diagnostics, parts);
}
Expand All @@ -86,7 +89,9 @@ BinaryExpressionSyntax asExpression when asExpression.Kind() == SyntaxKind.AsExp
private (EquatableArray<DiagnosticInfo> diagnostics, List<IPathPart> parts) HandleMemberBindingExpression(MemberBindingExpressionSyntax memberBinding)
{
var member = memberBinding.Name.Identifier.Text;
IPathPart part = new MemberAccess(member);
var typeInfo = Context.SemanticModel.GetTypeInfo(memberBinding).Type;
var isReferenceType = typeInfo?.IsReferenceType ?? false;
IPathPart part = new MemberAccess(member, !isReferenceType);
part = new ConditionalAccess(part);

return ([], new List<IPathPart>([part]));
Expand All @@ -95,7 +100,9 @@ BinaryExpressionSyntax asExpression when asExpression.Kind() == SyntaxKind.AsExp
private (EquatableArray<DiagnosticInfo> diagnostics, List<IPathPart> parts) HandleElementBindingExpression(ElementBindingExpressionSyntax elementBinding)
{
var elementAccessSymbol = Context.SemanticModel.GetSymbolInfo(elementBinding).Symbol;
var (elementAccessDiagnostics, elementAccessParts) = HandleElementAccessSymbol(elementAccessSymbol, elementBinding.ArgumentList.Arguments, elementBinding.GetLocation());
var elementType = Context.SemanticModel.GetTypeInfo(elementBinding).Type;

var (elementAccessDiagnostics, elementAccessParts) = CreateIndexAccess(elementAccessSymbol, elementType, elementBinding.ArgumentList.Arguments, elementBinding.GetLocation());
if (elementAccessDiagnostics.Length > 0)
{
return (elementAccessDiagnostics, elementAccessParts);
Expand Down Expand Up @@ -147,7 +154,7 @@ BinaryExpressionSyntax asExpression when asExpression.Kind() == SyntaxKind.AsExp
return (new EquatableArray<DiagnosticInfo>([DiagnosticsFactory.UnableToResolvePath(Context.Node.GetLocation())]), new List<IPathPart>());
}

private (EquatableArray<DiagnosticInfo>, List<IPathPart>) HandleElementAccessSymbol(ISymbol? elementAccessSymbol, SeparatedSyntaxList<ArgumentSyntax> argumentList, Location location)
private (EquatableArray<DiagnosticInfo>, List<IPathPart>) CreateIndexAccess(ISymbol? elementAccessSymbol, ITypeSymbol? typeSymbol, SeparatedSyntaxList<ArgumentSyntax> argumentList, Location location)
{
if (argumentList.Count != 1)
{
Expand All @@ -162,7 +169,8 @@ BinaryExpressionSyntax asExpression when asExpression.Kind() == SyntaxKind.AsExp
}

var name = GetIndexerName(elementAccessSymbol);
IPathPart part = new IndexAccess(name, indexValue);
var isReferenceType = typeSymbol?.IsReferenceType ?? false;
IPathPart part = new IndexAccess(name, indexValue, !isReferenceType);

return ([], [part]);
}
Expand Down
18 changes: 3 additions & 15 deletions src/Controls/src/BindingSourceGen/SetterBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,22 @@ namespace Microsoft.Maui.Controls.BindingSourceGen;
public sealed record Setter(string[] PatternMatchingExpressions, string AssignmentStatement)
{
public static Setter From(
TypeDescription sourceTypeDescription,
EquatableArray<IPathPart> path,
IEnumerable<IPathPart> path,
string sourceVariableName = "source",
string assignedValueExpression = "value")
{
var builder = new SetterBuilder(sourceVariableName, assignedValueExpression);

if (path.Length > 0)
foreach (var part in path)
{
if (sourceTypeDescription.IsNullable)
{
builder.AddIsExpression("{}");
}

foreach (var part in path)
{
builder.AddPart(part);
}
builder.AddPart(part);
}

return builder.Build();
}

private sealed class SetterBuilder
{
private readonly string _sourceVariableName;
private readonly string _assignedValueExpression;

private string _expression;
Expand All @@ -38,9 +28,7 @@ private sealed class SetterBuilder

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

_expression = sourceVariableName;
}

Expand Down
18 changes: 12 additions & 6 deletions src/Controls/tests/BindingSourceGen.UnitTests/BindingCodeWriterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public void BuildsWholeDocument()
new ConditionalAccess(new MemberAccess("B")),
new ConditionalAccess(new MemberAccess("C")),
]),
SetterOptions: new(IsWritable: true, AcceptsNullValue: false)));
SetterOptions: new(IsWritable: true, AcceptsNullValue: false),
NullableContextEnabled: true));

var code = codeWriter.GenerateCode();
AssertExtensions.CodeIsEqual(
Expand Down Expand Up @@ -140,7 +141,8 @@ public void CorrectlyFormatsSimpleBinding()
new ConditionalAccess(new MemberAccess("B")),
new ConditionalAccess(new MemberAccess("C")),
]),
SetterOptions: new(IsWritable: true, AcceptsNullValue: false)));
SetterOptions: new(IsWritable: true, AcceptsNullValue: false),
NullableContextEnabled: true));

var code = codeBuilder.ToString();
AssertExtensions.CodeIsEqual(
Expand Down Expand Up @@ -210,7 +212,8 @@ public void CorrectlyFormatsBindingWithoutAnyNullablesInPath()
new MemberAccess("B"),
new MemberAccess("C"),
]),
SetterOptions: new(IsWritable: true, AcceptsNullValue: false)));
SetterOptions: new(IsWritable: true, AcceptsNullValue: false),
NullableContextEnabled: true));

var code = codeBuilder.ToString();
AssertExtensions.CodeIsEqual(
Expand Down Expand Up @@ -276,7 +279,8 @@ public void CorrectlyFormatsBindingWithoutSetter()
new MemberAccess("B"),
new MemberAccess("C"),
]),
SetterOptions: new(IsWritable: false)));
SetterOptions: new(IsWritable: false),
NullableContextEnabled: true));

var code = codeBuilder.ToString();
AssertExtensions.CodeIsEqual(
Expand Down Expand Up @@ -339,7 +343,8 @@ public void CorrectlyFormatsBindingWithIndexers()
new ConditionalAccess(new IndexAccess("Indexer", "Abc")),
new IndexAccess("Item", 0),
]),
SetterOptions: new(IsWritable: true, AcceptsNullValue: false)));
SetterOptions: new(IsWritable: true, AcceptsNullValue: false),
NullableContextEnabled: true));

var code = codeBuilder.ToString();
AssertExtensions.CodeIsEqual(
Expand Down Expand Up @@ -417,7 +422,8 @@ public void CorrectlyFormatsBindingWithCasts()
new Cast(new TypeDescription("Z", IsValueType: true, IsNullable: true, IsGenericParameter: false)),
new ConditionalAccess(new MemberAccess("D")),
]),
SetterOptions: new(IsWritable: true, AcceptsNullValue: false)));
SetterOptions: new(IsWritable: true, AcceptsNullValue: false),
NullableContextEnabled: true));

var code = codeBuilder.ToString();

Expand Down
Loading

0 comments on commit bff3429

Please sign in to comment.