From 4adfb65ee4362f0d53a73854d41b5871d02decf6 Mon Sep 17 00:00:00 2001 From: Fred Silberberg Date: Mon, 14 Feb 2022 19:17:06 -0800 Subject: [PATCH] Add support for decoding required members in metadata (#59288) Added support for reading the RequiredMemberAttribute from metadata appropriately and returning the right result for IsRequired and HasDeclaredRequiredMembers for metadata infos. --- .../CSharp/Portable/CSharpResources.resx | 2 +- .../Symbols/Metadata/PE/PEFieldSymbol.cs | 39 +++- .../Symbols/Metadata/PE/PENamedTypeSymbol.cs | 26 ++- .../Symbols/Metadata/PE/PEPropertySymbol.cs | 86 +++++-- ...berContainerSymbol_ImplementationChecks.cs | 2 +- .../Portable/xlf/CSharpResources.cs.xlf | 6 +- .../Portable/xlf/CSharpResources.de.xlf | 6 +- .../Portable/xlf/CSharpResources.es.xlf | 6 +- .../Portable/xlf/CSharpResources.fr.xlf | 6 +- .../Portable/xlf/CSharpResources.it.xlf | 6 +- .../Portable/xlf/CSharpResources.ja.xlf | 6 +- .../Portable/xlf/CSharpResources.ko.xlf | 6 +- .../Portable/xlf/CSharpResources.pl.xlf | 6 +- .../Portable/xlf/CSharpResources.pt-BR.xlf | 6 +- .../Portable/xlf/CSharpResources.ru.xlf | 6 +- .../Portable/xlf/CSharpResources.tr.xlf | 6 +- .../Portable/xlf/CSharpResources.zh-Hans.xlf | 6 +- .../Portable/xlf/CSharpResources.zh-Hant.xlf | 6 +- .../Symbol/Symbols/RequiredMembersTests.cs | 219 ++++++++++++++---- 19 files changed, 337 insertions(+), 115 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index 22f1681ce31a0..9e8f50c20a240 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -6948,7 +6948,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ required members - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' Required member '{0}' cannot be hidden by '{1}'. diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs index f5d35611fd6f4..27efdc935350e 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs @@ -29,9 +29,10 @@ internal sealed class PEFieldSymbol : FieldSymbol private struct PackedFlags { // Layout: - // |..............................|vvvvv| + // |............................|rr|vvvvv| // // f = FlowAnalysisAnnotations. 5 bits (4 value bits + 1 completion bit). + // r = Required members. 2 bits (1 value bit + 1 completion bit). private const int HasDisallowNullAttribute = 0x1 << 0; private const int HasAllowNullAttribute = 0x1 << 1; @@ -39,6 +40,9 @@ private struct PackedFlags private const int HasNotNullAttribute = 0x1 << 3; private const int FlowAnalysisAnnotationsCompletionBit = 0x1 << 4; + private const int HasRequiredMemberAttribute = 0x1 << 5; + private const int RequiredMemberCompletionBit = 0x1 << 6; + private int _bits; public bool SetFlowAnalysisAnnotations(FlowAnalysisAnnotations value) @@ -67,6 +71,24 @@ public bool TryGetFlowAnalysisAnnotations(out FlowAnalysisAnnotations value) Debug.Assert(value == 0 || result); return result; } + + public bool SetHasRequiredMemberAttribute(bool isRequired) + { + int bitsToSet = RequiredMemberCompletionBit | (isRequired ? HasRequiredMemberAttribute : 0); + return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet); + } + + public bool TryGetHasRequiredMemberAttribute(out bool hasRequiredMemberAttribute) + { + if ((_bits & RequiredMemberCompletionBit) != 0) + { + hasRequiredMemberAttribute = (_bits & HasRequiredMemberAttribute) != 0; + return true; + } + + hasRequiredMemberAttribute = false; + return false; + } } private readonly FieldDefinitionHandle _handle; @@ -588,7 +610,18 @@ internal override ObsoleteAttributeData ObsoleteAttributeData get { return null; } } - // PROTOTYPE(req): Implement - internal override bool IsRequired => false; + internal override bool IsRequired + { + get + { + if (!_packedFlags.TryGetHasRequiredMemberAttribute(out bool hasRequiredMemberAttribute)) + { + hasRequiredMemberAttribute = ContainingPEModule.Module.HasAttribute(_handle, AttributeDescription.RequiredMemberAttribute); + _packedFlags.SetHasRequiredMemberAttribute(hasRequiredMemberAttribute); + } + + return hasRequiredMemberAttribute; + } + } } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs index dc8229e580697..eee021718a380 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs @@ -143,6 +143,7 @@ private class UncommonProperties internal NamedTypeSymbol lazyComImportCoClassType = ErrorTypeSymbol.UnknownResultType; internal ThreeState lazyHasEmbeddedAttribute = ThreeState.Unknown; internal ThreeState lazyHasInterpolatedStringHandlerAttribute = ThreeState.Unknown; + internal ThreeState lazyHasRequiredMembers = ThreeState.Unknown; #if DEBUG internal bool IsDefaultValue() @@ -157,7 +158,8 @@ internal bool IsDefaultValue() lazyDefaultMemberName == null && (object)lazyComImportCoClassType == (object)ErrorTypeSymbol.UnknownResultType && !lazyHasEmbeddedAttribute.HasValue() && - !lazyHasInterpolatedStringHandlerAttribute.HasValue(); + !lazyHasInterpolatedStringHandlerAttribute.HasValue() && + !lazyHasRequiredMembers.HasValue(); } #endif } @@ -824,8 +826,26 @@ private static ICollection CreateReadOnlyMemberNames(HashSet nam } } - // PROTOTYPE(req): Implement - internal override bool HasDeclaredRequiredMembers => false; + internal override bool HasDeclaredRequiredMembers + { + get + { + var uncommon = GetUncommonProperties(); + if (uncommon == s_noUncommonProperties) + { + return false; + } + + if (uncommon.lazyHasRequiredMembers.HasValue()) + { + return uncommon.lazyHasRequiredMembers.Value(); + } + + var hasRequiredMemberAttribute = ContainingPEModule.Module.HasAttribute(_handle, AttributeDescription.RequiredMemberAttribute); + uncommon.lazyHasRequiredMembers = hasRequiredMemberAttribute.ToThreeState(); + return hasRequiredMemberAttribute; + } + } public override ImmutableArray GetMembers() { diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs index 8e24f9904c191..6ea1f50775670 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs @@ -17,6 +17,7 @@ using Microsoft.CodeAnalysis.CSharp.DocumentationComments; using Microsoft.CodeAnalysis.CSharp.Emit; using Microsoft.CodeAnalysis.PooledObjects; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE { @@ -47,14 +48,53 @@ internal class PEPropertySymbol private const int UnsetAccessibility = -1; private int _declaredAccessibility = UnsetAccessibility; - private readonly Flags _flags; + private PackedFlags _flags; - [Flags] - private enum Flags : byte + private struct PackedFlags { - IsSpecialName = 1, - IsRuntimeSpecialName = 2, - CallMethodsDirectly = 4 + // Layout: + // |...........................|rr|c|n|s| + // + // s = special name flag. 1 bit + // n = runtime special name flag. 1 bit + // c = call methods directly flag. 1 bit + // r = Required member. 2 bits (1 bit for value + 1 completion bit). + private const int IsSpecialNameFlag = 1 << 0; + private const int IsRuntimeSpecialNameFlag = 1 << 1; + private const int CallMethodsDirectlyFlag = 1 << 2; + private const int HasRequiredMemberAttribute = 1 << 4; + private const int RequiredMemberCompletionBit = 1 << 5; + + private int _bits; + + public PackedFlags(bool isSpecialName, bool isRuntimeSpecialName, bool callMethodsDirectly) + { + _bits = (isSpecialName ? IsSpecialNameFlag : 0) + | (isRuntimeSpecialName ? IsRuntimeSpecialNameFlag : 0) + | (callMethodsDirectly ? CallMethodsDirectlyFlag : 0); + } + + public void SetHasRequiredMemberAttribute(bool isRequired) + { + var bitsToSet = (isRequired ? HasRequiredMemberAttribute : 0) | RequiredMemberCompletionBit; + ThreadSafeFlagOperations.Set(ref _bits, bitsToSet); + } + + public bool TryGetHasRequiredMemberAttribute(out bool hasRequiredMemberAttribute) + { + if ((_bits & RequiredMemberCompletionBit) != 0) + { + hasRequiredMemberAttribute = (_bits & HasRequiredMemberAttribute) != 0; + return true; + } + + hasRequiredMemberAttribute = false; + return false; + } + + public bool IsSpecialName => (_bits & IsSpecialNameFlag) != 0; + public bool IsRuntimeSpecialName => (_bits & IsRuntimeSpecialNameFlag) != 0; + public bool CallMethodsDirectly => (_bits & CallMethodsDirectlyFlag) != 0; } internal static PEPropertySymbol Create( @@ -204,20 +244,10 @@ private PEPropertySymbol( } } - if (callMethodsDirectly) - { - _flags |= Flags.CallMethodsDirectly; - } - - if ((mdFlags & PropertyAttributes.SpecialName) != 0) - { - _flags |= Flags.IsSpecialName; - } - - if ((mdFlags & PropertyAttributes.RTSpecialName) != 0) - { - _flags |= Flags.IsRuntimeSpecialName; - } + _flags = new PackedFlags( + isSpecialName: (mdFlags & PropertyAttributes.SpecialName) != 0, + isRuntimeSpecialName: (mdFlags & PropertyAttributes.RTSpecialName) != 0, + callMethodsDirectly); static bool anyUnexpectedRequiredModifiers(ParamInfo[] propertyParams) { @@ -277,7 +307,7 @@ public override string Name internal override bool HasSpecialName { - get { return (_flags & Flags.IsSpecialName) != 0; } + get { return _flags.IsSpecialName; } } public override string MetadataName @@ -466,8 +496,14 @@ internal override bool IsRequired { get { - // PROTOTYPE(req): Implement - return false; + if (!_flags.TryGetHasRequiredMemberAttribute(out bool hasRequiredMemberAttribute)) + { + var containingPEModuleSymbol = (PEModuleSymbol)this.ContainingModule; + hasRequiredMemberAttribute = containingPEModuleSymbol.Module.HasAttribute(_handle, AttributeDescription.RequiredMemberAttribute); + _flags.SetHasRequiredMemberAttribute(hasRequiredMemberAttribute); + } + + return hasRequiredMemberAttribute; } } @@ -632,7 +668,7 @@ public override ImmutableArray ExplicitInterfaceImplementations internal override bool MustCallMethodsDirectly { - get { return (_flags & Flags.CallMethodsDirectly) != 0; } + get { return _flags.CallMethodsDirectly; } } private static bool DoSignaturesMatch( @@ -767,7 +803,7 @@ internal override bool HasRuntimeSpecialName { get { - return (_flags & Flags.IsRuntimeSpecialName) != 0; + return _flags.IsRuntimeSpecialName; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs index 5c14d9644ef29..134c3b1e236ef 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs @@ -908,7 +908,7 @@ void checkSingleOverriddenMember(Symbol overridingMember, Symbol overriddenMembe } else if (overriddenMember is PropertySymbol { IsRequired: true } && overridingMember is PropertySymbol { IsRequired: false }) { - // '{0}': cannot remove 'required' from '{1}' when overriding + // '{0}' must be required because it overrides required member '{1}' diagnostics.Add(ErrorCode.ERR_OverrideMustHaveRequired, overridingMemberLocation, overridingMember, overriddenMember); } else diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf index a19f085033ea8..e2ff29ddddcc2 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ Pokud chcete odstranit toto varování, můžete místo toho použít /reference - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf index fefcc9f658271..f298ae69df379 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ Um die Warnung zu beheben, können Sie stattdessen /reference verwenden (Einbett - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf index dd5d8f16dd59d..cd891365c5719 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ Para eliminar la advertencia puede usar /reference (establezca la propiedad Embe - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf index 60d4172ca9027..8e76e509495f0 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ Pour supprimer l'avertissement, vous pouvez utiliser la commande /reference (dé - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf index 6bdd6d1fe5a71..c47fb03ef21aa 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ Per rimuovere l'avviso, è invece possibile usare /reference (impostare la propr - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf index ffb1a49ea0bb4..056a11080142a 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf index 23fad227a5071..9e04e2983966b 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11433,4 +11433,4 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf index 56990cd668e6d..e63aeadcf0f96 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ Aby usunąć ostrzeżenie, możesz zamiast tego użyć opcji /reference (ustaw w - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf index bb83790d13313..27d01ea964fd3 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ Para incorporar informações de tipo de interoperabilidade para os dois assembl - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf index 86f3b39cb1899..4d1605ea58f06 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf index 8d4394caf493b..e57aeda4a8658 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ Uyarıyı kaldırmak için, /reference kullanabilirsiniz (Birlikte Çalışma T - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf index d8ba71d3cf382..9ec1f386e046e 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11439,4 +11439,4 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf index 03669f0a62a8c..1dfbb00bff7c8 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf @@ -983,8 +983,8 @@ - '{0}': cannot remove 'required' from '{1}' when overriding - '{0}': cannot remove 'required' from '{1}' when overriding + '{0}' must be required because it overrides required member '{1}' + '{0}' must be required because it overrides required member '{1}' @@ -11434,4 +11434,4 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - \ No newline at end of file + diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index 7d16b5d73599d..51ebef1477f6d 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -28,8 +28,8 @@ public RequiredMemberAttribute() } "; - private static CSharpCompilation CreateCompilationWithRequiredMembers(CSharpTestSource source, CSharpParseOptions? parseOptions = null, CSharpCompilationOptions? options = null, TargetFramework tfm = TargetFramework.NetCoreApp) - => CreateCompilation(new[] { source, RequiredMemberAttribute }, options: options, parseOptions: parseOptions); + private static CSharpCompilation CreateCompilationWithRequiredMembers(CSharpTestSource source, CSharpParseOptions? parseOptions = null, CSharpCompilationOptions? options = null, string? assemblyName = null) + => CreateCompilation(new[] { source, RequiredMemberAttribute }, options: options, parseOptions: parseOptions, assemblyName: assemblyName); private static Action ValidateRequiredMembersInModule(string[] memberPaths, string expectedAttributeLayout) { @@ -39,19 +39,23 @@ private static Action ValidateRequiredMembersInModule(string[] mem { var actualAttributes = RequiredMemberAttributesVisitor.GetString(peModule); AssertEx.AssertEqualToleratingWhitespaceDifferences(expectedAttributeLayout, actualAttributes); - return; } foreach (var memberPath in memberPaths) { var member = module.GlobalNamespace.GetMember(memberPath); + 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.True(((NamedTypeSymbol)member.ContainingSymbol).HasDeclaredRequiredMembers); } }; } @@ -404,65 +408,106 @@ [RequiredMember] System.Int32 C.Field ); } - [Fact] - public void RequiredMemberAttributeEmitted_OverrideRequiredProperty_MissingRequiredOnOverride01() + [Theory] + [CombinatorialData] + public void RequiredMemberAttributeEmitted_OverrideRequiredProperty_MissingRequiredOnOverride01(bool useMetadataReference) { - var comp = CreateCompilationWithRequiredMembers(@" -class Base + var @base = @" +public class Base { public virtual required int Prop { get; set; } -} +}"; + + var derived = @" class Derived : Base { public override int Prop { get; set; } } -"); +"; + + var comp = CreateCompilationWithRequiredMembers(@base + derived); comp.VerifyDiagnostics( - // (8,25): error CS9501: 'Derived.Prop': cannot remove 'required' from 'Base.Prop' when overriding + // (8,25): error CS9501: 'Derived.Prop' must be required because it overrides required member 'Base.Prop' // public override int Prop { get; set; } Diagnostic(ErrorCode.ERR_OverrideMustHaveRequired, "Prop").WithArguments("Derived.Prop", "Base.Prop").WithLocation(8, 25) ); + + var baseComp = CreateCompilationWithRequiredMembers(@base); + baseComp.VerifyDiagnostics(); + + comp = CreateCompilation(derived, references: new[] { useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference() }); + comp.VerifyDiagnostics( + // (4,25): error CS9501: 'Derived.Prop' must be required because it overrides required member 'Base.Prop' + // public override int Prop { get; set; } + Diagnostic(ErrorCode.ERR_OverrideMustHaveRequired, "Prop").WithArguments("Derived.Prop", "Base.Prop").WithLocation(4, 25) + ); } - [Fact] - public void RequiredMemberAttributeEmitted_OverrideRequiredProperty_MissingRequiredOnOverride02() + [Theory] + [CombinatorialData] + public void RequiredMemberAttributeEmitted_OverrideRequiredProperty_MissingRequiredOnOverride02(bool useMetadataReference) { - var comp = CreateCompilationWithRequiredMembers(@" -class Base + var @base = @" +public class Base { public virtual int Prop { get; set; } -} -class Derived : Base +}"; + + var derived = @" +public class Derived : Base { public override required int Prop { get; set; } -} +}"; + + var derivedDerived = @" class DerivedDerived : Derived { public override int Prop { get; set; } } -"); +"; + + var comp = CreateCompilationWithRequiredMembers(@base + derived + derivedDerived); comp.VerifyDiagnostics( - // (12,25): error CS9501: 'DerivedDerived.Prop': cannot remove 'required' from 'Derived.Prop' when overriding + // (12,25): error CS9501: 'DerivedDerived.Prop' must be required because it overrides required member 'Derived.Prop' // public override int Prop { get; set; } Diagnostic(ErrorCode.ERR_OverrideMustHaveRequired, "Prop").WithArguments("DerivedDerived.Prop", "Derived.Prop").WithLocation(12, 25) ); + + var baseComp = CreateCompilationWithRequiredMembers(@base); + baseComp.VerifyDiagnostics(); + + MetadataReference baseReference = useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference(); + var derivedComp = CreateCompilation(derived, references: new[] { baseReference }); + derivedComp.VerifyDiagnostics(); + + comp = CreateCompilation(derivedDerived, new[] { baseReference, useMetadataReference ? derivedComp.ToMetadataReference() : derivedComp.EmitToImageReference() }); + comp.VerifyDiagnostics( + // (4,25): error CS9501: 'DerivedDerived.Prop' must be required because it overrides required member 'Derived.Prop' + // public override int Prop { get; set; } + Diagnostic(ErrorCode.ERR_OverrideMustHaveRequired, "Prop").WithArguments("DerivedDerived.Prop", "Derived.Prop").WithLocation(4, 25) + ); } - [Fact] - public void RequiredMemberAttributeEmitted_OverrideRequiredProperty() + [Theory] + [CombinatorialData] + public void RequiredMemberAttributeEmitted_OverrideRequiredProperty(bool useMetadataReference) { - var comp = CreateCompilationWithRequiredMembers(@" -class Base + var @base = @" +public class Base { public virtual required int Prop { get; set; } } +"; + + string derived = @" class Derived : Base { public override required int Prop { get; set; } } -"); +"; + var comp = CreateCompilationWithRequiredMembers(@base + derived); var expectedRequiredMembers = new[] { "Base.Prop", "Derived.Prop" }; @@ -475,25 +520,45 @@ [RequiredMember] Derived var symbolValidator = ValidateRequiredMembersInModule(expectedRequiredMembers, expectedAttributeLayout); var verifier = CompileAndVerify(comp, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); verifier.VerifyDiagnostics(); + + var baseComp = CreateCompilationWithRequiredMembers(@base, assemblyName: "test"); + baseComp.VerifyDiagnostics(); + + comp = CreateCompilation(derived, new[] { useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference() }); + expectedAttributeLayout = @" +[RequiredMember] Derived + [RequiredMember] System.Int32 Derived.Prop { get; set; } +"; + symbolValidator = ValidateRequiredMembersInModule(new[] { "Derived.Prop" }, expectedAttributeLayout); + verifier = CompileAndVerify(comp, symbolValidator: symbolValidator, sourceSymbolValidator: symbolValidator); + verifier.VerifyDiagnostics(); } - [Fact] - public void RequiredMemberAttributeEmitted_AddRequiredOnOverride() + [Theory] + [CombinatorialData] + public void RequiredMemberAttributeEmitted_AddRequiredOnOverride(bool useMetadataReference) { - var comp = CreateCompilationWithRequiredMembers(@" -class Base + var @base = @" +public class Base { public virtual int Prop { get; set; } } -class Derived : Base +"; + + var derived = @" +public class Derived : Base { public override required int Prop { get; set; } } +"; + + var derivedDerived = @" class DerivedDerived : Derived { public override required int Prop { get; set; } } -"); +"; + var comp = CreateCompilationWithRequiredMembers(@base + derived + derivedDerived); var expectedRequiredMembers = new[] { "Derived.Prop", "DerivedDerived.Prop" }; @@ -507,6 +572,22 @@ [RequiredMember] DerivedDerived var symbolValidator = ValidateRequiredMembersInModule(expectedRequiredMembers, expectedAttributeLayout); var verifier = CompileAndVerify(comp, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); verifier.VerifyDiagnostics(); + + var baseComp = CreateCompilationWithRequiredMembers(@base); + baseComp.VerifyDiagnostics(); + var baseReference = useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference(); + + var derivedComp = CreateCompilation(derived, new[] { baseReference }); + derivedComp.VerifyDiagnostics(); + + comp = CreateCompilation(derivedDerived, new[] { baseReference, useMetadataReference ? derivedComp.ToMetadataReference() : derivedComp.EmitToImageReference() }); + expectedAttributeLayout = @" +[RequiredMember] DerivedDerived + [RequiredMember] System.Int32 DerivedDerived.Prop { get; set; } +"; + symbolValidator = ValidateRequiredMembersInModule(new[] { "DerivedDerived.Prop" }, expectedAttributeLayout); + verifier = CompileAndVerify(comp, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); + verifier.VerifyDiagnostics(); } [Fact] @@ -540,19 +621,23 @@ [RequiredMember] System.Int32 Outer.Inner.Field ); } - [Fact] - public void RequiredMemberAttributeEmitted_AbstractProperty() + [Theory] + [CombinatorialData] + public void RequiredMemberAttributeEmitted_AbstractProperty(bool useMetadataReference) { - var comp = CreateCompilationWithRequiredMembers(@" -abstract class Base + var @base = @" +public abstract class Base { public required abstract int Prop { get; set; } -} +}"; + + var derived = @" class Derived : Base { public override required int Prop { get; set; } } -"); +"; + var comp = CreateCompilationWithRequiredMembers(@base + derived); var expectedRequiredMembers = new[] { "Base.Prop", "Derived.Prop" }; @@ -566,18 +651,37 @@ [RequiredMember] Derived var symbolValidator = ValidateRequiredMembersInModule(expectedRequiredMembers, expectedAttributeLayout); var verifier = CompileAndVerify(comp, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); verifier.VerifyDiagnostics(); + + var baseComp = CreateCompilationWithRequiredMembers(@base, assemblyName: "base"); + baseComp.VerifyDiagnostics(); + var baseReference = useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference(); + + comp = CreateCompilation(derived, new[] { baseReference }, assemblyName: "derived"); + + expectedAttributeLayout = @" +[RequiredMember] Derived + [RequiredMember] System.Int32 Derived.Prop { get; set; } +"; + + symbolValidator = ValidateRequiredMembersInModule(new[] { "Derived.Prop" }, expectedAttributeLayout); + verifier = CompileAndVerify(comp, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); + verifier.VerifyDiagnostics(); } - [Fact] - public void HidingRequiredMembers() + [Theory] + [CombinatorialData] + public void HidingRequiredMembers(bool useMetadataReference) { - var comp = CreateCompilationWithRequiredMembers(@" + var @base = +@" #pragma warning disable CS0649 // Never assigned -class Base +public class Base { public required int Field; public required int Prop { get; set; } -} +}"; + + var derived = @" class Derived1 : Base { public new int Field; // 1 @@ -593,7 +697,8 @@ class Derived3 : Base public int Field; // 1 public int Prop { get; set; } // 2 } -"); +"; + var comp = CreateCompilationWithRequiredMembers(@base + derived); comp.VerifyDiagnostics( // (10,20): error CS9502: Required member 'Base.Field' cannot be hidden by 'Derived1.Field'. @@ -618,6 +723,34 @@ class Derived3 : Base // public int Prop { get; set; } // 2 Diagnostic(ErrorCode.ERR_RequiredMemberCannotBeHidden, "Prop").WithArguments("Base.Prop", "Derived3.Prop").WithLocation(21, 16) ); + + var baseComp = CreateCompilationWithRequiredMembers(@base); + baseComp.VerifyDiagnostics(); + + comp = CreateCompilation("#pragma warning disable CS0649 // Never assigned" + derived, new[] { useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference() }); + comp.VerifyDiagnostics( + // (4,20): error CS9502: Required member 'Base.Field' cannot be hidden by 'Derived1.Field'. + // public new int Field; // 1 + Diagnostic(ErrorCode.ERR_RequiredMemberCannotBeHidden, "Field").WithArguments("Base.Field", "Derived1.Field").WithLocation(4, 20), + // (5,20): error CS9502: Required member 'Base.Prop' cannot be hidden by 'Derived1.Prop'. + // public new int Prop { get; set; } // 2 + Diagnostic(ErrorCode.ERR_RequiredMemberCannotBeHidden, "Prop").WithArguments("Base.Prop", "Derived1.Prop").WithLocation(5, 20), + // (9,20): error CS9502: Required member 'Base.Prop' cannot be hidden by 'Derived2.Prop'. + // public new int Prop; // 3 + Diagnostic(ErrorCode.ERR_RequiredMemberCannotBeHidden, "Prop").WithArguments("Base.Prop", "Derived2.Prop").WithLocation(9, 20), + // (10,20): error CS9502: Required member 'Base.Field' cannot be hidden by 'Derived2.Field'. + // public new int Field { get; set; } // 4 + Diagnostic(ErrorCode.ERR_RequiredMemberCannotBeHidden, "Field").WithArguments("Base.Field", "Derived2.Field").WithLocation(10, 20), + // (14,16): warning CS0108: 'Derived3.Field' hides inherited member 'Base.Field'. Use the new keyword if hiding was intended. + // public int Field; // 1 + Diagnostic(ErrorCode.WRN_NewRequired, "Field").WithArguments("Derived3.Field", "Base.Field").WithLocation(14, 16), + // (14,16): error CS9502: Required member 'Base.Field' cannot be hidden by 'Derived3.Field'. + // public int Field; // 1 + Diagnostic(ErrorCode.ERR_RequiredMemberCannotBeHidden, "Field").WithArguments("Base.Field", "Derived3.Field").WithLocation(14, 16), + // (15,16): error CS9502: Required member 'Base.Prop' cannot be hidden by 'Derived3.Prop'. + // public int Prop { get; set; } // 2 + Diagnostic(ErrorCode.ERR_RequiredMemberCannotBeHidden, "Prop").WithArguments("Base.Prop", "Derived3.Prop").WithLocation(15, 16) + ); } [Fact]