Fix crash in 'use object/collection initializer' in VB. #70028
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -89,6 +89,7 @@ | |||
| <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs" /> | |||
There was a problem hiding this comment.
view PR with whitespace off.
| public static bool IsInitializerOfLocalDeclarationStatement( | ||
| LocalDeclarationStatementSyntax localDeclarationStatement, | ||
| BaseObjectCreationExpressionSyntax rootExpression, | ||
| [NotNullWhen(true)] out VariableDeclaratorSyntax? variableDeclarator) |
There was a problem hiding this comment.
crux of the change was that we had some suspect code working at the raw node level, trying to understand c# and vb nodes in a uniform way. That code was simply broken. We now defer to each language to handle this case as VB has some unique constructs we can't handle uniformly.
There was a problem hiding this comment.
this function is shared between UseCollectionInitializer and UseObjectInitializer, which is why it's a shared helper they can both use.
| ExpressionStatementSyntax, | ||
| LocalDeclarationStatementSyntax, | ||
| VariableDeclaratorSyntax, | ||
| CSharpUseNamedMemberInitializerAnalyzer> |
There was a problem hiding this comment.
previously UseNamedMemberInitializerAnalyzer was a concrete type that worked on both C# and VB. but it was handling this case wrong, and each language has specific logic it needs. So this moved to become an abstract type, and there is now a C# and VB specific subclass.
| CSharpUseNamedMemberInitializerAnalyzer> | ||
| { | ||
| protected override bool IsInitializerOfLocalDeclarationStatement(LocalDeclarationStatementSyntax localDeclarationStatement, BaseObjectCreationExpressionSyntax rootExpression, out VariableDeclaratorSyntax variableDeclarator) | ||
| => CSharpObjectCreationHelpers.IsInitializerOfLocalDeclarationStatement(localDeclarationStatement, rootExpression, out variableDeclarator); |
There was a problem hiding this comment.
calls into shared helper.
| @@ -24,6 +24,7 @@ internal sealed class CSharpUseCollectionInitializerDiagnosticAnalyzer : | |||
| MemberAccessExpressionSyntax, | |||
| InvocationExpressionSyntax, | |||
| ExpressionStatementSyntax, | |||
| LocalDeclarationStatementSyntax, | |||
There was a problem hiding this comment.
passing this around as a strongly typed type-argument so that the signatures are correct in each language.
| TStatementSyntax containingStatement, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| if (containingStatement is not TLocalDeclarationStatementSyntax localDeclarationStatement) |
There was a problem hiding this comment.
this function update is the meat of the change. Previously it made assumptions about the shape that didn't hold in VB. now it calls into each language to deal with the specific shapes, and only uses shared code once it knows it can.
| { | ||
| var seenInvocation = false; | ||
| var seenIndexAssignment = false; | ||
| protected sealed override bool TryAddMatches( |
There was a problem hiding this comment.
sealed members in abstract classes to make inheritance easier to reason about.
| internal class UseNamedMemberInitializerAnalyzer< | ||
| namespace Microsoft.CodeAnalysis.UseObjectInitializer; | ||
|
|
||
| internal abstract class AbstractUseNamedMemberInitializerAnalyzer< |
There was a problem hiding this comment.
this pattern matches what we do in AbstractUseCOllectionInitializerAnalyzer. It's verbose in terms of the decl here and the generic constraints, but it makes the typing of all the subclasses nicer.
|
|
||
| Imports Microsoft.CodeAnalysis.VisualBasic.Syntax | ||
|
|
||
| Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer |
There was a problem hiding this comment.
VB impl of the helper.
| Imports Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer | ||
|
|
||
| Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer | ||
| Friend NotInheritable Class VisualBasicUseNamedMemberInitializerAnalyzer |
There was a problem hiding this comment.
VB impl of the abstract analyzer type.
|
@ryzngard ptal. Thanks! |
| protected abstract bool IsInitializerOfLocalDeclarationStatement( | ||
| TLocalDeclarationStatementSyntax localDeclarationStatement, TObjectCreationExpressionSyntax rootExpression, [NotNullWhen(true)] out TVariableDeclaratorSyntax? variableDeclarator); | ||
|
|
||
| private static readonly ObjectPool<TAnalyzer> s_pool = SharedPools.Default<TAnalyzer>(); |
There was a problem hiding this comment.
this (and the two methods below) moved up to here from derived type.
| TVariableDeclaratorSyntax, | ||
| TAnalyzer>, new() | ||
| { | ||
| private static readonly ObjectPool<TAnalyzer> s_pool = SharedPools.Default<TAnalyzer>(); |
There was a problem hiding this comment.
move to base type. works for 'use collection init' and 'use object init'.
ghost
added this to the 
Fixes #70015