diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs index 91bbb1b0e1f74..1f182b73abc9b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs @@ -458,10 +458,15 @@ AttributeLocation IAttributeTargetSymbol.AllowedAttributeLocations /// /// Symbol to copy bound attributes from, or null if the attributes are not shared among multiple source parameter symbols. /// + /// + /// 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. + /// private SourceParameterSymbol? BoundAttributesSource - => PartialDefinitionPart; + => PartialImplementationPart; - private SourceParameterSymbol? PartialImplementationPart + protected SourceParameterSymbol? PartialImplementationPart { get { @@ -481,7 +486,7 @@ private SourceParameterSymbol? PartialImplementationPart } } - private SourceParameterSymbol? PartialDefinitionPart + protected SourceParameterSymbol? PartialDefinitionPart { get { @@ -501,7 +506,7 @@ private SourceParameterSymbol? PartialDefinitionPart } } - internal sealed override SyntaxList AttributeDeclarationList + internal override SyntaxList AttributeDeclarationList { get { @@ -515,11 +520,13 @@ internal sealed override SyntaxList AttributeDeclarationLis /// internal virtual OneOrMany> GetAttributeDeclarations() { - if (PartialImplementationPart is { } implementationPart) + // TODO2: should this be non-virtual and subtypes override 'AttributeDeclarationList' instead? + Debug.Assert(PartialImplementationPart is null); + if (PartialDefinitionPart is { } definitionPart) { return OneOrMany.Create( AttributeDeclarationList, - implementationPart.AttributeDeclarationList); + definitionPart.AttributeDeclarationList); } else { diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs index db0e62af2e2c6..43a62f8f8efaf 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs @@ -401,6 +401,7 @@ protected sealed override SourceMemberMethodSymbol BoundAttributesSource internal sealed override OneOrMany> GetAttributeDeclarations() { + Debug.Assert(PartialDefinitionPart is null); if ((object)this.SourcePartialImplementation != null) { return OneOrMany.Create(ImmutableArray.Create(AttributeDeclarationSyntaxList, this.SourcePartialImplementation.AttributeDeclarationSyntaxList)); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs index 6c21244fb9c49..acf4d7ab312d3 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs @@ -631,6 +631,7 @@ public sealed override ImmutableArray ExplicitInterfaceImplementat internal sealed override OneOrMany> GetAttributeDeclarations() { + Debug.Assert(PartialDefinitionPart is null); if (PartialImplementationPart is { } implementation) { return OneOrMany.Create(AttributeDeclarationList, ((SourcePropertyAccessorSymbol)implementation).AttributeDeclarationList); @@ -639,7 +640,7 @@ internal sealed override OneOrMany> GetAttribute return OneOrMany.Create(AttributeDeclarationList); } - private SyntaxList AttributeDeclarationList + internal SyntaxList AttributeDeclarationList { get { diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index ee17b1fbe541e..abafeb68ea878 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -175,6 +175,7 @@ public override OneOrMany> AttributeDeclarationL { get { + Debug.Assert(PartialDefinitionPart is null); if (PartialImplementationPart is { } implementationPart) { return OneOrMany.Create( diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedAccessorValueParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedAccessorValueParameterSymbol.cs index 7252c0fee9068..bfe1cb85ababc 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedAccessorValueParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedAccessorValueParameterSymbol.cs @@ -70,11 +70,13 @@ protected override IAttributeTargetSymbol AttributeOwner get { return (SourceMemberMethodSymbol)this.ContainingSymbol; } } - internal override OneOrMany> GetAttributeDeclarations() + internal sealed override SyntaxList AttributeDeclarationList { - // Bind the attributes on the accessor's attribute syntax list with "param" target specifier. - var accessor = (SourceMemberMethodSymbol)this.ContainingSymbol; - return accessor.GetAttributeDeclarations(); + get + { + var accessor = (SourcePropertyAccessorSymbol)this.ContainingSymbol; + return accessor.AttributeDeclarationList; + } } internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder attributes) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index 9f498acdca7a5..7b4aa9c1fcea6 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3197,6 +3197,8 @@ public partial int P [Fact] public void Attributes_SetValueParam() { + // Just as with parameter attributes on partial methods, + // the implementation part attributes are emitted before the definition part attributes. var source = """ #pragma warning disable CS9113 // Primary constructor parameter is unread @@ -3231,8 +3233,8 @@ public partial int P var accessor = comp.GetMember("C.set_P"); AssertEx.Equal([], accessor.GetAttributes().SelectAsArray(a => a.ToString())); AssertEx.Equal([], accessor.PartialImplementationPart.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.Parameters.Single().GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.PartialImplementationPart!.Parameters.Single().GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(2)", "B(2)", "A(1)", "B(1)"], accessor.Parameters.Single().GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(2)", "B(2)", "A(1)", "B(1)"], accessor.PartialImplementationPart!.Parameters.Single().GetAttributes().SelectAsArray(a => a.ToString())); } [Fact] @@ -3269,6 +3271,8 @@ partial class C [Fact] public void Attributes_IndexerParameter() { + // Unlike other symbol kinds, for parameters, the implementation part attributes are emitted first, then the definition part attributes. + // This is consistent with partial methods. var source = """ #pragma warning disable CS9113 // Primary constructor parameter is unread @@ -3302,7 +3306,7 @@ partial class C void verify(ParameterSymbol param) { - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], param.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(2)", "B(2)", "A(1)", "B(1)"], param.GetAttributes().SelectAsArray(a => a.ToString())); } } @@ -3317,27 +3321,27 @@ class Attr : Attribute { } partial class C { [Attr] - public partial int P { [Attr] get; [Attr] [param: Attr] set; } + public partial int P { [Attr] get; [Attr] [param: Attr] set; } // 1 (param) - [Attr] - public partial int P { [Attr] get => 1; [Attr] [param: Attr] set { } } + [Attr] // 2 + public partial int P { [Attr] get => 1; [Attr] [param: Attr] set { } } // 3, 4 (accessors) } """; var comp = CreateCompilation(source); comp.VerifyEmitDiagnostics( + // (8,55): error CS0579: Duplicate 'Attr' attribute + // public partial int P { [Attr] get; [Attr] [param: Attr] set; } // 1 (param) + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(8, 55), // (10,6): error CS0579: Duplicate 'Attr' attribute - // [Attr] + // [Attr] // 2 Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(10, 6), // (11,29): error CS0579: Duplicate 'Attr' attribute - // public partial int P { [Attr] get => 1; [Attr] [param: Attr] set { } } + // public partial int P { [Attr] get => 1; [Attr] [param: Attr] set { } } // 3, 4 (accessors) Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 29), // (11,46): error CS0579: Duplicate 'Attr' attribute - // public partial int P { [Attr] get => 1; [Attr] [param: Attr] set { } } - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 46), - // (11,60): error CS0579: Duplicate 'Attr' attribute - // public partial int P { [Attr] get => 1; [Attr] [param: Attr] set { } } - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 60)); + // public partial int P { [Attr] get => 1; [Attr] [param: Attr] set { } } // 3, 4 (accessors) + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 46)); var property = comp.GetMember("C.P"); AssertEx.Equal(["Attr", "Attr"], property.GetAttributes().SelectAsArray(a => a.ToString())); @@ -3357,33 +3361,33 @@ class Attr : Attribute { } partial class C { [Attr] - public partial int this[[Attr] int x, [Attr] int y] { [Attr] get; [Attr] [param: Attr] set; } + public partial int this[[Attr] int x, [Attr] int y] { [Attr] get; [Attr] [param: Attr] set; } // 1, 2, 3 (param) - [Attr] - public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } + [Attr] // 4 + public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } // 5, 6 (accessor) } """; var comp = CreateCompilation(source); comp.VerifyEmitDiagnostics( + // (8,30): error CS0579: Duplicate 'Attr' attribute + // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get; [Attr] [param: Attr] set; } // 1, 2, 3 (param) + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(8, 30), + // (8,44): error CS0579: Duplicate 'Attr' attribute + // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get; [Attr] [param: Attr] set; } // 1, 2, 3 (param) + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(8, 44), + // (8,86): error CS0579: Duplicate 'Attr' attribute + // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get; [Attr] [param: Attr] set; } // 1, 2, 3 (param) + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(8, 86), // (10,6): error CS0579: Duplicate 'Attr' attribute - // [Attr] + // [Attr] // 4 Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(10, 6), - // (11,30): error CS0579: Duplicate 'Attr' attribute - // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 30), - // (11,44): error CS0579: Duplicate 'Attr' attribute - // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 44), // (11,60): error CS0579: Duplicate 'Attr' attribute - // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } + // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } // 5, 6 (accessor) Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 60), // (11,77): error CS0579: Duplicate 'Attr' attribute - // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 77), - // (11,91): error CS0579: Duplicate 'Attr' attribute - // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 91)); + // public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } // 5, 6 (accessor) + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 77)); var property = (SourcePropertySymbol)comp.GetMember("C").Indexers.Single(); AssertEx.Equal(["Attr", "Attr"], property.GetAttributes().SelectAsArray(a => a.ToString()));