Skip to content
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

Jump to bottom

Allow parsing ref readonly parameters in cref #69104

Merged
merged 9 commits into from
Jul 26, 2023
Merged

Allow parsing ref readonly parameters in cref #69104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d2c7205
Allow parsing `ref readonly` parameters in `cref`
jjonescz Jul 19, 2023
4ec46b2
Test `readonly ref` in `cref`
jjonescz Jul 20, 2023
3cf3671
Attach unexpected readonly in cref as trivia
jjonescz Jul 20, 2023
c64e631
Preserve previous Update method
jjonescz Jul 21, 2023
ad4962d
Remove assert to support manually created invalid nodes
jjonescz Jul 21, 2023
c10bd30
Check feature availability when binding `cref`
jjonescz Jul 21, 2023
01edf55
Test more invalid cref permutations
jjonescz Jul 21, 2023
e630934
Turn errors into warnings for doc comments
jjonescz Jul 21, 2023
e1a7480
Assert that errors are not added to missing nodes via the new overload
jjonescz Jul 21, 2023
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
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,11 @@ private ImmutableArray<ParameterSymbol> BindCrefParameters(BaseCrefParameterList
foreach (CrefParameterSyntax parameter in parameterListSyntax.Parameters)
{
RefKind refKind = parameter.RefKindKeyword.Kind().GetRefKind();
if (refKind == RefKind.Ref && parameter.ReadOnlyKeyword.IsKind(SyntaxKind.ReadOnlyKeyword))
{
CheckFeatureAvailability(parameter.ReadOnlyKeyword, MessageID.IDS_FeatureRefReadonlyParameters, diagnostics, forceWarning: true);
refKind = RefKind.RefReadOnlyParameter;
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
}

Debug.Assert(parameterListSyntax.Parent is object);
TypeSymbol type = BindCrefParameterOrReturnType(parameter.Type, (MemberCrefSyntax)parameterListSyntax.Parent, diagnostics);
Expand Down
20 changes: 14 additions & 6 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2696,8 +2696,8 @@ protected AssemblySymbol GetForwardedToAssembly(string name, int arity, ref Name
internal static bool CheckFeatureAvailability(SyntaxNode syntax, MessageID feature, BindingDiagnosticBag diagnostics, Location? location = null)
=> CheckFeatureAvailability(syntax, feature, diagnostics.DiagnosticBag, location);

internal static bool CheckFeatureAvailability(SyntaxToken syntax, MessageID feature, BindingDiagnosticBag diagnostics, Location? location = null)
=> CheckFeatureAvailability(syntax, feature, diagnostics.DiagnosticBag, location);
internal static bool CheckFeatureAvailability(SyntaxToken syntax, MessageID feature, BindingDiagnosticBag diagnostics, Location? location = null, bool forceWarning = false)
=> CheckFeatureAvailability(syntax, feature, diagnostics.DiagnosticBag, location, forceWarning: forceWarning);

internal static bool CheckFeatureAvailability(SyntaxNodeOrToken syntax, MessageID feature, BindingDiagnosticBag diagnostics, Location? location = null)
=> CheckFeatureAvailability(syntax, feature, diagnostics.DiagnosticBag, location);
Expand All @@ -2708,8 +2708,8 @@ internal static bool CheckFeatureAvailability(SyntaxTree tree, MessageID feature
private static bool CheckFeatureAvailability(SyntaxNode syntax, MessageID feature, DiagnosticBag? diagnostics, Location? location = null)
=> CheckFeatureAvailability(syntax.SyntaxTree, feature, diagnostics, (location, syntax), static tuple => tuple.location ?? tuple.syntax.GetLocation());

private static bool CheckFeatureAvailability(SyntaxToken syntax, MessageID feature, DiagnosticBag? diagnostics, Location? location = null)
=> CheckFeatureAvailability(syntax.SyntaxTree!, feature, diagnostics, (location, syntax), static tuple => tuple.location ?? tuple.syntax.GetLocation());
private static bool CheckFeatureAvailability(SyntaxToken syntax, MessageID feature, DiagnosticBag? diagnostics, Location? location = null, bool forceWarning = false)
=> CheckFeatureAvailability(syntax.SyntaxTree!, feature, diagnostics, (location, syntax), static tuple => tuple.location ?? tuple.syntax.GetLocation(), forceWarning: forceWarning);

private static bool CheckFeatureAvailability(SyntaxNodeOrToken syntax, MessageID feature, DiagnosticBag? diagnostics, Location? location = null)
=> CheckFeatureAvailability(syntax.SyntaxTree!, feature, diagnostics, (location, syntax), static tuple => tuple.location ?? tuple.syntax.GetLocation()!);
Expand All @@ -2720,11 +2720,19 @@ private static bool CheckFeatureAvailability(SyntaxTree tree, MessageID feature,
/// <param name="getLocation">Callback function that computes the location to report the diagnostics at
/// <em>if</em> a diagnostic should be reported. Should always be passed a static/cached callback to prevent
/// allocations of the delegate.</param>
private static bool CheckFeatureAvailability<TData>(SyntaxTree tree, MessageID feature, DiagnosticBag? diagnostics, TData data, Func<TData, Location> getLocation)
private static bool CheckFeatureAvailability<TData>(SyntaxTree tree, MessageID feature, DiagnosticBag? diagnostics, TData data, Func<TData, Location> getLocation, bool forceWarning = false)
{
if (feature.GetFeatureAvailabilityDiagnosticInfo((CSharpParseOptions)tree.Options) is { } diagInfo)
{
diagnostics?.Add(diagInfo, getLocation(data));
if (forceWarning)
{
diagnostics?.Add(ErrorCode.WRN_ErrorOverride, getLocation(data), diagInfo, (int)diagInfo.Code);
}
else
{
diagnostics?.Add(diagInfo, getLocation(data));
}

return false;
}
return true;
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Generated/CSharp.Generated.g4
View file
Open in desktop

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

86 changes: 65 additions & 21 deletions ...arpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30195,62 +30195,81 @@ static CrefBracketedParameterListSyntax()

/// <summary>
/// An element of a BaseCrefParameterListSyntax.
/// Unlike a regular parameter, a cref parameter has only an optional ref or out keyword and a type -
/// Unlike a regular parameter, a cref parameter has only an optional ref, in, out keyword,
/// an optional readonly keyword, and a type -
/// there is no name and there are no attributes or other modifiers.
/// </summary>
internal sealed partial class CrefParameterSyntax : CSharpSyntaxNode
{
internal readonly SyntaxToken? refKindKeyword;
internal readonly SyntaxToken? readOnlyKeyword;
Copy link
Member

Choose a reason for hiding this comment

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

another, possibly cleaner, way to do this is to just changet this to a TokenList of Modifiers.

internal readonly TypeSyntax type;

internal CrefParameterSyntax(SyntaxKind kind, SyntaxToken? refKindKeyword, TypeSyntax type, DiagnosticInfo[]? diagnostics, SyntaxAnnotation[]? annotations)
internal CrefParameterSyntax(SyntaxKind kind, SyntaxToken? refKindKeyword, SyntaxToken? readOnlyKeyword, TypeSyntax type, DiagnosticInfo[]? diagnostics, SyntaxAnnotation[]? annotations)
: base(kind, diagnostics, annotations)
{
this.SlotCount = 2;
this.SlotCount = 3;
if (refKindKeyword != null)
{
this.AdjustFlagsAndWidth(refKindKeyword);
this.refKindKeyword = refKindKeyword;
}
if (readOnlyKeyword != null)
{
this.AdjustFlagsAndWidth(readOnlyKeyword);
this.readOnlyKeyword = readOnlyKeyword;
}
this.AdjustFlagsAndWidth(type);
this.type = type;
}

internal CrefParameterSyntax(SyntaxKind kind, SyntaxToken? refKindKeyword, TypeSyntax type, SyntaxFactoryContext context)
internal CrefParameterSyntax(SyntaxKind kind, SyntaxToken? refKindKeyword, SyntaxToken? readOnlyKeyword, TypeSyntax type, SyntaxFactoryContext context)
: base(kind)
{
this.SetFactoryContext(context);
this.SlotCount = 2;
this.SlotCount = 3;
if (refKindKeyword != null)
{
this.AdjustFlagsAndWidth(refKindKeyword);
this.refKindKeyword = refKindKeyword;
}
if (readOnlyKeyword != null)
{
this.AdjustFlagsAndWidth(readOnlyKeyword);
this.readOnlyKeyword = readOnlyKeyword;
}
this.AdjustFlagsAndWidth(type);
this.type = type;
}

internal CrefParameterSyntax(SyntaxKind kind, SyntaxToken? refKindKeyword, TypeSyntax type)
internal CrefParameterSyntax(SyntaxKind kind, SyntaxToken? refKindKeyword, SyntaxToken? readOnlyKeyword, TypeSyntax type)
: base(kind)
{
this.SlotCount = 2;
this.SlotCount = 3;
if (refKindKeyword != null)
{
this.AdjustFlagsAndWidth(refKindKeyword);
this.refKindKeyword = refKindKeyword;
}
if (readOnlyKeyword != null)
{
this.AdjustFlagsAndWidth(readOnlyKeyword);
this.readOnlyKeyword = readOnlyKeyword;
}
this.AdjustFlagsAndWidth(type);
this.type = type;
}

public SyntaxToken? RefKindKeyword => this.refKindKeyword;
public SyntaxToken? ReadOnlyKeyword => this.readOnlyKeyword;
public TypeSyntax Type => this.type;

internal override GreenNode? GetSlot(int index)
=> index switch
{
0 => this.refKindKeyword,
1 => this.type,
1 => this.readOnlyKeyword,
2 => this.type,
_ => null,
};

Expand All @@ -30259,11 +30278,11 @@ internal CrefParameterSyntax(SyntaxKind kind, SyntaxToken? refKindKeyword, TypeS
public override void Accept(CSharpSyntaxVisitor visitor) => visitor.VisitCrefParameter(this);
public override TResult Accept<TResult>(CSharpSyntaxVisitor<TResult> visitor) => visitor.VisitCrefParameter(this);

public CrefParameterSyntax Update(SyntaxToken refKindKeyword, TypeSyntax type)
public CrefParameterSyntax Update(SyntaxToken refKindKeyword, SyntaxToken readOnlyKeyword, TypeSyntax type)
{
if (refKindKeyword != this.RefKindKeyword || type != this.Type)
if (refKindKeyword != this.RefKindKeyword || readOnlyKeyword != this.ReadOnlyKeyword || type != this.Type)
{
var newNode = SyntaxFactory.CrefParameter(refKindKeyword, type);
var newNode = SyntaxFactory.CrefParameter(refKindKeyword, readOnlyKeyword, type);
var diags = GetDiagnostics();
if (diags?.Length > 0)
newNode = newNode.WithDiagnosticsGreen(diags);
Expand All @@ -30277,21 +30296,27 @@ public CrefParameterSyntax Update(SyntaxToken refKindKeyword, TypeSyntax type)
}

internal override GreenNode SetDiagnostics(DiagnosticInfo[]? diagnostics)
=> new CrefParameterSyntax(this.Kind, this.refKindKeyword, this.type, diagnostics, GetAnnotations());
=> new CrefParameterSyntax(this.Kind, this.refKindKeyword, this.readOnlyKeyword, this.type, diagnostics, GetAnnotations());

internal override GreenNode SetAnnotations(SyntaxAnnotation[]? annotations)
=> new CrefParameterSyntax(this.Kind, this.refKindKeyword, this.type, GetDiagnostics(), annotations);
=> new CrefParameterSyntax(this.Kind, this.refKindKeyword, this.readOnlyKeyword, this.type, GetDiagnostics(), annotations);

internal CrefParameterSyntax(ObjectReader reader)
: base(reader)
{
this.SlotCount = 2;
this.SlotCount = 3;
var refKindKeyword = (SyntaxToken?)reader.ReadValue();
if (refKindKeyword != null)
{
AdjustFlagsAndWidth(refKindKeyword);
this.refKindKeyword = refKindKeyword;
}
var readOnlyKeyword = (SyntaxToken?)reader.ReadValue();
if (readOnlyKeyword != null)
{
AdjustFlagsAndWidth(readOnlyKeyword);
this.readOnlyKeyword = readOnlyKeyword;
}
var type = (TypeSyntax)reader.ReadValue();
AdjustFlagsAndWidth(type);
this.type = type;
Expand All @@ -30301,6 +30326,7 @@ internal override void WriteTo(ObjectWriter writer)
{
base.WriteTo(writer);
writer.WriteValue(this.refKindKeyword);
writer.WriteValue(this.readOnlyKeyword);
writer.WriteValue(this.type);
}

Expand Down Expand Up @@ -35985,7 +36011,7 @@ public override CSharpSyntaxNode VisitCrefBracketedParameterList(CrefBracketedPa
=> node.Update((SyntaxToken)Visit(node.OpenBracketToken), VisitList(node.Parameters), (SyntaxToken)Visit(node.CloseBracketToken));

public override CSharpSyntaxNode VisitCrefParameter(CrefParameterSyntax node)
=> node.Update((SyntaxToken)Visit(node.RefKindKeyword), (TypeSyntax)Visit(node.Type));
=> node.Update((SyntaxToken)Visit(node.RefKindKeyword), (SyntaxToken)Visit(node.ReadOnlyKeyword), (TypeSyntax)Visit(node.Type));

public override CSharpSyntaxNode VisitXmlElement(XmlElementSyntax node)
=> node.Update((XmlElementStartTagSyntax)Visit(node.StartTag), VisitList(node.Content), (XmlElementEndTagSyntax)Visit(node.EndTag));
Expand Down Expand Up @@ -40645,7 +40671,7 @@ public CrefBracketedParameterListSyntax CrefBracketedParameterList(SyntaxToken o
return result;
}

public CrefParameterSyntax CrefParameter(SyntaxToken? refKindKeyword, TypeSyntax type)
public CrefParameterSyntax CrefParameter(SyntaxToken? refKindKeyword, SyntaxToken? readOnlyKeyword, TypeSyntax type)
{
#if DEBUG
if (refKindKeyword != null)
Expand All @@ -40659,14 +40685,23 @@ public CrefParameterSyntax CrefParameter(SyntaxToken? refKindKeyword, TypeSyntax
default: throw new ArgumentException(nameof(refKindKeyword));
}
}
if (readOnlyKeyword != null)
{
switch (readOnlyKeyword.Kind)
{
case SyntaxKind.ReadOnlyKeyword:
case SyntaxKind.None: break;
default: throw new ArgumentException(nameof(readOnlyKeyword));
}
}
if (type == null) throw new ArgumentNullException(nameof(type));
#endif

int hash;
var cached = CSharpSyntaxNodeCache.TryGetNode((int)SyntaxKind.CrefParameter, refKindKeyword, type, this.context, out hash);
var cached = CSharpSyntaxNodeCache.TryGetNode((int)SyntaxKind.CrefParameter, refKindKeyword, readOnlyKeyword, type, this.context, out hash);
if (cached != null) return (CrefParameterSyntax)cached;

var result = new CrefParameterSyntax(SyntaxKind.CrefParameter, refKindKeyword, type, this.context);
var result = new CrefParameterSyntax(SyntaxKind.CrefParameter, refKindKeyword, readOnlyKeyword, type, this.context);
if (hash >= 0)
{
SyntaxNodeCache.AddNode(result, hash);
Expand Down Expand Up @@ -45852,7 +45887,7 @@ public static CrefBracketedParameterListSyntax CrefBracketedParameterList(Syntax
return result;
}

public static CrefParameterSyntax CrefParameter(SyntaxToken? refKindKeyword, TypeSyntax type)
public static CrefParameterSyntax CrefParameter(SyntaxToken? refKindKeyword, SyntaxToken? readOnlyKeyword, TypeSyntax type)
{
#if DEBUG
if (refKindKeyword != null)
Expand All @@ -45866,14 +45901,23 @@ public static CrefParameterSyntax CrefParameter(SyntaxToken? refKindKeyword, Typ
default: throw new ArgumentException(nameof(refKindKeyword));
}
}
if (readOnlyKeyword != null)
{
switch (readOnlyKeyword.Kind)
{
case SyntaxKind.ReadOnlyKeyword:
case SyntaxKind.None: break;
default: throw new ArgumentException(nameof(readOnlyKeyword));
}
}
if (type == null) throw new ArgumentNullException(nameof(type));
#endif

int hash;
var cached = SyntaxNodeCache.TryGetNode((int)SyntaxKind.CrefParameter, refKindKeyword, type, out hash);
var cached = SyntaxNodeCache.TryGetNode((int)SyntaxKind.CrefParameter, refKindKeyword, readOnlyKeyword, type, out hash);
if (cached != null) return (CrefParameterSyntax)cached;

var result = new CrefParameterSyntax(SyntaxKind.CrefParameter, refKindKeyword, type);
var result = new CrefParameterSyntax(SyntaxKind.CrefParameter, refKindKeyword, readOnlyKeyword, type);
if (hash >= 0)
{
SyntaxNodeCache.AddNode(result, hash);
Expand Down
18 changes: 14 additions & 4 deletions .../CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Main.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2089,7 +2089,7 @@ public partial class CSharpSyntaxRewriter : CSharpSyntaxVisitor<SyntaxNode?>
=> node.Update(VisitToken(node.OpenBracketToken), VisitList(node.Parameters), VisitToken(node.CloseBracketToken));

public override SyntaxNode? VisitCrefParameter(CrefParameterSyntax node)
=> node.Update(VisitToken(node.RefKindKeyword), (TypeSyntax?)Visit(node.Type) ?? throw new ArgumentNullException("type"));
=> node.Update(VisitToken(node.RefKindKeyword), VisitToken(node.ReadOnlyKeyword), (TypeSyntax?)Visit(node.Type) ?? throw new ArgumentNullException("type"));

public override SyntaxNode? VisitXmlElement(XmlElementSyntax node)
=> node.Update((XmlElementStartTagSyntax?)Visit(node.StartTag) ?? throw new ArgumentNullException("startTag"), VisitList(node.Content), (XmlElementEndTagSyntax?)Visit(node.EndTag) ?? throw new ArgumentNullException("endTag"));
Expand Down Expand Up @@ -5860,7 +5860,7 @@ public static CrefBracketedParameterListSyntax CrefBracketedParameterList(Separa
=> SyntaxFactory.CrefBracketedParameterList(SyntaxFactory.Token(SyntaxKind.OpenBracketToken), parameters, SyntaxFactory.Token(SyntaxKind.CloseBracketToken));

/// <summary>Creates a new CrefParameterSyntax instance.</summary>
public static CrefParameterSyntax CrefParameter(SyntaxToken refKindKeyword, TypeSyntax type)
public static CrefParameterSyntax CrefParameter(SyntaxToken refKindKeyword, SyntaxToken readOnlyKeyword, TypeSyntax type)
{
switch (refKindKeyword.Kind())
{
Expand All @@ -5870,13 +5870,23 @@ public static CrefParameterSyntax CrefParameter(SyntaxToken refKindKeyword, Type
case SyntaxKind.None: break;
default: throw new ArgumentException(nameof(refKindKeyword));
}
switch (readOnlyKeyword.Kind())
{
case SyntaxKind.ReadOnlyKeyword:
case SyntaxKind.None: break;
default: throw new ArgumentException(nameof(readOnlyKeyword));
}
if (type == null) throw new ArgumentNullException(nameof(type));
return (CrefParameterSyntax)Syntax.InternalSyntax.SyntaxFactory.CrefParameter((Syntax.InternalSyntax.SyntaxToken?)refKindKeyword.Node, (Syntax.InternalSyntax.TypeSyntax)type.Green).CreateRed();
return (CrefParameterSyntax)Syntax.InternalSyntax.SyntaxFactory.CrefParameter((Syntax.InternalSyntax.SyntaxToken?)refKindKeyword.Node, (Syntax.InternalSyntax.SyntaxToken?)readOnlyKeyword.Node, (Syntax.InternalSyntax.TypeSyntax)type.Green).CreateRed();
}

/// <summary>Creates a new CrefParameterSyntax instance.</summary>
public static CrefParameterSyntax CrefParameter(SyntaxToken refKindKeyword, TypeSyntax type)
=> SyntaxFactory.CrefParameter(refKindKeyword, default, type);

/// <summary>Creates a new CrefParameterSyntax instance.</summary>
public static CrefParameterSyntax CrefParameter(TypeSyntax type)
=> SyntaxFactory.CrefParameter(default, type);
=> SyntaxFactory.CrefParameter(default, default, type);

/// <summary>Creates a new XmlElementSyntax instance.</summary>
public static XmlElementSyntax XmlElement(XmlElementStartTagSyntax startTag, SyntaxList<XmlNodeSyntax> content, XmlElementEndTagSyntax endTag)
Expand Down
Loading