Skip to content

Disallow complex forms of IndexerName attributes in extensions #77781

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

Merged
merged 1 commit into from
Mar 25, 2025
Merged

Disallow complex forms of IndexerName attributes in extensions #77781

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
12 changes: 9 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/Source/QuickAttributeChecker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ private static QuickAttributeChecker CreatePredefinedQuickAttributeChecker()
var result = new QuickAttributeChecker();
result.AddName(AttributeDescription.TypeIdentifierAttribute.Name, QuickAttributes.TypeIdentifier);
result.AddName(AttributeDescription.TypeForwardedToAttribute.Name, QuickAttributes.TypeForwardedTo);
result.AddName(AttributeDescription.IndexerNameAttribute.Name, QuickAttributes.IndexerName);
result.AddName(AttributeDescription.AssemblyKeyNameAttribute.Name, QuickAttributes.AssemblyKeyName);
result.AddName(AttributeDescription.AssemblyKeyFileAttribute.Name, QuickAttributes.AssemblyKeyFile);
result.AddName(AttributeDescription.AssemblySignatureKeyAttribute.Name, QuickAttributes.AssemblySignatureKey);
Expand Down Expand Up @@ -144,9 +145,10 @@ internal enum QuickAttributes : byte
None = 0,
TypeIdentifier = 1 << 0,
TypeForwardedTo = 1 << 1,
AssemblyKeyName = 1 << 2,
AssemblyKeyFile = 1 << 3,
AssemblySignatureKey = 1 << 4,
IndexerName = 1 << 2,
AssemblyKeyName = 1 << 3,
AssemblyKeyFile = 1 << 4,
AssemblySignatureKey = 1 << 5,
Last = AssemblySignatureKey,
}

Expand All @@ -171,6 +173,10 @@ public static QuickAttributes GetQuickAttributes(string name, bool inAttribute)
{
result |= QuickAttributes.TypeForwardedTo;
}
else if (matches(AttributeDescription.IndexerNameAttribute))
{
result |= QuickAttributes.IndexerName;
}
else if (matches(AttributeDescription.AssemblyKeyNameAttribute))
{
result |= QuickAttributes.AssemblyKeyName;
Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1666,7 +1666,7 @@ internal CommonAssemblyWellKnownAttributeData GetSourceDecodedWellKnownAttribute
}

attributesBag = null;
Func<AttributeSyntax, bool> attributeMatches = attribute switch
Func<AttributeSyntax, Binder?, bool> attributeMatches = attribute switch
{
QuickAttributes.AssemblySignatureKey => isPossibleAssemblySignatureKeyAttribute,
QuickAttributes.AssemblyKeyName => isPossibleAssemblyKeyNameAttribute,
Expand All @@ -1678,19 +1678,19 @@ internal CommonAssemblyWellKnownAttributeData GetSourceDecodedWellKnownAttribute

return (CommonAssemblyWellKnownAttributeData?)attributesBag?.DecodedWellKnownAttributeData;

bool isPossibleAssemblySignatureKeyAttribute(AttributeSyntax node)
bool isPossibleAssemblySignatureKeyAttribute(AttributeSyntax node, Binder? rootBinderOpt)
{
QuickAttributeChecker checker = this.DeclaringCompilation.GetBinderFactory(node.SyntaxTree).GetBinder(node).QuickAttributeChecker;
return checker.IsPossibleMatch(node, QuickAttributes.AssemblySignatureKey);
}

bool isPossibleAssemblyKeyNameAttribute(AttributeSyntax node)
bool isPossibleAssemblyKeyNameAttribute(AttributeSyntax node, Binder? rootBinderOpt)
{
QuickAttributeChecker checker = this.DeclaringCompilation.GetBinderFactory(node.SyntaxTree).GetBinder(node).QuickAttributeChecker;
return checker.IsPossibleMatch(node, QuickAttributes.AssemblyKeyName);
}

bool isPossibleAssemblyKeyFileAttribute(AttributeSyntax node)
bool isPossibleAssemblyKeyFileAttribute(AttributeSyntax node, Binder? rootBinderOpt)
{
QuickAttributeChecker checker = this.DeclaringCompilation.GetBinderFactory(node.SyntaxTree).GetBinder(node).QuickAttributeChecker;
return checker.IsPossibleMatch(node, QuickAttributes.AssemblyKeyFile);
Expand Down Expand Up @@ -1756,7 +1756,7 @@ private static void AfterPossibleForwardedTypesAttributePartBound(AttributeSynta
Debug.Assert(removed);
}

private bool IsPossibleForwardedTypesAttribute(AttributeSyntax node)
private bool IsPossibleForwardedTypesAttribute(AttributeSyntax node, Binder rootBinderOpt)
{
QuickAttributeChecker checker =
this.DeclaringCompilation.GetBinderFactory(node.SyntaxTree).GetBinder(node).QuickAttributeChecker;
Expand Down
30 changes: 29 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,11 @@ internal string SourceName
// always use the real attribute bag of this symbol and modify LoadAndValidateAttributes to
// handle partially filled bags.
CustomAttributesBag<CSharpAttributeData>? temp = null;
LoadAndValidateAttributes(OneOrMany.Create(indexerNameAttributeLists), ref temp, earlyDecodingOnly: true);
Binder rootBinder = GetAttributeBinder(indexerNameAttributeLists, DeclaringCompilation);
LoadAndValidateAttributes(
OneOrMany.Create(indexerNameAttributeLists), ref temp, earlyDecodingOnly: true,
binderOpt: rootBinder,
attributeMatchesOpt: this.GetIsNewExtensionMember() ? isPossibleIndexerNameAttributeInExtension : isPossibleIndexerNameAttribute);
if (temp != null)
{
Debug.Assert(temp.IsEarlyDecodedWellKnownAttributeDataComputed);
Expand All @@ -487,6 +491,24 @@ internal string SourceName
}

return _lazySourceName;

static bool isPossibleIndexerNameAttribute(AttributeSyntax node, Binder? rootBinderOpt)
{
Debug.Assert(rootBinderOpt is not null);
QuickAttributeChecker checker = rootBinderOpt.QuickAttributeChecker;
return checker.IsPossibleMatch(node, QuickAttributes.IndexerName);
}

static bool isPossibleIndexerNameAttributeInExtension(AttributeSyntax node, Binder? rootBinderOpt)
{
// PROTOTYPE: Temporarily limit binding to a string literal argument in order to avoid a binding cycle.
if (node.ArgumentList?.Arguments is not [{ NameColon: null, NameEquals: null, Expression: LiteralExpressionSyntax { RawKind: (int)SyntaxKind.StringLiteralExpression } }])
{
return false;
}

return isPossibleIndexerNameAttribute(node, rootBinderOpt);
}
}
}
#nullable disable
Expand Down Expand Up @@ -1696,6 +1718,12 @@ private void ValidateIndexerNameAttribute(CSharpAttributeData attribute, Attribu
{
diagnostics.Add(ErrorCode.ERR_BadArgumentToAttribute, node.ArgumentList.Arguments[0].Location, node.GetErrorDisplayName());
}
else if (this.GetIsNewExtensionMember() && SourceName != indexerName)
{
// PROTOTYPE: Report more descriptive error
// error CS8078: An expression is too long or complex to compile
diagnostics.Add(ErrorCode.ERR_InsufficientStack, node.ArgumentList.Arguments[0].Location);
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/Symbol_Attributes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ internal bool LoadAndValidateAttributes(
AttributeLocation symbolPart = AttributeLocation.None,
bool earlyDecodingOnly = false,
Binder? binderOpt = null,
Func<AttributeSyntax, bool>? attributeMatchesOpt = null,
Func<AttributeSyntax, Binder?, bool>? attributeMatchesOpt = null,
Action<AttributeSyntax>? beforeAttributePartBound = null,
Action<AttributeSyntax>? afterAttributePartBound = null)
{
Expand Down Expand Up @@ -586,7 +586,7 @@ private ImmutableArray<AttributeSyntax> GetAttributesToBind(
AttributeLocation symbolPart,
BindingDiagnosticBag diagnostics,
CSharpCompilation compilation,
Func<AttributeSyntax, bool> attributeMatchesOpt,
Func<AttributeSyntax, Binder, bool> attributeMatchesOpt,
Binder rootBinderOpt,
out ImmutableArray<Binder> binders)
{
Expand Down Expand Up @@ -624,7 +624,7 @@ private ImmutableArray<AttributeSyntax> GetAttributesToBind(
{
foreach (var attribute in attributesToBind)
{
if (attributeMatchesOpt(attribute))
if (attributeMatchesOpt(attribute, rootBinderOpt))
{
syntaxBuilder.Add(attribute);
attributesToBindCount++;
Expand Down Expand Up @@ -666,7 +666,7 @@ protected virtual bool ShouldBindAttributes(AttributeListSyntax attributeDeclara
}

#nullable enable
private Binder GetAttributeBinder(SyntaxList<AttributeListSyntax> attributeDeclarationSyntaxList, CSharpCompilation compilation, Binder? rootBinder = null)
protected Binder GetAttributeBinder(SyntaxList<AttributeListSyntax> attributeDeclarationSyntaxList, CSharpCompilation compilation, Binder? rootBinder = null)
{
var binder = rootBinder ?? compilation.GetBinderFactory(attributeDeclarationSyntaxList.Node!.SyntaxTree).GetBinder(attributeDeclarationSyntaxList.Node);
binder = new ContextualAttributeBinder(binder, this);
Expand Down
20 changes: 15 additions & 5 deletions src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22839,15 +22839,15 @@ public MyAttr(int p) {}
);
}

[Fact(Skip = "Cycle")] // PROTOTYPE: There is a cycle due to the attribute
[Fact]
public void ReceiverParameterScope_07_InAttribute()
{
var src = @"
static class Extensions
{
extension(int p)
{
[System.Runtime.CompilerServices.IndexerName(nameof(p))]
[MyAttr(nameof(p))]
Copy link
Member

@jjonescz jjonescz Mar 25, 2025 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

Could we keep the original test with [IndexerName(nameof(p))]? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we keep the original test with [IndexerName(nameof(p))]?

The purpose of the test is to confirm that use of receiver parameter in an attribute inside nameof is allowed. Keeping the original code defeats this purpose because of the limitations applied to IndexerName attribute specifically. That limitation is tested by other dedicated test.

int this[int y]
{
get
Expand All @@ -22857,6 +22857,11 @@ int this[int y]
}
}
}

class MyAttr : System.Attribute
{
public MyAttr(string p) {}
}
";
var comp = CreateCompilation(src);

Expand Down Expand Up @@ -22989,13 +22994,13 @@ string M3()
);
}

[Fact(Skip = "Cycle")] // PROTOTYPE: There is a cycle due to the attribute
[Fact]
public void CycleInAttribute_01()
{
var src = @"
static class Extensions
{
static const string Str = ""val""
const string Str = ""val"";
extension(string p)
{
[System.Runtime.CompilerServices.IndexerName(Str)]
Expand All @@ -23011,7 +23016,12 @@ int this[int y]
";
var comp = CreateCompilation(src);

comp.VerifyEmitDiagnostics();
// PROTOTYPE: We do not allow complex forms of IndexerName attribute due to a possible binding cycle
comp.VerifyEmitDiagnostics(
// (7,54): error CS8078: An expression is too long or complex to compile
// [System.Runtime.CompilerServices.IndexerName(Str)]
Diagnostic(ErrorCode.ERR_InsufficientStack, "Str").WithLocation(7, 54)
);
}

[Fact]
Expand Down
66 changes: 64 additions & 2 deletions src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5107,7 +5107,37 @@ partial struct S1
""";

var comp = CreateCompilation(source);
comp.VerifyEmitDiagnostics();
CompileAndVerify(comp).VerifyDiagnostics().VerifyTypeIL("S1",
"""
.class private sequential ansi sealed beforefieldinit S1
extends [mscorlib]System.ValueType
{
.custom instance void [mscorlib]System.Reflection.DefaultMemberAttribute::.ctor(string) = (
01 00 06 4d 79 4e 61 6d 65 00 00
)
.pack 0
.size 1
// Methods
.method public hidebysig specialname
instance int32 get_MyName (
int32 x
) cil managed
{
// Method begins at RVA 0x2067
// Code size 2 (0x2)
.maxstack 8
IL_0000: ldarg.1
IL_0001: ret
} // end of method S1::get_MyName
// Properties
.property instance int32 MyName(
int32 x
)
{
.get instance int32 S1::get_MyName(int32)
}
} // end of class S1
""".Replace("[mscorlib]", ExecutionConditionUtil.IsMonoOrCoreClr ? "[netstandard]" : "[mscorlib]"));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/76842")]
Expand All @@ -5126,7 +5156,39 @@ partial struct S1
""";

var comp = CreateCompilation(source);
comp.VerifyEmitDiagnostics();

// Note, the indexer name in metadata is "Item", expected "MyName"
CompileAndVerify(comp).VerifyDiagnostics().VerifyTypeIL("S1",
"""
.class private sequential ansi sealed beforefieldinit S1
extends [mscorlib]System.ValueType
{
.custom instance void [mscorlib]System.Reflection.DefaultMemberAttribute::.ctor(string) = (
01 00 04 49 74 65 6d 00 00
)
.pack 0
.size 1
// Methods
.method public hidebysig specialname
instance int32 get_Item (
int32 x
) cil managed
{
// Method begins at RVA 0x2067
// Code size 2 (0x2)
.maxstack 8
IL_0000: ldarg.1
IL_0001: ret
} // end of method S1::get_Item
// Properties
.property instance int32 Item(
int32 x
)
{
.get instance int32 S1::get_Item(int32)
}
} // end of class S1
""".Replace("[mscorlib]", ExecutionConditionUtil.IsMonoOrCoreClr ? "[netstandard]" : "[mscorlib]"));
}
}
}