Make extracted parameters non-nullable when possible.

src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.Analyzer.cs

@@ -29,13 +29,9 @@ protected override bool IsInPrimaryConstructorBaseType()
             => this.SelectionResult.GetContainingScopeOf<PrimaryConstructorBaseTypeSyntax>() != null;
 
         protected override VariableInfo CreateFromSymbol(
-            Compilation compilation,
-            ISymbol symbol,
-            ITypeSymbol type,
-            VariableStyle style,
-            bool variableDeclared)
+            ISymbol symbol, ITypeSymbol type, VariableStyle style, bool variableDeclared)
         {
-            return CreateFromSymbolCommon<LocalDeclarationStatementSyntax>(compilation, symbol, type, style, s_nonNoisySyntaxKindSet);
+            return CreateFromSymbolCommon<LocalDeclarationStatementSyntax>(symbol, type, style, s_nonNoisySyntaxKindSet);
         }
 
         protected override ITypeSymbol? GetRangeVariableType(SemanticModel model, IRangeVariableSymbol symbol)

src/Features/CSharpTest/ExtractMethod/ExtractLocalFunctionTests.cs

@@ -2928,7 +2928,7 @@ class C
             {
                 public string? M()
                 {
-                    string? x = {|Rename:NewMethod|}();
+                    string x = {|Rename:NewMethod|}();
 
                     return x;
 

src/Features/CSharpTest/ExtractMethod/ExtractMethodTests2.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Security;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeRefactorings;
 using Microsoft.CodeAnalysis.CodeRefactorings.ExtractMethod;
@@ -11,6 +12,7 @@
 using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
 using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
 using Microsoft.CodeAnalysis.Test.Utilities;
+using Microsoft.CodeAnalysis.Testing;
 using Roslyn.Test.Utilities;
 using Xunit;
 
@@ -3302,7 +3304,7 @@ class C
             {
                 public string? M()
                 {
-                    string? x = {|Rename:NewMethod|}();
+                    string x = {|Rename:NewMethod|}();
 
                     return x;
                 }
@@ -5867,6 +5869,101 @@ public async Task Goo()
             """,
             options: ImplicitTypeEverywhere());
 
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/61555")]
+    public async Task TestKnownNotNullParameter()
+        => await new VerifyCS.Test
+        {
+            TestCode = """
+                #nullable enable
+
+                public class C
+                {
+                    public void M(C? c)
+                    {
+                        if (c == null)
+                        {
+                            return;
+                        }
+
+                        [|c.ToString();|]
+                    }
+                }
+                """,
+            FixedCode = """
+                #nullable enable
+
+                public class C
+                {
+                    public void M(C? c)
+                    {
+                        if (c == null)
+                        {
+                            return;
+                        }
+
+                        NewMethod(c);
+                    }
+
+                    private static void NewMethod(C c)
+                    {
+                        c.ToString();
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp13,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net90,
+        }.RunAsync();
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/61555")]
+    public async Task TestKnownNotNullParameter_AssignedNull()
+        => await new VerifyCS.Test
+        {
+            TestCode = """
+                #nullable enable
+
+                public class C
+                {
+                    public void M(C? c)
+                    {
+                        if (c == null)
+                        {
+                            return;
+                        }
+
+                        [|c.ToString();
+                        c = null;
+                        c?.ToString();|]
+                    }
+                }
+                """,
+            FixedCode = """
+                #nullable enable
+
+                public class C
+                {
+                    public void M(C? c)
+                    {
+                        if (c == null)
+                        {
+                            return;
+                        }
+
+                        c = NewMethod(c);
+                    }
+
+                    private static C? NewMethod(C? c)
+                    {
+                        {|CS8602:c|}.ToString();
+                        c = null;
+                        c?.ToString();
+                        return c;
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp13,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net90,
+        }.RunAsync();
+
     [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/67017")]
     public async Task TestPrimaryConstructorBaseList(bool withBody)
     {

src/Features/Core/Portable/ExtractMethod/MethodExtractor.Analyzer.cs

@@ -2,7 +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.
 
-using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
@@ -69,7 +68,7 @@ protected Analyzer(TSelectionResult selectionResult, bool localFunction, Cancell
         /// <summary>
         /// create VariableInfo type
         /// </summary>
-        protected abstract VariableInfo CreateFromSymbol(Compilation compilation, ISymbol symbol, ITypeSymbol type, VariableStyle variableStyle, bool variableDeclared);
+        protected abstract VariableInfo CreateFromSymbol(ISymbol symbol, ITypeSymbol type, VariableStyle variableStyle, bool variableDeclared);
 
         protected virtual bool IsReadOutside(ISymbol symbol, HashSet<ISymbol> readOutsideMap)
             => readOutsideMap.Contains(symbol);
@@ -311,9 +310,12 @@ private bool IsInExpressionOrHasReturnStatement(SemanticModel model)
         }
 
         private OperationStatus GetOperationStatus(
-            SemanticModel model, Dictionary<ISymbol, List<SyntaxToken>> symbolMap,
-            IList<VariableInfo> parameters, IList<ISymbol> failedVariables,
-            bool unsafeAddressTakenUsed, bool returnTypeHasAnonymousType,
+            SemanticModel model,
+            Dictionary<ISymbol, List<SyntaxToken>> symbolMap,
+            IList<VariableInfo> parameters,
+            IList<ISymbol> failedVariables,
+            bool unsafeAddressTakenUsed,
+            bool returnTypeHasAnonymousType,
             bool containsAnyLocalFunctionCallNotWithinSpan)
         {
             var readonlyFieldStatus = CheckReadOnlyFields(model, symbolMap);
@@ -606,7 +608,59 @@ private void GenerateVariableInfoMap(
                     continue;
                 }
 
-                AddVariableToMap(variableInfoMap, symbol, CreateFromSymbol(model.Compilation, symbol, type, variableStyle, variableDeclared));
+                type = FixNullability(symbol, type, variableStyle);
+
+                AddVariableToMap(
+                    variableInfoMap,
+                    symbol,
+                    CreateFromSymbol(symbol, type, variableStyle, variableDeclared));
+            }
+
+            ITypeSymbol FixNullability(ISymbol symbol, ITypeSymbol type, VariableStyle style)
+            {
+                if (style.ParameterStyle.ParameterBehavior != ParameterBehavior.None &&
+                    type.NullableAnnotation is NullableAnnotation.Annotated &&
+                    !IsMaybeNullWithinSelection(symbol))
+                {
+                    // We're going to pass this nullable variable in as a parameter to the new method we're creating. If the
+                    // variable is actually never null within the section we're extracting, then change its return type to
+                    // be non-nullable so that any usage of it within the new method will not warn.
+                    return type.WithNullableAnnotation(NullableAnnotation.NotAnnotated);
+                }
+
+                return type;
+            }
+
+            bool IsMaybeNullWithinSelection(ISymbol symbol)
+            {
+                if (!symbolMap.TryGetValue(symbol, out var tokens))
+                    return true;
+
+                var syntaxFacts = this.SyntaxFacts;
+                foreach (var token in tokens)
+                {
+                    if (token.Parent is not TExpressionSyntax expression)
+                        continue;
+
+                    // a = b;
+                    //
+                    // Need to ask what the nullability of 'b' is since 'a' may be currently non-null but may be
+                    // assigned a null value.
+                    if (syntaxFacts.IsLeftSideOfAssignment(expression))
+                    {
+                        syntaxFacts.GetPartsOfAssignmentExpressionOrStatement(expression.GetRequiredParent(), out _, out _, out var right);
+                        expression = (TExpressionSyntax)right;
+                    }
+
+                    var typeInfo = model.GetTypeInfo(expression, this.CancellationToken);
+                    if (typeInfo.Nullability.FlowState == NullableFlowState.MaybeNull ||
+                        typeInfo.ConvertedNullability.FlowState == NullableFlowState.MaybeNull)
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
             }
         }
 
@@ -994,13 +1048,15 @@ private OperationStatus CheckReadOnlyFields(SemanticModel semanticModel, Diction
             return OperationStatus.SucceededStatus;
         }
 
-        protected static VariableInfo CreateFromSymbolCommon<T>(
-            Compilation compilation,
+        protected VariableInfo CreateFromSymbolCommon<T>(
             ISymbol symbol,
             ITypeSymbol type,
             VariableStyle style,
             HashSet<int> nonNoisySyntaxKindSet) where T : SyntaxNode
         {
+            var semanticModel = _semanticDocument.SemanticModel;
+            var compilation = semanticModel.Compilation;
+
             return symbol switch
             {
                 ILocalSymbol local => new VariableInfo(

src/Features/Core/Portable/ExtractMethod/MethodExtractor.CodeGenerator.cs

@@ -349,7 +349,7 @@ protected ImmutableArray<ITypeParameterSymbol> CreateMethodTypeParameters()
 
         protected ImmutableArray<IParameterSymbol> CreateMethodParameters()
         {
-            var parameters = ArrayBuilder<IParameterSymbol>.GetInstance();
+            using var _ = ArrayBuilder<IParameterSymbol>.GetInstance(out var parameters);
             var isLocalFunction = LocalFunction && ShouldLocalFunctionCaptureParameter(SemanticDocument.Root);
             foreach (var parameter in AnalyzerResult.MethodParameters)
             {
@@ -368,7 +368,7 @@ protected ImmutableArray<IParameterSymbol> CreateMethodParameters()
                 }
             }
 
-            return parameters.ToImmutableAndFree();
+            return parameters.ToImmutableAndClear();
         }
 
         private static RefKind GetRefKind(ParameterBehavior parameterBehavior)

src/Features/VisualBasic/Portable/ExtractMethod/VisualBasicMethodExtractor.Analyzer.vb

@@ -31,14 +31,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExtractMethod
             End Function
 
             Protected Overrides Function CreateFromSymbol(
-                compilation As Compilation, symbol As ISymbol,
-                type As ITypeSymbol, style As VariableStyle, requiresDeclarationExpressionRewrite As Boolean) As VariableInfo
+                    symbol As ISymbol,
+                    type As ITypeSymbol,
+                    style As VariableStyle,
+                    requiresDeclarationExpressionRewrite As Boolean) As VariableInfo
                 If symbol.IsFunctionValue() AndAlso style.ParameterStyle.DeclarationBehavior <> DeclarationBehavior.None Then
                     Contract.ThrowIfFalse(style.ParameterStyle.DeclarationBehavior = DeclarationBehavior.MoveIn OrElse style.ParameterStyle.DeclarationBehavior = DeclarationBehavior.SplitIn)
                     style = AlwaysReturn(style)
                 End If
 
-                Return CreateFromSymbolCommon(Of LocalDeclarationStatementSyntax)(compilation, symbol, type, style, s_nonNoisySyntaxKindSet)
+                Return CreateFromSymbolCommon(Of LocalDeclarationStatementSyntax)(symbol, type, style, s_nonNoisySyntaxKindSet)
             End Function
 
             Protected Overrides Function GetRangeVariableType(semanticModel As SemanticModel, symbol As IRangeVariableSymbol) As ITypeSymbol