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.

Sign up for GitHub

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 property/indexer to be provided as part of its declaration. #18110

Merged
merged 1 commit into from
Mar 24, 2017
Merged

Enable a default implementation of an interface property/indexer to be provided as part of its declaration. #18110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
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
View file
Open in desktop

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct if one accessor is abstract and one virtual?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see that is left as a future work item.

}

if (isIndexer)
Expand Down
36 changes: 18 additions & 18 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
View file
Open in desktop
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use CSharpCompilation.LanguageVersion. Partial types cannot have declarations coming from different trees with different language versions. The compiler checks up front that all input trees have the same language version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still there is no special syntax at this site.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree that there is no special syntax at this site. But there is a new language feature being used. In any case I agree with your decision to document it as an open issue with a PROTOTYPE comment.

// 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