Skip to content

Commit

Permalink
Calculate containing type effective accessibility for required member…
Browse files Browse the repository at this point in the history 
… enforcement (#61643)

* Calculate containing type effective accessibility for required member enforcement

Implements the spec change in dotnet/csharplang#6190. Fixes #61527 and #61528.
  • Loading branch information
@333fred
333fred authored Jun 14, 2022
1 parent 1c13b39 commit e7a1a08
Show file tree
Hide file tree
Showing 11 changed files with 210 additions and 115 deletions.
28 changes: 20 additions & 8 deletions src/Compilers/CSharp/Portable/Symbols/Source/ModifierUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,12 +416,13 @@ private static void ReportDuplicateModifiers(
}
}

internal static CSDiagnosticInfo CheckAccessibility(DeclarationModifiers modifiers, Symbol symbol, bool isExplicitInterfaceImplementation)
internal static bool CheckAccessibility(DeclarationModifiers modifiers, Symbol symbol, bool isExplicitInterfaceImplementation, BindingDiagnosticBag diagnostics, Location errorLocation)
{
if (!IsValidAccessibility(modifiers))
{
// error CS0107: More than one protection modifier
return new CSDiagnosticInfo(ErrorCode.ERR_BadMemberProtection);
diagnostics.Add(ErrorCode.ERR_BadMemberProtection, errorLocation);
return true;
}

if (!isExplicitInterfaceImplementation &&
Expand All @@ -436,28 +437,39 @@ internal static CSDiagnosticInfo CheckAccessibility(DeclarationModifiers modifie

if (symbol.ContainingType?.IsInterface == true && !symbol.ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
{
return new CSDiagnosticInfo(ErrorCode.ERR_RuntimeDoesNotSupportProtectedAccessForInterfaceMember);
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportProtectedAccessForInterfaceMember, errorLocation);
return true;
}
break;
}
}

if ((modifiers & DeclarationModifiers.Required) != 0)
{
var useSiteInfo = new CompoundUseSiteInfo<AssemblySymbol>(futureDestination: diagnostics, assemblyBeingBuilt: symbol.ContainingAssembly);
bool reportedError = false;

switch (symbol)
{
case FieldSymbol or PropertySymbol when symbol.DeclaredAccessibility < symbol.ContainingType.DeclaredAccessibility:
case PropertySymbol { SetMethod.DeclaredAccessibility: var accessibility } when accessibility < symbol.ContainingType.DeclaredAccessibility:
case FieldSymbol when !symbol.IsAsRestrictive(symbol.ContainingType, ref useSiteInfo):
case PropertySymbol { SetMethod: { } method } when !method.IsAsRestrictive(symbol.ContainingType, ref useSiteInfo):
// Required member '{0}' cannot be less visible or have a setter less visible than the containing type '{1}'.
return new CSDiagnosticInfo(ErrorCode.ERR_RequiredMemberCannotBeLessVisibleThanContainingType, symbol, symbol.ContainingType);
diagnostics.Add(ErrorCode.ERR_RequiredMemberCannotBeLessVisibleThanContainingType, errorLocation, symbol, symbol.ContainingType);
reportedError = true;
break;
case PropertySymbol { SetMethod: null }:
case FieldSymbol when (modifiers & DeclarationModifiers.ReadOnly) != 0:
// Required member '{0}' must be settable.
return new CSDiagnosticInfo(ErrorCode.ERR_RequiredMemberMustBeSettable, symbol);
diagnostics.Add(ErrorCode.ERR_RequiredMemberMustBeSettable, errorLocation, symbol);
reportedError = true;
break;
}

diagnostics.Add(errorLocation, useSiteInfo);
return reportedError;
}

return null;
return false;
}

// Returns declared accessibility.
Expand Down
6 changes: 1 addition & 5 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,7 @@ private SourceConstructorSymbol(
CheckFeatureAvailabilityAndRuntimeSupport(syntax, location, hasBody, diagnostics);
}

var info = ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation: false);
if (info != null)
{
diagnostics.Add(info, location);
}
ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation: false, diagnostics, location);

if (!modifierErrors)
{
Expand Down
6 changes: 1 addition & 5 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,11 +425,7 @@ internal DeclarationModifiers Modifiers

private void CheckAccessibility(Location location, BindingDiagnosticBag diagnostics, bool isExplicitInterfaceImplementation)
{
var info = ModifierUtils.CheckAccessibility(_modifiers, this, isExplicitInterfaceImplementation);
if (info != null)
{
diagnostics.Add(new CSDiagnostic(info, location));
}
ModifierUtils.CheckAccessibility(_modifiers, this, isExplicitInterfaceImplementation, diagnostics, location);
}

private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool explicitInterfaceImplementation,
Expand Down
6 changes: 1 addition & 5 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceFieldSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,7 @@ internal bool IsNew

protected void CheckAccessibility(BindingDiagnosticBag diagnostics)
{
var info = ModifierUtils.CheckAccessibility(Modifiers, this, isExplicitInterfaceImplementation: false);
if (info != null)
{
diagnostics.Add(new CSDiagnostic(info, this.ErrorLocation));
}
ModifierUtils.CheckAccessibility(Modifiers, this, isExplicitInterfaceImplementation: false, diagnostics, ErrorLocation);
}

protected void ReportModifiersDiagnostics(BindingDiagnosticBag diagnostics)
Expand Down
7 changes: 1 addition & 6 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,12 +395,7 @@ private DeclarationModifiers MakeAndCheckTypeModifiers(
// It is an error for the same modifier to appear multiple times.
if (!modifierErrors)
{
var info = ModifierUtils.CheckAccessibility(mods, this, isExplicitInterfaceImplementation: false);
if (info != null)
{
diagnostics.Add(info, this.Locations[0]);
modifierErrors = true;
}
modifierErrors = ModifierUtils.CheckAccessibility(mods, this, isExplicitInterfaceImplementation: false, diagnostics, Locations[0]);
}
}

Expand Down
6 changes: 1 addition & 5 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbolBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,7 @@ protected SourceOrdinaryMethodSymbolBase(
CheckModifiersForBody(location, diagnostics);
}

var info = ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation: isExplicitInterfaceImplementation);
if (info != null)
{
diagnostics.Add(info, location);
}
ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation: isExplicitInterfaceImplementation, diagnostics, location);
}

protected abstract ImmutableArray<TypeParameterSymbol> MakeTypeParameters(CSharpSyntaxNode node, BindingDiagnosticBag diagnostics);
Expand Down
12 changes: 2 additions & 10 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,7 @@ private SourcePropertyAccessorSymbol(
CheckFeatureAvailabilityAndRuntimeSupport(syntax, location, hasBody: true, diagnostics: diagnostics);
CheckModifiersForBody(location, diagnostics);

var info = ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation);
if (info != null)
{
diagnostics.Add(info, location);
}
ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation, diagnostics, location);

this.CheckModifiers(location, hasBody: true, isAutoPropertyOrExpressionBodied: true, diagnostics: diagnostics);
}
Expand Down Expand Up @@ -230,11 +226,7 @@ protected SourcePropertyAccessorSymbol(
CheckModifiersForBody(location, diagnostics);
}

var info = ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation);
if (info != null)
{
diagnostics.Add(info, location);
}
ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation, diagnostics, location);

if (!modifierErrors)
{
Expand Down
6 changes: 1 addition & 5 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,11 +846,7 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions,

private void CheckAccessibility(Location location, BindingDiagnosticBag diagnostics, bool isExplicitInterfaceImplementation)
{
var info = ModifierUtils.CheckAccessibility(_modifiers, this, isExplicitInterfaceImplementation);
if (info != null)
{
diagnostics.Add(new CSDiagnostic(info, location));
}
ModifierUtils.CheckAccessibility(_modifiers, this, isExplicitInterfaceImplementation, diagnostics, location);
}

private void CheckModifiers(bool isExplicitInterfaceImplementation, Location location, bool isIndexer, BindingDiagnosticBag diagnostics)
Expand Down
6 changes: 1 addition & 5 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceUserDefinedOperatorSymbolBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,7 @@ protected SourceUserDefinedOperatorSymbolBase(

// SPEC: It is an error for the same modifier to appear multiple times in an
// SPEC: operator declaration.
var info = ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation: false);
if (info != null)
{
diagnostics.Add(info, location);
}
ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation: false, diagnostics, location);
}

protected static DeclarationModifiers MakeDeclarationModifiers(MethodKind methodKind, bool inInterface, BaseMethodDeclarationSyntax syntax, Location location, BindingDiagnosticBag diagnostics)
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ private static bool IsTypeLessVisibleThan(TypeSymbol type, Symbol sym, ref Compo
}
}

private static bool IsAsRestrictive(NamedTypeSymbol s1, Symbol sym2, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
internal static bool IsAsRestrictive(this Symbol s1, Symbol sym2, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
Accessibility acc1 = s1.DeclaredAccessibility;

Expand Down
Loading

0 comments on commit e7a1a08

Please sign in to comment.