Skip to content

Commit

Permalink
Enable a default implementation of an interface property/indexer to b…
Browse files Browse the repository at this point in the history
…e provided as part of its declaration. (#18110)
  • Loading branch information
AlekseyTs authored Mar 24, 2017
1 parent 50210ff commit deba3ea
Show file tree
Hide file tree
Showing 11 changed files with 2,951 additions and 265 deletions.
2 changes: 2 additions & 0 deletions docs/features/DefaultInterfaceImplementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,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.


**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 @@ -1484,9 +1484,6 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_CycleInInterfaceInheritance" xml:space="preserve">
<value>Inherited interface '{1}' causes a cycle in the interface hierarchy of '{0}'</value>
</data>
<data name="ERR_InterfaceMemberHasBody" xml:space="preserve">
<value>'{0}': interface members cannot have a definition</value>
</data>
<data name="ERR_HidingAbstractMethod" xml:space="preserve">
<value>'{0}' hides inherited abstract member '{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 @@ -351,7 +351,7 @@ internal enum ErrorCode
ERR_NonInterfaceInInterfaceList = 527,
ERR_DuplicateInterfaceInBaseList = 528,
ERR_CycleInInterfaceInheritance = 529,
ERR_InterfaceMemberHasBody = 531,
//ERR_InterfaceMemberHasBody = 531,
ERR_HidingAbstractMethod = 533,
ERR_UnimplementedAbstractMethod = 534,
ERR_UnimplementedInterfaceMember = 535,
Expand Down
13 changes: 3 additions & 10 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1562,18 +1562,11 @@ protected void CheckModifiersForBody(SyntaxNode declarationSyntax, Location loca
{
if (_containingType.IsInterface)
{
if (!this.IsAccessor())
{
Binder.CheckFeatureAvailability(declarationSyntax, MessageID.IDS_DefaultInterfaceImplementation, diagnostics, location);
Binder.CheckFeatureAvailability(declarationSyntax, MessageID.IDS_DefaultInterfaceImplementation, diagnostics, location);

if (!ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
{
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, location);
}
}
else
if (!ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
{
diagnostics.Add(ErrorCode.ERR_InterfaceMemberHasBody, location, this);
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, location);
}
}
else if (IsExtern && !IsAbstract)
Expand Down
153 changes: 90 additions & 63 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,37 @@ private SourcePropertySymbol(
bodyBinder = bodyBinder.WithUnsafeRegionIfNecessary(modifiers);
bodyBinder = bodyBinder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks, this);

var propertySyntax = syntax as PropertyDeclarationSyntax;
var arrowExpression = propertySyntax != null
? propertySyntax.ExpressionBody
: ((IndexerDeclarationSyntax)syntax).ExpressionBody;
bool hasExpressionBody = arrowExpression != null;
bool hasInitializer = !isIndexer && propertySyntax.Initializer != null;

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

bool accessorsHaveImplementation;
if (hasAccessorList)
{
accessorsHaveImplementation = (getSyntax != null && (getSyntax.Body != null || getSyntax.ExpressionBody != null)) ||
(setSyntax != null && (setSyntax.Body != null || setSyntax.ExpressionBody != null));
}
else
{
accessorsHaveImplementation = hasExpressionBody;
}

bool modifierErrors;
_modifiers = MakeModifiers(modifiers, isExplicitInterfaceImplementation, isIndexer, location, diagnostics, out modifierErrors);
_modifiers = MakeModifiers(modifiers, isExplicitInterfaceImplementation, isIndexer,
accessorsHaveImplementation, location,
diagnostics, out modifierErrors);
this.CheckAccessibility(location, diagnostics);

this.CheckModifiers(location, isIndexer, diagnostics);

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

if (isIndexer && !isExplicitInterfaceImplementation)
{
// Evaluate the attributes immediately in case the IndexerNameAttribute has been applied.
Expand Down Expand Up @@ -107,66 +132,6 @@ private SourcePropertySymbol(
_name = isIndexer ? ExplicitInterfaceHelpers.GetMemberName(WellKnownMemberNames.Indexer, _explicitInterfaceType, aliasQualifierOpt) : _sourceName;
_isExpressionBodied = false;

bool hasAccessorList = syntax.AccessorList != null;
var propertySyntax = syntax as PropertyDeclarationSyntax;
var arrowExpression = propertySyntax != null
? propertySyntax.ExpressionBody
: ((IndexerDeclarationSyntax)syntax).ExpressionBody;
bool hasExpressionBody = arrowExpression != null;
bool hasInitializer = !isIndexer && propertySyntax.Initializer != null;

bool notRegularProperty = (!IsAbstract && !IsExtern && !isIndexer && hasAccessorList);
AccessorDeclarationSyntax getSyntax = null;
AccessorDeclarationSyntax setSyntax = null;
if (hasAccessorList)
{
foreach (var accessor in syntax.AccessorList.Accessors)
{
switch (accessor.Kind())
{
case SyntaxKind.GetAccessorDeclaration:
if (getSyntax == null)
{
getSyntax = accessor;
}
else
{
diagnostics.Add(ErrorCode.ERR_DuplicateAccessor, accessor.Keyword.GetLocation());
}
break;
case SyntaxKind.SetAccessorDeclaration:
if (setSyntax == null)
{
setSyntax = accessor;
}
else
{
diagnostics.Add(ErrorCode.ERR_DuplicateAccessor, accessor.Keyword.GetLocation());
}
break;
case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
diagnostics.Add(ErrorCode.ERR_GetOrSetExpected, accessor.Keyword.GetLocation());
continue;
case SyntaxKind.UnknownAccessorDeclaration:
// We don't need to report an error here as the parser will already have
// done that for us.
continue;
default:
throw ExceptionUtilities.UnexpectedValue(accessor.Kind());
}

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

if (hasInitializer)
{
CheckInitializer(notRegularProperty, location, diagnostics);
Expand Down Expand Up @@ -383,6 +348,65 @@ private SourcePropertySymbol(
syntax.AccessorList, syntax.GetExpressionBodySyntax(), syntax, diagnostics);
}

private static void GetAcessorDeclarations(BasePropertyDeclarationSyntax syntax, DiagnosticBag diagnostics,
out bool notRegularProperty, out bool hasAccessorList,
out AccessorDeclarationSyntax getSyntax, out AccessorDeclarationSyntax setSyntax)
{
notRegularProperty = true;
hasAccessorList = syntax.AccessorList != null;
getSyntax = null;
setSyntax = null;

if (hasAccessorList)
{
foreach (var accessor in syntax.AccessorList.Accessors)
{
switch (accessor.Kind())
{
case SyntaxKind.GetAccessorDeclaration:
if (getSyntax == null)
{
getSyntax = accessor;
}
else
{
diagnostics.Add(ErrorCode.ERR_DuplicateAccessor, accessor.Keyword.GetLocation());
}
break;
case SyntaxKind.SetAccessorDeclaration:
if (setSyntax == null)
{
setSyntax = accessor;
}
else
{
diagnostics.Add(ErrorCode.ERR_DuplicateAccessor, accessor.Keyword.GetLocation());
}
break;
case SyntaxKind.AddAccessorDeclaration:
case SyntaxKind.RemoveAccessorDeclaration:
diagnostics.Add(ErrorCode.ERR_GetOrSetExpected, accessor.Keyword.GetLocation());
continue;
case SyntaxKind.UnknownAccessorDeclaration:
// We don't need to report an error here as the parser will already have
// done that for us.
continue;
default:
throw ExceptionUtilities.UnexpectedValue(accessor.Kind());
}

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

internal bool IsExpressionBodied
{
get
Expand Down Expand Up @@ -714,7 +738,9 @@ private void CheckAccessibility(Location location, DiagnosticBag diagnostics)
}
}

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

if (isIndexer)
Expand Down
36 changes: 18 additions & 18 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,24 @@ private static void ReportImplicitImplementationMatchDiagnostics(Symbol interfac
ReportAnyMismatchedConstraints(interfaceMethod, implementingType, implicitImplMethod, diagnostics);
}
}

if (implicitImpl.ContainingType.IsInterface && implementingType.ContainingModule != implicitImpl.ContainingModule)
{
// PROTOTYPE(DefaultInterfaceImplementation): Should we check language version as well?
// Usually, it is done based on specific syntax that targets a new feature, but in this case
// no special syntax is used. Also, partial types can have declarations coming from different
// trees with different options. We could use options from the tree the result of
// GetInterfaceLocation() is based on.

// The default implementation is coming from a different module, which means that we probably won't check
// for the required runtime capability
if (!implementingType.ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
{
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementationForMember,
GetInterfaceLocation(interfaceMember, implementingType),
implicitImpl, interfaceMember, implementingType);
}
}
}

if (MemberSignatureComparer.ConsideringTupleNamesCreatesDifference(implicitImpl, interfaceMember))
Expand All @@ -1115,24 +1133,6 @@ private static void ReportImplicitImplementationMatchDiagnostics(Symbol interfac
}
}
}

if (implicitImpl.ContainingType.IsInterface && implementingType.ContainingModule != implicitImpl.ContainingModule)
{
// PROTOTYPE(DefaultInterfaceImplementation): Should we check language version as well?
// Usually, it is done based on specific syntax that targets a new feature, but in this case
// no special syntax is used. Also, partial types can have declarations coming from different
// trees with different options. We could use options from the tree the result of
// GetInterfaceLocation() is based on.

// The default implementation is coming from a different module, which means that we probably won't check
// for the required runtime capability
if (!implementingType.ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
{
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementationForMember,
GetInterfaceLocation(interfaceMember, implementingType),
implicitImpl, interfaceMember, implementingType);
}
}
}

/// <summary>
Expand Down
Loading

0 comments on commit deba3ea

Please sign in to comment.