Skip to content

Commit

Permalink
Add support for emitting RequiredMembersAttribute in source
Browse files Browse the repository at this point in the history
Adding `required` to a member now results in the type having a `RequiredMembersAttribute` emitted with the name of that member as the contents. Reading this data from metadata is not yet supported, nor is adding the requisite `ObsoleteAttribute` to constructors that depend on such contracts. The rules for when required is allowed and when it is disallowed are documented in dotnet/csharplang#5566, pending LDM review.

Test plan: dotnet#57046
  • Loading branch information
333fred committed Jan 20, 2022
1 parent 7238484 commit e69a49e
Show file tree
Hide file tree
Showing 45 changed files with 1,267 additions and 29 deletions.
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6947,4 +6947,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeatureRequiredMembers" xml:space="preserve">
<value>required members</value>
</data>
<data name="ERR_OverrideMustHaveRequired" xml:space="preserve">
<value>'{0}': cannot remove 'required' from '{1}' when overriding</value>
</data>
<data name="ERR_RequiredMembersCannotBeHidden" xml:space="preserve">
<value>Required member '{0}' cannot be hidden by '{1}'.</value>
</data>
<data name="ERR_RequiredMembersCannotBeLessVisibleThanContainingType" xml:space="preserve">
<value>Required member '{0}' cannot be less visible than the containing type '{1}'.</value>
</data>
<data name="ERR_ExplicitRequiredMembers" xml:space="preserve">
<value>Do not use 'System.Runtime.CompilerSerives.RequiredMembersAttribute'. Use the 'required' keyword on required fields and properties instead.</value>
</data>
</root>
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2033,5 +2033,9 @@ internal enum ErrorCode
// PROTOTYPE(req): Move above the comment and condense before merge

ERR_RequiredNameDisallowed = 9500,
ERR_OverrideMustHaveRequired = 9501,
ERR_RequiredMembersCannotBeHidden = 9502,
ERR_RequiredMembersCannotBeLessVisibleThanContainingType = 9503,
ERR_ExplicitRequiredMembers = 9504,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ public override bool IsExtern
get { return false; }
}

internal override bool IsRequired => false;

internal override ObsoleteAttributeData ObsoleteAttributeData
{
get { return null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
ImmutableArray.Create(
new TypedConstant(manager.System_Diagnostics_DebuggerBrowsableState, TypedConstantKind.Enum, DebuggerBrowsableState.Never))));
}

internal override bool IsRequired => false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ public override bool IsAbstract
get { return false; }
}

internal override bool IsRequired => false;

internal sealed override ObsoleteAttributeData ObsoleteAttributeData
{
get { return null; }
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ErrorPropertySymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public ErrorPropertySymbol(Symbol containingSymbol, TypeSymbol type, string name

public override bool IsExtern { get { return false; } }

internal override bool IsRequired => false;

internal sealed override ObsoleteAttributeData ObsoleteAttributeData { get { return null; } }

public override ImmutableArray<ParameterSymbol> Parameters { get { return ImmutableArray<ParameterSymbol>.Empty; } }
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,11 @@ internal virtual FieldSymbol AsMember(NamedTypeSymbol newOwner)
return newOwner.IsDefinition ? this : new SubstitutedFieldSymbol(newOwner as SubstitutedNamedTypeSymbol, this);
}

/// <summary>
/// Returns true if this field is required to be set in an object initializer on object creation.
/// </summary>
internal abstract bool IsRequired { get; }

#region Use-Site Diagnostics

internal override UseSiteInfo<AssemblySymbol> GetUseSiteInfo()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,5 +587,8 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
{
get { return null; }
}

// PROTOTYPE(req): Implement
internal override bool IsRequired => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,15 @@ public override bool IsStatic
}
}

internal override bool IsRequired
{
get
{
// PROTOTYPE(req): Implement
return false;
}
}

public override ImmutableArray<ParameterSymbol> Parameters
{
get { return _parameters; }
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ public bool IsWriteOnly
}
}

/// <summary>
/// Returns true if this property is required to be set in an object initializer on object creation.
/// </summary>
internal abstract bool IsRequired { get; }

/// <summary>
/// True if the property itself is excluded from code coverage instrumentation.
/// True for source properties marked with <see cref="AttributeDescription.ExcludeFromCodeCoverageAttribute"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ public SignatureOnlyPropertySymbol(

public override bool IsExtern { get { throw ExceptionUtilities.Unreachable; } }

internal override bool IsRequired => throw ExceptionUtilities.Unreachable;

internal override ObsoleteAttributeData ObsoleteAttributeData { get { throw ExceptionUtilities.Unreachable; } }

public override AssemblySymbol ContainingAssembly { get { throw ExceptionUtilities.Unreachable; } }
Expand Down
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Source/ModifierUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,14 @@ internal static CSDiagnosticInfo CheckAccessibility(DeclarationModifiers modifie
}
}

if ((modifiers & DeclarationModifiers.Required) != 0
&& symbol.Kind is SymbolKind.Property or SymbolKind.Field
&& symbol.DeclaredAccessibility < symbol.ContainingType.DeclaredAccessibility)
{
// Required member '{0}' cannot be less visible than the containing type '{1}'.
return new CSDiagnosticInfo(ErrorCode.ERR_RequiredMembersCannotBeLessVisibleThanContainingType, symbol, symbol.ContainingType);
}

return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ internal sealed override bool HasRuntimeSpecialName
return this.Name == WellKnownMemberNames.EnumBackingFieldName;
}
}

internal override bool IsRequired => (Modifiers & DeclarationModifiers.Required) != 0;
}

internal abstract class SourceFieldSymbolWithSyntaxReference : SourceFieldSymbol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,7 @@ protected void AfterMembersChecks(BindingDiagnosticBag diagnostics)
CheckSequentialOnPartialType(diagnostics);
CheckForProtectedInStaticClass(diagnostics);
CheckForUnmatchedOperators(diagnostics);
CheckForRequiredMembers(diagnostics);

var location = Locations[0];
var compilation = DeclaringCompilation;
Expand Down Expand Up @@ -2352,6 +2353,15 @@ private void CheckForEqualityAndGetHashCode(BindingDiagnosticBag diagnostics)
}
}

private void CheckForRequiredMembers(BindingDiagnosticBag diagnostics)
{
if (GetMembersUnordered().Any(SymbolExtensions.IsRequired))
{
// Ensure that an error is reported if the required constructor isn't present.
_ = Binder.GetWellKnownTypeMember(DeclaringCompilation, WellKnownMember.System_Runtime_CompilerServices_RequiredMembersAttribute__ctor, diagnostics, Locations[0]);
}
}

private bool TypeOverridesObjectMethod(string name)
{
foreach (var method in this.GetMembers(name).OfType<MethodSymbol>())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,12 @@ private void CheckNewModifier(Symbol symbol, bool isNew, BindingDiagnosticBag di

AddHidingAbstractDiagnostic(symbol, symbolLocation, hiddenMember, diagnostics, ref unused);

if (hiddenMember.IsRequired())
{
// Required member '{0}' cannot be hidden by '{1}'.
diagnostics.Add(ErrorCode.ERR_RequiredMembersCannotBeHidden, symbolLocation, hiddenMember, symbol);
}

return;
}
}
Expand Down Expand Up @@ -900,6 +906,11 @@ void checkSingleOverriddenMember(Symbol overridingMember, Symbol overriddenMembe
// it is ok to override with no tuple names, for compatibility with C# 6, but otherwise names should match
diagnostics.Add(ErrorCode.ERR_CantChangeTupleNamesOnOverride, overridingMemberLocation, overridingMember, overriddenMember);
}
else if (overriddenMember is PropertySymbol { IsRequired: true } && overridingMember is PropertySymbol { IsRequired: false })
{
// '{0}': cannot remove 'required' from '{1}' when overriding
diagnostics.Add(ErrorCode.ERR_OverrideMustHaveRequired, overridingMemberLocation, overridingMember, overriddenMember);
}
else
{
// As in dev11, we don't compare obsoleteness to the immediately-overridden member,
Expand Down Expand Up @@ -1409,6 +1420,12 @@ private static void CheckNonOverrideMember(
diagnostics.Add(ErrorCode.WRN_NewOrOverrideExpected, hidingMemberLocation, hidingMember, hiddenMember);
diagnosticAdded = true;
}
else if (hiddenMember.IsRequired())
{
// Required member '{0}' cannot be hidden by '{1}'.
diagnostics.Add(ErrorCode.ERR_RequiredMembersCannotBeHidden, hidingMemberLocation, hiddenMember, hidingMember);
diagnosticAdded = true;
}

if (diagnosticAdded)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,16 @@ internal sealed override void DecodeWellKnownAttribute(ref DecodeWellKnownAttrib
diagnostics.Add(ErrorCode.ERR_CantUseRequiredAttribute, arguments.AttributeSyntaxOpt.Name.Location);
}
else if (ReportExplicitUseOfReservedAttributes(in arguments,
ReservedAttributes.DynamicAttribute | ReservedAttributes.IsReadOnlyAttribute | ReservedAttributes.IsUnmanagedAttribute | ReservedAttributes.IsByRefLikeAttribute | ReservedAttributes.TupleElementNamesAttribute | ReservedAttributes.NullableAttribute | ReservedAttributes.NullableContextAttribute | ReservedAttributes.NativeIntegerAttribute | ReservedAttributes.CaseSensitiveExtensionAttribute))
ReservedAttributes.DynamicAttribute
| ReservedAttributes.IsReadOnlyAttribute
| ReservedAttributes.IsUnmanagedAttribute
| ReservedAttributes.IsByRefLikeAttribute
| ReservedAttributes.TupleElementNamesAttribute
| ReservedAttributes.NullableAttribute
| ReservedAttributes.NullableContextAttribute
| ReservedAttributes.NativeIntegerAttribute
| ReservedAttributes.CaseSensitiveExtensionAttribute
| ReservedAttributes.RequiredMemberAttribute))
{
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.SecurityCriticalAttribute)
Expand Down Expand Up @@ -1583,6 +1592,39 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
AddSynthesizedAttribute(ref attributes,
this.DeclaringCompilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor));
}

var requiredMembers = ArrayBuilder<Symbol>.GetInstance();
foreach (var member in GetMembers())
{
switch (member)
{
case SourceFieldSymbol { IsRequired: true }:
case SourcePropertySymbol { IsRequired: true, IsOverride: false }:
case SourcePropertySymbol { IsRequired: true, IsOverride: true, OverriddenProperty.IsRequired: false }:
requiredMembers.Add(member);
break;
}
}

if (requiredMembers.Any())
{
var stringType = compilation.GetSpecialType(SpecialType.System_String);
// Because GetMembers() is already sorted in lexical order, we don't need to do
// any additional sorting here.
var nameConstants = requiredMembers.SelectAsArray(
static (member, stringType) => new TypedConstant(stringType, TypedConstantKind.Primitive, member.Name),
stringType);
var stringArrayType = ArrayTypeSymbol.CreateSZArray(stringType.ContainingAssembly, TypeWithAnnotations.Create(stringType));

AddSynthesizedAttribute(
ref attributes,
compilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_RequiredMembersAttribute__ctor,
ImmutableArray.Create(new TypedConstant(stringArrayType, nameConstants))));

// PROTOTYPE(req): Add obsolete marker to constructors if required members and Obsolete hasn't already been emitted
}

requiredMembers.Free();
}

#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ protected SourcePropertySymbolBase(
{
Debug.Assert(!isExpressionBodied || !isAutoProperty);
Debug.Assert(!isExpressionBodied || !hasInitializer);
Debug.Assert((modifiers & DeclarationModifiers.Required) == 0 || this is SourcePropertySymbol);

_syntaxRef = syntax.GetReference();
Location = location;
Expand Down Expand Up @@ -518,6 +519,8 @@ public override bool IsVirtual
get { return (_modifiers & DeclarationModifiers.Virtual) != 0; }
}

internal sealed override bool IsRequired => (_modifiers & DeclarationModifiers.Required) != 0;

internal bool IsNew
{
get { return (_modifiers & DeclarationModifiers.New) != 0; }
Expand Down
7 changes: 7 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Symbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,7 @@ internal enum ReservedAttributes
NullablePublicOnlyAttribute = 1 << 8,
NativeIntegerAttribute = 1 << 9,
CaseSensitiveExtensionAttribute = 1 << 10,
RequiredMemberAttribute = 1 << 11,
}

internal bool ReportExplicitUseOfReservedAttributes(in DecodeWellKnownAttributeArguments<AttributeSyntax, CSharpAttributeData, AttributeLocation> arguments, ReservedAttributes reserved)
Expand Down Expand Up @@ -1421,6 +1422,12 @@ internal bool ReportExplicitUseOfReservedAttributes(in DecodeWellKnownAttributeA
// ExtensionAttribute should not be set explicitly.
diagnostics.Add(ErrorCode.ERR_ExplicitExtension, arguments.AttributeSyntaxOpt.Location);
}
else if ((reserved & ReservedAttributes.RequiredMemberAttribute) != 0 &&
attribute.IsTargetAttribute(this, AttributeDescription.RequiredMembersAttribute))
{
// Do not use 'System.Runtime.CompilerSerives.RequiredMembersAttribute'. Use the 'required' keyword on required fields and properties instead.
diagnostics.Add(ErrorCode.ERR_ExplicitRequiredMembers, arguments.AttributeSyntaxOpt.Location);
}
else
{
return false;
Expand Down
7 changes: 7 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -839,5 +839,12 @@ internal static bool HasAsyncMethodBuilderAttribute(this Symbol symbol, [NotNull
builderArgument = null;
return false;
}

internal static bool IsRequired(this Symbol symbol)
=> symbol switch
{
FieldSymbol { IsRequired: true } or PropertySymbol { IsRequired: true } => true,
_ => false
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,5 +151,7 @@ private void CheckForFieldTargetedAttribute(BindingDiagnosticBag diagnostics)
}
}
}

internal override bool IsRequired => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -192,5 +192,7 @@ public override bool IsImplicitlyDeclared
{
get { return true; }
}

internal override bool IsRequired => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -204,5 +204,7 @@ public override bool IsStatic
return _underlyingField.IsStatic;
}
}

internal override bool IsRequired => _underlyingField.IsRequired;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ public override bool IsExtern
}
}

internal override bool IsRequired => _underlyingProperty.IsRequired;

internal override ObsoleteAttributeData ObsoleteAttributeData
{
get
Expand Down
20 changes: 20 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit e69a49e

Please sign in to comment.