Partial properties: attributes

src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs

@@ -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>> GetAttributeDeclarations();
 
         protected abstract IAttributeTargetSymbol AttributeOwner { get; }
 
@@ -86,7 +86,7 @@ private CustomAttributesBag<CSharpAttributeData> GetAttributesBag()
                 return bag;
             }
 
-            if (LoadAndValidateAttributes(OneOrMany.Create(this.AttributeDeclarationSyntaxList), ref _lazyCustomAttributesBag))
+            if (LoadAndValidateAttributes(this.GetAttributeDeclarations(), ref _lazyCustomAttributesBag))
             {
                 var completed = state.NotePartComplete(CompletionPart.Attributes);
                 Debug.Assert(completed);

src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs

@@ -56,7 +56,7 @@ internal class GlobalExpressionVariable : SourceMemberFieldSymbol
                 : new GlobalExpressionVariable(containingType, modifiers, typeSyntax, name, syntaxReference, locationSpan);
         }
 
-        protected override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList => default(SyntaxList<AttributeListSyntax>);
+        protected override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations() => OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;
         protected override TypeSyntax TypeSyntax => (TypeSyntax)_typeSyntaxOpt?.GetSyntax();
         protected override SyntaxTokenList ModifiersTokenList => default(SyntaxTokenList);
         public override bool HasInitializer => false;

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

@@ -454,30 +454,57 @@ 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 { PartialImplementationPart.Parameters: { } parameters } => parameters,
+                    SourcePropertySymbol { PartialImplementationPart.Parameters: { } parameters } => parameters,
+                    _ => default
+                };
+
+                if (implParameters.IsDefault)
                 {
                     return null;
                 }
 
-                var impl = sourceMethod.SourcePartialImplementation;
-                if ((object)impl == null)
+                Debug.Assert(!this.ContainingSymbol.IsPartialImplementation());
+                return (SourceParameterSymbol)implParameters[this.Ordinal];
+            }
+        }
+
+        protected SourceParameterSymbol? PartialDefinitionPart
+        {
+            get
+            {
+                ImmutableArray<ParameterSymbol> defParameters = this.ContainingSymbol switch
+                {
+                    SourceMemberMethodSymbol { PartialDefinitionPart.Parameters: { } parameters } => parameters,
+                    SourcePropertySymbol { PartialDefinitionPart.Parameters: { } parameters } => parameters,
+                    _ => default
+                };
+
+                if (defParameters.IsDefault)
                 {
                     return null;
                 }
 
-                return (SourceParameterSymbol)impl.Parameters[this.Ordinal];
+                Debug.Assert(!this.ContainingSymbol.IsPartialDefinition());
+                return (SourceParameterSymbol)defParameters[this.Ordinal];
             }
         }
 
@@ -495,44 +522,22 @@ 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)
-            {
-                return OneOrMany.Create(attributes);
-            }
+            // Attributes on parameters in partial members are owned by the parameter in the implementation part.
+            // If this symbol has a non-null PartialImplementationPart, we should have accessed this method through that implementation symbol.
+            Debug.Assert(PartialImplementationPart is null);
 
-            SyntaxList<AttributeListSyntax> otherAttributes;
-
-            // if this is a definition get the implementation and vice versa
-            SourceOrdinaryMethodSymbol otherPart = sourceMethod.OtherPartOfPartial;
-            if ((object)otherPart != null)
+            if (PartialDefinitionPart is { } definitionPart)
             {
-                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.
@@ -1030,33 +1035,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)
+            {
+                return false;
+            }
+
+            return implParameterAttributeList.Any(attrList => attrList.Attributes.Contains(node));
         }
+#nullable disable
 
         private void ValidateCallerLineNumberAttribute(AttributeSyntax node, BindingDiagnosticBag diagnostics)
         {

src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs

@@ -86,17 +86,14 @@ public new EnumMemberDeclarationSyntax SyntaxNode
             }
         }
 
-        protected override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
+        protected override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
         {
-            get
+            if (this.containingType.AnyMemberHasAttributes)
             {
-                if (this.containingType.AnyMemberHasAttributes)
-                {
-                    return this.SyntaxNode.AttributeLists;
-                }
-
-                return default(SyntaxList<AttributeListSyntax>);
+                return OneOrMany.Create(this.SyntaxNode.AttributeLists);
             }
+
+            return OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;
         }
 
 #nullable enable

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs

@@ -408,17 +408,14 @@ private static BaseFieldDeclarationSyntax GetFieldDeclaration(CSharpSyntaxNode d
             return (BaseFieldDeclarationSyntax)declarator.Parent.Parent;
         }
 
-        protected override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
+        protected override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
         {
-            get
+            if (this.containingType.AnyMemberHasAttributes)
             {
-                if (this.containingType.AnyMemberHasAttributes)
-                {
-                    return GetFieldDeclaration(this.SyntaxNode).AttributeLists;
-                }
-
-                return default(SyntaxList<AttributeListSyntax>);
+                return OneOrMany.Create(GetFieldDeclaration(this.SyntaxNode).AttributeLists);
             }
+
+            return OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;
         }
 
         public sealed override RefKind RefKind => GetTypeAndRefKind(ConsList<FieldSymbol>.Empty).RefKind;

src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs

@@ -335,10 +335,6 @@ internal bool IsPartialWithoutImplementation
             }
         }
 
-        /// <summary>
-        /// Returns the implementation part of a partial method definition, 
-        /// or null if this is not a partial method or it is the definition part.
-        /// </summary>
         internal SourceOrdinaryMethodSymbol SourcePartialDefinition
         {
             get
@@ -347,10 +343,6 @@ internal SourceOrdinaryMethodSymbol SourcePartialDefinition
             }
         }
 
-        /// <summary>
-        /// Returns the definition part of a partial method implementation, 
-        /// or null if this is not a partial method or it is the implementation part.
-        /// </summary>
         internal SourceOrdinaryMethodSymbol SourcePartialImplementation
         {
             get
@@ -401,6 +393,10 @@ protected sealed override SourceMemberMethodSymbol BoundAttributesSource
 
         internal sealed override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
         {
+            // Attributes on partial methods are owned by the definition part.
+            // If this symbol has a non-null PartialDefinitionPart, we should have accessed this method through that definition symbol instead
+            Debug.Assert(PartialDefinitionPart is null);
+
             if ((object)this.SourcePartialImplementation != null)
             {
                 return OneOrMany.Create(ImmutableArray.Create(AttributeDeclarationSyntaxList, this.SourcePartialImplementation.AttributeDeclarationSyntaxList));

src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs

@@ -631,16 +631,38 @@ 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.
+            if (PartialDefinitionPart is { } definition)
+            {
+                Debug.Assert(MethodKind == MethodKind.PropertySet);
+                return OneOrMany.Create(AttributeDeclarationList, ((SourcePropertyAccessorSymbol)definition).AttributeDeclarationList);
+            }
+
+            return OneOrMany.Create(AttributeDeclarationList);
+        }
+
+        private SyntaxList<AttributeListSyntax> AttributeDeclarationList
+        {
+            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
@@ -805,6 +827,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;

src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs

@@ -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>> GetAttributeDeclarations()
+        {
+            // Attributes on partial properties are owned by the definition part.
+            // If this symbol has a non-null PartialDefinitionPart, we should have accessed this method through that definition symbol instead
+            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;