Skip to content

Commit 40c98b2

Browse files
vbreussvbreuss
authored
fix: preserve protected internal accessibility on overridden members (#788)
When overriding a `protected internal virtual` member, the override must remain `protected internal` whenever the overriding code can see the declaring assembly's internals (same assembly or InternalsVisibleTo). Mapping unconditionally to `protected` produced CS0507 in those cases. Resolve the override accessibility once at entity construction with the source assembly in scope, and emit the resolved string from the method, property, accessor, and event sites.
1 parent e61557f commit 40c98b2

7 files changed

Lines changed: 93 additions & 25 deletions

File tree

‎Source/Mockolate.SourceGenerators/Entities/Event.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ internal record Event
99
public Event(IEventSymbol eventSymbol, IMethodSymbol delegateInvokeMethod, List<Event>? alreadyDefinedEvents, IAssemblySymbol? sourceAssembly = null)
1010
{
1111
Accessibility = eventSymbol.DeclaredAccessibility;
12+
OverrideAccessibility = Helpers.ResolveOverrideVisibility(
13+
Accessibility, eventSymbol.ContainingAssembly, sourceAssembly);
1214
UseOverride = eventSymbol.IsVirtual || eventSymbol.IsAbstract;
1315
IsAbstract = eventSymbol.IsAbstract;
1416
Name = Helpers.EscapeIfKeyword(eventSymbol.ExplicitInterfaceImplementations.Length > 0 ? eventSymbol.ExplicitInterfaceImplementations[0].Name : eventSymbol.Name);
@@ -53,6 +55,7 @@ public Event(IEventSymbol eventSymbol, IMethodSymbol delegateInvokeMethod, List<
5355
};
5456

5557
public Accessibility Accessibility { get; }
58+
public string OverrideAccessibility { get; }
5659
public string Name { get; }
5760
public string? ExplicitImplementation { get; }
5861

‎Source/Mockolate.SourceGenerators/Entities/Method.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ internal record Method
99
public Method(IMethodSymbol methodSymbol, List<Method>? alreadyDefinedMethods, IAssemblySymbol? sourceAssembly = null)
1010
{
1111
Accessibility = methodSymbol.DeclaredAccessibility;
12+
OverrideAccessibility = Helpers.ResolveOverrideVisibility(
13+
Accessibility, methodSymbol.ContainingAssembly, sourceAssembly);
1214
UseOverride = methodSymbol.IsVirtual || methodSymbol.IsAbstract;
1315
IsAbstract = methodSymbol.IsAbstract;
1416
IsStatic = methodSymbol.IsStatic;
@@ -67,6 +69,7 @@ public Method(IMethodSymbol methodSymbol, List<Method>? alreadyDefinedMethods, I
6769
};
6870

6971
public Accessibility Accessibility { get; }
72+
public string OverrideAccessibility { get; }
7073
public Type ReturnType { get; }
7174
public string Name { get; }
7275
public string ContainingType { get; }

‎Source/Mockolate.SourceGenerators/Entities/Property.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ internal record Property
99
public Property(IPropertySymbol propertySymbol, List<Property>? alreadyDefinedProperties, IAssemblySymbol? sourceAssembly = null)
1010
{
1111
Accessibility = propertySymbol.DeclaredAccessibility;
12+
OverrideAccessibility = Helpers.ResolveOverrideVisibility(
13+
Accessibility, propertySymbol.ContainingAssembly, sourceAssembly);
1214
UseOverride = propertySymbol.IsVirtual || propertySymbol.IsAbstract;
1315
string rawName = propertySymbol.ExplicitInterfaceImplementations.Length > 0 ? propertySymbol.ExplicitInterfaceImplementations[0].Name : propertySymbol.Name;
1416
Name = propertySymbol.IsIndexer ? rawName : Helpers.EscapeIfKeyword(rawName);
@@ -72,6 +74,7 @@ public Property(IPropertySymbol propertySymbol, List<Property>? alreadyDefinedPr
7274
public EquatableArray<Attribute>? Attributes { get; }
7375

7476
public Accessibility Accessibility { get; }
77+
public string OverrideAccessibility { get; }
7578
public string Name { get; }
7679
public string? ExplicitImplementation { get; }
7780

‎Source/Mockolate.SourceGenerators/Helpers.cs‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,12 @@ static bool HasIndexedConflict(string @base, string parameterName, int count)
7878
}
7979
}
8080

81-
// A member (or accessor) declared in another assembly is overridable only if the overriding
82-
// assembly can actually see it. `internal` and `private protected` are invisible across assembly
83-
// boundaries unless the declaring assembly grants InternalsVisibleTo. `protected internal`
84-
// (= protected OR internal) is always reachable via the protected half from a derived class.
81+
/// <summary>
82+
/// A member (or accessor) declared in another assembly is overridable only if the overriding
83+
/// assembly can actually see it. `internal` and `private protected` are invisible across assembly
84+
/// boundaries unless the declaring assembly grants InternalsVisibleTo. `protected internal`
85+
/// (= protected OR internal) is always reachable via the protected half from a derived class.
86+
/// </summary>
8587
public static bool IsOverridableFrom(ISymbol member, IAssemblySymbol? sourceAssembly)
8688
{
8789
if (sourceAssembly is null ||
@@ -99,6 +101,41 @@ member.DeclaredAccessibility is not (Accessibility.Internal or Accessibility.Pro
99101
return containingAssembly.GivesAccessTo(sourceAssembly);
100102
}
101103

104+
/// <summary>
105+
/// C# requires an override to match the base member's declared accessibility, with one
106+
/// exception: when overriding a `protected internal` member from an assembly that cannot see
107+
/// `internal` (i.e. neither the same assembly nor an InternalsVisibleTo target), the override
108+
/// must drop the internal half and use plain `protected`.
109+
/// </summary>
110+
public static string ResolveOverrideVisibility(Accessibility accessibility,
111+
IAssemblySymbol? containingAssembly, IAssemblySymbol? sourceAssembly)
112+
=> accessibility switch
113+
{
114+
Accessibility.Public => "public",
115+
Accessibility.Protected => "protected",
116+
Accessibility.Internal => "internal",
117+
Accessibility.ProtectedAndInternal => "private protected",
118+
Accessibility.ProtectedOrInternal => HasInternalAccess(containingAssembly, sourceAssembly)
119+
? "protected internal"
120+
: "protected",
121+
_ => "private",
122+
};
123+
124+
private static bool HasInternalAccess(IAssemblySymbol? containingAssembly, IAssemblySymbol? sourceAssembly)
125+
{
126+
if (sourceAssembly is null || containingAssembly is null)
127+
{
128+
return false;
129+
}
130+
131+
if (SymbolEqualityComparer.Default.Equals(containingAssembly, sourceAssembly))
132+
{
133+
return true;
134+
}
135+
136+
return containingAssembly.GivesAccessTo(sourceAssembly);
137+
}
138+
102139
public static string ToTypeOrWrapper(this Type type)
103140
{
104141
if (type.SpecialGenericType == SpecialGenericType.Span)
@@ -146,8 +183,8 @@ public static bool NeedsRefStructPipeline(this MethodParameter parameter)
146183
}
147184

148185
return parameter.RefKind == RefKind.RefReadOnlyParameter
149-
&& parameter.Type.IsRefStruct
150-
&& parameter.Type.SpecialGenericType is (SpecialGenericType.Span or SpecialGenericType.ReadOnlySpan);
186+
&& parameter.Type.IsRefStruct
187+
&& parameter.Type.SpecialGenericType is SpecialGenericType.Span or SpecialGenericType.ReadOnlySpan;
151188
}
152189

153190
extension(ITypeSymbol typeSymbol)

‎Source/Mockolate.SourceGenerators/Sources/Sources.MockClass.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,7 +1486,7 @@ private static void AppendMockSubject_ImplementClass_AddEvent(StringBuilder sb,
14861486
{
14871487
if (@event.ExplicitImplementation is null)
14881488
{
1489-
sb.Append("\t\t").Append(@event.Accessibility.ToVisibilityString()).Append(' ');
1489+
sb.Append("\t\t").Append(@event.OverrideAccessibility).Append(' ');
14901490
if (@event.IsStatic)
14911491
{
14921492
sb.Append("static ");
@@ -1625,7 +1625,7 @@ property.IndexerParameters is not null
16251625
{
16261626
if (property.ExplicitImplementation is null)
16271627
{
1628-
sb.Append("\t\t").Append(property.Accessibility.ToVisibilityString()).Append(' ');
1628+
sb.Append("\t\t").Append(property.OverrideAccessibility).Append(' ');
16291629
if (property.IsStatic)
16301630
{
16311631
sb.Append("static ");
@@ -1671,7 +1671,7 @@ property.IndexerParameters is not null
16711671
sb.Append("\t\t\t");
16721672
if (property.Getter.Accessibility != property.Accessibility)
16731673
{
1674-
sb.Append(property.Getter.Accessibility.ToVisibilityString()).Append(' ');
1674+
sb.Append(property.Getter.OverrideAccessibility).Append(' ');
16751675
}
16761676

16771677
sb.AppendLine("get");
@@ -1906,7 +1906,7 @@ property.IndexerParameters is not null
19061906
sb.Append("\t\t\t");
19071907
if (property.Setter.Accessibility != property.Accessibility)
19081908
{
1909-
sb.Append(property.Setter.Accessibility.ToVisibilityString()).Append(' ');
1909+
sb.Append(property.Setter.OverrideAccessibility).Append(' ');
19101910
}
19111911

19121912
sb.AppendLine(property.Setter.IsInitOnly ? "init" : "set");
@@ -2143,7 +2143,7 @@ private static void AppendMockSubject_ImplementClass_AddMethod(StringBuilder sb,
21432143
sb.Append("\t\t");
21442144
if (method.ExplicitImplementation is null)
21452145
{
2146-
sb.Append(method.Accessibility.ToVisibilityString()).Append(' ');
2146+
sb.Append(method.OverrideAccessibility).Append(' ');
21472147
if (method.IsStatic)
21482148
{
21492149
sb.Append("static ");

‎Source/Mockolate.SourceGenerators/Sources/Sources.cs‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -671,20 +671,6 @@ static bool IsIdentifierPart(char c)
671671
}
672672
}
673673

674-
extension(Accessibility accessibility)
675-
{
676-
internal string ToVisibilityString()
677-
=> accessibility switch
678-
{
679-
Accessibility.Protected => "protected",
680-
Accessibility.Internal => "internal",
681-
Accessibility.ProtectedOrInternal => "protected",
682-
Accessibility.Public => "public",
683-
Accessibility.ProtectedAndInternal => "private protected",
684-
_ => "private",
685-
};
686-
}
687-
688674
extension(RefKind refKind)
689675
{
690676
internal string GetString(bool replaceRefReadonlyWithIn = false)

‎Tests/Mockolate.SourceGenerators.Tests/MockTests.cs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,42 @@ await That(result.Sources["Mock.MyDerivedClass.g.cs"])
699699
.DoesNotContain("override bool Equals");
700700
}
701701

702+
[Fact]
703+
public async Task ShouldPreserveProtectedInternalAccessibilityOnOverriddenMembers()
704+
{
705+
GeneratorResult result = Generator
706+
.Run("""
707+
using Mockolate;
708+
using System;
709+
710+
namespace MyCode;
711+
712+
public class Program
713+
{
714+
public static void Main(string[] args) => _ = MyClass.CreateMock();
715+
}
716+
717+
public class MyClass
718+
{
719+
protected internal virtual void ProtectedInternalMethod() { }
720+
protected internal virtual int ProtectedInternalProperty { get; set; }
721+
protected internal virtual event EventHandler? ProtectedInternalEvent;
722+
public virtual int MixedAccessorProperty { get; protected internal set; }
723+
}
724+
""");
725+
726+
await That(result.Sources).ContainsKey("Mock.MyClass.g.cs");
727+
string generated = result.Sources["Mock.MyClass.g.cs"];
728+
await That(generated)
729+
.Contains("protected internal override void ProtectedInternalMethod()").And
730+
.Contains("protected internal override int ProtectedInternalProperty").And
731+
.Contains("protected internal override event global::System.EventHandler? ProtectedInternalEvent").And
732+
.Contains("protected internal set").And
733+
.DoesNotContain("protected override void ProtectedInternalMethod").And
734+
.DoesNotContain("protected override int ProtectedInternalProperty").And
735+
.DoesNotContain("protected override event");
736+
}
737+
702738
[Fact]
703739
public async Task ShouldSupportSpecialTypes()
704740
{

0 commit comments

Comments
 (0)