Fix NRE in Generate Constructor from VB (#51239)

src/EditorFeatures/VisualBasicTest/GenerateConstructor/GenerateConstructorTests.vb

@@ -2179,5 +2179,117 @@ Class C
 End Class
 ")
         End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateConstructor)>
+        <WorkItem(51040, "https://github.com/dotnet/roslyn/issues/51040")>
+        Public Async Function TestOmittedParameter() As Task
+
+            Await TestInRegularAndScriptAsync(
+"Class C
+    Private _a As Integer
+
+    Public Sub New(Optional a As Integer = 1)
+        Me._a = a
+    End Sub
+
+    Public Function M() As C
+        Return New C(, [||]2)
+    End Function
+End Class
+",
+"Class C
+    Private _a As Integer
+    Private v As Integer
+
+    Public Sub New(Optional a As Integer = 1)
+        Me._a = a
+    End Sub
+
+    Public Sub New(Optional a As Integer = 1, Optional v As Integer = Nothing)
+        Me.New(a)
+        Me.v = v
+    End Sub
+
+    Public Function M() As C
+        Return New C(, 2)
+    End Function
+End Class
+")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateConstructor)>
+        <WorkItem(51040, "https://github.com/dotnet/roslyn/issues/51040")>
+        Public Async Function TestOmittedParameterAtEnd() As Task
+
+            Await TestInRegularAndScriptAsync(
+"Class C
+    Private _a As Integer
+
+    Public Sub New(Optional a As Integer = 1)
+        Me._a = a
+    End Sub
+
+    Public Function M() As C
+        Return New C(1,[||])
+    End Function
+End Class
+",
+"Class C
+    Private _a As Integer
+    Private p As Object
+
+    Public Sub New(Optional a As Integer = 1)
+        Me._a = a
+    End Sub
+
+    Public Sub New(Optional a As Integer = 1, Optional p As Object = Nothing)
+        Me.New(a)
+        Me.p = p
+    End Sub
+
+    Public Function M() As C
+        Return New C(1,)
+    End Function
+End Class
+")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateConstructor)>
+        <WorkItem(51040, "https://github.com/dotnet/roslyn/issues/51040")>
+        Public Async Function TestOmittedParameterAtStartAndEnd() As Task
+
+            Await TestInRegularAndScriptAsync(
+"Class C
+    Private _a As Integer
+
+    Public Sub New(Optional a As Integer = 1)
+        Me._a = a
+    End Sub
+
+    Public Function M() As C
+        Return New C(,[||])
+    End Function
+End Class
+",
+"Class C
+    Private _a As Integer
+    Private p As Object
+
+    Public Sub New(Optional a As Integer = 1)
+        Me._a = a
+    End Sub
+
+    Public Sub New(Optional a As Integer = 1, Optional p As Object = Nothing)
+        Me.New(a)
+        Me.p = p
+    End Sub
+
+    Public Function M() As C
+        Return New C(,)
+    End Function
+End Class
+")
+        End Function
+
     End Class
 End Namespace

src/Features/Core/Portable/CodeFixes/GenerateMember/AbstractGenerateMemberCodeFixProvider.cs

@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
@@ -19,7 +17,7 @@ namespace Microsoft.CodeAnalysis.CodeFixes.GenerateMember
 {
     internal abstract class AbstractGenerateMemberCodeFixProvider : CodeFixProvider
     {
-        public override FixAllProvider GetFixAllProvider()
+        public override FixAllProvider? GetFixAllProvider()
         {
             // Fix All is not supported by this code fix
             return null;
@@ -39,9 +37,9 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
             var diagnostic = context.Diagnostics.First();
             var document = context.Document;
-            var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
+            var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
 
-            var root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+            var root = await document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
             var names = GetTargetNodes(syntaxFacts, root, context.Span, diagnostic);
             foreach (var name in names)
             {

src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.State.cs

@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
@@ -39,18 +37,18 @@ protected internal class State
             private ImmutableArray<Argument> _arguments;
 
             // The type we're creating a constructor for.  Will be a class or struct type.
-            public INamedTypeSymbol TypeToGenerateIn { get; private set; }
+            public INamedTypeSymbol? TypeToGenerateIn { get; private set; }
 
             private ImmutableArray<RefKind> _parameterRefKinds;
             public ImmutableArray<ITypeSymbol> ParameterTypes;
 
             public SyntaxToken Token { get; private set; }
             public bool IsConstructorInitializerGeneration { get; private set; }
 
-            private IMethodSymbol _delegatedConstructor;
+            private IMethodSymbol? _delegatedConstructor;
 
             private ImmutableArray<IParameterSymbol> _parameters;
-            private ImmutableDictionary<string, ISymbol> _parameterToExistingMemberMap;
+            private ImmutableDictionary<string, ISymbol>? _parameterToExistingMemberMap;
 
             public ImmutableDictionary<string, string> ParameterToNewFieldMap { get; private set; }
             public ImmutableDictionary<string, string> ParameterToNewPropertyMap { get; private set; }
@@ -63,9 +61,12 @@ private State(TService service, SemanticDocument document, NamingRule fieldNamin
                 _fieldNamingRule = fieldNamingRule;
                 _propertyNamingRule = propertyNamingRule;
                 _parameterNamingRule = parameterNamingRule;
+
+                ParameterToNewFieldMap = ImmutableDictionary<string, string>.Empty;
+                ParameterToNewPropertyMap = ImmutableDictionary<string, string>.Empty;
             }
 
-            public static async Task<State> GenerateAsync(
+            public static async Task<State?> GenerateAsync(
                 TService service,
                 SemanticDocument document,
                 SyntaxNode node,
@@ -107,6 +108,7 @@ private async Task<bool> TryInitializeAsync(
                     return false;
                 }
 
+                Contract.ThrowIfNull(TypeToGenerateIn);
                 if (!CodeGenerator.CanAdd(_document.Project.Solution, TypeToGenerateIn, cancellationToken))
                     return false;
 
@@ -173,11 +175,14 @@ private bool TryInitializeDelegatedConstructor(CancellationToken cancellationTok
                 return true;
             }
 
-            private IMethodSymbol FindConstructorToDelegateTo(
+            private IMethodSymbol? FindConstructorToDelegateTo(
                 ImmutableArray<IParameterSymbol> allParameters,
-                ImmutableArray<TExpressionSyntax> allExpressions,
+                ImmutableArray<TExpressionSyntax?> allExpressions,
                 CancellationToken cancellationToken)
             {
+                Contract.ThrowIfNull(TypeToGenerateIn);
+                Contract.ThrowIfNull(TypeToGenerateIn.BaseType);
+
                 for (var i = allParameters.Length; i > 0; i--)
                 {
                     var parameters = allParameters.TakeAsArray(i);
@@ -191,12 +196,14 @@ private IMethodSymbol FindConstructorToDelegateTo(
                 return null;
             }
 
-            private IMethodSymbol FindConstructorToDelegateTo(
+            private IMethodSymbol? FindConstructorToDelegateTo(
                 ImmutableArray<IParameterSymbol> parameters,
-                ImmutableArray<TExpressionSyntax> expressions,
+                ImmutableArray<TExpressionSyntax?> expressions,
                 ImmutableArray<IMethodSymbol> constructors,
                 CancellationToken cancellationToken)
             {
+                Contract.ThrowIfNull(TypeToGenerateIn);
+
                 foreach (var constructor in constructors)
                 {
                     // Don't bother delegating to an implicit constructor. We don't want to add `: base()` as that's just
@@ -226,8 +233,10 @@ private IMethodSymbol FindConstructorToDelegateTo(
 
             private bool ClashesWithExistingConstructor()
             {
+                Contract.ThrowIfNull(TypeToGenerateIn);
+
                 var destinationProvider = _document.Project.Solution.Workspace.Services.GetLanguageServices(TypeToGenerateIn.Language);
-                var syntaxFacts = destinationProvider.GetService<ISyntaxFactsService>();
+                var syntaxFacts = destinationProvider.GetRequiredService<ISyntaxFactsService>();
                 return TypeToGenerateIn.InstanceConstructors.Any(c => Matches(c, syntaxFacts));
             }
 
@@ -508,6 +517,8 @@ where m.Name.Equals(expectedFieldName, StringComparison.OrdinalIgnoreCase)
 
             private IEnumerable<string> GetUnavailableMemberNames()
             {
+                Contract.ThrowIfNull(TypeToGenerateIn);
+
                 return TypeToGenerateIn.MemberNames.Concat(
                     from type in TypeToGenerateIn.GetBaseTypes()
                     from member in type.GetMembers()
@@ -557,12 +568,14 @@ public async Task<Document> GetChangedDocumentAsync(
                        await GenerateMemberDelegatingConstructorAsync(document, withFields, withProperties, cancellationToken).ConfigureAwait(false);
             }
 
-            private async Task<Document> GenerateThisOrBaseDelegatingConstructorAsync(
+            private async Task<Document?> GenerateThisOrBaseDelegatingConstructorAsync(
                 Document document, bool withFields, bool withProperties, CancellationToken cancellationToken)
             {
                 if (_delegatedConstructor == null)
                     return null;
 
+                Contract.ThrowIfNull(TypeToGenerateIn);
+
                 var provider = document.Project.Solution.Workspace.Services.GetLanguageServices(TypeToGenerateIn.Language);
                 var (members, assignments) = await GenerateMembersAndAssignmentsAsync(document, withFields, withProperties, cancellationToken).ConfigureAwait(false);
                 var isThis = _delegatedConstructor.ContainingType.OriginalDefinition.Equals(TypeToGenerateIn.OriginalDefinition);
@@ -581,7 +594,7 @@ private async Task<Document> GenerateThisOrBaseDelegatingConstructorAsync(
                     baseConstructorArguments: isThis ? default : delegatingArguments,
                     thisConstructorArguments: isThis ? delegatingArguments : default);
 
-                return await provider.GetService<ICodeGenerationService>().AddMembersAsync(
+                return await provider.GetRequiredService<ICodeGenerationService>().AddMembersAsync(
                     document.Project.Solution,
                     TypeToGenerateIn,
                     members.Concat(constructor),
@@ -594,6 +607,8 @@ private async Task<Document> GenerateThisOrBaseDelegatingConstructorAsync(
             private async Task<(ImmutableArray<ISymbol>, ImmutableArray<SyntaxNode>)> GenerateMembersAndAssignmentsAsync(
                 Document document, bool withFields, bool withProperties, CancellationToken cancellationToken)
             {
+                Contract.ThrowIfNull(TypeToGenerateIn);
+
                 var provider = document.Project.Solution.Workspace.Services.GetLanguageServices(TypeToGenerateIn.Language);
 
                 var members = withFields ? SyntaxGeneratorExtensions.CreateFieldsForParameters(_parameters, ParameterToNewFieldMap, IsContainedInUnsafeType) :
@@ -615,6 +630,8 @@ private async Task<Document> GenerateThisOrBaseDelegatingConstructorAsync(
             private async Task<Document> GenerateMemberDelegatingConstructorAsync(
                 Document document, bool withFields, bool withProperties, CancellationToken cancellationToken)
             {
+                Contract.ThrowIfNull(TypeToGenerateIn);
+
                 var provider = document.Project.Solution.Workspace.Services.GetLanguageServices(TypeToGenerateIn.Language);
                 var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
 
@@ -623,7 +640,7 @@ private async Task<Document> GenerateMemberDelegatingConstructorAsync(
                     withProperties ? ParameterToNewPropertyMap :
                     ImmutableDictionary<string, string>.Empty;
 
-                return await provider.GetService<ICodeGenerationService>().AddMembersAsync(
+                return await provider.GetRequiredService<ICodeGenerationService>().AddMembersAsync(
                     document.Project.Solution,
                     TypeToGenerateIn,
                     provider.GetService<SyntaxGenerator>().CreateMemberDelegatingConstructor(

src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.cs

@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
@@ -86,6 +84,8 @@ public async Task<ImmutableArray<CodeAction>> GenerateConstructorAsync(Document
                 var state = await State.GenerateAsync((TService)this, semanticDocument, node, cancellationToken).ConfigureAwait(false);
                 if (state != null)
                 {
+                    Contract.ThrowIfNull(state.TypeToGenerateIn);
+
                     using var _ = ArrayBuilder<CodeAction>.GetInstance(out var result);
 
                     // If we have any fields we'd like to generate, offer a code action to do that.
@@ -116,7 +116,7 @@ public async Task<ImmutableArray<CodeAction>> GenerateConstructorAsync(Document
             return ImmutableArray<CodeAction>.Empty;
         }
 
-        protected static bool IsSymbolAccessible(ISymbol symbol, SemanticDocument document)
+        protected static bool IsSymbolAccessible(ISymbol? symbol, SemanticDocument document)
         {
             if (symbol == null)
             {
@@ -157,6 +157,9 @@ protected string GenerateNameForArgument(SemanticModel semanticModel, Argument a
             if (argument.IsNamed)
                 return argument.Name;
 
+            if (argument.Expression is null)
+                return ITypeSymbolExtensions.DefaultParameterName;
+
             var name = this.GenerateNameForExpression(semanticModel, argument.Expression, cancellationToken);
             return string.IsNullOrEmpty(name) ? ITypeSymbolExtensions.DefaultParameterName : name;
         }

src/Features/Core/Portable/GenerateMember/GenerateConstructor/Argument.cs

@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 namespace Microsoft.CodeAnalysis.GenerateMember.GenerateConstructor
 {
     internal abstract partial class AbstractGenerateConstructorService<TService, TExpressionSyntax>
@@ -12,9 +10,9 @@ protected readonly struct Argument
         {
             public readonly RefKind RefKind;
             public readonly string Name;
-            public readonly TExpressionSyntax Expression;
+            public readonly TExpressionSyntax? Expression;
 
-            public Argument(RefKind refKind, string name, TExpressionSyntax expression)
+            public Argument(RefKind refKind, string? name, TExpressionSyntax? expression)
             {
                 RefKind = refKind;
                 Name = name ?? "";

src/Features/Core/Portable/GenerateMember/GenerateConstructor/GenerateConstructorHelpers.cs

@@ -18,7 +18,7 @@ internal static class GenerateConstructorHelpers
         public static bool CanDelegateTo<TExpressionSyntax>(
             SemanticDocument document,
             ImmutableArray<IParameterSymbol> parameters,
-            ImmutableArray<TExpressionSyntax> expressions,
+            ImmutableArray<TExpressionSyntax?> expressions,
             IMethodSymbol constructor)
             where TExpressionSyntax : SyntaxNode
         {
@@ -73,7 +73,7 @@ private static bool IsCompatible<TExpressionSyntax>(
             ISemanticFactsService semanticFacts,
             SemanticModel semanticModel,
             IMethodSymbol constructor,
-            ImmutableArray<TExpressionSyntax> expressions)
+            ImmutableArray<TExpressionSyntax?> expressions)
             where TExpressionSyntax : SyntaxNode
         {
             Debug.Assert(constructor.Parameters.Length == expressions.Length);
@@ -107,6 +107,10 @@ private static bool IsCompatible<TExpressionSyntax>(
                 if (constructorParameter == null)
                     return false;
 
+                // In VB the argument may not have been specified at all if the parameter is optional
+                if (expressions[i] is null && constructorParameter.IsOptional)
+                    continue;
+
                 var conversion = semanticFacts.ClassifyConversion(semanticModel, expressions[i], constructorParameter.Type);
                 if (!conversion.IsIdentity && !conversion.IsImplicit)
                     return false;