Remove need to pass generator parameter to helper methods #76627
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
dotnet-policy-service
bot
added
VSCode
Needs API Review
| @@ -3134,17 +3134,6 @@ private static StatementSyntax AsStatement(SyntaxNode node) | |||
| public override SyntaxNode ExpressionStatement(SyntaxNode expression) | |||
| => SyntaxFactory.ExpressionStatement((ExpressionSyntax)expression); | |||
|
|
|||
| internal override SyntaxNode MemberAccessExpressionWorker(SyntaxNode? expression, SyntaxNode simpleName) | |||
There was a problem hiding this comment.
lots of impls moved to CSharpSyntaxGeneratorInternal.
| @@ -1776,7 +1776,9 @@ internal SyntaxNode InterpolationFormatClause(string format) | |||
| /// This is typically a null value for reference types or a zero-filled value for value types. | |||
| /// </summary> | |||
| public abstract SyntaxNode DefaultExpression(SyntaxNode type); | |||
| public abstract SyntaxNode DefaultExpression(ITypeSymbol type); | |||
There was a problem hiding this comment.
note: it is safe to make an abstract method a non-virtual method in a shipped API. That's because:
- callers to this already use callvirt, which works fine on an instance method.
- this is a type that is not publicly extendible (it has internal constructors). So no one but us could have been overriding this.
| Microsoft.CodeAnalysis.Editing.SyntaxGenerator.ConvertExpression(Microsoft.CodeAnalysis.SyntaxNode type, Microsoft.CodeAnalysis.SyntaxNode expression) -> Microsoft.CodeAnalysis.SyntaxNode | ||
| Microsoft.CodeAnalysis.Editing.SyntaxGenerator.DefaultExpression(Microsoft.CodeAnalysis.ITypeSymbol type) -> Microsoft.CodeAnalysis.SyntaxNode | ||
| Microsoft.CodeAnalysis.Editing.SyntaxGenerator.IdentifierName(string identifier) -> Microsoft.CodeAnalysis.SyntaxNode | ||
| Microsoft.CodeAnalysis.Editing.SyntaxGenerator.MemberAccessExpression(Microsoft.CodeAnalysis.SyntaxNode expression, Microsoft.CodeAnalysis.SyntaxNode memberName) -> Microsoft.CodeAnalysis.SyntaxNode |
There was a problem hiding this comment.
all these changes are safe, and just reflect these methods no longer being abstract. they are now instnace methods that defer to methods on SyntaxGeneratorInternal.
There was a problem hiding this comment.
(this also means this needs no public API review as there is no actual effective public api change).
| @@ -16,14 +15,16 @@ private CSharpFlagsEnumGenerator() | |||
| { | |||
| } | |||
|
|
|||
| protected override SyntaxGeneratorInternal SyntaxGenerator | |||
| => CSharpSyntaxGeneratorInternal.Instance; | |||
There was a problem hiding this comment.
instead of the flags-generator needing to be passed the syntax-generator, it can jsut ask its subclass to provide it. t he subclass provides the internal impl (which can be used from workspace/code-fixes layer), as it supplies everything needed.
| @@ -239,4 +241,97 @@ public override SyntaxNode UnaryPattern(SyntaxToken operatorToken, SyntaxNode pa | |||
| => SyntaxFactory.UnaryPattern(operatorToken, (PatternSyntax)Parenthesize(pattern)); | |||
|
|
|||
| #endregion | |||
|
|
|||
| public override SyntaxNode CastExpression(SyntaxNode type, SyntaxNode expression) | |||
There was a problem hiding this comment.
these are all moves. From CSharpSyntaxGenerator to CSharpSyntaxGeneratorInternal.
|
@ToddGrun @JoeRobich ptal |
|
@Cosifne ptal :) |
Found while doing extract method work. We have a lot of helpers that have to contort themselves to pass other helpers around due to the split between public/internal in syntax generator. Addressed this by makign sure the funcitonality we need is available on both interfaces, with the public side always deferring to the internal side (which can be accessed through a static singleton instance).