Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
28 changes: 18 additions & 10 deletions src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@
Symbol symbol,
NamedTypeSymbol within,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
TypeSymbol throughTypeOpt = null)
TypeSymbol throughTypeOpt = null,
bool skipContainingTypeCheck = false)
{
bool failedThroughTypeCheck;
return IsSymbolAccessibleCore(symbol, within, throughTypeOpt, out failedThroughTypeCheck, within.DeclaringCompilation, ref useSiteInfo);
return IsSymbolAccessibleCore(symbol, within, throughTypeOpt, out failedThroughTypeCheck, within.DeclaringCompilation, ref useSiteInfo, basesBeingResolved: null, skipContainingTypeCheck);
}

/// <summary>
Expand All @@ -55,9 +56,10 @@
TypeSymbol throughTypeOpt,
out bool failedThroughTypeCheck,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
ConsList<TypeSymbol> basesBeingResolved = null)
ConsList<TypeSymbol> basesBeingResolved = null,
bool skipContainingTypeCheck = false)
{
return IsSymbolAccessibleCore(symbol, within, throughTypeOpt, out failedThroughTypeCheck, within.DeclaringCompilation, ref useSiteInfo, basesBeingResolved);
return IsSymbolAccessibleCore(symbol, within, throughTypeOpt, out failedThroughTypeCheck, within.DeclaringCompilation, ref useSiteInfo, basesBeingResolved, skipContainingTypeCheck);
}

/// <summary>
Expand Down Expand Up @@ -137,7 +139,8 @@
out bool failedThroughTypeCheck,
CSharpCompilation compilation,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
ConsList<TypeSymbol> basesBeingResolved = null)
ConsList<TypeSymbol> basesBeingResolved = null,
bool skipContainingTypeCheck = false)
{
Debug.Assert((object)symbol != null);
Debug.Assert((object)within != null);
Expand Down Expand Up @@ -209,7 +212,7 @@
throughTypeOpt = null;
}

return IsMemberAccessible(symbol.ContainingType, symbol.DeclaredAccessibility, within, throughTypeOpt, out failedThroughTypeCheck, compilation, ref useSiteInfo);
return IsMemberAccessible(symbol.ContainingType, symbol.DeclaredAccessibility, within, throughTypeOpt, out failedThroughTypeCheck, compilation, ref useSiteInfo, basesBeingResolved, skipContainingTypeCheck);

default:
throw ExceptionUtilities.UnexpectedValue(symbol.Kind);
Expand Down Expand Up @@ -302,7 +305,8 @@
out bool failedThroughTypeCheck,
CSharpCompilation compilation,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
ConsList<TypeSymbol> basesBeingResolved = null)
ConsList<TypeSymbol> basesBeingResolved = null,
bool skipContainingTypeCheck = false)
{
Debug.Assert(within is NamedTypeSymbol || within is AssemblySymbol);
Debug.Assert((object)containingType != null);
Expand All @@ -316,7 +320,9 @@
}

// A nested symbol is only accessible to us if its container is accessible as well.
if (!IsNamedTypeAccessible(containingType, within, ref useSiteInfo, basesBeingResolved))
// Skip this check when checking overridden members, as the containing type could have
// type arguments from unrelated assemblies.
if (!skipContainingTypeCheck && !IsNamedTypeAccessible(containingType, within, ref useSiteInfo, basesBeingResolved))
{
return false;
}
Expand All @@ -336,7 +342,8 @@
out failedThroughTypeCheck,
compilation,
ref useSiteInfo,
basesBeingResolved);
basesBeingResolved,
skipContainingTypeCheck);
}

private static bool IsNonPublicMemberAccessible(
Expand All @@ -347,7 +354,8 @@
out bool failedThroughTypeCheck,
CSharpCompilation compilation,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
ConsList<TypeSymbol> basesBeingResolved = null)
ConsList<TypeSymbol> basesBeingResolved = null,
bool skipContainingTypeCheck = false)

Check failure on line 358 in src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs#L358

src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs(358,18): error IDE0060: (NETCORE_ENGINEERING_TELEMETRY=Build) Remove unused parameter 'skipContainingTypeCheck' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0060)

Check failure on line 358 in src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs#L358

src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs(358,18): error IDE0060: (NETCORE_ENGINEERING_TELEMETRY=Build) Remove unused parameter 'skipContainingTypeCheck' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0060)
{
failedThroughTypeCheck = false;

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/EventSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ internal EventSymbol GetLeastOverriddenEvent(NamedTypeSymbol? accessingTypeOpt)
EventSymbol? overridden = e.OverriddenEvent;
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
if ((object?)overridden == null ||
(accessingTypeOpt is { } && !AccessCheck.IsSymbolAccessible(overridden, accessingTypeOpt, ref discardedUseSiteInfo)))
(accessingTypeOpt is { } && !AccessCheck.IsSymbolAccessible(overridden, accessingTypeOpt, ref discardedUseSiteInfo, skipContainingTypeCheck: true)))
{
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ private MethodSymbol GetLeastOverriddenMethodCore(NamedTypeSymbol accessingTypeO
MethodSymbol overridden = m.OverriddenMethod;
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
if ((object)overridden == null ||
(accessingTypeOpt is { } && !AccessCheck.IsSymbolAccessible(overridden, accessingTypeOpt, ref discardedUseSiteInfo)) ||
(accessingTypeOpt is { } && !AccessCheck.IsSymbolAccessible(overridden, accessingTypeOpt, ref discardedUseSiteInfo, skipContainingTypeCheck: true)) ||
(requireSameReturnType && !this.ReturnType.Equals(overridden.ReturnType, TypeCompareKind.AllIgnoreOptions)))
{
break;
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ internal PropertySymbol GetLeastOverriddenProperty(NamedTypeSymbol accessingType
PropertySymbol overridden = p.OverriddenProperty;
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
if ((object)overridden == null ||
(accessingTypeOpt is { } && !AccessCheck.IsSymbolAccessible(overridden, accessingTypeOpt, ref discardedUseSiteInfo)))
(accessingTypeOpt is { } && !AccessCheck.IsSymbolAccessible(overridden, accessingTypeOpt, ref discardedUseSiteInfo, skipContainingTypeCheck: true)))
{
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3096,5 +3096,50 @@ static void validate(ModuleSymbol m)
m.GlobalNamespace.GetMember("Test1.<P1>k__BackingField").GetAttributes().Select(a => a.ToString()));
}
}

[Fact]
public void PropertyIsReadOnly_WithTypeArgumentFromUnrelatedAssembly()
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot Please add a similar test for VB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added VB test in commit 1d31135. The test follows the same pattern as the C# version and verifies that checking PropertySymbol.IsReadOnly doesn't trigger an assertion when the containing type has type arguments from unrelated assemblies.

{
// This test verifies that checking PropertySymbol.IsReadOnly doesn't trigger
// an assertion when the containing type has type arguments from unrelated assemblies.
// Based on issue #81937 pattern where overridden members cause accessibility checks
// on type arguments that may be from unrelated assemblies.
string source1 = """
public class C0<T>
{
public virtual int P { get; set; }
}

public class C1<T> : C0<T>
{
public override int P { get; set; }
}
""";
var comp1 = CreateCompilation(source1);
comp1.VerifyDiagnostics();

string source2 = """
internal class C2 { }

internal class C3 : C1<C2>
{
public override int P { get; set; }
}
""";

var comp2 = CreateCompilation(source2, references: [comp1.ToMetadataReference()]);
comp2.VerifyDiagnostics();

// Get the property symbol and check IsReadOnly
var c3 = comp2.GlobalNamespace.GetTypeMember("C3");
var property = c3.GetMember<PropertySymbol>("P");

// This should not throw an assertion
var isReadOnly = property.IsReadOnly;
Assert.False(isReadOnly);

var isWriteOnly = property.IsWriteOnly;
Assert.False(isWriteOnly);
}
}
}
Loading