From 98e4536bc1edb52dfa1f9dbeb8a5c67886ea67ab Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Sat, 11 May 2024 11:15:49 -0700 Subject: [PATCH 01/15] Partial properties: attributes --- .../FieldSymbolWithAttributesAndModifiers.cs | 4 +- .../Source/GlobalExpressionVariable.cs | 2 +- .../Source/SourceComplexParameterSymbol.cs | 111 ++- .../Source/SourceEnumConstantSymbol.cs | 6 +- .../Symbols/Source/SourceMemberFieldSymbol.cs | 6 +- .../Source/SourceOrdinaryMethodSymbol.cs | 1 + .../Source/SourcePropertyAccessorSymbol.cs | 37 +- .../Symbols/Source/SourcePropertySymbol.cs | 21 +- .../Source/SourcePropertySymbolBase.cs | 33 +- ...yConstructorParameterBackingFieldSymbol.cs | 4 +- ...nthesizedRecordEqualityContractProperty.cs | 6 +- .../SynthesizedRecordPropertySymbol.cs | 7 +- .../SynthesizedBackingFieldSymbol.cs | 21 +- .../AttributeTests_CallerInfoAttributes.cs | 98 ++ .../Symbol/Symbols/PartialPropertiesTests.cs | 876 +++++++++++++++++- 15 files changed, 1133 insertions(+), 100 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs b/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs index 67d199cf3ff56..fd53afe79eaec 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs @@ -26,7 +26,7 @@ internal abstract class FieldSymbolWithAttributesAndModifiers : FieldSymbol, IAt /// /// Gets the syntax list of custom attributes applied on the symbol. /// - protected abstract SyntaxList AttributeDeclarationSyntaxList { get; } + protected abstract OneOrMany> AttributeDeclarationLists { get; } protected abstract IAttributeTargetSymbol AttributeOwner { get; } @@ -86,7 +86,7 @@ private CustomAttributesBag 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); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs b/src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs index 64b9a72c48268..00b797813975b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs @@ -56,7 +56,7 @@ internal static GlobalExpressionVariable Create( : new GlobalExpressionVariable(containingType, modifiers, typeSyntax, name, syntaxReference, locationSpan); } - protected override SyntaxList AttributeDeclarationSyntaxList => default(SyntaxList); + protected override OneOrMany> AttributeDeclarationLists => OneOrMany>.Empty; protected override TypeSyntax TypeSyntax => (TypeSyntax)_typeSyntaxOpt?.GetSyntax(); protected override SyntaxTokenList ModifiersTokenList => default(SyntaxTokenList); public override bool HasInitializer => false; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs index 27167398d80fe..4687e5b5a54d3 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs @@ -454,30 +454,55 @@ AttributeLocation IAttributeTargetSymbol.AllowedAttributeLocations } } +#nullable enable /// /// Symbol to copy bound attributes from, or null if the attributes are not shared among multiple source parameter symbols. /// /// - /// 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. /// - private SourceParameterSymbol BoundAttributesSource + private SourceParameterSymbol? BoundAttributesSource + => PartialImplementationPart; + + protected SourceParameterSymbol? PartialImplementationPart { get { - var sourceMethod = this.ContainingSymbol as SourceOrdinaryMethodSymbol; - if ((object)sourceMethod == null) + ImmutableArray implParameters = this.ContainingSymbol switch + { + SourceMemberMethodSymbol method => method.PartialImplementationPart?.Parameters ?? default, + SourcePropertySymbol property => property.PartialImplementationPart?.Parameters ?? default, + _ => default + }; + + if (implParameters.IsDefault) { return null; } - var impl = sourceMethod.SourcePartialImplementation; - if ((object)impl == null) + return (SourceParameterSymbol)implParameters[this.Ordinal]; + } + } + + protected SourceParameterSymbol? PartialDefinitionPart + { + get + { + ImmutableArray defParameters = this.ContainingSymbol switch + { + SourceMemberMethodSymbol method => method.PartialDefinitionPart?.Parameters ?? default, + SourcePropertySymbol property => property.PartialDefinitionPart?.Parameters ?? default, + _ => default + }; + + if (defParameters.IsDefault) { return null; } - return (SourceParameterSymbol)impl.Parameters[this.Ordinal]; + return (SourceParameterSymbol)defParameters[this.Ordinal]; } } @@ -495,44 +520,19 @@ internal sealed override SyntaxList AttributeDeclarationLis /// internal virtual OneOrMany> 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 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 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); - } - - if (attributes.Equals(default(SyntaxList))) - { - return OneOrMany.Create(otherAttributes); - } - else if (otherAttributes.Equals(default(SyntaxList))) - { - return OneOrMany.Create(attributes); + return OneOrMany.Create(AttributeDeclarationList); } - - return OneOrMany.Create(ImmutableArray.Create(attributes, otherAttributes)); } +#nullable disable /// /// Returns data decoded from well-known attributes applied to the symbol or null if there are no applied attributes. @@ -1030,33 +1030,26 @@ private bool IsValidCallerInfoContext(AttributeSyntax node) => !ContainingSymbol && !ContainingSymbol.IsOperator() && !IsOnPartialImplementation(node); +#nullable enable /// /// 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. /// - /// - /// 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) { diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs index 4216680ce2790..73f55d007390d 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs @@ -86,16 +86,16 @@ protected sealed override DeclarationModifiers Modifiers } } - protected override SyntaxList AttributeDeclarationSyntaxList + protected override OneOrMany> AttributeDeclarationLists { get { if (this.containingType.AnyMemberHasAttributes) { - return this.SyntaxNode.AttributeLists; + return OneOrMany.Create(this.SyntaxNode.AttributeLists); } - return default(SyntaxList); + return OneOrMany>.Empty; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs index 804ac34303400..fd0fb0ace4372 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs @@ -408,16 +408,16 @@ private static BaseFieldDeclarationSyntax GetFieldDeclaration(CSharpSyntaxNode d return (BaseFieldDeclarationSyntax)declarator.Parent.Parent; } - protected override SyntaxList AttributeDeclarationSyntaxList + protected override OneOrMany> AttributeDeclarationLists { get { if (this.containingType.AnyMemberHasAttributes) { - return GetFieldDeclaration(this.SyntaxNode).AttributeLists; + return OneOrMany.Create(GetFieldDeclaration(this.SyntaxNode).AttributeLists); } - return default(SyntaxList); + return OneOrMany>.Empty; } } 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 5d19c49bfd0cc..7460e057701c0 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs @@ -631,16 +631,37 @@ public sealed override ImmutableArray ExplicitInterfaceImplementat internal sealed override OneOrMany> 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) + { + return OneOrMany.Create(AttributeDeclarationList, ((SourcePropertyAccessorSymbol)definition).AttributeDeclarationList); + } + + return OneOrMany.Create(AttributeDeclarationList); + } + + internal SyntaxList 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 +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; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index d9f9672b670d5..abafeb68ea878 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/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 AttributeDeclarationSyntaxList - => ((BasePropertyDeclarationSyntax)CSharpSyntaxNode).AttributeLists; + public override OneOrMany> 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; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs index 1749008b67547..db04d310bc31d 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs @@ -1060,7 +1060,13 @@ private SynthesizedSealedPropertyAccessor MakeSynthesizedSealedAccessor() #region Attributes - public abstract SyntaxList AttributeDeclarationSyntaxList { get; } + public abstract OneOrMany> AttributeDeclarationLists { get; } + + /// + /// Symbol to copy bound attributes from, or null if the attributes are not shared among multiple source property symbols. + /// Analogous to . + /// + protected abstract SourcePropertySymbolBase BoundAttributesSource { get; } public abstract IAttributeTargetSymbol AttributesOwner { get; } @@ -1087,10 +1093,29 @@ private CustomAttributesBag GetAttributesBag() return bag; } - // The property is responsible for completion of the backing field - _ = BackingField?.GetAttributes(); + var copyFrom = this.BoundAttributesSource; + + // 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); diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs index 6552a867d6ae9..5aed26232b860 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs @@ -33,8 +33,8 @@ protected override IAttributeTargetSymbol AttributeOwner internal override Location ErrorLocation => ParameterSymbol.TryGetFirstLocation() ?? NoLocation.Singleton; - protected override SyntaxList AttributeDeclarationSyntaxList - => default; + protected override OneOrMany> AttributeDeclarationLists + => OneOrMany>.Empty; public override Symbol? AssociatedSymbol => null; diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs index 63afef8d2f803..c67530d0158fd 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs @@ -49,8 +49,10 @@ public SynthesizedRecordEqualityContractProperty(SourceMemberContainerTypeSymbol public override ImmutableArray DeclaringSyntaxReferences => ImmutableArray.Empty; - public override SyntaxList AttributeDeclarationSyntaxList - => new SyntaxList(); + public override OneOrMany> AttributeDeclarationLists + => OneOrMany>.Empty; + + protected override SourcePropertySymbolBase? BoundAttributesSource => null; public override IAttributeTargetSymbol AttributesOwner => this; diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs index 1a3554d59e402..23fdad438c495 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using System.Diagnostics; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.Symbols { @@ -42,13 +43,15 @@ public SynthesizedRecordPropertySymbol( BackingParameter = (SourceParameterSymbol)backingParameter; } + protected override SourcePropertySymbolBase? BoundAttributesSource => null; + public override IAttributeTargetSymbol AttributesOwner => BackingParameter as IAttributeTargetSymbol ?? this; protected override Location TypeLocation => ((ParameterSyntax)CSharpSyntaxNode).Type!.Location; - public override SyntaxList AttributeDeclarationSyntaxList - => BackingParameter.AttributeDeclarationList; + public override OneOrMany> AttributeDeclarationLists + => OneOrMany.Create(BackingParameter.AttributeDeclarationList); protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics) { diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs index cf1ddb35d1427..7d7b14cac2c93 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs @@ -100,8 +100,8 @@ protected override IAttributeTargetSymbol AttributeOwner internal override Location ErrorLocation => _property.Location; - protected override SyntaxList AttributeDeclarationSyntaxList - => _property.AttributeDeclarationSyntaxList; + protected override OneOrMany> AttributeDeclarationLists + => _property.AttributeDeclarationLists; public override Symbol AssociatedSymbol => _property; @@ -163,15 +163,18 @@ private void CheckForFieldTargetedAttribute(BindingDiagnosticBag diagnostics) return; } - foreach (var attribute in AttributeDeclarationSyntaxList) + foreach (var attributeList in AttributeDeclarationLists) { - if (attribute.Target?.GetAttributeLocation() == AttributeLocation.Field) + foreach (var attribute in attributeList) { - diagnostics.Add( - new CSDiagnosticInfo(ErrorCode.WRN_AttributesOnBackingFieldsNotAvailable, - languageVersion.ToDisplayString(), - new CSharpRequiredLanguageVersion(MessageID.IDS_FeatureAttributesOnBackingFields.RequiredVersion())), - attribute.Target.Location); + if (attribute.Target?.GetAttributeLocation() == AttributeLocation.Field) + { + diagnostics.Add( + new CSDiagnosticInfo(ErrorCode.WRN_AttributesOnBackingFieldsNotAvailable, + languageVersion.ToDisplayString(), + new CSharpRequiredLanguageVersion(MessageID.IDS_FeatureAttributesOnBackingFields.RequiredVersion())), + attribute.Target.Location); + } } } } diff --git a/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs b/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs index dd9319f97b459..db4cc1d6ccc50 100644 --- a/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs @@ -3231,6 +3231,19 @@ public static void Main() // (14,10): warning CS4025: The CallerFilePathAttribute applied to parameter 'path' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments // [CallerFilePath] string path) { } Diagnostic(ErrorCode.WRN_CallerFilePathParamForUnconsumedLocation, "CallerFilePath").WithArguments("path")); + + CompileAndVerify(source, options: TestOptions.DebugExe.WithMetadataImportOptions(MetadataImportOptions.All), symbolValidator: verify); + + void verify(ModuleSymbol module) + { + // https://github.com/dotnet/roslyn/issues/73482 + // These are ignored in source but they still get written out to metadata. + // This means if the method is accessible from another compilation, then the attribute will be respected there, but not in the declaring compilation. + var goo = module.GlobalNamespace.GetMember("D.Goo"); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerLineNumberAttribute"], goo.Parameters[0].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerMemberNameAttribute"], goo.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerFilePathAttribute"], goo.Parameters[2].GetAttributes().SelectAsArray(attr => attr.ToString())); + } } [Fact] @@ -5836,5 +5849,90 @@ public CallerArgumentExpressionAttribute([CallerArgumentExpression(nameof(parame // public CallerArgumentExpressionAttribute([CallerArgumentExpression(nameof(parameterName))] string parameterName) Diagnostic(ErrorCode.ERR_BadCallerArgumentExpressionParamWithoutDefaultValue, "CallerArgumentExpression").WithLocation(5, 51)); } + + [Fact] + public void CallerMemberName_SetterValueParam() + { + // There is no way in C# to call a setter without passing an argument for the value, so the CallerMemberName effectively does nothing. + var source = """ + using System; + using System.Runtime.CompilerServices; + using System.Runtime.InteropServices; + + partial class C + { + public static void Main() + { + var c = new C(); + c[1] = "1"; + } + + public string this[int x] + { + [param: Optional, DefaultParameterValue("0")] + [param: CallerMemberName] + set + { + Console.Write(value); + } + } + } + """; + + var verifier = CompileAndVerify(source, expectedOutput: "1"); + verifier.VerifyDiagnostics(); + } + + [Fact] + public void CallerArgumentExpression_SetterValueParam() + { + var source = """ + using System; + using System.Runtime.CompilerServices; + + namespace System.Runtime.CompilerServices + { + [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = true, Inherited = false)] + public sealed class CallerArgumentExpressionAttribute : Attribute + { + public CallerArgumentExpressionAttribute(string parameterName) + { + ParameterName = parameterName; + } + public string ParameterName { get; } + } + } + + partial class C + { + public static void Main() + { + var c = new C(); + c[1] = GetNumber(); + } + + public static int GetNumber() => 1; + + public int this[int x, [CallerArgumentExpression("value")] string argumentExpression = "0"] + { + set + { + Console.Write(argumentExpression); + } + } + } + """; + var verifier = CompileAndVerify(source, expectedOutput: "0", symbolValidator: verify); + verifier.VerifyDiagnostics( + // (27,29): warning CS8963: The CallerArgumentExpressionAttribute applied to parameter 'argumentExpression' will have no effect. It is applied with an invalid parameter name. + // public int this[int x, [CallerArgumentExpression("value")] string argumentExpression = "0"] + Diagnostic(ErrorCode.WRN_CallerArgumentExpressionAttributeHasInvalidParameterName, "CallerArgumentExpression").WithArguments("argumentExpression").WithLocation(27, 29)); + + void verify(ModuleSymbol module) + { + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["""System.Runtime.CompilerServices.CallerArgumentExpressionAttribute("value")"""], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } } } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index 1be88ef4740e1..b29b5cedca8db 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3017,11 +3017,879 @@ partial class C Diagnostic(ErrorCode.ERR_PartialMemberInconsistentTupleNames, "this").WithArguments("C.this[(int x, int y, int z)]", "C.this[(int a, int b, int c)]").WithLocation(13, 37)); } + [Theory] + [InlineData("A(1)", "B(2)")] + [InlineData("B(2)", "A(1)")] + public void Attributes_Property_01(string declAttribute, string implAttribute) + { + // Name or arguments to attributes doesn't affect order of emit. + // Attributes on the declaration part precede attributes on the implementation part. + var source = $$""" + #pragma warning disable CS9113 // Primary constructor parameter is unread + + using System; + + class A(int i) : Attribute { } + class B(int i) : Attribute { } + + partial class C + { + [{{declAttribute}}] + public partial int P { get; set; } + + [{{implAttribute}}] + public partial int P { get => 1; set { } } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + var property = comp.GetMember("C.P"); + AssertEx.Equal([declAttribute, implAttribute], property.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal([declAttribute, implAttribute], property.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + } + + [Theory] + [InlineData("A(1)", "B(2)")] + [InlineData("B(2)", "A(1)")] + public void Attributes_Property_02(string declAttribute, string implAttribute) + { + // Lexical order of the partial declarations themselves doesn't affect order that attributes are emitted. + var source = $$""" + #pragma warning disable CS9113 // Primary constructor parameter is unread + + using System; + + class A(int i) : Attribute { } + class B(int i) : Attribute { } + + partial class C + { + [{{implAttribute}}] + public partial int P { get => 1; set { } } + + [{{declAttribute}}] + public partial int P { get; set; } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + var property = comp.GetMember("C.P"); + AssertEx.Equal([declAttribute, implAttribute], property.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal([declAttribute, implAttribute], property.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + } + + [Fact] + public void Attributes_Property_03() + { + // Order of attributes within a part is preserved. + var source = """ + #pragma warning disable CS9113 // Primary constructor parameter is unread + + using System; + + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class A(int i) : Attribute { } + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class B(int i) : Attribute { } + + partial class C + { + [A(2), B(2)] + public partial int P { get => 1; set { } } + + [A(1), B(1)] + public partial int P { get; set; } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + var property = comp.GetMember("C.P"); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], property.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], property.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + } + + [Fact] + public void Attributes_GetAccessor() + { + // Order of attributes within a part is preserved. + var source = """ + #pragma warning disable CS9113 // Primary constructor parameter is unread + + using System; + + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class A(int i) : Attribute { } + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class B(int i) : Attribute { } + + partial class C + { + public partial int P + { + [A(2), B(2)] + get => 1; + set { } + } + + public partial int P + { + [A(1), B(1)] + get; + set; + } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + var accessor = comp.GetMember("C.get_P"); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + } + + [Fact] + public void Attributes_SetAccessor() + { + var source = """ + #pragma warning disable CS9113 // Primary constructor parameter is unread + + using System; + + + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class A(int i) : Attribute { } + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class B(int i) : Attribute { } + + partial class C + { + public partial int P + { + get => 1; + [A(2), B(2)] + set { } + } + + public partial int P + { + get; + [A(1), B(1)] + set; + } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + var accessor = comp.GetMember("C.set_P"); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + } + + [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 + + using System; + + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class A(int i) : Attribute { } + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class B(int i) : Attribute { } + + partial class C + { + public partial int P + { + get => 1; + [param: A(2), B(2)] + set { } + } + + public partial int P + { + get; + [param: A(1), B(1)] + set; + } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + 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(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] + public void Attributes_Indexer() + { + var source = """ + #pragma warning disable CS9113 // Primary constructor parameter is unread + + using System; + + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class A(int i) : Attribute { } + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class B(int i) : Attribute { } + + partial class C + { + [A(2), B(2)] + public partial int this[int i] { get => 1; set { } } + + [A(1), B(1)] + public partial int this[int i] { get; set; } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + var indexer = (SourcePropertySymbol)comp.GetMember("C").Indexers.Single(); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], indexer.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], indexer.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + } + + [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 + + using System; + + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class A(int i) : Attribute { } + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] + class B(int i) : Attribute { } + + partial class C + { + public partial int this[[A(2), B(2)] int i] { get => 1; set { } } + + public partial int this[[A(1), B(1)] int i] { get; set; } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + var indexer = (SourcePropertySymbol)comp.GetMember("C").Indexers.Single(); + + verify(indexer.Parameters.Single()); + verify(indexer.GetMethod!.Parameters.Single()); + verify(indexer.SetMethod!.Parameters[0]); + + verify(indexer.PartialImplementationPart!.Parameters.Single()); + verify(indexer.PartialImplementationPart!.GetMethod!.Parameters.Single()); + verify(indexer.PartialImplementationPart!.SetMethod!.Parameters[0]); + + void verify(ParameterSymbol param) + { + AssertEx.Equal(["A(2)", "B(2)", "A(1)", "B(1)"], param.GetAttributes().SelectAsArray(a => a.ToString())); + } + } + + [Fact] + public void Attributes_Property_DisallowedDuplicates() + { + var source = """ + using System; + + class Attr : Attribute { } + + partial class C + { + [Attr] + public partial int P { [Attr] get; [Attr] [param: Attr] set; } // 1 (param) + + [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] // 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 { } } // 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 { } } // 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())); + AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters.Single().GetAttributes().SelectAsArray(a => a.ToString())); + } + + [Fact] + public void Attributes_Indexer_DisallowedDuplicates() + { + var source = """ + using System; + + class Attr : Attribute { } + + partial class C + { + [Attr] + public partial int this[[Attr] int x, [Attr] int y] { [Attr] get; [Attr] [param: Attr] set; } // 1, 2, 3 (param) + + [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] // 4 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(10, 6), + // (11,60): error CS0579: Duplicate 'Attr' attribute + // 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 { } } // 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())); + AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.Parameters[0].GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.Parameters[1].GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[0].GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[1].GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[2].GetAttributes().SelectAsArray(a => a.ToString())); + } + + [Fact] + public void Attributes_Property_BackingField() + { + var source = """ + using System; + + class Attr : Attribute { } + + partial class C + { + [field: Attr] + public partial int P { get; set; } + + [field: Attr] + public partial int P { get => 1; set { } } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics( + // (7,6): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'property'. All attributes in this block will be ignored. + // [field: Attr] + Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "property").WithLocation(7, 6), + // (10,6): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'property'. All attributes in this block will be ignored. + // [field: Attr] + Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "property").WithLocation(10, 6)); + + var property = comp.GetMember("C.P"); + AssertEx.Equal([], property.GetAttributes().SelectAsArray(a => a.ToString())); + } + + [Theory] + [InlineData("", "[UnscopedRef] ")] + [InlineData("[UnscopedRef] ", "")] + public void Attributes_UnscopedRef(string defAttrs, string implAttrs) + { + // There aren't many interesting scenarios to test with UnscopedRef because: + // - no out parameters (so no removing the implicit 'scoped' from them) + // - no ref parameters either (so no interesting differences in ref safety analysis for ref readonlys marked with `[UnscopedRef]`) + var source = $$""" + #pragma warning disable CS9113 // Primary constructor parameter is unread + + using System.Diagnostics.CodeAnalysis; + + public ref struct RS([UnscopedRef] ref readonly int ri) { } + + partial class C + { + public partial RS this[{{defAttrs}}ref readonly int i] { get; } + public partial RS this[{{implAttrs}}ref readonly int i] + { + get => new RS(in i); + } + } + """; + + var comp = CreateCompilation([source, UnscopedRefAttributeDefinition]); + comp.VerifyEmitDiagnostics(); + + var indexer = (SourcePropertySymbol)comp.GetMember("C").Indexers.Single(); + Assert.True(indexer.Parameters[0].HasUnscopedRefAttribute); + Assert.True(indexer.PartialImplementationPart!.Parameters[0].HasUnscopedRefAttribute); + Assert.True(indexer.GetMethod!.Parameters[0].HasUnscopedRefAttribute); + Assert.True(indexer.GetMethod!.PartialImplementationPart!.Parameters[0].HasUnscopedRefAttribute); + } + + [Fact] + public void Attributes_CallerLineNumber_OnDefinition() + { + var source = $$""" + using System; + using System.Runtime.CompilerServices; + + partial class C + { + public static void Main() + { + var c = new C(); + #line 1 + Console.Write(c[2]); + } + + public partial int this[int x, [CallerLineNumber] int lineNumber = 0] { get; } + public partial int this[int x, int lineNumber] + { + get + { + Console.Write(lineNumber); + return x; + } + } + } + """; + var verifier = CompileAndVerify(source, expectedOutput: "12", symbolValidator: verify); + verifier.VerifyDiagnostics(); + + void verify(ModuleSymbol module) + { + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerLineNumberAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } + + [Fact] + public void Attributes_CallerLineNumber_OnImplementation() + { + var source = $$""" + using System; + using System.Runtime.CompilerServices; + + partial class C + { + public static void Main() + { + var c = new C(); + #line 1 + Console.Write(c[2]); + } + + public partial int this[int x, int lineNumber = 0] { get; } + public partial int this[int x, [CallerLineNumber] int lineNumber] + { + get + { + Console.Write(lineNumber); + return x; + } + } + } + """; + var verifier = CompileAndVerify(source, expectedOutput: "02", symbolValidator: verify); + verifier.VerifyDiagnostics( + // (5,37): warning CS4024: The CallerLineNumberAttribute applied to parameter 'lineNumber' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments + // public partial int this[int x, [CallerLineNumber] int lineNumber] + Diagnostic(ErrorCode.WRN_CallerLineNumberParamForUnconsumedLocation, "CallerLineNumber").WithArguments("lineNumber").WithLocation(5, 37)); + + void verify(ModuleSymbol module) + { + // https://github.com/dotnet/roslyn/issues/73482 + // The attribute is still written out to metadata even though it is ignored in source. + // We could consider changing this for both properties and methods. + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerLineNumberAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } + + [Fact] + public void Attributes_CallerFilePath_OnDefinition() + { + var source = ($$""" + using System; + using System.Runtime.CompilerServices; + + partial class C + { + public static void Main() + { + var c = new C(); + Console.Write(c[2]); + } + + public partial int this[int x, [CallerFilePath] string filePath = "0"] { get; } + public partial int this[int x, string filePath] + { + get + { + Console.Write(filePath); + return x; + } + } + } + """, filePath: "Program.cs"); + var verifier = CompileAndVerify(source, expectedOutput: "Program.cs2", symbolValidator: verify); + verifier.VerifyDiagnostics(); + + void verify(ModuleSymbol module) + { + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerFilePathAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } + + [Fact] + public void Attributes_CallerFilePath_OnImplementation() + { + var source = ($$""" + using System; + using System.Runtime.CompilerServices; + + partial class C + { + public static void Main() + { + var c = new C(); + Console.Write(c[2]); + } + + public partial int this[int x, string filePath = "0"] { get; } + public partial int this[int x, [CallerFilePath] string filePath] + { + get + { + Console.Write(filePath); + return x; + } + } + } + """, filePath: "Program.cs"); + var verifier = CompileAndVerify(source, expectedOutput: "02", symbolValidator: verify); + verifier.VerifyDiagnostics( + // Program.cs(13,37): warning CS4025: The CallerFilePathAttribute applied to parameter 'filePath' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments + // public partial int this[int x, [CallerFilePath] string filePath] + Diagnostic(ErrorCode.WRN_CallerFilePathParamForUnconsumedLocation, "CallerFilePath").WithArguments("filePath").WithLocation(13, 37)); + + void verify(ModuleSymbol module) + { + // https://github.com/dotnet/roslyn/issues/73482 + // The attribute is still written out to metadata even though it is ignored in source. + // We could consider changing this for both properties and methods. + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerFilePathAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } + + [Fact] + public void Attributes_CallerMemberName_OnDefinition() + { + var source = $$""" + using System; + using System.Runtime.CompilerServices; + + partial class C + { + public static void Main() + { + var c = new C(); + Console.Write(c[2]); + } + + public partial int this[int x, [CallerMemberName] string filePath = "0"] { get; } + public partial int this[int x, string filePath] + { + get + { + Console.Write(filePath); + return x; + } + } + } + """; + var verifier = CompileAndVerify(source, expectedOutput: "Main2", symbolValidator: verify); + verifier.VerifyDiagnostics(); + + void verify(ModuleSymbol module) + { + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerMemberNameAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } + + [Fact] + public void Attributes_CallerMemberName_OnImplementation() + { + var source = $$""" + using System; + using System.Runtime.CompilerServices; + + partial class C + { + public static void Main() + { + var c = new C(); + Console.Write(c[2]); + } + + public partial int this[int x, string filePath = "0"] { get; } + public partial int this[int x, [CallerMemberName] string filePath] + { + get + { + Console.Write(filePath); + return x; + } + } + } + """; + var verifier = CompileAndVerify(source, expectedOutput: "02", symbolValidator: verify); + verifier.VerifyDiagnostics( + // (13,37): warning CS4026: The CallerMemberNameAttribute applied to parameter 'filePath' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments + // public partial int this[int x, [CallerMemberName] string filePath] + Diagnostic(ErrorCode.WRN_CallerMemberNameParamForUnconsumedLocation, "CallerMemberName").WithArguments("filePath").WithLocation(13, 37)); + + void verify(ModuleSymbol module) + { + // https://github.com/dotnet/roslyn/issues/73482 + // The attribute is still written out to metadata even though it is ignored in source. + // We could consider changing this for both properties and methods. + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerMemberNameAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } + + [Fact] + public void Attributes_CallerArgumentExpression_OnDefinition() + { + var source = $$""" + using System; + using System.Runtime.CompilerServices; + + namespace System.Runtime.CompilerServices + { + [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = true, Inherited = false)] + public sealed class CallerArgumentExpressionAttribute : Attribute + { + public CallerArgumentExpressionAttribute(string parameterName) + { + ParameterName = parameterName; + } + public string ParameterName { get; } + } + } + + partial class C + { + public static void Main() + { + var c = new C(); + Console.Write(c[GetNumber()]); + } + + public static int GetNumber() => 2; + + public partial int this[int x, [CallerArgumentExpression("x")] string argumentExpression = "0"] { get; } + public partial int this[int x, string argumentExpression] + { + get + { + Console.Write(argumentExpression); + return x; + } + } + } + """; + var verifier = CompileAndVerify(source, expectedOutput: "GetNumber()2", symbolValidator: verify); + verifier.VerifyDiagnostics(); + + void verify(ModuleSymbol module) + { + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["""System.Runtime.CompilerServices.CallerArgumentExpressionAttribute("x")"""], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } + + [Fact] + public void Attributes_CallerArgumentExpression_OnImplementation() + { + var source = """ + using System; + using System.Runtime.CompilerServices; + + namespace System.Runtime.CompilerServices + { + [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = true, Inherited = false)] + public sealed class CallerArgumentExpressionAttribute : Attribute + { + public CallerArgumentExpressionAttribute(string parameterName) + { + ParameterName = parameterName; + } + public string ParameterName { get; } + } + } + + partial class C + { + public static void Main() + { + var c = new C(); + Console.Write(c[GetNumber()]); + } + + public static int GetNumber() => 2; + + public partial int this[int x, string argumentExpression = "0"] { get; } + public partial int this[int x, [CallerArgumentExpression("x")] string argumentExpression] + { + get + { + Console.Write(argumentExpression); + return x; + } + } + } + """; + var verifier = CompileAndVerify(source, expectedOutput: "02", symbolValidator: verify); + verifier.VerifyDiagnostics( + // (28,37): warning CS8966: The CallerArgumentExpressionAttribute applied to parameter 'argumentExpression' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments + // public partial int this[int x, [CallerArgumentExpression("x")] string argumentExpression] + Diagnostic(ErrorCode.WRN_CallerArgumentExpressionParamForUnconsumedLocation, "CallerArgumentExpression").WithArguments("argumentExpression").WithLocation(28, 37)); + + void verify(ModuleSymbol module) + { + // https://github.com/dotnet/roslyn/issues/73482 + // The attribute is still written out to metadata even though it is ignored in source. + // We could consider changing this for both properties and methods. + var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); + AssertEx.Equal(["""System.Runtime.CompilerServices.CallerArgumentExpressionAttribute("x")"""], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + } + } + + [Fact] + public void CallerMemberName_SetterValueParam_ImplementationPart() + { + // Counterpart to test 'AttributeTests_CallerInfoAttributes.CallerMemberName_SetterValueParam' + // There is no way in C# to call a setter without passing an argument for the value, so the CallerMemberName effectively does nothing. + // It would be reasonable to also warn here about the CallerInfo attribute on implementation, + // but this is a corner case that clearly won't work regardless of which part the attribute is on, so it's not a big deal that the warning is missing. + // Verify that our checks for caller-info attributes on implementation part parameters behave gracefully here. + var source = """ + using System; + using System.Runtime.CompilerServices; + using System.Runtime.InteropServices; + + partial class C + { + public static void Main() + { + var c = new C(); + c[1] = "1"; + } + + public partial string this[int x] { set; } + public partial string this[int x] + { + [param: Optional, DefaultParameterValue("0")] + [param: CallerMemberName] + set + { + Console.Write(value); + } + } + } + """; + + var verifier = CompileAndVerify(source, expectedOutput: "1"); + verifier.VerifyDiagnostics(); + } + + [Theory] + [InlineData("[AllowNull] ", "")] + [InlineData("", "[AllowNull] ")] + public void AllowNull(string defAttrs, string implAttrs) + { + var source = $$""" + #nullable enable + + using System; + using System.Diagnostics.CodeAnalysis; + + partial class C + { + public static void Main() + { + var c = new C(); + try + { + _ = c[null]; // no warning + } + catch + { + Console.Write(1); + } + } + + public partial string this[{{defAttrs}}string s] { get; } + public partial string this[{{implAttrs}}string s] + { + get + { + return s.ToString(); // https://github.com/dotnet/roslyn/issues/73484: missing a warning here + } + } + } + """; + + var verifier = CompileAndVerify([source, AllowNullAttributeDefinition], expectedOutput: "1"); + verifier.VerifyDiagnostics(); + } + // PROTOTYPE(partial-properties): override partial property where base has modopt - // PROTOTYPE(partial-properties): test indexers incl parameters with attributes - // PROTOTYPE(partial-properties): test merging property attributes - // PROTOTYPE(partial-properties): [UnscopedRef]+scoped difference across partials // PROTOTYPE(partial-properties): test that doc comments work consistently with partial methods (and probably spec it as well) - // PROTOTYPE(partial-properties): test CallerInfo attributes applied to either definition or implementation part } } From 3ce6777f9f43f6d3fe337b4889dfcc28767e7807 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 09:44:01 -0700 Subject: [PATCH 02/15] Fix formatting --- .../Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs b/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs index db4cc1d6ccc50..95f36130f3493 100644 --- a/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs @@ -3231,7 +3231,7 @@ public static void Main() // (14,10): warning CS4025: The CallerFilePathAttribute applied to parameter 'path' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments // [CallerFilePath] string path) { } Diagnostic(ErrorCode.WRN_CallerFilePathParamForUnconsumedLocation, "CallerFilePath").WithArguments("path")); - + CompileAndVerify(source, options: TestOptions.DebugExe.WithMetadataImportOptions(MetadataImportOptions.All), symbolValidator: verify); void verify(ModuleSymbol module) From 782ac67ba047dd144ea054534f784edb18f90051 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 10:21:54 -0700 Subject: [PATCH 03/15] I need to start auto-highlighting these spaces in my editor --- .../CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index b29b5cedca8db..54313992de469 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3495,7 +3495,7 @@ public static void Main() """; var verifier = CompileAndVerify(source, expectedOutput: "12", symbolValidator: verify); verifier.VerifyDiagnostics(); - + void verify(ModuleSymbol module) { var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); @@ -3535,7 +3535,7 @@ public static void Main() // (5,37): warning CS4024: The CallerLineNumberAttribute applied to parameter 'lineNumber' will have no effect because it applies to a member that is used in contexts that do not allow optional arguments // public partial int this[int x, [CallerLineNumber] int lineNumber] Diagnostic(ErrorCode.WRN_CallerLineNumberParamForUnconsumedLocation, "CallerLineNumber").WithArguments("lineNumber").WithLocation(5, 37)); - + void verify(ModuleSymbol module) { // https://github.com/dotnet/roslyn/issues/73482 From a29a3aafdac933c440c0ce943571c6e2dc092c56 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:08:44 -0700 Subject: [PATCH 04/15] make 'SourcePropertyAccessorSymbol.AttributeDeclarationList' private --- .../Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs index 7460e057701c0..c33c6d94e7f9f 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs @@ -647,7 +647,7 @@ internal sealed override OneOrMany> GetAttribute return OneOrMany.Create(AttributeDeclarationList); } - internal SyntaxList AttributeDeclarationList + private SyntaxList AttributeDeclarationList { get { From d9458a90dfb10b45a72b105408abb1347da99dd1 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:10:05 -0700 Subject: [PATCH 05/15] FieldSymbolWithAttributesAndModifiers.AttributeDeclarationLists -> GetAttributeDeclarations() --- .../Source/FieldSymbolWithAttributesAndModifiers.cs | 4 ++-- .../Symbols/Source/GlobalExpressionVariable.cs | 2 +- .../Symbols/Source/SourceEnumConstantSymbol.cs | 13 +++++-------- .../Symbols/Source/SourceMemberFieldSymbol.cs | 13 +++++-------- ...PrimaryConstructorParameterBackingFieldSymbol.cs | 2 +- .../Synthesized/SynthesizedBackingFieldSymbol.cs | 4 ++-- 6 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs b/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs index fd53afe79eaec..bfec737594640 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs @@ -26,7 +26,7 @@ internal abstract class FieldSymbolWithAttributesAndModifiers : FieldSymbol, IAt /// /// Gets the syntax list of custom attributes applied on the symbol. /// - protected abstract OneOrMany> AttributeDeclarationLists { get; } + protected abstract OneOrMany> GetAttributeDeclarations(); protected abstract IAttributeTargetSymbol AttributeOwner { get; } @@ -86,7 +86,7 @@ private CustomAttributesBag GetAttributesBag() return bag; } - if (LoadAndValidateAttributes(this.AttributeDeclarationLists, ref _lazyCustomAttributesBag)) + if (LoadAndValidateAttributes(this.GetAttributeDeclarations(), ref _lazyCustomAttributesBag)) { var completed = state.NotePartComplete(CompletionPart.Attributes); Debug.Assert(completed); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs b/src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs index 00b797813975b..3403885c3079f 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/GlobalExpressionVariable.cs @@ -56,7 +56,7 @@ internal static GlobalExpressionVariable Create( : new GlobalExpressionVariable(containingType, modifiers, typeSyntax, name, syntaxReference, locationSpan); } - protected override OneOrMany> AttributeDeclarationLists => OneOrMany>.Empty; + protected override OneOrMany> GetAttributeDeclarations() => OneOrMany>.Empty; protected override TypeSyntax TypeSyntax => (TypeSyntax)_typeSyntaxOpt?.GetSyntax(); protected override SyntaxTokenList ModifiersTokenList => default(SyntaxTokenList); public override bool HasInitializer => false; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs index 73f55d007390d..a2f5dd94a71f9 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs @@ -86,17 +86,14 @@ protected sealed override DeclarationModifiers Modifiers } } - protected override OneOrMany> AttributeDeclarationLists + protected override OneOrMany> GetAttributeDeclarations() { - get + if (this.containingType.AnyMemberHasAttributes) { - if (this.containingType.AnyMemberHasAttributes) - { - return OneOrMany.Create(this.SyntaxNode.AttributeLists); - } - - return OneOrMany>.Empty; + return OneOrMany.Create(this.SyntaxNode.AttributeLists); } + + return OneOrMany>.Empty; } #nullable enable diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs index fd0fb0ace4372..13a1bda935111 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs +++ b/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 OneOrMany> AttributeDeclarationLists + protected override OneOrMany> GetAttributeDeclarations() { - get + if (this.containingType.AnyMemberHasAttributes) { - if (this.containingType.AnyMemberHasAttributes) - { - return OneOrMany.Create(GetFieldDeclaration(this.SyntaxNode).AttributeLists); - } - - return OneOrMany>.Empty; + return OneOrMany.Create(GetFieldDeclaration(this.SyntaxNode).AttributeLists); } + + return OneOrMany>.Empty; } public sealed override RefKind RefKind => GetTypeAndRefKind(ConsList.Empty).RefKind; diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs index 5aed26232b860..5f3d127451425 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs @@ -33,7 +33,7 @@ protected override IAttributeTargetSymbol AttributeOwner internal override Location ErrorLocation => ParameterSymbol.TryGetFirstLocation() ?? NoLocation.Singleton; - protected override OneOrMany> AttributeDeclarationLists + protected override OneOrMany> GetAttributeDeclarations() => OneOrMany>.Empty; public override Symbol? AssociatedSymbol diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs index 7d7b14cac2c93..acb6d380fdd71 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs @@ -100,7 +100,7 @@ protected override IAttributeTargetSymbol AttributeOwner internal override Location ErrorLocation => _property.Location; - protected override OneOrMany> AttributeDeclarationLists + protected override OneOrMany> GetAttributeDeclarations() => _property.AttributeDeclarationLists; public override Symbol AssociatedSymbol @@ -163,7 +163,7 @@ private void CheckForFieldTargetedAttribute(BindingDiagnosticBag diagnostics) return; } - foreach (var attributeList in AttributeDeclarationLists) + foreach (var attributeList in GetAttributeDeclarations()) { foreach (var attribute in attributeList) { From 806e3cda93da903bcfcae06ea1078d7c762ad865 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:12:46 -0700 Subject: [PATCH 06/15] SourcePropertySymbolBase.AttributeDeclarationLists -> GetAttributeDeclarations() --- .../Symbols/Source/SourcePropertySymbol.cs | 23 ++++++++----------- .../Source/SourcePropertySymbolBase.cs | 4 ++-- ...nthesizedRecordEqualityContractProperty.cs | 2 +- .../SynthesizedRecordPropertySymbol.cs | 2 +- .../SynthesizedBackingFieldSymbol.cs | 2 +- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index abafeb68ea878..52cccb693b73c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -171,21 +171,18 @@ private static SyntaxTokenList GetModifierTokensSyntax(SyntaxNode syntax) private static bool HasInitializer(SyntaxNode syntax) => syntax is PropertyDeclarationSyntax { Initializer: { } }; - public override OneOrMany> AttributeDeclarationLists + public override OneOrMany> GetAttributeDeclarations() { - get + Debug.Assert(PartialDefinitionPart is null); + if (PartialImplementationPart is { } implementationPart) { - 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); - } + return OneOrMany.Create( + ((BasePropertyDeclarationSyntax)CSharpSyntaxNode).AttributeLists, + ((BasePropertyDeclarationSyntax)implementationPart.CSharpSyntaxNode).AttributeLists); + } + else + { + return OneOrMany.Create(((BasePropertyDeclarationSyntax)CSharpSyntaxNode).AttributeLists); } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs index db04d310bc31d..a003de34a608b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs @@ -1060,7 +1060,7 @@ private SynthesizedSealedPropertyAccessor MakeSynthesizedSealedAccessor() #region Attributes - public abstract OneOrMany> AttributeDeclarationLists { get; } + public abstract OneOrMany> GetAttributeDeclarations(); /// /// Symbol to copy bound attributes from, or null if the attributes are not shared among multiple source property symbols. @@ -1112,7 +1112,7 @@ private CustomAttributesBag GetAttributesBag() { // The property is responsible for completion of the backing field _ = BackingField?.GetAttributes(); - bagCreatedOnThisThread = LoadAndValidateAttributes(AttributeDeclarationLists, ref _lazyCustomAttributesBag); + bagCreatedOnThisThread = LoadAndValidateAttributes(GetAttributeDeclarations(), ref _lazyCustomAttributesBag); } if (bagCreatedOnThisThread) diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs index c67530d0158fd..1dc7444d03efb 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs @@ -49,7 +49,7 @@ public SynthesizedRecordEqualityContractProperty(SourceMemberContainerTypeSymbol public override ImmutableArray DeclaringSyntaxReferences => ImmutableArray.Empty; - public override OneOrMany> AttributeDeclarationLists + public override OneOrMany> GetAttributeDeclarations() => OneOrMany>.Empty; protected override SourcePropertySymbolBase? BoundAttributesSource => null; diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs index 23fdad438c495..2d905f6daf0f8 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs @@ -50,7 +50,7 @@ public SynthesizedRecordPropertySymbol( protected override Location TypeLocation => ((ParameterSyntax)CSharpSyntaxNode).Type!.Location; - public override OneOrMany> AttributeDeclarationLists + public override OneOrMany> GetAttributeDeclarations() => OneOrMany.Create(BackingParameter.AttributeDeclarationList); protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics) diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs index acb6d380fdd71..323447d1aa226 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs @@ -101,7 +101,7 @@ internal override Location ErrorLocation => _property.Location; protected override OneOrMany> GetAttributeDeclarations() - => _property.AttributeDeclarationLists; + => _property.GetAttributeDeclarations(); public override Symbol AssociatedSymbol => _property; From e29d708ec863231f93ea4446f7395038b866fbe3 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:13:38 -0700 Subject: [PATCH 07/15] Remove comments on SourcePartialDefinition/SourcePartialImplementation --- .../Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs index 43a62f8f8efaf..ac50d31c43070 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs @@ -335,10 +335,6 @@ internal bool IsPartialWithoutImplementation } } - /// - /// Returns the implementation part of a partial method definition, - /// or null if this is not a partial method or it is the definition part. - /// internal SourceOrdinaryMethodSymbol SourcePartialDefinition { get @@ -347,10 +343,6 @@ internal SourceOrdinaryMethodSymbol SourcePartialDefinition } } - /// - /// Returns the definition part of a partial method implementation, - /// or null if this is not a partial method or it is the implementation part. - /// internal SourceOrdinaryMethodSymbol SourcePartialImplementation { get From b8eb66fa6d51f38f1d525dbdcc1e6cea08c49cd2 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:19:17 -0700 Subject: [PATCH 08/15] Add test AllowNull_Property --- .../Symbol/Symbols/PartialPropertiesTests.cs | 50 ++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index 54313992de469..1ef664bdb817b 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3851,7 +3851,55 @@ public partial string this[int x] [Theory] [InlineData("[AllowNull] ", "")] [InlineData("", "[AllowNull] ")] - public void AllowNull(string defAttrs, string implAttrs) + public void AllowNull_Property(string defAttrs, string implAttrs) + { + var source = $$""" + #nullable enable + + using System; + using System.Diagnostics.CodeAnalysis; + + partial class C + { + public static void Main() + { + var c = new C(); + try + { + c.Prop = null; // no warning + } + catch + { + Console.Write(1); + } + } + + {{defAttrs}} + public partial string Prop { get; set; } + + {{implAttrs}} + public partial string Prop + { + get => ""; + set + { + value.ToString(); // warning + } + } + } + """; + + var verifier = CompileAndVerify([source, AllowNullAttributeDefinition], expectedOutput: "1"); + verifier.VerifyDiagnostics( + // (30,13): warning CS8602: Dereference of a possibly null reference. + // value.ToString(); // warning + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "value").WithLocation(30, 13)); + } + + [Theory] + [InlineData("[AllowNull] ", "")] + [InlineData("", "[AllowNull] ")] + public void AllowNull_IndexerParam(string defAttrs, string implAttrs) { var source = $$""" #nullable enable From 41d786e8868792b4b05df2ec10df9840ff5304e4 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:23:17 -0700 Subject: [PATCH 09/15] verify attributes in metadata --- .../Symbol/Symbols/PartialPropertiesTests.cs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index 1ef664bdb817b..a68be83a1b53a 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3074,12 +3074,18 @@ partial class C } """; - var comp = CreateCompilation(source); - comp.VerifyEmitDiagnostics(); + var verifier = CompileAndVerify(source, symbolValidator: module => verify(module, isSource: false), sourceSymbolValidator: module => verify(module, isSource: true)); + verifier.VerifyDiagnostics(); - var property = comp.GetMember("C.P"); - AssertEx.Equal([declAttribute, implAttribute], property.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal([declAttribute, implAttribute], property.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + void verify(ModuleSymbol module, bool isSource) + { + var property = module.GlobalNamespace.GetMember("C.P"); + AssertEx.Equal([declAttribute, implAttribute], property.GetAttributes().SelectAsArray(a => a.ToString())); + if (isSource) + { + AssertEx.Equal([declAttribute, implAttribute], ((SourcePropertySymbol)property).PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + } + } } [Fact] From 87a59f64bcdbb44ba56053d3e94380c3489a5d21 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:32:28 -0700 Subject: [PATCH 10/15] Test Obsolete --- .../Symbol/Symbols/PartialPropertiesTests.cs | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index a68be83a1b53a..630600750c932 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3943,6 +3943,101 @@ public partial string this[{{implAttrs}}string s] verifier.VerifyDiagnostics(); } + [Fact] + public void Obsolete_01() + { + var source = """ + using System; + + partial class C + { + [Obsolete] + public partial int Prop { get; } + + public partial int Prop { get => M(); } // no diagnostic for use of obsolete member + + [Obsolete] + int M() => 1; + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + } + + [Fact] + public void Obsolete_02() + { + var source = """ + using System; + + partial class C + { + public partial int this[int x, int y = VALUE] { get; } // no diagnostic for use of obsolete const + + [Obsolete] + public partial int this[int x, int y] => x; + + [Obsolete] + public const int VALUE = 1; + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + } + + [Fact] + public void Obsolete_03() + { + var source = """ + using System; + + partial class C + { + public partial int this[int x, int y = VALUE] { get; } // 1 + + public partial int this[int x, int y] { [Obsolete] get => x; } + + [Obsolete] + public const int VALUE = 1; + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics( + // (5,44): warning CS0612: 'C.VALUE' is obsolete + // public partial int this[int x, int y = VALUE] { get; } // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "VALUE").WithArguments("C.VALUE").WithLocation(5, 44)); + } + + [Theory] + [CombinatorialData] + public void Obsolete_04( + [CombinatorialValues("public partial int Prop { [Obsolete] get; }", "[Obsolete] public partial int Prop { get; }")] + string declPart, + [CombinatorialValues("public partial int Prop { get => M(); }", "public partial int Prop => M();")] + string implPart) + { + // note that one of the combinations here is redundant with Obsolete_01, but that seems fine, + // as a failure in Obsolete_01 may be easier to triage. + var source = $$""" + using System; + + partial class C + { + {{declPart}} + {{implPart}} // no diagnostic for use of obsolete member + + [Obsolete] + int M() => 1; + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + } + // PROTOTYPE(partial-properties): override partial property where base has modopt // PROTOTYPE(partial-properties): test that doc comments work consistently with partial methods (and probably spec it as well) } From 3e5b6e89ba5afb94e0c48c94a99a4c834b3e5dea Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:38:33 -0700 Subject: [PATCH 11/15] add comments to asserts --- .../Portable/Symbols/Source/SourceComplexParameterSymbol.cs | 2 ++ .../Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs | 2 ++ .../CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs index 4687e5b5a54d3..d287859ce7543 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs @@ -520,7 +520,9 @@ internal sealed override SyntaxList AttributeDeclarationLis /// internal virtual OneOrMany> GetAttributeDeclarations() { + // If this symbol has a non-null PartialImplementationPart, we should have accessed this method through that implementation symbol instead Debug.Assert(PartialImplementationPart is null); + if (PartialDefinitionPart is { } definitionPart) { return OneOrMany.Create( diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs index ac50d31c43070..4dbd46fe0de7e 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs @@ -393,7 +393,9 @@ protected sealed override SourceMemberMethodSymbol BoundAttributesSource internal sealed override OneOrMany> GetAttributeDeclarations() { + // 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)); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index 52cccb693b73c..ca065b27dbdf0 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -173,7 +173,9 @@ private static bool HasInitializer(SyntaxNode syntax) public override OneOrMany> GetAttributeDeclarations() { + // 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( From 03f46c2fbbcdb100187f2adc3253063c3909374e Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:46:02 -0700 Subject: [PATCH 12/15] Add ToStrings() helper --- .../Symbol/Symbols/PartialPropertiesTests.cs | 60 +++++++++---------- .../Test/Utilities/CSharp/Extensions.cs | 3 + 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index 630600750c932..9ba9e78fafeca 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3046,8 +3046,8 @@ partial class C comp.VerifyEmitDiagnostics(); var property = comp.GetMember("C.P"); - AssertEx.Equal([declAttribute, implAttribute], property.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal([declAttribute, implAttribute], property.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal([declAttribute, implAttribute], property.GetAttributes().ToStrings()); + AssertEx.Equal([declAttribute, implAttribute], property.PartialImplementationPart!.GetAttributes().ToStrings()); } [Theory] @@ -3080,10 +3080,10 @@ partial class C void verify(ModuleSymbol module, bool isSource) { var property = module.GlobalNamespace.GetMember("C.P"); - AssertEx.Equal([declAttribute, implAttribute], property.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal([declAttribute, implAttribute], property.GetAttributes().ToStrings()); if (isSource) { - AssertEx.Equal([declAttribute, implAttribute], ((SourcePropertySymbol)property).PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal([declAttribute, implAttribute], ((SourcePropertySymbol)property).PartialImplementationPart!.GetAttributes().ToStrings()); } } } @@ -3116,8 +3116,8 @@ partial class C comp.VerifyEmitDiagnostics(); var property = comp.GetMember("C.P"); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], property.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], property.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], property.GetAttributes().ToStrings()); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], property.PartialImplementationPart!.GetAttributes().ToStrings()); } [Fact] @@ -3156,8 +3156,8 @@ public partial int P comp.VerifyEmitDiagnostics(); var accessor = comp.GetMember("C.get_P"); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.GetAttributes().ToStrings()); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.PartialImplementationPart!.GetAttributes().ToStrings()); } [Fact] @@ -3196,8 +3196,8 @@ public partial int P comp.VerifyEmitDiagnostics(); var accessor = comp.GetMember("C.set_P"); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.GetAttributes().ToStrings()); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.PartialImplementationPart!.GetAttributes().ToStrings()); } [Fact] @@ -3237,10 +3237,10 @@ public partial int P comp.VerifyEmitDiagnostics(); 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(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())); + AssertEx.Equal([], accessor.GetAttributes().ToStrings()); + AssertEx.Equal([], accessor.PartialImplementationPart.GetAttributes().ToStrings()); + AssertEx.Equal(["A(2)", "B(2)", "A(1)", "B(1)"], accessor.Parameters.Single().GetAttributes().ToStrings()); + AssertEx.Equal(["A(2)", "B(2)", "A(1)", "B(1)"], accessor.PartialImplementationPart!.Parameters.Single().GetAttributes().ToStrings()); } [Fact] @@ -3270,8 +3270,8 @@ partial class C comp.VerifyEmitDiagnostics(); var indexer = (SourcePropertySymbol)comp.GetMember("C").Indexers.Single(); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], indexer.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], indexer.PartialImplementationPart!.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], indexer.GetAttributes().ToStrings()); + AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], indexer.PartialImplementationPart!.GetAttributes().ToStrings()); } [Fact] @@ -3312,7 +3312,7 @@ partial class C void verify(ParameterSymbol param) { - AssertEx.Equal(["A(2)", "B(2)", "A(1)", "B(1)"], param.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["A(2)", "B(2)", "A(1)", "B(1)"], param.GetAttributes().ToStrings()); } } @@ -3350,10 +3350,10 @@ partial class C 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())); - AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters.Single().GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters.Single().GetAttributes().ToStrings()); } [Fact] @@ -3396,14 +3396,14 @@ partial class C 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())); - AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.Parameters[0].GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.Parameters[1].GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[0].GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[1].GetAttributes().SelectAsArray(a => a.ToString())); - AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[2].GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal(["Attr", "Attr"], property.GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.Parameters[0].GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.GetMethod!.Parameters[1].GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[0].GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[1].GetAttributes().ToStrings()); + AssertEx.Equal(["Attr", "Attr"], property.SetMethod!.Parameters[2].GetAttributes().ToStrings()); } [Fact] @@ -3434,7 +3434,7 @@ partial class C Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "property").WithLocation(10, 6)); var property = comp.GetMember("C.P"); - AssertEx.Equal([], property.GetAttributes().SelectAsArray(a => a.ToString())); + AssertEx.Equal([], property.GetAttributes().ToStrings()); } [Theory] diff --git a/src/Compilers/Test/Utilities/CSharp/Extensions.cs b/src/Compilers/Test/Utilities/CSharp/Extensions.cs index 76fa6c8d66365..6b4e838a76726 100644 --- a/src/Compilers/Test/Utilities/CSharp/Extensions.cs +++ b/src/Compilers/Test/Utilities/CSharp/Extensions.cs @@ -416,6 +416,9 @@ public static void VerifyNamedArgumentValue(this CSharpAttributeData attr, in Assert.True(IsEqual(arg, v)); } + public static ImmutableArray ToStrings(this ImmutableArray attributes) + => attributes.SelectAsArray(a => a.ToString()); + internal static bool IsEqual(TypedConstant arg, object expected) { switch (arg.Kind) From baadc959ad0558ce7d26b23d86f3378f243812f0 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 15:49:29 -0700 Subject: [PATCH 13/15] ToStrings() more things --- .../Symbol/Symbols/PartialPropertiesTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index 9ba9e78fafeca..54294332d9fa2 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3505,7 +3505,7 @@ public static void Main() void verify(ModuleSymbol module) { var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); - AssertEx.Equal(["System.Runtime.CompilerServices.CallerLineNumberAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerLineNumberAttribute"], indexer.Parameters[1].GetAttributes().ToStrings()); } } @@ -3548,7 +3548,7 @@ void verify(ModuleSymbol module) // The attribute is still written out to metadata even though it is ignored in source. // We could consider changing this for both properties and methods. var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); - AssertEx.Equal(["System.Runtime.CompilerServices.CallerLineNumberAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerLineNumberAttribute"], indexer.Parameters[1].GetAttributes().ToStrings()); } } @@ -3584,7 +3584,7 @@ public static void Main() void verify(ModuleSymbol module) { var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); - AssertEx.Equal(["System.Runtime.CompilerServices.CallerFilePathAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerFilePathAttribute"], indexer.Parameters[1].GetAttributes().ToStrings()); } } @@ -3626,7 +3626,7 @@ void verify(ModuleSymbol module) // The attribute is still written out to metadata even though it is ignored in source. // We could consider changing this for both properties and methods. var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); - AssertEx.Equal(["System.Runtime.CompilerServices.CallerFilePathAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerFilePathAttribute"], indexer.Parameters[1].GetAttributes().ToStrings()); } } @@ -3662,7 +3662,7 @@ public static void Main() void verify(ModuleSymbol module) { var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); - AssertEx.Equal(["System.Runtime.CompilerServices.CallerMemberNameAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerMemberNameAttribute"], indexer.Parameters[1].GetAttributes().ToStrings()); } } @@ -3704,7 +3704,7 @@ void verify(ModuleSymbol module) // The attribute is still written out to metadata even though it is ignored in source. // We could consider changing this for both properties and methods. var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); - AssertEx.Equal(["System.Runtime.CompilerServices.CallerMemberNameAttribute"], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["System.Runtime.CompilerServices.CallerMemberNameAttribute"], indexer.Parameters[1].GetAttributes().ToStrings()); } } @@ -3755,7 +3755,7 @@ public static void Main() void verify(ModuleSymbol module) { var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); - AssertEx.Equal(["""System.Runtime.CompilerServices.CallerArgumentExpressionAttribute("x")"""], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["""System.Runtime.CompilerServices.CallerArgumentExpressionAttribute("x")"""], indexer.Parameters[1].GetAttributes().ToStrings()); } } @@ -3812,7 +3812,7 @@ void verify(ModuleSymbol module) // The attribute is still written out to metadata even though it is ignored in source. // We could consider changing this for both properties and methods. var indexer = (PropertySymbol)module.GlobalNamespace.GetMember("C").Indexers.Single(); - AssertEx.Equal(["""System.Runtime.CompilerServices.CallerArgumentExpressionAttribute("x")"""], indexer.Parameters[1].GetAttributes().SelectAsArray(attr => attr.ToString())); + AssertEx.Equal(["""System.Runtime.CompilerServices.CallerArgumentExpressionAttribute("x")"""], indexer.Parameters[1].GetAttributes().ToStrings()); } } From 179203f8861c77c2826b1a90f36b973195bc2ec1 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 15 May 2024 22:30:10 -0700 Subject: [PATCH 14/15] Address feedback --- .../Source/SourceComplexParameterSymbol.cs | 13 +- .../Source/SourceOrdinaryMethodSymbol.cs | 1 + .../Source/SourcePropertyAccessorSymbol.cs | 1 + .../Symbols/Source/SourcePropertySymbol.cs | 1 + .../AttributeTests_CallerInfoAttributes.cs | 18 +- .../Symbol/Symbols/PartialPropertiesTests.cs | 239 ++++++++++++++---- 6 files changed, 220 insertions(+), 53 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs index d287859ce7543..707da3435ec61 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs @@ -472,8 +472,8 @@ protected SourceParameterSymbol? PartialImplementationPart { ImmutableArray implParameters = this.ContainingSymbol switch { - SourceMemberMethodSymbol method => method.PartialImplementationPart?.Parameters ?? default, - SourcePropertySymbol property => property.PartialImplementationPart?.Parameters ?? default, + SourceMemberMethodSymbol { PartialImplementationPart.Parameters: { } parameters } => parameters, + SourcePropertySymbol { PartialImplementationPart.Parameters: { } parameters } => parameters, _ => default }; @@ -482,6 +482,7 @@ protected SourceParameterSymbol? PartialImplementationPart return null; } + Debug.Assert(!this.ContainingSymbol.IsPartialImplementation()); return (SourceParameterSymbol)implParameters[this.Ordinal]; } } @@ -492,8 +493,8 @@ protected SourceParameterSymbol? PartialDefinitionPart { ImmutableArray defParameters = this.ContainingSymbol switch { - SourceMemberMethodSymbol method => method.PartialDefinitionPart?.Parameters ?? default, - SourcePropertySymbol property => property.PartialDefinitionPart?.Parameters ?? default, + SourceMemberMethodSymbol { PartialDefinitionPart.Parameters: { } parameters } => parameters, + SourcePropertySymbol { PartialDefinitionPart.Parameters: { } parameters } => parameters, _ => default }; @@ -502,6 +503,7 @@ protected SourceParameterSymbol? PartialDefinitionPart return null; } + Debug.Assert(!this.ContainingSymbol.IsPartialDefinition()); return (SourceParameterSymbol)defParameters[this.Ordinal]; } } @@ -520,7 +522,8 @@ internal sealed override SyntaxList AttributeDeclarationLis /// internal virtual OneOrMany> GetAttributeDeclarations() { - // If this symbol has a non-null PartialImplementationPart, we should have accessed this method through that implementation symbol instead + // 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); if (PartialDefinitionPart is { } definitionPart) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs index 4dbd46fe0de7e..2b14c397e27bf 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs @@ -393,6 +393,7 @@ protected sealed override SourceMemberMethodSymbol BoundAttributesSource internal sealed override OneOrMany> 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); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs index c33c6d94e7f9f..c625366d197a9 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs @@ -641,6 +641,7 @@ internal sealed override OneOrMany> GetAttribute // 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); } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index ca065b27dbdf0..46d4da363c7a8 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -173,6 +173,7 @@ private static bool HasInitializer(SyntaxNode syntax) public override OneOrMany> 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); diff --git a/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs b/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs index 95f36130f3493..bf4053c6fd9f2 100644 --- a/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_CallerInfoAttributes.cs @@ -5859,7 +5859,7 @@ public void CallerMemberName_SetterValueParam() using System.Runtime.CompilerServices; using System.Runtime.InteropServices; - partial class C + public partial class C { public static void Main() { @@ -5881,6 +5881,22 @@ public string this[int x] var verifier = CompileAndVerify(source, expectedOutput: "1"); verifier.VerifyDiagnostics(); + + var source1 = """ + class D + { + void M() + { + var c = new C(); + c.set_Item(1); + } + } + """; + var comp1 = CreateCompilation(source1, references: [verifier.Compilation.EmitToImageReference()]); + comp1.VerifyEmitDiagnostics( + // (6,11): error CS0571: 'C.this[int].set': cannot explicitly call operator or accessor + // c.set_Item(1); + Diagnostic(ErrorCode.ERR_CantCallSpecialMethod, "set_Item").WithArguments("C.this[int].set").WithLocation(6, 11)); } [Fact] diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index 54294332d9fa2..425a31d3d855c 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3160,10 +3160,29 @@ public partial int P AssertEx.Equal(["A(1)", "B(1)", "A(2)", "B(2)"], accessor.PartialImplementationPart!.GetAttributes().ToStrings()); } - [Fact] - public void Attributes_SetAccessor() + [Theory] + [CombinatorialData] + public void Attributes_SetAccessor(bool definitionFirst) { - var source = """ + var definitionPart = """ + public partial int P + { + get; + [A(1), B(1)] + set; + } + """; + + var implementationPart = """ + public partial int P + { + get => 1; + [A(2), B(2)] + set { } + } + """; + + var source = $$""" #pragma warning disable CS9113 // Primary constructor parameter is unread using System; @@ -3176,19 +3195,9 @@ class B(int i) : Attribute { } partial class C { - public partial int P - { - get => 1; - [A(2), B(2)] - set { } - } + {{(definitionFirst ? definitionPart : implementationPart)}} - public partial int P - { - get; - [A(1), B(1)] - set; - } + {{(definitionFirst ? implementationPart : definitionPart)}} } """; @@ -3327,27 +3336,41 @@ class Attr : Attribute { } partial class C { [Attr] - public partial int P { [Attr] get; [Attr] [param: Attr] set; } // 1 (param) + public partial int P + { + [Attr] + get; + + [Attr] + [param: Attr] set; // 1 + } [Attr] // 2 - public partial int P { [Attr] get => 1; [Attr] [param: Attr] set { } } // 3, 4 (accessors) + public partial int P + { + [Attr] // 3 + get => 1; + + [Attr] // 4 + [param: Attr] set { } + } } """; 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 + // (14,17): error CS0579: Duplicate 'Attr' attribute + // [param: Attr] set; // 1 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(14, 17), + // (17,6): error CS0579: Duplicate 'Attr' attribute // [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 { } } // 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 { } } // 3, 4 (accessors) - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 46)); + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(17, 6), + // (20,10): error CS0579: Duplicate 'Attr' attribute + // [Attr] // 3 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(20, 10), + // (23,10): error CS0579: Duplicate 'Attr' attribute + // [Attr] // 4 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(23, 10)); var property = comp.GetMember("C.P"); AssertEx.Equal(["Attr", "Attr"], property.GetAttributes().ToStrings()); @@ -3367,33 +3390,49 @@ class Attr : Attribute { } partial class C { [Attr] - public partial int this[[Attr] int x, [Attr] int y] { [Attr] get; [Attr] [param: Attr] set; } // 1, 2, 3 (param) + public partial int this[ + [Attr] int x, // 1 + [Attr] int y] // 2 + { + [Attr] get; + [Attr] + [param: Attr] set; // 3 + } [Attr] // 4 - public partial int this[[Attr] int x, [Attr] int y] { [Attr] get => 1; [Attr] [param: Attr] set { } } // 5, 6 (accessor) + public partial int this[ + [Attr] int x, + [Attr] int y] + { + [Attr] // 5 + get => 1; + + [Attr] // 6 + [param: Attr] set { } + } } """; 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 + // (9,10): error CS0579: Duplicate 'Attr' attribute + // [Attr] int x, // 1 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(9, 10), + // (10,10): error CS0579: Duplicate 'Attr' attribute + // [Attr] int y] // 2 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(10, 10), + // (14,17): error CS0579: Duplicate 'Attr' attribute + // [param: Attr] set; // 3 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(14, 17), + // (17,6): error CS0579: Duplicate 'Attr' attribute // [Attr] // 4 - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(10, 6), - // (11,60): error CS0579: Duplicate 'Attr' attribute - // 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 { } } // 5, 6 (accessor) - Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(11, 77)); + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(17, 6), + // (22,10): error CS0579: Duplicate 'Attr' attribute + // [Attr] // 5 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(22, 10), + // (25,10): error CS0579: Duplicate 'Attr' attribute + // [Attr] // 6 + Diagnostic(ErrorCode.ERR_DuplicateAttribute, "Attr").WithArguments("Attr").WithLocation(25, 10)); var property = (SourcePropertySymbol)comp.GetMember("C").Indexers.Single(); AssertEx.Equal(["Attr", "Attr"], property.GetAttributes().ToStrings()); @@ -4038,6 +4077,112 @@ partial class C comp.VerifyEmitDiagnostics(); } + [Theory] + [InlineData("[Obsolete]", "")] + [InlineData("", "[Obsolete]")] + public void Obsolete_05(string defAttrs, string implAttrs) + { + var source = $$""" + using System; + + partial class C + { + {{defAttrs}} + public partial int this[int x] { get; } + + {{implAttrs}} + public partial int this[int x] { get => x; } + } + + class D + { + void M() + { + var c = new C(); + _ = c[1]; // 1 + } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics( + // (17,13): warning CS0612: 'C.this[int]' is obsolete + // _ = c[1]; // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "c[1]").WithArguments("C.this[int]").WithLocation(17, 13)); + } + + [Theory] + [InlineData("[Obsolete]", "")] + [InlineData("", "[Obsolete]")] + public void Obsolete_06(string defAttrs, string implAttrs) + { + var source = $$""" + using System; + + partial class C + { + public partial int this[int x] { {{defAttrs}} get; } + + public partial int this[int x] { {{implAttrs}} get => x; } + } + + class D + { + void M() + { + var c = new C(); + _ = c[1]; // 1 + } + } + """; + + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics( + // (15,13): warning CS0612: 'C.this[int].get' is obsolete + // _ = c[1]; // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "c[1]").WithArguments("C.this[int].get").WithLocation(15, 13)); + } + + [Fact] + public void UnmanagedCallersOnly() + { + var source = $$""" + using System.Runtime.InteropServices; + + partial class C + { + [UnmanagedCallersOnly] // 1 + public partial int P1 { get; } + public partial int P1 => 1; + + public partial int P2 { get; } + [UnmanagedCallersOnly] // 2 + public partial int P2 => 1; + + public partial int P3 { [UnmanagedCallersOnly] get; } // 3 + public partial int P3 => 1; + + public partial int P4 { get; } + public partial int P4 { [UnmanagedCallersOnly] get => 1; } // 4 + } + """; + + var comp = CreateCompilation([source, UnmanagedCallersOnlyAttributeDefinition]); + comp.VerifyEmitDiagnostics( + // (5,6): error CS0592: Attribute 'UnmanagedCallersOnly' is not valid on this declaration type. It is only valid on 'method' declarations. + // [UnmanagedCallersOnly] // 1 + Diagnostic(ErrorCode.ERR_AttributeOnBadSymbolType, "UnmanagedCallersOnly").WithArguments("UnmanagedCallersOnly", "method").WithLocation(5, 6), + // (10,6): error CS0592: Attribute 'UnmanagedCallersOnly' is not valid on this declaration type. It is only valid on 'method' declarations. + // [UnmanagedCallersOnly] // 2 + Diagnostic(ErrorCode.ERR_AttributeOnBadSymbolType, "UnmanagedCallersOnly").WithArguments("UnmanagedCallersOnly", "method").WithLocation(10, 6), + // (13,30): error CS8896: 'UnmanagedCallersOnly' can only be applied to ordinary static non-abstract, non-virtual methods or static local functions. + // public partial int P3 { [UnmanagedCallersOnly] get; } // 3 + Diagnostic(ErrorCode.ERR_UnmanagedCallersOnlyRequiresStatic, "UnmanagedCallersOnly").WithLocation(13, 30), + // (17,30): error CS8896: 'UnmanagedCallersOnly' can only be applied to ordinary static non-abstract, non-virtual methods or static local functions. + // public partial int P4 { [UnmanagedCallersOnly] get => 1; } // 4 + Diagnostic(ErrorCode.ERR_UnmanagedCallersOnlyRequiresStatic, "UnmanagedCallersOnly").WithLocation(17, 30)); + } + // PROTOTYPE(partial-properties): override partial property where base has modopt // PROTOTYPE(partial-properties): test that doc comments work consistently with partial methods (and probably spec it as well) } From b1901538f3917218f9b4e53bc7eac4e2763982c8 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 16 May 2024 14:39:22 -0700 Subject: [PATCH 15/15] Add note for field keyword feature --- .../CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs index 425a31d3d855c..e4d5d591b05a2 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs @@ -3472,6 +3472,8 @@ partial class C // [field: Attr] Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "field").WithArguments("field", "property").WithLocation(10, 6)); + // https://github.com/dotnet/roslyn/issues/57012 + // 'field' keyword in properties feature should test partial properties where the implementation uses 'field' and one or both parts have 'field:' targeted attribute lists. var property = comp.GetMember("C.P"); AssertEx.Equal([], property.GetAttributes().ToStrings()); }