Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable a default implementation of an interface event to be provided as part of its declaration. #18183

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/features/DefaultInterfaceImplementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,7 @@ class Test1 : I1

- Supplying an implementation along with declaration of a property or an indexer and recognizing that implementation as default implementation for them when a type implements the interface.

- Supplying an implementation along with declaration of an event and recognizing that implementation as default implementation for the event when a type implements the interface.


**Open issues and work items** are tracked in https://github.com/dotnet/roslyn/issues/17952.
9 changes: 0 additions & 9 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

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

3 changes: 0 additions & 3 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,6 @@
<data name="ERR_InterfaceEventInitializer" xml:space="preserve">
<value>'{0}': event in interface cannot have initializer</value>
</data>
<data name="ERR_EventPropertyInInterface" xml:space="preserve">
<value>An event in an interface cannot have add or remove accessors</value>
</data>
<data name="ERR_BadEventUsage" xml:space="preserve">
<value>The event '{0}' can only appear on the left hand side of += or -= (except when used from within the type '{1}')</value>
</data>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ internal enum ErrorCode
ERR_EventNotDelegate = 66,
WRN_UnreferencedEvent = 67,
ERR_InterfaceEventInitializer = 68,
ERR_EventPropertyInInterface = 69,
//ERR_EventPropertyInInterface = 69,
ERR_BadEventUsage = 70,
ERR_ExplicitEventFieldImpl = 71,
ERR_CantOverrideNonEvent = 72,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,26 +62,28 @@ internal SourceCustomEventAccessorSymbol(
isExtensionMethod: false,
isMetadataVirtualIgnoringModifiers: explicitInterfaceImplementations.Any());

if (@event.ContainingType.IsInterface)
var bodyOpt = syntax.Body;
if (bodyOpt != null)
{
diagnostics.Add(ErrorCode.ERR_EventPropertyInInterface, this.Location);
}
else
{
var bodyOpt = syntax.Body;
if (bodyOpt != null)
if (IsExtern && !IsAbstract)
{
if (IsExtern && !IsAbstract)
{
diagnostics.Add(ErrorCode.ERR_ExternHasBody, this.Location, this);
}
else if (IsAbstract && !IsExtern)
diagnostics.Add(ErrorCode.ERR_ExternHasBody, this.Location, this);
}
else if (IsAbstract && !IsExtern)
{
diagnostics.Add(ErrorCode.ERR_AbstractHasBody, this.Location, this);
}
else if (@event.ContainingType.IsInterface)
{
Binder.CheckFeatureAvailability(syntax, MessageID.IDS_DefaultInterfaceImplementation, diagnostics, this.Location);

if (!ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
{
diagnostics.Add(ErrorCode.ERR_AbstractHasBody, this.Location, this);
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, this.Location);
}
// Do not report error for IsAbstract && IsExtern. Dev10 reports CS0180 only
// in that case ("member cannot be both extern and abstract").
}
// Do not report error for IsAbstract && IsExtern. Dev10 reports CS0180 only
// in that case ("member cannot be both extern and abstract").
}

_name = GetOverriddenAccessorName(@event, isAdder) ?? _name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ internal sealed class SourceCustomEventSymbol : SourceEventSymbol
private readonly ImmutableArray<EventSymbol> _explicitInterfaceImplementations;

internal SourceCustomEventSymbol(SourceMemberContainerTypeSymbol containingType, Binder binder, EventDeclarationSyntax syntax, DiagnosticBag diagnostics) :
base(containingType, syntax, syntax.Modifiers, syntax.ExplicitInterfaceSpecifier, syntax.Identifier, diagnostics)
base(containingType, syntax, syntax.Modifiers, isFieldLike:false,
interfaceSpecifierSyntaxOpt:syntax.ExplicitInterfaceSpecifier,
nameTokenSyntax:syntax.Identifier, diagnostics:diagnostics)
{
ExplicitInterfaceSpecifierSyntax interfaceSpecifier = syntax.ExplicitInterfaceSpecifier;
SyntaxToken nameToken = syntax.Identifier;
Expand Down Expand Up @@ -113,21 +115,9 @@ internal SourceCustomEventSymbol(SourceMemberContainerTypeSymbol containingType,
_addMethod = CreateAccessorSymbol(addSyntax, explicitlyImplementedEvent, aliasQualifierOpt, diagnostics);
_removeMethod = CreateAccessorSymbol(removeSyntax, explicitlyImplementedEvent, aliasQualifierOpt, diagnostics);

if (containingType.IsInterfaceType())
if (addSyntax == null || removeSyntax == null)
{
if (addSyntax == null && removeSyntax == null) //NOTE: AND - different error code produced if one is present
{
// CONSIDER: we're matching dev10, but it would probably be more helpful to give
// an error like ERR_EventPropertyInInterface.
diagnostics.Add(ErrorCode.ERR_EventNeedsBothAccessors, this.Locations[0], this);
}
}
else
{
if (addSyntax == null || removeSyntax == null)
{
diagnostics.Add(ErrorCode.ERR_EventNeedsBothAccessors, this.Locations[0], this);
}
diagnostics.Add(ErrorCode.ERR_EventNeedsBothAccessors, this.Locations[0], this);
}

_explicitInterfaceImplementations =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ internal SourceEventSymbol(
SourceMemberContainerTypeSymbol containingType,
CSharpSyntaxNode syntax,
SyntaxTokenList modifiers,
bool isFieldLike,
ExplicitInterfaceSpecifierSyntax interfaceSpecifierSyntaxOpt,
SyntaxToken nameTokenSyntax,
DiagnosticBag diagnostics)
Expand All @@ -49,7 +50,7 @@ internal SourceEventSymbol(

var isExplicitInterfaceImplementation = interfaceSpecifierSyntaxOpt != null;
bool modifierErrors;
_modifiers = MakeModifiers(modifiers, isExplicitInterfaceImplementation, _location, diagnostics, out modifierErrors);
_modifiers = MakeModifiers(modifiers, isExplicitInterfaceImplementation, isFieldLike, _location, diagnostics, out modifierErrors);
this.CheckAccessibility(_location, diagnostics);
}

Expand Down Expand Up @@ -398,7 +399,9 @@ private void CheckAccessibility(Location location, DiagnosticBag diagnostics)
}
}

private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool explicitInterfaceImplementation, Location location, DiagnosticBag diagnostics, out bool modifierErrors)
private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool explicitInterfaceImplementation,
bool isFieldLike, Location location,
DiagnosticBag diagnostics, out bool modifierErrors)
{
bool isInterface = this.ContainingType.IsInterface;
var defaultAccess = isInterface ? DeclarationModifiers.Public : DeclarationModifiers.Private;
Expand Down Expand Up @@ -434,7 +437,8 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool expli
// Proper errors must have been reported by now.
if (isInterface)
{
mods = (mods & ~DeclarationModifiers.AccessibilityMask) | DeclarationModifiers.Abstract | DeclarationModifiers.Public;
mods = (mods & ~DeclarationModifiers.AccessibilityMask) | DeclarationModifiers.Public |
(isFieldLike ? DeclarationModifiers.Abstract : DeclarationModifiers.Virtual);
}

return mods;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ internal sealed class SourceFieldLikeEventSymbol : SourceEventSymbol
private readonly SynthesizedFieldLikeEventAccessorSymbol _removeMethod;

internal SourceFieldLikeEventSymbol(SourceMemberContainerTypeSymbol containingType, Binder binder, SyntaxTokenList modifiers, VariableDeclaratorSyntax declaratorSyntax, DiagnosticBag diagnostics)
: base(containingType, declaratorSyntax, modifiers, null, declaratorSyntax.Identifier, diagnostics)
: base(containingType, declaratorSyntax, modifiers, isFieldLike:true, interfaceSpecifierSyntaxOpt:null,
nameTokenSyntax:declaratorSyntax.Identifier, diagnostics:diagnostics)
{
_name = declaratorSyntax.Identifier.ValueText;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private SourcePropertySymbol(
bool hasExpressionBody = arrowExpression != null;
bool hasInitializer = !isIndexer && propertySyntax.Initializer != null;

GetAcessorDeclarations(syntax, diagnostics, out bool notRegularProperty, out bool hasAccessorList,
GetAcessorDeclarations(syntax, diagnostics, out bool isAutoProperty, out bool hasAccessorList,
out AccessorDeclarationSyntax getSyntax, out AccessorDeclarationSyntax setSyntax);

bool accessorsHaveImplementation;
Expand All @@ -101,7 +101,7 @@ private SourcePropertySymbol(

this.CheckModifiers(location, isIndexer, diagnostics);

notRegularProperty = notRegularProperty && (!containingType.IsInterface && !IsAbstract && !IsExtern && !isIndexer && hasAccessorList);
isAutoProperty = isAutoProperty && (!containingType.IsInterface && !IsAbstract && !IsExtern && !isIndexer && hasAccessorList);

if (isIndexer && !isExplicitInterfaceImplementation)
{
Expand Down Expand Up @@ -134,13 +134,13 @@ private SourcePropertySymbol(

if (hasInitializer)
{
CheckInitializer(notRegularProperty, location, diagnostics);
CheckInitializer(isAutoProperty, location, diagnostics);
}

if (notRegularProperty || hasInitializer)
if (isAutoProperty || hasInitializer)
{
var hasGetSyntax = getSyntax != null;
_isAutoProperty = notRegularProperty && hasGetSyntax;
_isAutoProperty = isAutoProperty && hasGetSyntax;
bool isReadOnly = hasGetSyntax && setSyntax == null;

if (_isAutoProperty || hasInitializer)
Expand All @@ -165,7 +165,7 @@ private SourcePropertySymbol(
hasInitializer);
}

if (notRegularProperty)
if (isAutoProperty)
{
Binder.CheckFeatureAvailability(
syntax,
Expand Down Expand Up @@ -261,8 +261,8 @@ private SourcePropertySymbol(
}
else
{
_getMethod = CreateAccessorSymbol(getSyntax, explicitlyImplementedProperty, aliasQualifierOpt, notRegularProperty, diagnostics);
_setMethod = CreateAccessorSymbol(setSyntax, explicitlyImplementedProperty, aliasQualifierOpt, notRegularProperty, diagnostics);
_getMethod = CreateAccessorSymbol(getSyntax, explicitlyImplementedProperty, aliasQualifierOpt, isAutoProperty, diagnostics);
_setMethod = CreateAccessorSymbol(setSyntax, explicitlyImplementedProperty, aliasQualifierOpt, isAutoProperty, diagnostics);

if ((getSyntax == null) || (setSyntax == null))
{
Expand All @@ -277,7 +277,7 @@ private SourcePropertySymbol(
diagnostics.Add(ErrorCode.ERR_RefPropertyMustHaveGetAccessor, location, this);
}
}
else if (notRegularProperty)
else if (isAutoProperty)
{
var accessor = _getMethod ?? _setMethod;
if (getSyntax == null)
Expand Down Expand Up @@ -349,10 +349,10 @@ private SourcePropertySymbol(
}

private static void GetAcessorDeclarations(BasePropertyDeclarationSyntax syntax, DiagnosticBag diagnostics,
out bool notRegularProperty, out bool hasAccessorList,
out bool isAutoProperty, out bool hasAccessorList,
out AccessorDeclarationSyntax getSyntax, out AccessorDeclarationSyntax setSyntax)
{
notRegularProperty = true;
isAutoProperty = true;
hasAccessorList = syntax.AccessorList != null;
getSyntax = null;
setSyntax = null;
Expand Down Expand Up @@ -397,13 +397,13 @@ private static void GetAcessorDeclarations(BasePropertyDeclarationSyntax syntax,

if (accessor.Body != null || accessor.ExpressionBody != null)
{
notRegularProperty = false;
isAutoProperty = false;
}
}
}
else
{
notRegularProperty = false;
isAutoProperty = false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2006,40 +2006,30 @@ public void CS0069ERR_EventPropertyInInterface()
var text = @"
interface I
{
event System.Action E1 { add; } // CS0069 on add
event System.Action E2 { remove; } // CS0069 on remove
event System.Action E3 { add; remove; } // CS0069 on both (i.e. x 2)
event System.Action E1 { add; }
event System.Action E2 { remove; }
event System.Action E3 { add; remove; }
}
";
CreateCompilationWithMscorlib(text).VerifyDiagnostics(
// (4,30): error CS0069: An event in an interface cannot have add or remove accessors
// event System.Action E1 { add; } // CS0069 on add
Diagnostic(ErrorCode.ERR_EventPropertyInInterface, "add"),
// (5,30): error CS0069: An event in an interface cannot have add or remove accessors
// event System.Action E2 { remove; } // CS0069 on remove
Diagnostic(ErrorCode.ERR_EventPropertyInInterface, "remove"),
// (6,30): error CS0069: An event in an interface cannot have add or remove accessors
// event System.Action E3 { add; remove; } // CS0069 on both (i.e. x 2)
Diagnostic(ErrorCode.ERR_EventPropertyInInterface, "add"),
// (6,35): error CS0069: An event in an interface cannot have add or remove accessors
// event System.Action E3 { add; remove; } // CS0069 on both (i.e. x 2)
Diagnostic(ErrorCode.ERR_EventPropertyInInterface, "remove"),

// CONSIDER: dev10 doesn't report these, but we report them in the parser so they're
// hard to suppress.

// (4,33): error CS0073: An add or remove accessor must have a body
// event System.Action E1 { add; } // CS0069 on add
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";"),
// event System.Action E1 { add; }
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";").WithLocation(4, 33),
// (5,36): error CS0073: An add or remove accessor must have a body
// event System.Action E2 { remove; } // CS0069 on remove
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";"),
// event System.Action E2 { remove; }
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";").WithLocation(5, 36),
// (6,33): error CS0073: An add or remove accessor must have a body
// event System.Action E3 { add; remove; } // CS0069 on both (i.e. x 2)
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";"),
// event System.Action E3 { add; remove; }
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";").WithLocation(6, 33),
// (6,41): error CS0073: An add or remove accessor must have a body
// event System.Action E3 { add; remove; } // CS0069 on both (i.e. x 2)
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";"));
// event System.Action E3 { add; remove; }
Diagnostic(ErrorCode.ERR_AddRemoveMustHaveBody, ";").WithLocation(6, 41),
// (4,25): error CS0065: 'I.E1': event property must have both add and remove accessors
// event System.Action E1 { add; }
Diagnostic(ErrorCode.ERR_EventNeedsBothAccessors, "E1").WithArguments("I.E1").WithLocation(4, 25),
// (5,25): error CS0065: 'I.E2': event property must have both add and remove accessors
// event System.Action E2 { remove; }
Diagnostic(ErrorCode.ERR_EventNeedsBothAccessors, "E2").WithArguments("I.E2").WithLocation(5, 25));
}

[Fact]
Expand Down
Loading