Skip to content
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
140 changes: 140 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ private ImmutableArray<Symbol> BindCrefInternal(CrefSyntax syntax, out Symbol? a
case SyntaxKind.IndexerMemberCref:
case SyntaxKind.OperatorMemberCref:
case SyntaxKind.ConversionOperatorMemberCref:
case SyntaxKind.ExtensionMemberCref:
return BindMemberCref((MemberCrefSyntax)syntax, containerOpt: null, ambiguityWinner: out ambiguityWinner, diagnostics: diagnostics);
default:
throw ExceptionUtilities.UnexpectedValue(syntax.Kind());
Expand Down Expand Up @@ -125,6 +126,9 @@ private ImmutableArray<Symbol> BindMemberCref(MemberCrefSyntax syntax, Namespace
case SyntaxKind.ConversionOperatorMemberCref:
result = BindConversionOperatorMemberCref((ConversionOperatorMemberCrefSyntax)syntax, containerOpt, out ambiguityWinner, diagnostics);
break;
case SyntaxKind.ExtensionMemberCref:
result = BindExtensionMemberCref((ExtensionMemberCrefSyntax)syntax, containerOpt, out ambiguityWinner, diagnostics);
break;
default:
throw ExceptionUtilities.UnexpectedValue(syntax.Kind());
}
Expand Down Expand Up @@ -216,6 +220,142 @@ private ImmutableArray<Symbol> BindIndexerMemberCref(IndexerMemberCrefSyntax syn
diagnostics: diagnostics);
}

private ImmutableArray<Symbol> BindExtensionMemberCref(ExtensionMemberCrefSyntax syntax, NamespaceOrTypeSymbol? containerOpt, out Symbol? ambiguityWinner, BindingDiagnosticBag diagnostics)
{
// Tracked by https://github.com/dotnet/roslyn/issues/76130 : handle extension operators
CheckFeatureAvailability(syntax, MessageID.IDS_FeatureExtensions, diagnostics);
Copy link
Member

@jjonescz jjonescz Jun 11, 2025

Choose a reason for hiding this comment

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

Is there a test for this langversion check? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

See Cref_54


if (containerOpt is not NamedTypeSymbol namedContainer)
{
ambiguityWinner = null;
return ImmutableArray<Symbol>.Empty;
}

ImmutableArray<Symbol> sortedSymbols = default;
int arity = 0;
TypeArgumentListSyntax? typeArgumentListSyntax = null;
CrefParameterListSyntax? parameters = null;

if (syntax.Member is NameMemberCrefSyntax { Name: SimpleNameSyntax simpleName } nameMember)
{
arity = simpleName.Arity;
typeArgumentListSyntax = simpleName is GenericNameSyntax genericName ? genericName.TypeArgumentList : null;
parameters = nameMember.Parameters;

TypeArgumentListSyntax? extensionTypeArguments = syntax.TypeArgumentList;
int extensionArity = extensionTypeArguments?.Arguments.Count ?? 0;
sortedSymbols = computeSortedAndFilteredCrefExtensionMembers(namedContainer, simpleName.Identifier.ValueText, extensionArity, arity, extensionTypeArguments, diagnostics, syntax);
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

ValueText

Do we have a test with escaped identifier at this position? #Closed

}

if (sortedSymbols.IsDefaultOrEmpty)
{
ambiguityWinner = null;
return [];
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

return [];

Consider leaving a follow up comment to handle operators. #Closed

}

Debug.Assert(sortedSymbols.All(s => s.GetIsNewExtensionMember()));

return ProcessCrefMemberLookupResults(sortedSymbols, arity, syntax, typeArgumentListSyntax, parameters, out ambiguityWinner, diagnostics);

ImmutableArray<Symbol> computeSortedAndFilteredCrefExtensionMembers(NamedTypeSymbol container, string name, int extensionArity, int arity, TypeArgumentListSyntax? extensionTypeArguments, BindingDiagnosticBag diagnostics, ExtensionMemberCrefSyntax syntax)
{
Debug.Assert(name is not null);

Debug.Assert(syntax.Parameters is not null);
ImmutableArray<ParameterSymbol> extensionParameterSymbols = BindCrefParameters(syntax.Parameters, diagnostics);

// Use signature method symbols to match extension blocks
var providedExtensionSignature = new SignatureOnlyMethodSymbol(
methodKind: MethodKind.Ordinary,
typeParameters: IndexedTypeParameterSymbol.TakeSymbols(extensionArity),
parameters: extensionParameterSymbols,
callingConvention: Cci.CallingConvention.Default,
// These are ignored by this specific MemberSignatureComparer.
containingType: null,
name: null,
refKind: RefKind.None,
isInitOnly: false,
isStatic: false,
returnType: default,
refCustomModifiers: [],
explicitInterfaceImplementations: []);

LookupOptions options = LookupOptions.AllMethodsOnArityZero | LookupOptions.MustNotBeParameter;
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = this.GetNewCompoundUseSiteInfo(diagnostics);
ArrayBuilder<Symbol>? sortedSymbolsBuilder = null;

foreach (var nested in container.GetTypeMembers())
{
if (!nested.IsExtension || nested.Arity != extensionArity || nested.ExtensionParameter is null)
{
continue;
}

var constructedNested = (NamedTypeSymbol)ConstructWithCrefTypeParameters(extensionArity, extensionTypeArguments, nested);

var candidateExtensionSignature = new SignatureOnlyMethodSymbol(
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

var candidateExtensionSignature = new SignatureOnlyMethodSymbol(

Consider adding a comment that we are using signature method symbols to match extension blocks #Closed

methodKind: MethodKind.Ordinary,
typeParameters: IndexedTypeParameterSymbol.TakeSymbols(constructedNested.Arity),
parameters: [constructedNested.ExtensionParameter],
callingConvention: Cci.CallingConvention.Default,
// These are ignored by this specific MemberSignatureComparer.
containingType: null,
name: null,
refKind: RefKind.None,
isInitOnly: false,
isStatic: false,
returnType: default,
refCustomModifiers: [],
explicitInterfaceImplementations: []);

if (!MemberSignatureComparer.CrefComparer.Equals(candidateExtensionSignature, providedExtensionSignature))
{
continue;
}

var candidates = constructedNested.GetMembers(name);

foreach (var candidate in candidates)
{
if (!SourceMemberContainerTypeSymbol.IsAllowedExtensionMember(candidate))
{
continue;
}

if (arity != 0 && candidate.GetArity() != arity)
{
continue;
}

// Note: we bypass the arity check here, as it would check for total arity (extension + member arity)
SingleLookupResult result = this.CheckViability(candidate, arity: 0, options, accessThroughType: null, diagnose: true, useSiteInfo: ref useSiteInfo);

if (result.Kind == LookupResultKind.Viable)
{
sortedSymbolsBuilder ??= ArrayBuilder<Symbol>.GetInstance();
sortedSymbolsBuilder.Add(result.Symbol);
}
}
}

diagnostics.Add(syntax, useSiteInfo);

if (sortedSymbolsBuilder is null)
{
return ImmutableArray<Symbol>.Empty;
}

// Since we resolve ambiguities by just picking the first symbol we encounter,
// the order of the symbols matters for repeatability.
if (sortedSymbolsBuilder.Count > 1)
{
sortedSymbolsBuilder.Sort(ConsistentSymbolOrder.Instance);
}

return sortedSymbolsBuilder.ToImmutableAndFree();
}
}

// NOTE: not guaranteed to be a method (e.g. class op_Addition)
// NOTE: constructor fallback logic applies
private ImmutableArray<Symbol> BindOperatorMemberCref(OperatorMemberCrefSyntax syntax, NamespaceOrTypeSymbol? containerOpt, out Symbol? ambiguityWinner, BindingDiagnosticBag diagnostics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private void AddTypeParameters(TypeSyntax typeSyntax, MultiDictionary<string, Ty
AddTypeParameters(qualifiedNameSyntax.Left, map);
break;
case SyntaxKind.GenericName:
AddTypeParameters((GenericNameSyntax)typeSyntax, map);
AddTypeParameters(((GenericNameSyntax)typeSyntax).TypeArgumentList.Arguments, map);
break;
case SyntaxKind.IdentifierName:
case SyntaxKind.PredefinedType:
Expand All @@ -103,20 +103,28 @@ private void AddTypeParameters(TypeSyntax typeSyntax, MultiDictionary<string, Ty
private void AddTypeParameters(MemberCrefSyntax memberSyntax, MultiDictionary<string, TypeParameterSymbol> map)
{
// Other members have arity 0.
if (memberSyntax.Kind() == SyntaxKind.NameMemberCref)
if (memberSyntax is NameMemberCrefSyntax nameMemberCref)
{
AddTypeParameters(((NameMemberCrefSyntax)memberSyntax).Name, map);
AddTypeParameters(nameMemberCref.Name, map);
}
else if (memberSyntax is ExtensionMemberCrefSyntax extensionCref)
{
if (extensionCref.TypeArgumentList is { } extensionTypeArguments)
{
AddTypeParameters(extensionTypeArguments.Arguments, map);
}

AddTypeParameters(extensionCref.Member, map);
}
}

private static void AddTypeParameters(GenericNameSyntax genericNameSyntax, MultiDictionary<string, TypeParameterSymbol> map)
private static void AddTypeParameters(SeparatedSyntaxList<TypeSyntax> typeArguments, MultiDictionary<string, TypeParameterSymbol> map)
{
// NOTE: Dev11 does not warn about duplication, it just matches parameter types to the
// *last* type parameter with the same name. That's why we're iterating backwards and
// skipping subsequent symbols with the same name. This can result in some surprising
// behavior. For example, both 'T's in "A<T>.B<T>" bind to the second implicitly
// declared type parameter.
SeparatedSyntaxList<TypeSyntax> typeArguments = genericNameSyntax.TypeArgumentList.Arguments;
for (int i = typeArguments.Count - 1; i >= 0; i--)
{
// Other types (non-identifiers) are allowed in error scenarios, but they do not introduce new
Expand Down
5 changes: 4 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -8096,7 +8096,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<value>Extension declarations may not have a name.</value>
</data>
<data name="ERR_ExtensionDisallowsMember" xml:space="preserve">
<value>Extension declarations can include only methods or properties</value>
<value>This member is not allowed in an extension block</value>
</data>
<data name="ERR_BadExtensionContainingType" xml:space="preserve">
<value>Extensions must be declared in a top-level, non-generic, static class</value>
Expand Down Expand Up @@ -8185,4 +8185,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_PPShebangInProjectBasedProgram" xml:space="preserve">
<value>'#!' directives can be only used in scripts or file-based programs</value>
</data>
<data name="ERR_MisplacedExtension" xml:space="preserve">
<value>An extension member syntax is disallowed in nested position within an extension member syntax</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,8 @@ internal static bool HasParameterList(CrefSyntax crefSyntax)
return ((OperatorMemberCrefSyntax)crefSyntax).Parameters != null;
case SyntaxKind.ConversionOperatorMemberCref:
return ((ConversionOperatorMemberCrefSyntax)crefSyntax).Parameters != null;
case SyntaxKind.ExtensionMemberCref:
return HasParameterList(((ExtensionMemberCrefSyntax)crefSyntax).Member);
Copy link
Contributor

@AlekseyTs AlekseyTs May 30, 2025

Choose a reason for hiding this comment

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

return HasParameterList(((ExtensionMemberCrefSyntax)crefSyntax).Member);

What about ExtensionMemberCrefSyntax.Parameters? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Parameters on extension are intentionally not considered here.
The value from HasParameterList affects the result kind from GetCrefSymbolInfo below, to report "ambiguous" vs. "overload resolution failure". In E.extension(int).M... we only allow the last parameter list to be omitted, so the presence/absence of the parameter list on extension is not relevant to was result kind we report.

}

return false;
Expand All @@ -379,7 +381,7 @@ private static SymbolInfo GetCrefSymbolInfo(OneOrMany<Symbol> symbols, SymbolInf

LookupResultKind resultKind = LookupResultKind.Ambiguous;

// The boundary between Ambiguous and OverloadResolutionFailure is let clear-cut for crefs.
// The boundary between Ambiguous and OverloadResolutionFailure is less clear-cut for crefs.
// We'll say that overload resolution failed if the syntax has a parameter list and if
// all of the candidates have the same kind.
SymbolKind firstCandidateKind = symbols[0].Kind;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public override void DefaultVisit(SyntaxNode node)

// Do this for the diagnostics, even if it won't be written.
BindingDiagnosticBag diagnostics = diagnose ? _diagnostics : BindingDiagnosticBag.GetInstance(withDiagnostics: false, withDependencies: _diagnostics.AccumulatesDependencies);
string docCommentId = GetDocumentationCommentId(cref, binder, diagnostics);
string docCommentId = GetEscapedDocumentationCommentId(cref, binder, diagnostics);

if (!diagnose)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ private void BindAndReplaceCref(XAttribute attribute, CSharpSyntaxNode originati
Binder binder = BinderFactory.MakeCrefBinder(crefSyntax, memberDeclSyntax, _compilation.GetBinderFactory(memberDeclSyntax.SyntaxTree));

var crefDiagnostics = BindingDiagnosticBag.GetInstance(_diagnostics);
attribute.Value = GetDocumentationCommentId(crefSyntax, binder, crefDiagnostics); // NOTE: mutation (element must be a copy)
attribute.Value = GetEscapedDocumentationCommentId(crefSyntax, binder, crefDiagnostics); // NOTE: mutation (element must be a copy)
RecordBindingDiagnostics(crefDiagnostics, sourceLocation); // Respects DocumentationMode.
crefDiagnostics.Free();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ public override void VisitMethod(MethodSymbol symbol)
return;
}

WriteLine("<member name=\"{0}\">", symbol.GetDocumentationCommentId());
WriteLine("<member name=\"{0}\">", symbol.GetEscapedDocumentationCommentId());
Indent();
WriteLine("<inheritdoc cref=\"{0}\"/>", symbolForDocComment.GetDocumentationCommentId());
WriteLine("<inheritdoc cref=\"{0}\"/>", symbolForDocComment.GetEscapedDocumentationCommentId());
Unindent();
WriteLine("</member>");
return;
Expand Down Expand Up @@ -331,7 +331,7 @@ public override void DefaultVisit(Symbol symbol)
// If the XML in any of the doc comments is invalid, skip all further processing (for this symbol) and
// just write a comment saying that info was lost for this symbol.
string message = ErrorFacts.GetMessage(MessageID.IDS_XMLIGNORED, CultureInfo.CurrentUICulture);
WriteLine(string.Format(CultureInfo.CurrentUICulture, message, symbol.GetDocumentationCommentId()));
WriteLine(string.Format(CultureInfo.CurrentUICulture, message, symbol.GetEscapedDocumentationCommentId()));
return;
}

Expand Down Expand Up @@ -381,7 +381,7 @@ public override void DefaultVisit(Symbol symbol)
// If the XML in any of the doc comments is invalid, skip all further processing (for this symbol) and
// just write a comment saying that info was lost for this symbol.
string message = ErrorFacts.GetMessage(MessageID.IDS_XMLIGNORED, CultureInfo.CurrentUICulture);
WriteLine(string.Format(CultureInfo.CurrentUICulture, message, symbol.GetDocumentationCommentId()));
WriteLine(string.Format(CultureInfo.CurrentUICulture, message, symbol.GetEscapedDocumentationCommentId()));
return;
}

Expand Down Expand Up @@ -467,7 +467,7 @@ private bool TryProcessRecordPropertyDocumentation(
Debug.Assert(paramTags.Count > 0);

BeginTemporaryString();
WriteLine("<member name=\"{0}\">", recordPropertySymbol.GetDocumentationCommentId());
WriteLine("<member name=\"{0}\">", recordPropertySymbol.GetEscapedDocumentationCommentId());
Indent();
var substitutedTextBuilder = PooledStringBuilder.GetInstance();
var includeElementNodesBuilder = _processIncludes ? ArrayBuilder<CSharpSyntaxNode>.GetInstance() : null;
Expand Down Expand Up @@ -573,7 +573,7 @@ private bool TryProcessDocumentationCommentTriviaNodes(
// because we need to have XML to process.
if (!shouldSkipPartialDefinitionComments || _processIncludes)
{
WriteLine("<member name=\"{0}\">", symbol.GetDocumentationCommentId());
WriteLine("<member name=\"{0}\">", symbol.GetEscapedDocumentationCommentId());
Indent();
}

Expand Down Expand Up @@ -1096,7 +1096,7 @@ private static string LongestCommonPrefix(string str1, string str2)
/// <remarks>
/// Does not respect DocumentationMode, so use a temporary bag if diagnostics are not desired.
/// </remarks>
private static string GetDocumentationCommentId(CrefSyntax crefSyntax, Binder binder, BindingDiagnosticBag diagnostics)
private static string GetEscapedDocumentationCommentId(CrefSyntax crefSyntax, Binder binder, BindingDiagnosticBag diagnostics)
{
if (crefSyntax.ContainsDiagnostics)
{
Expand Down Expand Up @@ -1135,7 +1135,7 @@ private static string GetDocumentationCommentId(CrefSyntax crefSyntax, Binder bi
diagnostics.AddDependencies(symbol as TypeSymbol ?? symbol.ContainingType);
}

return symbol.OriginalDefinition.GetDocumentationCommentId();
return symbol.OriginalDefinition.GetEscapedDocumentationCommentId();
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2400,7 +2400,7 @@ internal enum ErrorCode
ERR_ExpressionTreeContainsNamedArgumentOutOfPosition = 9307,

ERR_OperatorsMustBePublic = 9308,
// available 9309,
ERR_MisplacedExtension = 9309,
ERR_OperatorMustReturnVoid = 9310,
ERR_CloseUnimplementedInterfaceMemberOperatorMismatch = 9311,
ERR_OperatorMismatchOnOverride = 9312,
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@ or ErrorCode.ERR_PartialWrongTypeParams
or ErrorCode.ERR_PartialWrongConstraints
or ErrorCode.ERR_NoImplicitConvCast
or ErrorCode.ERR_PartialMisplaced
or ErrorCode.ERR_MisplacedExtension
or ErrorCode.ERR_ImportedCircularBase
or ErrorCode.ERR_UseDefViolationOut
or ErrorCode.ERR_ArraySizeInDeclaration
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Generated/CSharp.Generated.g4

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading