From e2b23fe2592ed3817797a1c05a6ccc862d61685a Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 19 Aug 2022 17:19:59 -0700 Subject: [PATCH 1/7] Filter out `RequiredMemberAttribute` from metadata symbols Fixes https://github.com/dotnet/roslyn/issues/61511. --- .../Symbols/Metadata/PE/PEFieldSymbol.cs | 22 +++++--------- .../Symbols/Metadata/PE/PEMethodSymbol.cs | 4 ++- .../Symbols/Metadata/PE/PEModuleSymbol.cs | 29 ++++++++++++++----- .../Symbols/Metadata/PE/PENamedTypeSymbol.cs | 5 +++- .../Symbols/Metadata/PE/PEParameterSymbol.cs | 2 ++ .../Symbols/Metadata/PE/PEPropertySymbol.cs | 4 ++- .../MetadataAsSourceTests.CSharp.cs | 15 ++++++---- 7 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs index b3e9d89b6c80e..9f8ad8e09d3ec 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs @@ -581,20 +581,14 @@ public override ImmutableArray GetAttributes() { var containingPEModuleSymbol = (PEModuleSymbol)this.ContainingModule; - if (FilterOutDecimalConstantAttribute()) - { - // filter out DecimalConstantAttribute - var attributes = containingPEModuleSymbol.GetCustomAttributesForToken( - _handle, - out _, - AttributeDescription.DecimalConstantAttribute); - - ImmutableInterlocked.InterlockedInitialize(ref _lazyCustomAttributes, attributes); - } - else - { - containingPEModuleSymbol.LoadCustomAttributes(_handle, ref _lazyCustomAttributes); - } + var attributes = containingPEModuleSymbol.GetCustomAttributesForToken( + _handle, + out _, + FilterOutDecimalConstantAttribute() ? AttributeDescription.DecimalConstantAttribute : default, + out _, + IsRequired ? AttributeDescription.RequiredMemberAttribute : default); + + ImmutableInterlocked.InterlockedInitialize(ref _lazyCustomAttributes, attributes); } return _lazyCustomAttributes; } diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs index 4da3ee989e5c5..57acf3592cd9c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs @@ -982,7 +982,9 @@ public override ImmutableArray GetAttributes() filteredOutAttribute4: out _, filterOut4: (checkForRequiredMembers && ObsoleteAttributeData is null) ? AttributeDescription.ObsoleteAttribute : default, filteredOutAttribute5: out _, - filterOut5: default); + filterOut5: default, + filteredOutAttribute6: out _, + filterOut6: default); isExtensionMethod = !extensionAttribute.IsNil; isReadOnly = !isReadOnlyAttribute.IsNil; diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEModuleSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEModuleSymbol.cs index ff1f0d5a8cc13..2c1292bca6f4a 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEModuleSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEModuleSymbol.cs @@ -294,11 +294,20 @@ internal ImmutableArray GetCustomAttributesForToken(EntityH out CustomAttributeHandle filteredOutAttribute1, AttributeDescription filterOut1) { - return GetCustomAttributesForToken(token, out filteredOutAttribute1, filterOut1, out _, default, out _, default, out _, default, out _, default); + return GetCustomAttributesForToken(token, out filteredOutAttribute1, filterOut1, out _, default, out _, default, out _, default, out _, default, out _, default); + } + + internal ImmutableArray GetCustomAttributesForToken(EntityHandle token, + out CustomAttributeHandle filteredOutAttribute1, + AttributeDescription filterOut1, + out CustomAttributeHandle filteredOutAttribute2, + AttributeDescription filterOut2) + { + return GetCustomAttributesForToken(token, out filteredOutAttribute1, filterOut1, out filteredOutAttribute2, filterOut2, out _, default, out _, default, out _, default, out _, default); } /// - /// Returns attributes with up-to four filters applied. For each filter, the last application of the + /// Returns attributes with up-to 6 filters applied. For each filter, the last application of the /// attribute will be tracked and returned. /// internal ImmutableArray GetCustomAttributesForToken(EntityHandle token, @@ -311,13 +320,16 @@ internal ImmutableArray GetCustomAttributesForToken(EntityH out CustomAttributeHandle filteredOutAttribute4, AttributeDescription filterOut4, out CustomAttributeHandle filteredOutAttribute5, - AttributeDescription filterOut5) + AttributeDescription filterOut5, + out CustomAttributeHandle filteredOutAttribute6, + AttributeDescription filterOut6) { filteredOutAttribute1 = default; filteredOutAttribute2 = default; filteredOutAttribute3 = default; filteredOutAttribute4 = default; filteredOutAttribute5 = default; + filteredOutAttribute6 = default; ArrayBuilder customAttributesBuilder = null; try @@ -357,6 +369,12 @@ internal ImmutableArray GetCustomAttributesForToken(EntityH continue; } + if (matchesFilter(customAttributeHandle, filterOut6)) + { + filteredOutAttribute6 = customAttributeHandle; + continue; + } + if (customAttributesBuilder == null) { customAttributesBuilder = ArrayBuilder.GetInstance(); @@ -436,10 +454,7 @@ internal ImmutableArray GetCustomAttributesFilterCompilerAt filteredOutAttribute1: out CustomAttributeHandle extensionAttribute, filterOut1: AttributeDescription.CaseSensitiveExtensionAttribute, filteredOutAttribute2: out CustomAttributeHandle isReadOnlyAttribute, - filterOut2: AttributeDescription.IsReadOnlyAttribute, - filteredOutAttribute3: out _, filterOut3: default, - filteredOutAttribute4: out _, filterOut4: default, - filteredOutAttribute5: out _, filterOut5: default); + filterOut2: AttributeDescription.IsReadOnlyAttribute); foundExtension = !extensionAttribute.IsNil; foundReadOnly = !isReadOnlyAttribute.IsNil; diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs index b7aa01e6898ab..61e1e6791e65c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs @@ -680,7 +680,10 @@ public override ImmutableArray GetAttributes() IsRefLikeType ? AttributeDescription.IsByRefLikeAttribute : default, out _, // Filter out [CompilerFeatureRequired] - (IsRefLikeType && DeriveCompilerFeatureRequiredDiagnostic() is null) ? AttributeDescription.CompilerFeatureRequiredAttribute : default); + (IsRefLikeType && DeriveCompilerFeatureRequiredDiagnostic() is null) ? AttributeDescription.CompilerFeatureRequiredAttribute : default, + out _, + // Filter out [RequiredMember] + HasDeclaredRequiredMembers ? AttributeDescription.RequiredMemberAttribute : default); ImmutableInterlocked.InterlockedInitialize(ref uncommon.lazyCustomAttributes, loadedCustomAttributes); } diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs index 093cf2e7d8873..05de7bd44127c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs @@ -1040,6 +1040,8 @@ public override ImmutableArray GetAttributes() out _, AttributeDescription.ScopedRefAttribute, out _, + default, + out _, default); if (!paramArrayAttribute.IsNil || !constantAttribute.IsNil) diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs index 14224e42c3994..c3798f30de856 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs @@ -642,7 +642,9 @@ public override ImmutableArray GetAttributes() ImmutableArray attributes = containingPEModuleSymbol.GetCustomAttributesForToken( _handle, out _, - this.RefKind == RefKind.RefReadOnly ? AttributeDescription.IsReadOnlyAttribute : default); + this.RefKind == RefKind.RefReadOnly ? AttributeDescription.IsReadOnlyAttribute : default, + out _, + IsRequired ? AttributeDescription.RequiredMemberAttribute : default); ImmutableInterlocked.InterlockedInitialize(ref _lazyCustomAttributes, attributes); } diff --git a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.CSharp.cs b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.CSharp.cs index 1ee1da581f388..a340489bb147b 100644 --- a/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.CSharp.cs +++ b/src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.CSharp.cs @@ -758,7 +758,11 @@ public class C public async Task TestRequiredProperty(bool signaturesOnly) { var metadataSource = """ - public class C { public required int Property { get; set; } } + public class C + { + public required int Property { get; set; } + public required int Field; + } namespace System.Runtime.CompilerServices { public sealed class RequiredMemberAttribute : Attribute { } @@ -783,14 +787,12 @@ public CompilerFeatureRequiredAttribute(string featureName) // {CodeAnalysisResources.InMemoryAssembly} #endregion -using System.Runtime.CompilerServices; - -[RequiredMember] public class [|C|] {{ + public required int Field; + public C(); - [RequiredMember] public required int Property {{ get; set; }} }}", false => $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null @@ -804,6 +806,9 @@ public class [|C|] [RequiredMember] public class [|C|] {{ + [RequiredMember] + public int Field; + [RequiredMember] public int Property {{ get; set; }} From e8136299e029a3aaaad2dbf4d36a767e8cae4d2b Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Mon, 22 Aug 2022 13:38:18 -0700 Subject: [PATCH 2/7] Fix required member tests with new expectations. --- .../Symbol/Symbols/RequiredMembersTests.cs | 9 +--- .../CSharp/RequiredMemberAttributesVisitor.cs | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index df787d86012e3..46e1de85b6c3f 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -56,14 +56,7 @@ private static Action ValidateRequiredMembersInModule(string[] mem AssertEx.NotNull(member, $"Member {memberPath} was not found"); Assert.True(member is PropertySymbol or FieldSymbol, $"Unexpected member symbol type {member.Kind}"); Assert.True(member.IsRequired()); - if (module is SourceModuleSymbol) - { - Assert.All(member.GetAttributes(), attr => AssertEx.NotEqual("System.Runtime.CompilerServices.RequiredMemberAttribute", attr.AttributeClass.ToTestDisplayString())); - } - else - { - AssertEx.Any(member.GetAttributes(), attr => attr.AttributeClass.ToTestDisplayString() == "System.Runtime.CompilerServices.RequiredMemberAttribute"); - } + Assert.All(member.GetAttributes(), attr => AssertEx.NotEqual("System.Runtime.CompilerServices.RequiredMemberAttribute", attr.AttributeClass.ToTestDisplayString())); requiredTypes.Add((NamedTypeSymbol)member.ContainingType); } diff --git a/src/Compilers/Test/Utilities/CSharp/RequiredMemberAttributesVisitor.cs b/src/Compilers/Test/Utilities/CSharp/RequiredMemberAttributesVisitor.cs index 00c93aa972ba1..276978b475adc 100644 --- a/src/Compilers/Test/Utilities/CSharp/RequiredMemberAttributesVisitor.cs +++ b/src/Compilers/Test/Utilities/CSharp/RequiredMemberAttributesVisitor.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; +using System.Reflection.Metadata; using System.Text; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE; @@ -25,6 +26,47 @@ private RequiredMemberAttributesVisitor(StringBuilder builder) : base(builder) protected override SymbolDisplayFormat DisplayFormat => SymbolDisplayFormat.TestFormat; + protected override void ReportSymbol(Symbol symbol) + { + EntityHandle handle; + PEModule module; + + switch (symbol) + { + case PENamedTypeSymbol namedType: + handle = namedType.Handle; + module = ((PEModuleSymbol)namedType.ContainingModule).Module; + break; + + case PEFieldSymbol field: + handle = field.Handle; + module = ((PEModuleSymbol)field.ContainingModule).Module; + break; + + case PEPropertySymbol property: + handle = property.Handle; + module = ((PEModuleSymbol)property.ContainingModule).Module; + break; + + default: + base.ReportSymbol(symbol); + return; + } + + var attribute = module.GetAttributeHandle(handle, AttributeDescription.RequiredMemberAttribute); + + if (attribute.IsNil) + { + return; + } + + ReportContainingSymbols(symbol); + _builder.Append(GetIndentString(symbol)); + _builder.Append("[RequiredMember] "); + _builder.AppendLine(symbol.ToDisplayString(DisplayFormat)); + _reported.Add(symbol); + } + protected override CSharpAttributeData? GetTargetAttribute(ImmutableArray attributes) => GetAttribute(attributes, "System.Runtime.CompilerServices", "RequiredMemberAttribute"); From 21550abad3aec735689b4edcbb9e015eb3f13b92 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Mon, 22 Aug 2022 15:32:18 -0700 Subject: [PATCH 3/7] PR feedback. --- .../Portable/Symbols/Metadata/PE/PEFieldSymbol.cs | 5 +++-- .../Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs | 11 +++++++++-- .../Portable/Symbols/Metadata/PE/PEPropertySymbol.cs | 5 +++-- .../CSharp/RequiredMemberAttributesVisitor.cs | 3 +++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs index 9f8ad8e09d3ec..6c5843faa621a 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs @@ -585,10 +585,11 @@ public override ImmutableArray GetAttributes() _handle, out _, FilterOutDecimalConstantAttribute() ? AttributeDescription.DecimalConstantAttribute : default, - out _, - IsRequired ? AttributeDescription.RequiredMemberAttribute : default); + out CustomAttributeHandle required, + AttributeDescription.RequiredMemberAttribute); ImmutableInterlocked.InterlockedInitialize(ref _lazyCustomAttributes, attributes); + _packedFlags.SetHasRequiredMemberAttribute(!required.IsNil); } return _lazyCustomAttributes; } diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs index 61e1e6791e65c..5e52a4f88f75c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs @@ -681,11 +681,18 @@ public override ImmutableArray GetAttributes() out _, // Filter out [CompilerFeatureRequired] (IsRefLikeType && DeriveCompilerFeatureRequiredDiagnostic() is null) ? AttributeDescription.CompilerFeatureRequiredAttribute : default, - out _, + out CustomAttributeHandle requiredHandle, // Filter out [RequiredMember] - HasDeclaredRequiredMembers ? AttributeDescription.RequiredMemberAttribute : default); + AttributeDescription.RequiredMemberAttribute); ImmutableInterlocked.InterlockedInitialize(ref uncommon.lazyCustomAttributes, loadedCustomAttributes); + + if (!uncommon.lazyHasRequiredMembers.HasValue()) + { + uncommon.lazyHasRequiredMembers = (!requiredHandle.IsNil).ToThreeState(); + } + + Debug.Assert(uncommon.lazyHasRequiredMembers.Value() != requiredHandle.IsNil); } return uncommon.lazyCustomAttributes; diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs index c3798f30de856..57f6f9695c9b7 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs @@ -643,10 +643,11 @@ public override ImmutableArray GetAttributes() _handle, out _, this.RefKind == RefKind.RefReadOnly ? AttributeDescription.IsReadOnlyAttribute : default, - out _, - IsRequired ? AttributeDescription.RequiredMemberAttribute : default); + out CustomAttributeHandle required, + AttributeDescription.RequiredMemberAttribute); ImmutableInterlocked.InterlockedInitialize(ref _lazyCustomAttributes, attributes); + _flags.SetHasRequiredMemberAttribute(!required.IsNil); } return _lazyCustomAttributes; } diff --git a/src/Compilers/Test/Utilities/CSharp/RequiredMemberAttributesVisitor.cs b/src/Compilers/Test/Utilities/CSharp/RequiredMemberAttributesVisitor.cs index 276978b475adc..cf5d1f0023457 100644 --- a/src/Compilers/Test/Utilities/CSharp/RequiredMemberAttributesVisitor.cs +++ b/src/Compilers/Test/Utilities/CSharp/RequiredMemberAttributesVisitor.cs @@ -65,6 +65,9 @@ protected override void ReportSymbol(Symbol symbol) _builder.Append("[RequiredMember] "); _builder.AppendLine(symbol.ToDisplayString(DisplayFormat)); _reported.Add(symbol); + + // If attributes aren't filtered out, this will print extra data and cause an error in test assertion. + base.ReportSymbol(symbol); } protected override CSharpAttributeData? GetTargetAttribute(ImmutableArray attributes) From 2e4d922ae0b25338af3088226d76fe6431ae0fdc Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 23 Aug 2022 10:17:15 -0700 Subject: [PATCH 4/7] Add explicit test for ordering. --- .../Symbol/Symbols/RequiredMembersTests.cs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 46e1de85b6c3f..1c9e339e557de 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -4946,6 +4946,49 @@ class C // public required int Test { get; set; } Diagnostic(ErrorCode.ERR_DuplicateNameInClass, "Test").WithArguments("C", "Test").WithLocation(4, 25) ); + } + + [Theory, CombinatorialData] + public void FirstAccessOfIsRequiredDoesNotMatter(bool accessAttributesFirst) + { + // Accessing attributes will populate IsRequired if it's not already populated, so we want to test both codepaths explicitly. + var comp = CreateCompilationWithRequiredMembers(""" + public class C + { + public required int Field; + public required int Property { get; set; } + } + """); + + CompileAndVerify(comp, symbolValidator: module => + { + var c = module.ContainingAssembly.GetTypeByMetadataName("C"); + AssertEx.NotNull(c); + if (accessAttributesFirst) + { + AssertAttributesEmpty(); + AssertIsRequired(); + } + else + { + AssertIsRequired(); + AssertAttributesEmpty(); + } + + void AssertIsRequired() + { + Assert.True(c.HasDeclaredRequiredMembers); + Assert.True(c.GetMember("Field").IsRequired); + Assert.True(c.GetMember("Property").IsRequired); + } + + void AssertAttributesEmpty() + { + Assert.Empty(c.GetAttributes()); + Assert.Empty(c.GetMember("Field").GetAttributes()); + Assert.Empty(c.GetMember("Property").GetAttributes()); + } + }); } } From 8907e78a816c63ba4658a792280dddc986ac2eac Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 23 Aug 2022 10:20:43 -0700 Subject: [PATCH 5/7] Style refactoring. --- .../Symbol/Symbols/RequiredMembersTests.cs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 1c9e339e557de..121dd483d30d1 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -4964,30 +4964,32 @@ public class C { var c = module.ContainingAssembly.GetTypeByMetadataName("C"); AssertEx.NotNull(c); + FieldSymbol fieldSymbol = c.GetMember("Field"); + PropertySymbol propertySymbol = c.GetMember("Property"); if (accessAttributesFirst) { - AssertAttributesEmpty(); - AssertIsRequired(); + assertAttributesEmpty(); + assertIsRequired(); } else { - AssertIsRequired(); - AssertAttributesEmpty(); + assertIsRequired(); + assertAttributesEmpty(); } - void AssertIsRequired() + void assertIsRequired() { Assert.True(c.HasDeclaredRequiredMembers); - Assert.True(c.GetMember("Field").IsRequired); - Assert.True(c.GetMember("Property").IsRequired); + Assert.True(fieldSymbol.IsRequired); + Assert.True(propertySymbol.IsRequired); } - void AssertAttributesEmpty() + void assertAttributesEmpty() { Assert.Empty(c.GetAttributes()); - Assert.Empty(c.GetMember("Field").GetAttributes()); - Assert.Empty(c.GetMember("Property").GetAttributes()); + Assert.Empty(fieldSymbol.GetAttributes()); + Assert.Empty(propertySymbol.GetAttributes()); } }); } From b15f4fe6a6819b82fbf23d9e2e9cb494c23a17b5 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 24 Aug 2022 09:44:29 -0700 Subject: [PATCH 6/7] Test changes as requested. --- .../Symbol/Symbols/RequiredMembersTests.cs | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 121dd483d30d1..97021ae50ae32 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -4955,8 +4955,10 @@ public void FirstAccessOfIsRequiredDoesNotMatter(bool accessAttributesFirst) var comp = CreateCompilationWithRequiredMembers(""" public class C { - public required int Field; - public required int Property { get; set; } + public required int Field1; + public required int Property1 { get; set; } + public int Field2; + public int Property2 { get; set; } } """); @@ -4964,8 +4966,10 @@ public class C { var c = module.ContainingAssembly.GetTypeByMetadataName("C"); AssertEx.NotNull(c); - FieldSymbol fieldSymbol = c.GetMember("Field"); - PropertySymbol propertySymbol = c.GetMember("Property"); + FieldSymbol field1 = c.GetMember("Field1"); + PropertySymbol property1 = c.GetMember("Property1"); + FieldSymbol field2 = c.GetMember("Field2"); + PropertySymbol property2 = c.GetMember("Property2"); if (accessAttributesFirst) { @@ -4981,15 +4985,19 @@ public class C void assertIsRequired() { Assert.True(c.HasDeclaredRequiredMembers); - Assert.True(fieldSymbol.IsRequired); - Assert.True(propertySymbol.IsRequired); + Assert.True(field1.IsRequired); + Assert.True(property1.IsRequired); + Assert.False(field2.IsRequired); + Assert.False(property2.IsRequired); } void assertAttributesEmpty() { Assert.Empty(c.GetAttributes()); - Assert.Empty(fieldSymbol.GetAttributes()); - Assert.Empty(propertySymbol.GetAttributes()); + Assert.Empty(field1.GetAttributes()); + Assert.Empty(property1.GetAttributes()); + Assert.Empty(field2.GetAttributes()); + Assert.Empty(property2.GetAttributes()); } }); } From 7859bb957b168efdd077401d53befa8f8982f640 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 24 Aug 2022 13:53:21 -0700 Subject: [PATCH 7/7] More feedback. --- .../Test/Symbol/Symbols/RequiredMembersTests.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 97021ae50ae32..e9fafc59148b8 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -4957,6 +4957,10 @@ public class C { public required int Field1; public required int Property1 { get; set; } + } + + public class D + { public int Field2; public int Property2 { get; set; } } @@ -4968,8 +4972,10 @@ public class C AssertEx.NotNull(c); FieldSymbol field1 = c.GetMember("Field1"); PropertySymbol property1 = c.GetMember("Property1"); - FieldSymbol field2 = c.GetMember("Field2"); - PropertySymbol property2 = c.GetMember("Property2"); + var d = module.ContainingAssembly.GetTypeByMetadataName("D"); + AssertEx.NotNull(d); + FieldSymbol field2 = d.GetMember("Field2"); + PropertySymbol property2 = d.GetMember("Property2"); if (accessAttributesFirst) { @@ -4987,6 +4993,7 @@ void assertIsRequired() Assert.True(c.HasDeclaredRequiredMembers); Assert.True(field1.IsRequired); Assert.True(property1.IsRequired); + Assert.False(d.HasDeclaredRequiredMembers); Assert.False(field2.IsRequired); Assert.False(property2.IsRequired); } @@ -4996,6 +5003,7 @@ void assertAttributesEmpty() Assert.Empty(c.GetAttributes()); Assert.Empty(field1.GetAttributes()); Assert.Empty(property1.GetAttributes()); + Assert.Empty(d.GetAttributes()); Assert.Empty(field2.GetAttributes()); Assert.Empty(property2.GetAttributes()); }