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

Support implementing protected members in implement interface #76178

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
Original file line number Diff line number Diff line change
Expand Up @@ -8612,12 +8612,17 @@ interface IInterface
protected int P1 {get;}
}

class Class : {|CS0535:{|CS0535:IInterface|}|}
class Class : {|CS0535:IInterface|}
{
public void Method1()
{
throw new System.NotImplementedException();
}

void IInterface.M1()
{
throw new System.NotImplementedException();
}
}
""",
},
Expand Down Expand Up @@ -8715,9 +8720,14 @@ interface IInterface
protected int P1 {get;}
}

abstract class Class : {|CS0535:{|CS0535:IInterface|}|}
abstract class Class : {|CS0535:IInterface|}
{
public abstract void Method1();

void IInterface.M1()
{
throw new System.NotImplementedException();
}
}
""",
},
Expand Down Expand Up @@ -12130,6 +12140,96 @@ class Class : IEnumerator<int>
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72380")]
public async Task TestImplementProtectedAbstract_CSharp9()
{
await new VerifyCS.Test
{
TestCode = """
using System;

public interface I
{
public abstract void Another();
protected abstract void Method();
}

public class CI : {|CS0535:{|CS0535:I|}|}
{
}
""",
FixedCode = """
using System;

public interface I
{
public abstract void Another();
protected abstract void Method();
}

public class CI : I
{
public void Another()
{
throw new NotImplementedException();
}

void I.Method()
{
throw new NotImplementedException();
}
}
""",
ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
LanguageVersion = LanguageVersion.CSharp9,
}.RunAsync();
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72380")]
public async Task TestImplementProtectedAbstract_CSharp10()
{
await new VerifyCS.Test
{
TestCode = """
using System;

public interface I
{
public abstract void Another();
protected abstract void Method();
}

public class CI : {|CS0535:{|CS0535:I|}|}
{
}
""",
FixedCode = """
using System;

public interface I
{
public abstract void Another();
protected abstract void Method();
}

public class CI : I
{
public void Another()
{
throw new NotImplementedException();
}

public void Method()
{
throw new NotImplementedException();
}
}
""",
ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
LanguageVersion = LanguageVersion.CSharp10,
}.RunAsync();
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/19721")]
public async Task TestMatchPropertyAgainstPropertyWithMoreAccessors1()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.ImplementType;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;

Expand Down Expand Up @@ -125,7 +126,8 @@ private static async IAsyncEnumerable<ImplementInterfaceConfiguration> GetImplem
Document document, ImplementInterfaceInfo state, [EnumeratorCancellation] CancellationToken cancellationToken)
{
var compilation = await document.Project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false);

var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
var supportsImplicitImplementationOfNonPublicInterfaceMembers = syntaxFacts.SupportsImplicitImplementationOfNonPublicInterfaceMembers(document.Project.ParseOptions!);
if (state.MembersWithoutExplicitOrImplicitImplementationWhichCanBeImplicitlyImplemented.Length > 0)
{
var totalMemberCount = 0;
Expand All @@ -137,7 +139,7 @@ private static async IAsyncEnumerable<ImplementInterfaceConfiguration> GetImplem
{
totalMemberCount++;

if (IsLessAccessibleThan(member, state.ClassOrStructType))
if (ContainsTypeLessAccessibleThan(member, state.ClassOrStructType, supportsImplicitImplementationOfNonPublicInterfaceMembers))
inaccessibleMemberCount++;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ public ImmutableArray<ISymbol> ImplementInterfaceMember(
this, document, info, options, configuration);

var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();

var supportsImplementingLessAccessibleMember = syntaxFacts.SupportsImplicitImplementationOfNonPublicInterfaceMembers(document.Project.ParseOptions!);
var implementedMembers = generator.GenerateMembers(
compilation,
interfaceMember,
conflictingMember: null,
memberName: interfaceMember.Name,
generateInvisibly: generator.ShouldGenerateInvisibleMember(document.Project.ParseOptions!, interfaceMember, interfaceMember.Name),
generateInvisibly: generator.ShouldGenerateInvisibleMember(document.Project.ParseOptions!, interfaceMember, interfaceMember.Name, supportsImplementingLessAccessibleMember),
generateAbstractly: configuration.Abstractly,
addNew: false,
interfaceMember.RequiresUnsafeModifier() && !syntaxFacts.IsUnsafeContext(info.ContextNode),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ bool InitializerReferencesParameter(SyntaxNode? initializer, IParameterSymbol pa
}
}

public static bool IsLessAccessibleThan(ISymbol? first, INamedTypeSymbol second)
public static bool ContainsTypeLessAccessibleThan(
ISymbol? first, INamedTypeSymbol second, bool supportsImplicitImplementationOfNonPublicInterfaceMembers)
{
if (first is null)
return false;
Expand All @@ -127,7 +128,7 @@ public static bool IsLessAccessibleThan(ISymbol? first, INamedTypeSymbol second)
return false;
}

if (first.DeclaredAccessibility < second.DeclaredAccessibility)
if (!supportsImplicitImplementationOfNonPublicInterfaceMembers && first.DeclaredAccessibility < second.DeclaredAccessibility)
return true;

switch (first)
Expand All @@ -136,10 +137,10 @@ public static bool IsLessAccessibleThan(ISymbol? first, INamedTypeSymbol second)
if (IsTypeLessAccessibleThanOtherType(propertySymbol.Type, second, []))
return true;

if (IsLessAccessibleThan(propertySymbol.GetMethod, second))
if (ContainsTypeLessAccessibleThan(propertySymbol.GetMethod, second, supportsImplicitImplementationOfNonPublicInterfaceMembers))
return true;

if (IsLessAccessibleThan(propertySymbol.SetMethod, second))
if (ContainsTypeLessAccessibleThan(propertySymbol.SetMethod, second, supportsImplicitImplementationOfNonPublicInterfaceMembers))
return true;

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,10 @@ private string DetermineMemberName(ISymbol member, ArrayBuilder<ISymbol> impleme

// See if we need to generate an invisible member. If we do, then reset the name
// back to what then member wants it to be.
var generateInvisibleMember = ShouldGenerateInvisibleMember(options, member, memberName);
var supportsImplicitImplementationOfNonPublicInterfaceMembers = this.Document
.GetRequiredLanguageService<ISyntaxFactsService>()
.SupportsImplicitImplementationOfNonPublicInterfaceMembers(options);
var generateInvisibleMember = ShouldGenerateInvisibleMember(options, member, memberName, supportsImplicitImplementationOfNonPublicInterfaceMembers);
memberName = generateInvisibleMember ? member.Name : memberName;

// The language doesn't allow static abstract implementations of interface methods. i.e,
Expand All @@ -222,7 +225,8 @@ private string DetermineMemberName(ISymbol member, ArrayBuilder<ISymbol> impleme
addNew, addUnsafe, propertyGenerationBehavior);
}

public bool ShouldGenerateInvisibleMember(ParseOptions options, ISymbol member, string memberName)
public bool ShouldGenerateInvisibleMember(
ParseOptions options, ISymbol member, string memberName, bool supportsImplementingLessAccessibleMember)
{
if (Service.HasHiddenExplicitImplementation)
{
Expand All @@ -240,9 +244,9 @@ public bool ShouldGenerateInvisibleMember(ParseOptions options, ISymbol member,
if (member.Name != memberName)
return true;

// If the member is less accessible than type, for which we are implementing it,
// then only explicit implementation is valid.
if (IsLessAccessibleThan(member, State.ClassOrStructType))
// If the member contains a type is less accessible than type, for which we are implementing it, then
// only explicit implementation is valid.
if (ContainsTypeLessAccessibleThan(member, State.ClassOrStructType, supportsImplementingLessAccessibleMember))
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ public bool SupportsTupleDeconstruction(ParseOptions options)
public bool SupportsCollectionExpressionNaturalType(ParseOptions options)
=> false;

public bool SupportsImplicitImplementationOfNonPublicInterfaceMembers(ParseOptions options)
=> options.LanguageVersion() >= LanguageVersion.CSharp10;

public SyntaxToken ParseToken(string text)
=> SyntaxFactory.ParseToken(text);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,12 @@ static ImmutableArray<ISymbol> GetImplicitlyImplementableMembers(INamedTypeSymbo
{
if (type.TypeKind == TypeKind.Interface)
{
return type.GetMembers().WhereAsArray(m => m.DeclaredAccessibility == Accessibility.Public &&
m.Kind != SymbolKind.NamedType && IsImplementable(m) &&
!IsPropertyWithNonPublicImplementableAccessor(m) &&
IsImplicitlyImplementable(m, within));
return type.GetMembers().WhereAsArray(
m => m.DeclaredAccessibility is Accessibility.Public or Accessibility.Protected &&
m.Kind != SymbolKind.NamedType &&
IsImplementable(m) &&
!IsPropertyWithNonPublicImplementableAccessor(m) &&
IsImplicitlyImplementable(m, within));
}

return type.GetMembers();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ internal interface ISyntaxFacts
bool SupportsConstantInterpolatedStrings(ParseOptions options);
bool SupportsTupleDeconstruction(ParseOptions options);
bool SupportsCollectionExpressionNaturalType(ParseOptions options);
bool SupportsImplicitImplementationOfNonPublicInterfaceMembers(ParseOptions options);

SyntaxToken ParseToken(string text);
SyntaxTriviaList ParseLeadingTrivia(string text);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
Return False
End Function

Public Function SupportsImplicitImplementationOfNonPublicInterfaceMembers(options As ParseOptions) As Boolean Implements ISyntaxFacts.SupportsImplicitImplementationOfNonPublicInterfaceMembers
Return True
End Function

Public Function ParseToken(text As String) As SyntaxToken Implements ISyntaxFacts.ParseToken
Return SyntaxFactory.ParseToken(text, startStatement:=True)
End Function
Expand Down
Loading