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

Partial properties: attributes #73437

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal abstract class FieldSymbolWithAttributesAndModifiers : FieldSymbol, IAt
/// <summary>
/// Gets the syntax list of custom attributes applied on the symbol.
/// </summary>
protected abstract SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList { get; }
protected abstract OneOrMany<SyntaxList<AttributeListSyntax>> AttributeDeclarationLists { get; }
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved

protected abstract IAttributeTargetSymbol AttributeOwner { get; }

Expand Down Expand Up @@ -86,7 +86,7 @@ private CustomAttributesBag<CSharpAttributeData> GetAttributesBag()
return bag;
}

if (LoadAndValidateAttributes(OneOrMany.Create(this.AttributeDeclarationSyntaxList), ref _lazyCustomAttributesBag))
if (LoadAndValidateAttributes(this.AttributeDeclarationLists, ref _lazyCustomAttributesBag))
{
var completed = state.NotePartComplete(CompletionPart.Attributes);
Debug.Assert(completed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ internal static GlobalExpressionVariable Create(
: new GlobalExpressionVariable(containingType, modifiers, typeSyntax, name, syntaxReference, locationSpan);
}

protected override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList => default(SyntaxList<AttributeListSyntax>);
protected override OneOrMany<SyntaxList<AttributeListSyntax>> AttributeDeclarationLists => OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;
protected override TypeSyntax TypeSyntax => (TypeSyntax)_typeSyntaxOpt?.GetSyntax();
protected override SyntaxTokenList ModifiersTokenList => default(SyntaxTokenList);
public override bool HasInitializer => false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,30 +454,55 @@ AttributeLocation IAttributeTargetSymbol.AllowedAttributeLocations
}
}

#nullable enable
/// <summary>
/// Symbol to copy bound attributes from, or null if the attributes are not shared among multiple source parameter symbols.
/// </summary>
/// <remarks>
/// Used for parameters of partial implementation. We bind the attributes only on the definition
/// part and copy them over to the implementation.
/// This is inconsistent with analogous 'BoundAttributesSource' on other symbols.
/// Usually the definition part is the source, but for parameters the implementation part is the source.
/// This affects the location of diagnostics among other things.
/// </remarks>
private SourceParameterSymbol BoundAttributesSource
private SourceParameterSymbol? BoundAttributesSource
=> PartialImplementationPart;

protected SourceParameterSymbol? PartialImplementationPart
{
get
{
var sourceMethod = this.ContainingSymbol as SourceOrdinaryMethodSymbol;
if ((object)sourceMethod == null)
ImmutableArray<ParameterSymbol> implParameters = this.ContainingSymbol switch
{
SourceMemberMethodSymbol method => method.PartialImplementationPart?.Parameters ?? default,
SourcePropertySymbol property => property.PartialImplementationPart?.Parameters ?? default,
333fred marked this conversation as resolved.
Show resolved Hide resolved
_ => default
};

if (implParameters.IsDefault)
{
return null;
}

var impl = sourceMethod.SourcePartialImplementation;
if ((object)impl == null)
return (SourceParameterSymbol)implParameters[this.Ordinal];
333fred marked this conversation as resolved.
Show resolved Hide resolved
}
}

protected SourceParameterSymbol? PartialDefinitionPart
{
get
{
ImmutableArray<ParameterSymbol> defParameters = this.ContainingSymbol switch
{
SourceMemberMethodSymbol method => method.PartialDefinitionPart?.Parameters ?? default,
SourcePropertySymbol property => property.PartialDefinitionPart?.Parameters ?? default,
333fred marked this conversation as resolved.
Show resolved Hide resolved
_ => default
};

if (defParameters.IsDefault)
{
return null;
}

return (SourceParameterSymbol)impl.Parameters[this.Ordinal];
return (SourceParameterSymbol)defParameters[this.Ordinal];
333fred marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -495,44 +520,19 @@ internal sealed override SyntaxList<AttributeListSyntax> AttributeDeclarationLis
/// </summary>
internal virtual OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
{
// C# spec:
// The attributes on the parameters of the resulting method declaration
// are the combined attributes of the corresponding parameters of the defining
// and the implementing partial method declaration in unspecified order.
// Duplicates are not removed.

SyntaxList<AttributeListSyntax> attributes = AttributeDeclarationList;

var sourceMethod = this.ContainingSymbol as SourceOrdinaryMethodSymbol;
if ((object)sourceMethod == null)
Debug.Assert(PartialImplementationPart is null);
if (PartialDefinitionPart is { } definitionPart)
{
return OneOrMany.Create(attributes);
}

SyntaxList<AttributeListSyntax> otherAttributes;

// if this is a definition get the implementation and vice versa
SourceOrdinaryMethodSymbol otherPart = sourceMethod.OtherPartOfPartial;
if ((object)otherPart != null)
{
otherAttributes = ((SourceParameterSymbol)otherPart.Parameters[this.Ordinal]).AttributeDeclarationList;
return OneOrMany.Create(
AttributeDeclarationList,
definitionPart.AttributeDeclarationList);
}
else
{
otherAttributes = default(SyntaxList<AttributeListSyntax>);
}

if (attributes.Equals(default(SyntaxList<AttributeListSyntax>)))
{
return OneOrMany.Create(otherAttributes);
}
else if (otherAttributes.Equals(default(SyntaxList<AttributeListSyntax>)))
{
return OneOrMany.Create(attributes);
return OneOrMany.Create(AttributeDeclarationList);
}

return OneOrMany.Create(ImmutableArray.Create(attributes, otherAttributes));
}
#nullable disable

/// <summary>
/// Returns data decoded from well-known attributes applied to the symbol or null if there are no applied attributes.
Expand Down Expand Up @@ -1030,33 +1030,26 @@ private bool IsValidCallerInfoContext(AttributeSyntax node) => !ContainingSymbol
&& !ContainingSymbol.IsOperator()
&& !IsOnPartialImplementation(node);

#nullable enable
/// <summary>
/// Is the attribute syntax appearing on a parameter of a partial method implementation part?
/// Since attributes are merged between the parts of a partial, we need to look at the syntax where the
/// attribute appeared in the source to see if it corresponds to a partial method implementation part.
/// </summary>
/// <param name="node"></param>
/// <returns></returns>
private bool IsOnPartialImplementation(AttributeSyntax node)
{
var method = ContainingSymbol as MethodSymbol;
if ((object)method == null) return false;
var impl = method.IsPartialImplementation() ? method : method.PartialImplementationPart;
if ((object)impl == null) return false;
var paramList =
node // AttributeSyntax
.Parent // AttributeListSyntax
.Parent // ParameterSyntax
.Parent as ParameterListSyntax; // ParameterListSyntax
if (paramList == null) return false;
var methDecl = paramList.Parent as MethodDeclarationSyntax;
if (methDecl == null) return false;
foreach (var r in impl.DeclaringSyntaxReferences)
{
if (r.GetSyntax() == methDecl) return true;
}
return false;
// If we are asking this, the candidate attribute had better be contained in *some* attribute associated with this parameter syntactically
Debug.Assert(this.GetAttributeDeclarations().Any(attrLists => attrLists.Any(attrList => attrList.Contains(node))));

var implParameter = this.ContainingSymbol.IsPartialImplementation() ? this : PartialImplementationPart;
if (implParameter?.AttributeDeclarationList is not { } implParameterAttributeList)
{
333fred marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

return implParameterAttributeList.Any(attrList => attrList.Attributes.Contains(node));
}
#nullable disable

private void ValidateCallerLineNumberAttribute(AttributeSyntax node, BindingDiagnosticBag diagnostics)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,16 @@ protected sealed override DeclarationModifiers Modifiers
}
}

protected override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
protected override OneOrMany<SyntaxList<AttributeListSyntax>> AttributeDeclarationLists
{
get
{
if (this.containingType.AnyMemberHasAttributes)
{
return this.SyntaxNode.AttributeLists;
return OneOrMany.Create(this.SyntaxNode.AttributeLists);
}

return default(SyntaxList<AttributeListSyntax>);
return OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,16 +408,16 @@ private static BaseFieldDeclarationSyntax GetFieldDeclaration(CSharpSyntaxNode d
return (BaseFieldDeclarationSyntax)declarator.Parent.Parent;
}

protected override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
protected override OneOrMany<SyntaxList<AttributeListSyntax>> AttributeDeclarationLists
{
get
{
if (this.containingType.AnyMemberHasAttributes)
{
return GetFieldDeclaration(this.SyntaxNode).AttributeLists;
return OneOrMany.Create(GetFieldDeclaration(this.SyntaxNode).AttributeLists);
}

return default(SyntaxList<AttributeListSyntax>);
return OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ protected sealed override SourceMemberMethodSymbol BoundAttributesSource

internal sealed override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
{
Debug.Assert(PartialDefinitionPart is null);
333fred marked this conversation as resolved.
Show resolved Hide resolved
if ((object)this.SourcePartialImplementation != null)
{
return OneOrMany.Create(ImmutableArray.Create(AttributeDeclarationSyntaxList, this.SourcePartialImplementation.AttributeDeclarationSyntaxList));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,16 +631,37 @@ public sealed override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementat

internal sealed override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
{
var syntax = this.GetSyntax();
switch (syntax.Kind())
if (PartialImplementationPart is { } implementation)
{
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
return OneOrMany.Create(((AccessorDeclarationSyntax)syntax).AttributeLists);
return OneOrMany.Create(AttributeDeclarationList, ((SourcePropertyAccessorSymbol)implementation).AttributeDeclarationList);
}

return base.GetAttributeDeclarations();
// If we are asking this question on a partial implementation symbol,
// it must be from a context which prefers to order implementation attributes before definition attributes.
// For example, the 'value' parameter of a set accessor.
333fred marked this conversation as resolved.
Show resolved Hide resolved
if (PartialDefinitionPart is { } definition)
{
return OneOrMany.Create(AttributeDeclarationList, ((SourcePropertyAccessorSymbol)definition).AttributeDeclarationList);
}

return OneOrMany.Create(AttributeDeclarationList);
}

internal SyntaxList<AttributeListSyntax> AttributeDeclarationList
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
{
get
{
var syntax = this.GetSyntax();
switch (syntax.Kind())
{
case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
case SyntaxKind.InitAccessorDeclaration:
return ((AccessorDeclarationSyntax)syntax).AttributeLists;
}

return default;
}
}

#nullable enable
Expand Down Expand Up @@ -805,6 +826,8 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
}

#nullable enable
protected sealed override SourceMemberMethodSymbol? BoundAttributesSource => (SourceMemberMethodSymbol?)PartialDefinitionPart;

public sealed override MethodSymbol? PartialImplementationPart => _property is SourcePropertySymbol { IsPartialDefinition: true, OtherPartOfPartial: { } other }
? (MethodKind == MethodKind.PropertyGet ? other.GetMethod : other.SetMethod)
: null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,25 @@ private static SyntaxTokenList GetModifierTokensSyntax(SyntaxNode syntax)
private static bool HasInitializer(SyntaxNode syntax)
=> syntax is PropertyDeclarationSyntax { Initializer: { } };

public override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
=> ((BasePropertyDeclarationSyntax)CSharpSyntaxNode).AttributeLists;
public override OneOrMany<SyntaxList<AttributeListSyntax>> AttributeDeclarationLists
{
get
{
Debug.Assert(PartialDefinitionPart is null);
if (PartialImplementationPart is { } implementationPart)
{
return OneOrMany.Create(
((BasePropertyDeclarationSyntax)CSharpSyntaxNode).AttributeLists,
((BasePropertyDeclarationSyntax)implementationPart.CSharpSyntaxNode).AttributeLists);
}
else
{
return OneOrMany.Create(((BasePropertyDeclarationSyntax)CSharpSyntaxNode).AttributeLists);
}
}
}

protected override SourcePropertySymbolBase? BoundAttributesSource => PartialDefinitionPart;

public override IAttributeTargetSymbol AttributesOwner => this;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,13 @@ private SynthesizedSealedPropertyAccessor MakeSynthesizedSealedAccessor()

#region Attributes

public abstract SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList { get; }
public abstract OneOrMany<SyntaxList<AttributeListSyntax>> AttributeDeclarationLists { get; }

/// <summary>
/// Symbol to copy bound attributes from, or null if the attributes are not shared among multiple source property symbols.
/// Analogous to <see cref="SourceMethodSymbolWithAttributes.BoundAttributesSource"/>.
/// </summary>
protected abstract SourcePropertySymbolBase BoundAttributesSource { get; }

public abstract IAttributeTargetSymbol AttributesOwner { get; }

Expand All @@ -1087,10 +1093,29 @@ private CustomAttributesBag<CSharpAttributeData> GetAttributesBag()
return bag;
}

// The property is responsible for completion of the backing field
_ = BackingField?.GetAttributes();
var copyFrom = this.BoundAttributesSource;
Copy link
Contributor Author

@RikkiGibson RikkiGibson May 14, 2024

Choose a reason for hiding this comment

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


// prevent infinite recursion:
Debug.Assert(!ReferenceEquals(copyFrom, this));

bool bagCreatedOnThisThread;
if (copyFrom is not null)
{
// When partial properties get the ability to have a backing field,
// the implementer will have to decide how the BackingField symbol works in 'copyFrom' scenarios.
Debug.Assert(BackingField is null);

var attributesBag = copyFrom.GetAttributesBag();
bagCreatedOnThisThread = Interlocked.CompareExchange(ref _lazyCustomAttributesBag, attributesBag, null) == null;
}
else
{
// The property is responsible for completion of the backing field
_ = BackingField?.GetAttributes();
bagCreatedOnThisThread = LoadAndValidateAttributes(AttributeDeclarationLists, ref _lazyCustomAttributesBag);
}

if (LoadAndValidateAttributes(OneOrMany.Create(AttributeDeclarationSyntaxList), ref _lazyCustomAttributesBag))
if (bagCreatedOnThisThread)
{
var completed = _state.NotePartComplete(CompletionPart.Attributes);
Debug.Assert(completed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ protected override IAttributeTargetSymbol AttributeOwner
internal override Location ErrorLocation
=> ParameterSymbol.TryGetFirstLocation() ?? NoLocation.Singleton;

protected override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
=> default;
protected override OneOrMany<SyntaxList<AttributeListSyntax>> AttributeDeclarationLists
=> OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;

public override Symbol? AssociatedSymbol
=> null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ public SynthesizedRecordEqualityContractProperty(SourceMemberContainerTypeSymbol

public override ImmutableArray<SyntaxReference> DeclaringSyntaxReferences => ImmutableArray<SyntaxReference>.Empty;

public override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
=> new SyntaxList<AttributeListSyntax>();
public override OneOrMany<SyntaxList<AttributeListSyntax>> AttributeDeclarationLists
=> OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;

protected override SourcePropertySymbolBase? BoundAttributesSource => null;

public override IAttributeTargetSymbol AttributesOwner => this;

Expand Down
Loading
Loading