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.

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

[semi-auto-props]: Refactor and rename 'AccessingAutoPropertyFromConstructor' #58832

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
if (setMethod is null)
{
var containing = this.ContainingMemberOrLambda;
if (!AccessingAutoPropertyFromConstructor(receiver, propertySymbol, containing)
if (!IsPropertyAssignedThroughBackingField(receiver, propertySymbol, containing)
&& !isAllowedDespiteReadonly(receiver))
{
Error(diagnostics, ErrorCode.ERR_AssgReadonlyProp, node, propertySymbol);
Expand Down
24 changes: 15 additions & 9 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1668,26 +1668,32 @@ private DiagnosticInfo GetBadEventUsageDiagnosticInfo(EventSymbol eventSymbol)
new CSDiagnosticInfo(ErrorCode.ERR_BadEventUsageNoField, leastOverridden);
}

internal static bool AccessingAutoPropertyFromConstructor(BoundPropertyAccess propertyAccess, Symbol fromMember)
internal static bool IsPropertyAssignedThroughBackingField(BoundPropertyAccess propertyAccess, Symbol fromMember)
{
return AccessingAutoPropertyFromConstructor(propertyAccess.ReceiverOpt, propertyAccess.PropertySymbol, fromMember);
return IsPropertyAssignedThroughBackingField(propertyAccess.ReceiverOpt, propertyAccess.PropertySymbol, fromMember);
}

private static bool AccessingAutoPropertyFromConstructor(BoundExpression receiver, PropertySymbol propertySymbol, Symbol fromMember)
/// <summary>
/// Determines whether a property is assigned through the backing field within a symbol <paramref name="fromMember"/>
/// </summary>
/// <remarks>
/// A property is assigned through backing field within a constructor or a field assignment.
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
/// </remarks>
private static bool IsPropertyAssignedThroughBackingField(BoundExpression receiver, PropertySymbol propertySymbol, Symbol fromMember)
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
{
if (!propertySymbol.IsDefinition && propertySymbol.ContainingType.Equals(propertySymbol.ContainingType.OriginalDefinition, TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
propertySymbol = propertySymbol.OriginalDefinition;
}

var sourceProperty = propertySymbol as SourcePropertySymbolBase;
var propertyIsStatic = propertySymbol.IsStatic;
// PROTOTYPE(semi-auto-props): TODO: Support assigning semi auto prop from constructors.

return (object)sourceProperty != null &&
sourceProperty.IsAutoPropertyWithGetAccessor &&
return propertySymbol is SourcePropertySymbolBase sourceProperty &&
// To be assigned through backing field, either SetMethod is null, or it's equivalent to backing field write
sourceProperty.SetMethod is not SourcePropertyAccessorSymbol { IsEquivalentToBackingFieldAccess: false } &&
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
TypeSymbol.Equals(sourceProperty.ContainingType, fromMember.ContainingType, TypeCompareKind.ConsiderEverything2) &&
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
IsConstructorOrField(fromMember, isStatic: propertyIsStatic) &&
(propertyIsStatic || receiver.Kind == BoundKind.ThisReference);
IsConstructorOrField(fromMember, isStatic: sourceProperty.IsStatic) &&
(sourceProperty.IsStatic || receiver.Kind == BoundKind.ThisReference);
}

private static bool IsConstructorOrField(Symbol member, bool isStatic)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ protected void VisitLvalue(BoundExpression node)
case BoundKind.PropertyAccess:
var access = (BoundPropertyAccess)node;

if (Binder.AccessingAutoPropertyFromConstructor(access, _symbol))
if (Binder.IsPropertyAssignedThroughBackingField(access, _symbol))
{
var backingField = (access.PropertySymbol as SourcePropertySymbolBase)?.BackingField;
if (backingField != null)
Expand Down Expand Up @@ -1915,7 +1915,7 @@ private bool RegularPropertyAccess(BoundExpression expr) // PROTOTYPE(semi-auto-
return false;
}

return !Binder.AccessingAutoPropertyFromConstructor((BoundPropertyAccess)expr, _symbol);
return !Binder.IsPropertyAssignedThroughBackingField((BoundPropertyAccess)expr, _symbol); // PROTOTYPE(semi-auto-props): Revise this method call is the behavior we want and add unit tests..
}

public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node)
Expand Down Expand Up @@ -2051,7 +2051,7 @@ public override BoundNode VisitPropertyAccess(BoundPropertyAccess node)
{
var property = node.PropertySymbol;

if (Binder.AccessingAutoPropertyFromConstructor(node, _symbol))
if (Binder.IsPropertyAssignedThroughBackingField(node, _symbol)) // PROTOTYPE(semi-auto-props): Revise this method call is the behavior we want and add unit tests..
{
var backingField = (property as SourcePropertySymbolBase)?.BackingField;
if (backingField != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,10 @@ protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundE
{
var propAccess = (BoundPropertyAccess)expr;

if (Binder.AccessingAutoPropertyFromConstructor(propAccess, this.CurrentSymbol))
// PROTOTYPE(semi-auto-props): The call to IsPropertyAssignedThroughBackingField isn't sensible.
// We get here for property read.
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
// This applies to ALL IsPropertyAssignedThroughBackingField calls in this file.
if (Binder.IsPropertyAssignedThroughBackingField(propAccess, this.CurrentSymbol)) // PROTOTYPE(semi-auto-props): Revise this method call is the behavior we want and add unit tests..
{
var propSymbol = propAccess.PropertySymbol;
member = (propSymbol as SourcePropertySymbolBase)?.BackingField;
Expand Down Expand Up @@ -1189,7 +1192,7 @@ private bool IsAssigned(BoundExpression node, out int unassignedSlot)
case BoundKind.PropertyAccess:
{
var propertyAccess = (BoundPropertyAccess)node;
if (Binder.AccessingAutoPropertyFromConstructor(propertyAccess, this.CurrentSymbol))
if (Binder.IsPropertyAssignedThroughBackingField(propertyAccess, this.CurrentSymbol)) // PROTOTYPE(semi-auto-props): Revise this method call is the behavior we want and add unit tests..
{
var property = propertyAccess.PropertySymbol;
var backingField = (property as SourcePropertySymbolBase)?.BackingField;
Expand Down Expand Up @@ -2240,7 +2243,7 @@ public override BoundNode VisitFieldAccess(BoundFieldAccess node)
public override BoundNode VisitPropertyAccess(BoundPropertyAccess node)
{
var result = base.VisitPropertyAccess(node);
if (Binder.AccessingAutoPropertyFromConstructor(node, this.CurrentSymbol))
if (Binder.IsPropertyAssignedThroughBackingField(node, this.CurrentSymbol)) // PROTOTYPE(semi-auto-props): Revise this method call is the behavior we want and add unit tests..
{
var property = node.PropertySymbol;
var backingField = (property as SourcePropertySymbolBase)?.BackingField;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,11 @@ internal Accessibility LocalAccessibility
/// </summary>
internal bool BodyShouldBeSynthesized => _bodyShouldBeSynthesized;

/// <summary>
/// Indicates whether this get/set accessor is equivalent to a backing field read/write.
/// </summary>
internal bool IsEquivalentToBackingFieldAccess => _bodyShouldBeSynthesized;

/// <summary>
/// Indicates whether this accessor is readonly due to reasons scoped to itself and its containing property.
/// </summary>
Expand Down