Continued Extract method cleanup (#76577)

src/EditorFeatures/CSharpTest/ExtractMethod/SelectionValidatorTests.cs

@@ -1520,7 +1520,7 @@ public async Task AnonymousTypeMember1()
     {
         var code = """
             using System;
-            class C { void M() { {|r:var x = new { {|b:String|} = true }; |}} }
+            class C { void M() { {|r:var x = new { {|b:String|} = true };|} } }
             """;
         await TestSelectionAsync(code);
     }
@@ -1717,7 +1717,7 @@ class Program
                 static void Main(string[] args)
                 {
                     A a = new A();
-                    var l = {|r:a?.{|b:Prop|}?.Length |}?? 0;
+                    var l = {|r:a?.{|b:Prop|}?.Length|} ?? 0;
                 }
             }
             class A
@@ -1769,7 +1769,7 @@ class Program
                 static void Main(string[] args)
                 {
                     A a = new A();
-                    var l = {|r:a?.{|b:Method()|}?.Length |}?? 0;
+                    var l = {|r:a?.{|b:Method()|}?.Length|} ?? 0;
                 }
             }
             class A

src/Features/CSharp/Portable/ExtractMethod/CSharpExtractMethodService.cs

@@ -16,16 +16,13 @@ namespace Microsoft.CodeAnalysis.CSharp.ExtractMethod;
 [method: ImportingConstructor]
 [method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
 internal sealed partial class CSharpExtractMethodService() : AbstractExtractMethodService<
-    CSharpExtractMethodService.CSharpSelectionValidator,
-    CSharpExtractMethodService.CSharpMethodExtractor,
-    CSharpExtractMethodService.CSharpSelectionResult,
     StatementSyntax,
     StatementSyntax,
     ExpressionSyntax>
 {
-    protected override CSharpSelectionValidator CreateSelectionValidator(SemanticDocument document, TextSpan textSpan, bool localFunction)
-        => new(document, textSpan, localFunction);
+    protected override SelectionValidator CreateSelectionValidator(SemanticDocument document, TextSpan textSpan, bool localFunction)
+        => new CSharpSelectionValidator(document, textSpan, localFunction);
 
-    protected override CSharpMethodExtractor CreateMethodExtractor(CSharpSelectionResult selectionResult, ExtractMethodGenerationOptions options, bool localFunction)
-        => new(selectionResult, options, localFunction);
+    protected override MethodExtractor CreateMethodExtractor(SelectionResult selectionResult, ExtractMethodGenerationOptions options, bool localFunction)
+        => new CSharpMethodExtractor(selectionResult, options, localFunction);
 }

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

@@ -15,14 +15,9 @@ internal sealed partial class CSharpExtractMethodService
 {
     internal sealed partial class CSharpMethodExtractor
     {
-        private sealed class CSharpAnalyzer(CSharpSelectionResult selectionResult, bool localFunction, CancellationToken cancellationToken) : Analyzer(selectionResult, localFunction, cancellationToken)
+        private sealed class CSharpAnalyzer(SelectionResult selectionResult, bool localFunction, CancellationToken cancellationToken)
+            : Analyzer(selectionResult, localFunction, cancellationToken)
         {
-            public static AnalyzerResult Analyze(CSharpSelectionResult selectionResult, bool localFunction, CancellationToken cancellationToken)
-            {
-                var analyzer = new CSharpAnalyzer(selectionResult, localFunction, cancellationToken);
-                return analyzer.Analyze();
-            }
-
             protected override bool TreatOutAsRef
                 => false;
 

src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.ExpressionCodeGenerator.cs

@@ -24,7 +24,7 @@ internal sealed partial class CSharpMethodExtractor
         private abstract partial class CSharpCodeGenerator
         {
             private sealed class ExpressionCodeGenerator(
-                CSharpSelectionResult selectionResult,
+                SelectionResult selectionResult,
                 AnalyzerResult analyzerResult,
                 ExtractMethodGenerationOptions options,
                 bool localFunction) : CSharpCodeGenerator(selectionResult, analyzerResult, options, localFunction)
@@ -120,7 +120,7 @@ protected override ImmutableArray<StatementSyntax> GetInitialStatementsForMethod
 
                 private ExpressionSyntax WrapInCheckedExpressionIfNeeded(ExpressionSyntax expression)
                 {
-                    var kind = this.SelectionResult.UnderCheckedExpressionContext();
+                    var kind = UnderCheckedExpressionContext();
                     if (kind == SyntaxKind.None)
                     {
                         return expression;

src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.MultipleStatementsCodeGenerator.cs

@@ -23,7 +23,7 @@ internal sealed partial class CSharpMethodExtractor
         private abstract partial class CSharpCodeGenerator
         {
             public sealed class MultipleStatementsCodeGenerator(
-                CSharpSelectionResult selectionResult,
+                SelectionResult selectionResult,
                 AnalyzerResult analyzerResult,
                 ExtractMethodGenerationOptions options,
                 bool localFunction) : CSharpCodeGenerator(selectionResult, analyzerResult, options, localFunction)

src/Features/CSharp/Portable/ExtractMethod/CSharpMethodExtractor.CSharpCodeGenerator.SingleStatementCodeGenerator.cs

@@ -20,7 +20,7 @@ internal sealed partial class CSharpMethodExtractor
         private abstract partial class CSharpCodeGenerator
         {
             public sealed class SingleStatementCodeGenerator(
-                CSharpSelectionResult selectionResult,
+                SelectionResult selectionResult,
                 AnalyzerResult analyzerResult,
                 ExtractMethodGenerationOptions options,
                 bool localFunction) : CSharpCodeGenerator(selectionResult, analyzerResult, options, localFunction)

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

@@ -44,20 +44,8 @@ private abstract partial class CSharpCodeGenerator : CodeGenerator<StatementSynt
             private const string NewMethodPascalCaseStr = "NewMethod";
             private const string NewMethodCamelCaseStr = "newMethod";
 
-            public static Task<GeneratedCode> GenerateAsync(
-                InsertionPoint insertionPoint,
-                CSharpSelectionResult selectionResult,
-                AnalyzerResult analyzerResult,
-                ExtractMethodGenerationOptions options,
-                bool localFunction,
-                CancellationToken cancellationToken)
-            {
-                var codeGenerator = Create(selectionResult, analyzerResult, options, localFunction);
-                return codeGenerator.GenerateAsync(insertionPoint, cancellationToken);
-            }
-
             public static CSharpCodeGenerator Create(
-                CSharpSelectionResult selectionResult,
+                SelectionResult selectionResult,
                 AnalyzerResult analyzerResult,
                 ExtractMethodGenerationOptions options,
                 bool localFunction)
@@ -72,7 +60,7 @@ public static CSharpCodeGenerator Create(
             }
 
             protected CSharpCodeGenerator(
-                CSharpSelectionResult selectionResult,
+                SelectionResult selectionResult,
                 AnalyzerResult analyzerResult,
                 ExtractMethodGenerationOptions options,
                 bool localFunction)
@@ -202,9 +190,25 @@ protected override SyntaxNode GetCallSiteContainerFromOutermostMoveInVariable(Ca
                 return declStatement.Parent;
             }
 
+            private bool ShouldPutUnsafeModifier()
+            {
+                var token = this.SelectionResult.GetFirstTokenInSelection();
+                var ancestors = token.GetAncestors<SyntaxNode>();
+
+                // if enclosing type contains unsafe keyword, we don't need to put it again
+                if (ancestors.Where(a => CSharp.SyntaxFacts.IsTypeDeclaration(a.Kind()))
+                             .Cast<MemberDeclarationSyntax>()
+                             .Any(m => m.GetModifiers().Any(SyntaxKind.UnsafeKeyword)))
+                {
+                    return false;
+                }
+
+                return token.Parent.IsUnsafeContext();
+            }
+
             private DeclarationModifiers CreateMethodModifiers()
             {
-                var isUnsafe = this.SelectionResult.ShouldPutUnsafeModifier();
+                var isUnsafe = ShouldPutUnsafeModifier();
                 var isAsync = this.SelectionResult.CreateAsyncMethod();
                 var isStatic = !AnalyzerResult.UseInstanceMember;
                 var isReadOnly = AnalyzerResult.ShouldBeReadOnly;
@@ -269,9 +273,27 @@ private ImmutableArray<StatementSyntax> CreateMethodBody(
                 return statements;
             }
 
+            protected SyntaxKind UnderCheckedExpressionContext()
+                => UnderCheckedContext<CheckedExpressionSyntax>();
+
+            protected SyntaxKind UnderCheckedStatementContext()
+                => UnderCheckedContext<CheckedStatementSyntax>();
+
+            protected SyntaxKind UnderCheckedContext<T>() where T : SyntaxNode
+            {
+                var token = this.SelectionResult.GetFirstTokenInSelection();
+                var contextNode = token.Parent.GetAncestor<T>();
+                if (contextNode == null)
+                {
+                    return SyntaxKind.None;
+                }
+
+                return contextNode.Kind();
+            }
+
             private ImmutableArray<StatementSyntax> WrapInCheckStatementIfNeeded(ImmutableArray<StatementSyntax> statements)
             {
-                var kind = this.SelectionResult.UnderCheckedStatementContext();
+                var kind = UnderCheckedStatementContext();
                 if (kind == SyntaxKind.None)
                     return statements;
 

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

@@ -19,23 +19,9 @@ internal sealed partial class CSharpExtractMethodService
 {
     internal sealed partial class CSharpMethodExtractor
     {
-        private sealed class CSharpTriviaResult : TriviaResult
+        private sealed class CSharpTriviaResult(SemanticDocument document, ITriviaSavedResult result)
+            : TriviaResult(document, result, (int)SyntaxKind.EndOfLineTrivia, (int)SyntaxKind.WhitespaceTrivia)
         {
-            public static async Task<CSharpTriviaResult> ProcessAsync(CSharpSelectionResult selectionResult, CancellationToken cancellationToken)
-            {
-                var preservationService = selectionResult.SemanticDocument.Document.Project.Services.GetService<ISyntaxTriviaService>();
-                var root = selectionResult.SemanticDocument.Root;
-                var result = preservationService.SaveTriviaAroundSelection(root, selectionResult.FinalSpan);
-                return new CSharpTriviaResult(
-                    await selectionResult.SemanticDocument.WithSyntaxRootAsync(result.Root, cancellationToken).ConfigureAwait(false),
-                    result);
-            }
-
-            private CSharpTriviaResult(SemanticDocument document, ITriviaSavedResult result)
-                : base(document, result, (int)SyntaxKind.EndOfLineTrivia, (int)SyntaxKind.WhitespaceTrivia)
-            {
-            }
-
             protected override AnnotationResolver GetAnnotationResolver(SyntaxNode callsite, SyntaxNode method)
             {
                 var isMethodOrLocalFunction = method is MethodDeclarationSyntax or LocalFunctionStatementSyntax;

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

@@ -19,19 +19,23 @@ namespace Microsoft.CodeAnalysis.CSharp.ExtractMethod;
 
 internal sealed partial class CSharpExtractMethodService
 {
-    internal sealed partial class CSharpMethodExtractor(CSharpSelectionResult result, ExtractMethodGenerationOptions options, bool localFunction)
+    internal sealed partial class CSharpMethodExtractor(
+        SelectionResult result, ExtractMethodGenerationOptions options, bool localFunction)
         : MethodExtractor(result, options, localFunction)
     {
         protected override CodeGenerator CreateCodeGenerator(AnalyzerResult analyzerResult)
             => CSharpCodeGenerator.Create(this.OriginalSelectionResult, analyzerResult, this.Options, this.LocalFunction);
 
-        protected override AnalyzerResult Analyze(CSharpSelectionResult selectionResult, bool localFunction, CancellationToken cancellationToken)
-            => CSharpAnalyzer.Analyze(selectionResult, localFunction, cancellationToken);
+        protected override AnalyzerResult Analyze(CancellationToken cancellationToken)
+        {
+            var analyzer = new CSharpAnalyzer(this.OriginalSelectionResult, this.LocalFunction, cancellationToken);
+            return analyzer.Analyze();
+        }
 
         protected override SyntaxNode GetInsertionPointNode(
             AnalyzerResult analyzerResult, CancellationToken cancellationToken)
         {
-            var originalSpanStart = OriginalSelectionResult.OriginalSpan.Start;
+            var originalSpanStart = OriginalSelectionResult.FinalSpan.Start;
             Contract.ThrowIfFalse(originalSpanStart >= 0);
 
             var document = this.OriginalSelectionResult.SemanticDocument;
@@ -61,10 +65,10 @@ protected override SyntaxNode GetInsertionPointNode(
                 {
                     if (currentNode is AnonymousFunctionExpressionSyntax anonymousFunction)
                     {
-                        if (OriginalSelectionWithin(anonymousFunction.Body) || OriginalSelectionWithin(anonymousFunction.ExpressionBody))
+                        if (SelectionWithin(anonymousFunction.Body) || SelectionWithin(anonymousFunction.ExpressionBody))
                             return currentNode;
 
-                        if (!OriginalSelectionResult.OriginalSpan.Contains(anonymousFunction.Span))
+                        if (!OriginalSelectionResult.FinalSpan.Contains(anonymousFunction.Span))
                         {
                             // If we encountered a function but the selection isn't within the body, it's likely the user
                             // is attempting to move the function (which is behavior that is supported). Stop looking up the 
@@ -76,10 +80,10 @@ protected override SyntaxNode GetInsertionPointNode(
 
                     if (currentNode is LocalFunctionStatementSyntax localFunction)
                     {
-                        if (OriginalSelectionWithin(localFunction.ExpressionBody) || OriginalSelectionWithin(localFunction.Body))
+                        if (SelectionWithin(localFunction.ExpressionBody) || SelectionWithin(localFunction.Body))
                             return currentNode;
 
-                        if (!OriginalSelectionResult.OriginalSpan.Contains(localFunction.Span))
+                        if (!OriginalSelectionResult.FinalSpan.Contains(localFunction.Span))
                         {
                             // If we encountered a function but the selection isn't within the body, it's likely the user
                             // is attempting to move the function (which is behavior that is supported). Stop looking up the 
@@ -146,21 +150,31 @@ SyntaxNode GetInsertionPointForGlobalStatement(GlobalStatementSyntax globalState
             }
         }
 
-        private bool OriginalSelectionWithin(SyntaxNode node)
+        private bool SelectionWithin(SyntaxNode node)
         {
             if (node is null)
             {
                 return false;
             }
 
-            return node.Span.Contains(OriginalSelectionResult.OriginalSpan);
+            return node.Span.Contains(OriginalSelectionResult.FinalSpan);
         }
 
-        protected override async Task<TriviaResult> PreserveTriviaAsync(CSharpSelectionResult selectionResult, CancellationToken cancellationToken)
-            => await CSharpTriviaResult.ProcessAsync(selectionResult, cancellationToken).ConfigureAwait(false);
+        protected override async Task<TriviaResult> PreserveTriviaAsync(SyntaxNode root, CancellationToken cancellationToken)
+        {
+            var semanticDocument = this.OriginalSelectionResult.SemanticDocument;
+            var preservationService = semanticDocument.Document.Project.Services.GetService<ISyntaxTriviaService>();
+            var result = preservationService.SaveTriviaAroundSelection(root, this.OriginalSelectionResult.FinalSpan);
+            return new CSharpTriviaResult(
+                await semanticDocument.WithSyntaxRootAsync(result.Root, cancellationToken).ConfigureAwait(false),
+                result);
+        }
 
-        protected override Task<GeneratedCode> GenerateCodeAsync(InsertionPoint insertionPoint, CSharpSelectionResult selectionResult, AnalyzerResult analyzeResult, ExtractMethodGenerationOptions options, CancellationToken cancellationToken)
-            => CSharpCodeGenerator.GenerateAsync(insertionPoint, selectionResult, analyzeResult, options, LocalFunction, cancellationToken);
+        protected override Task<GeneratedCode> GenerateCodeAsync(InsertionPoint insertionPoint, SelectionResult selectionResult, AnalyzerResult analyzeResult, ExtractMethodGenerationOptions options, CancellationToken cancellationToken)
+        {
+            var codeGenerator = CSharpCodeGenerator.Create(selectionResult, analyzeResult, options, this.LocalFunction);
+            return codeGenerator.GenerateAsync(insertionPoint, cancellationToken);
+        }
 
         protected override AbstractFormattingRule GetCustomFormattingRule(Document document)
             => FormattingRule.Instance;

src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.ExpressionResult.cs

@@ -24,10 +24,9 @@ internal abstract partial class CSharpSelectionResult
         private sealed class ExpressionResult(
             SemanticDocument document,
             SelectionType selectionType,
-            TextSpan originalSpan,
             TextSpan finalSpan,
             bool selectionChanged) : CSharpSelectionResult(
-                document, selectionType, originalSpan, finalSpan, selectionChanged)
+                document, selectionType, finalSpan, selectionChanged)
         {
             public override bool ContainingScopeHasAsyncKeyword()
                 => false;

src/Features/CSharp/Portable/ExtractMethod/CSharpSelectionResult.StatementResult.cs

@@ -23,10 +23,9 @@ internal abstract partial class CSharpSelectionResult
         private sealed class StatementResult(
             SemanticDocument document,
             SelectionType selectionType,
-            TextSpan originalSpan,
             TextSpan finalSpan,
             bool selectionChanged)
-            : CSharpSelectionResult(document, selectionType, originalSpan, finalSpan, selectionChanged)
+            : CSharpSelectionResult(document, selectionType, finalSpan, selectionChanged)
         {
             public override bool ContainingScopeHasAsyncKeyword()
             {