Skip to content

Commit ccbc092

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Continued Extract method cleanup (#76577)
2 parents a7ff9bf + f11598a commit ccbc092

File tree

37 files changed

+553
-723
lines changed

37 files changed

+553
-723
lines changed

‎src/EditorFeatures/CSharpTest/ExtractMethod/SelectionValidatorTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,7 +1520,7 @@ public async Task AnonymousTypeMember1()
15201520
{
15211521
var code = """
15221522
using System;
1523-
class C { void M() { {|r:var x = new { {|b:String|} = true }; |}} }
1523+
class C { void M() { {|r:var x = new { {|b:String|} = true };|} } }
15241524
""";
15251525
await TestSelectionAsync(code);
15261526
}
@@ -1717,7 +1717,7 @@ class Program
17171717
static void Main(string[] args)
17181718
{
17191719
A a = new A();
1720-
var l = {|r:a?.{|b:Prop|}?.Length |}?? 0;
1720+
var l = {|r:a?.{|b:Prop|}?.Length|} ?? 0;
17211721
}
17221722
}
17231723
class A
@@ -1769,7 +1769,7 @@ class Program
17691769
static void Main(string[] args)
17701770
{
17711771
A a = new A();
1772-
var l = {|r:a?.{|b:Method()|}?.Length |}?? 0;
1772+
var l = {|r:a?.{|b:Method()|}?.Length|} ?? 0;
17731773
}
17741774
}
17751775
class A

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,13 @@ namespace Microsoft.CodeAnalysis.CSharp.ExtractMethod;
1616
[method: ImportingConstructor]
1717
[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
1818
internal sealed partial class CSharpExtractMethodService() : AbstractExtractMethodService<
19-
CSharpExtractMethodService.CSharpSelectionValidator,
20-
CSharpExtractMethodService.CSharpMethodExtractor,
21-
CSharpExtractMethodService.CSharpSelectionResult,
2219
StatementSyntax,
2320
StatementSyntax,
2421
ExpressionSyntax>
2522
{
26-
protected override CSharpSelectionValidator CreateSelectionValidator(SemanticDocument document, TextSpan textSpan, bool localFunction)
27-
=> new(document, textSpan, localFunction);
23+
protected override SelectionValidator CreateSelectionValidator(SemanticDocument document, TextSpan textSpan, bool localFunction)
24+
=> new CSharpSelectionValidator(document, textSpan, localFunction);
2825

29-
protected override CSharpMethodExtractor CreateMethodExtractor(CSharpSelectionResult selectionResult, ExtractMethodGenerationOptions options, bool localFunction)
30-
=> new(selectionResult, options, localFunction);
26+
protected override MethodExtractor CreateMethodExtractor(SelectionResult selectionResult, ExtractMethodGenerationOptions options, bool localFunction)
27+
=> new CSharpMethodExtractor(selectionResult, options, localFunction);
3128
}

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,9 @@ internal sealed partial class CSharpExtractMethodService
1515
{
1616
internal sealed partial class CSharpMethodExtractor
1717
{
18-
private sealed class CSharpAnalyzer(CSharpSelectionResult selectionResult, bool localFunction, CancellationToken cancellationToken) : Analyzer(selectionResult, localFunction, cancellationToken)
18+
private sealed class CSharpAnalyzer(SelectionResult selectionResult, bool localFunction, CancellationToken cancellationToken)
19+
: Analyzer(selectionResult, localFunction, cancellationToken)
1920
{
20-
public static AnalyzerResult Analyze(CSharpSelectionResult selectionResult, bool localFunction, CancellationToken cancellationToken)
21-
{
22-
var analyzer = new CSharpAnalyzer(selectionResult, localFunction, cancellationToken);
23-
return analyzer.Analyze();
24-
}
25-
2621
protected override bool TreatOutAsRef
2722
=> false;
2823

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ internal sealed partial class CSharpMethodExtractor
2424
private abstract partial class CSharpCodeGenerator
2525
{
2626
private sealed class ExpressionCodeGenerator(
27-
CSharpSelectionResult selectionResult,
27+
SelectionResult selectionResult,
2828
AnalyzerResult analyzerResult,
2929
ExtractMethodGenerationOptions options,
3030
bool localFunction) : CSharpCodeGenerator(selectionResult, analyzerResult, options, localFunction)
@@ -120,7 +120,7 @@ protected override ImmutableArray<StatementSyntax> GetInitialStatementsForMethod
120120

121121
private ExpressionSyntax WrapInCheckedExpressionIfNeeded(ExpressionSyntax expression)
122122
{
123-
var kind = this.SelectionResult.UnderCheckedExpressionContext();
123+
var kind = UnderCheckedExpressionContext();
124124
if (kind == SyntaxKind.None)
125125
{
126126
return expression;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ internal sealed partial class CSharpMethodExtractor
2323
private abstract partial class CSharpCodeGenerator
2424
{
2525
public sealed class MultipleStatementsCodeGenerator(
26-
CSharpSelectionResult selectionResult,
26+
SelectionResult selectionResult,
2727
AnalyzerResult analyzerResult,
2828
ExtractMethodGenerationOptions options,
2929
bool localFunction) : CSharpCodeGenerator(selectionResult, analyzerResult, options, localFunction)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ internal sealed partial class CSharpMethodExtractor
2020
private abstract partial class CSharpCodeGenerator
2121
{
2222
public sealed class SingleStatementCodeGenerator(
23-
CSharpSelectionResult selectionResult,
23+
SelectionResult selectionResult,
2424
AnalyzerResult analyzerResult,
2525
ExtractMethodGenerationOptions options,
2626
bool localFunction) : CSharpCodeGenerator(selectionResult, analyzerResult, options, localFunction)

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

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,8 @@ private abstract partial class CSharpCodeGenerator : CodeGenerator<StatementSynt
4444
private const string NewMethodPascalCaseStr = "NewMethod";
4545
private const string NewMethodCamelCaseStr = "newMethod";
4646

47-
public static Task<GeneratedCode> GenerateAsync(
48-
InsertionPoint insertionPoint,
49-
CSharpSelectionResult selectionResult,
50-
AnalyzerResult analyzerResult,
51-
ExtractMethodGenerationOptions options,
52-
bool localFunction,
53-
CancellationToken cancellationToken)
54-
{
55-
var codeGenerator = Create(selectionResult, analyzerResult, options, localFunction);
56-
return codeGenerator.GenerateAsync(insertionPoint, cancellationToken);
57-
}
58-
5947
public static CSharpCodeGenerator Create(
60-
CSharpSelectionResult selectionResult,
48+
SelectionResult selectionResult,
6149
AnalyzerResult analyzerResult,
6250
ExtractMethodGenerationOptions options,
6351
bool localFunction)
@@ -72,7 +60,7 @@ public static CSharpCodeGenerator Create(
7260
}
7361

7462
protected CSharpCodeGenerator(
75-
CSharpSelectionResult selectionResult,
63+
SelectionResult selectionResult,
7664
AnalyzerResult analyzerResult,
7765
ExtractMethodGenerationOptions options,
7866
bool localFunction)
@@ -202,9 +190,25 @@ protected override SyntaxNode GetCallSiteContainerFromOutermostMoveInVariable(Ca
202190
return declStatement.Parent;
203191
}
204192

193+
private bool ShouldPutUnsafeModifier()
194+
{
195+
var token = this.SelectionResult.GetFirstTokenInSelection();
196+
var ancestors = token.GetAncestors<SyntaxNode>();
197+
198+
// if enclosing type contains unsafe keyword, we don't need to put it again
199+
if (ancestors.Where(a => CSharp.SyntaxFacts.IsTypeDeclaration(a.Kind()))
200+
.Cast<MemberDeclarationSyntax>()
201+
.Any(m => m.GetModifiers().Any(SyntaxKind.UnsafeKeyword)))
202+
{
203+
return false;
204+
}
205+
206+
return token.Parent.IsUnsafeContext();
207+
}
208+
205209
private DeclarationModifiers CreateMethodModifiers()
206210
{
207-
var isUnsafe = this.SelectionResult.ShouldPutUnsafeModifier();
211+
var isUnsafe = ShouldPutUnsafeModifier();
208212
var isAsync = this.SelectionResult.CreateAsyncMethod();
209213
var isStatic = !AnalyzerResult.UseInstanceMember;
210214
var isReadOnly = AnalyzerResult.ShouldBeReadOnly;
@@ -269,9 +273,27 @@ private ImmutableArray<StatementSyntax> CreateMethodBody(
269273
return statements;
270274
}
271275

276+
protected SyntaxKind UnderCheckedExpressionContext()
277+
=> UnderCheckedContext<CheckedExpressionSyntax>();
278+
279+
protected SyntaxKind UnderCheckedStatementContext()
280+
=> UnderCheckedContext<CheckedStatementSyntax>();
281+
282+
protected SyntaxKind UnderCheckedContext<T>() where T : SyntaxNode
283+
{
284+
var token = this.SelectionResult.GetFirstTokenInSelection();
285+
var contextNode = token.Parent.GetAncestor<T>();
286+
if (contextNode == null)
287+
{
288+
return SyntaxKind.None;
289+
}
290+
291+
return contextNode.Kind();
292+
}
293+
272294
private ImmutableArray<StatementSyntax> WrapInCheckStatementIfNeeded(ImmutableArray<StatementSyntax> statements)
273295
{
274-
var kind = this.SelectionResult.UnderCheckedStatementContext();
296+
var kind = UnderCheckedStatementContext();
275297
if (kind == SyntaxKind.None)
276298
return statements;
277299

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

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,9 @@ internal sealed partial class CSharpExtractMethodService
1919
{
2020
internal sealed partial class CSharpMethodExtractor
2121
{
22-
private sealed class CSharpTriviaResult : TriviaResult
22+
private sealed class CSharpTriviaResult(SemanticDocument document, ITriviaSavedResult result)
23+
: TriviaResult(document, result, (int)SyntaxKind.EndOfLineTrivia, (int)SyntaxKind.WhitespaceTrivia)
2324
{
24-
public static async Task<CSharpTriviaResult> ProcessAsync(CSharpSelectionResult selectionResult, CancellationToken cancellationToken)
25-
{
26-
var preservationService = selectionResult.SemanticDocument.Document.Project.Services.GetService<ISyntaxTriviaService>();
27-
var root = selectionResult.SemanticDocument.Root;
28-
var result = preservationService.SaveTriviaAroundSelection(root, selectionResult.FinalSpan);
29-
return new CSharpTriviaResult(
30-
await selectionResult.SemanticDocument.WithSyntaxRootAsync(result.Root, cancellationToken).ConfigureAwait(false),
31-
result);
32-
}
33-
34-
private CSharpTriviaResult(SemanticDocument document, ITriviaSavedResult result)
35-
: base(document, result, (int)SyntaxKind.EndOfLineTrivia, (int)SyntaxKind.WhitespaceTrivia)
36-
{
37-
}
38-
3925
protected override AnnotationResolver GetAnnotationResolver(SyntaxNode callsite, SyntaxNode method)
4026
{
4127
var isMethodOrLocalFunction = method is MethodDeclarationSyntax or LocalFunctionStatementSyntax;

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

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,23 @@ namespace Microsoft.CodeAnalysis.CSharp.ExtractMethod;
1919

2020
internal sealed partial class CSharpExtractMethodService
2121
{
22-
internal sealed partial class CSharpMethodExtractor(CSharpSelectionResult result, ExtractMethodGenerationOptions options, bool localFunction)
22+
internal sealed partial class CSharpMethodExtractor(
23+
SelectionResult result, ExtractMethodGenerationOptions options, bool localFunction)
2324
: MethodExtractor(result, options, localFunction)
2425
{
2526
protected override CodeGenerator CreateCodeGenerator(AnalyzerResult analyzerResult)
2627
=> CSharpCodeGenerator.Create(this.OriginalSelectionResult, analyzerResult, this.Options, this.LocalFunction);
2728

28-
protected override AnalyzerResult Analyze(CSharpSelectionResult selectionResult, bool localFunction, CancellationToken cancellationToken)
29-
=> CSharpAnalyzer.Analyze(selectionResult, localFunction, cancellationToken);
29+
protected override AnalyzerResult Analyze(CancellationToken cancellationToken)
30+
{
31+
var analyzer = new CSharpAnalyzer(this.OriginalSelectionResult, this.LocalFunction, cancellationToken);
32+
return analyzer.Analyze();
33+
}
3034

3135
protected override SyntaxNode GetInsertionPointNode(
3236
AnalyzerResult analyzerResult, CancellationToken cancellationToken)
3337
{
34-
var originalSpanStart = OriginalSelectionResult.OriginalSpan.Start;
38+
var originalSpanStart = OriginalSelectionResult.FinalSpan.Start;
3539
Contract.ThrowIfFalse(originalSpanStart >= 0);
3640

3741
var document = this.OriginalSelectionResult.SemanticDocument;
@@ -61,10 +65,10 @@ protected override SyntaxNode GetInsertionPointNode(
6165
{
6266
if (currentNode is AnonymousFunctionExpressionSyntax anonymousFunction)
6367
{
64-
if (OriginalSelectionWithin(anonymousFunction.Body) || OriginalSelectionWithin(anonymousFunction.ExpressionBody))
68+
if (SelectionWithin(anonymousFunction.Body) || SelectionWithin(anonymousFunction.ExpressionBody))
6569
return currentNode;
6670

67-
if (!OriginalSelectionResult.OriginalSpan.Contains(anonymousFunction.Span))
71+
if (!OriginalSelectionResult.FinalSpan.Contains(anonymousFunction.Span))
6872
{
6973
// If we encountered a function but the selection isn't within the body, it's likely the user
7074
// is attempting to move the function (which is behavior that is supported). Stop looking up the
@@ -76,10 +80,10 @@ protected override SyntaxNode GetInsertionPointNode(
7680

7781
if (currentNode is LocalFunctionStatementSyntax localFunction)
7882
{
79-
if (OriginalSelectionWithin(localFunction.ExpressionBody) || OriginalSelectionWithin(localFunction.Body))
83+
if (SelectionWithin(localFunction.ExpressionBody) || SelectionWithin(localFunction.Body))
8084
return currentNode;
8185

82-
if (!OriginalSelectionResult.OriginalSpan.Contains(localFunction.Span))
86+
if (!OriginalSelectionResult.FinalSpan.Contains(localFunction.Span))
8387
{
8488
// If we encountered a function but the selection isn't within the body, it's likely the user
8589
// is attempting to move the function (which is behavior that is supported). Stop looking up the
@@ -146,21 +150,31 @@ SyntaxNode GetInsertionPointForGlobalStatement(GlobalStatementSyntax globalState
146150
}
147151
}
148152

149-
private bool OriginalSelectionWithin(SyntaxNode node)
153+
private bool SelectionWithin(SyntaxNode node)
150154
{
151155
if (node is null)
152156
{
153157
return false;
154158
}
155159

156-
return node.Span.Contains(OriginalSelectionResult.OriginalSpan);
160+
return node.Span.Contains(OriginalSelectionResult.FinalSpan);
157161
}
158162

159-
protected override async Task<TriviaResult> PreserveTriviaAsync(CSharpSelectionResult selectionResult, CancellationToken cancellationToken)
160-
=> await CSharpTriviaResult.ProcessAsync(selectionResult, cancellationToken).ConfigureAwait(false);
163+
protected override async Task<TriviaResult> PreserveTriviaAsync(SyntaxNode root, CancellationToken cancellationToken)
164+
{
165+
var semanticDocument = this.OriginalSelectionResult.SemanticDocument;
166+
var preservationService = semanticDocument.Document.Project.Services.GetService<ISyntaxTriviaService>();
167+
var result = preservationService.SaveTriviaAroundSelection(root, this.OriginalSelectionResult.FinalSpan);
168+
return new CSharpTriviaResult(
169+
await semanticDocument.WithSyntaxRootAsync(result.Root, cancellationToken).ConfigureAwait(false),
170+
result);
171+
}
161172

162-
protected override Task<GeneratedCode> GenerateCodeAsync(InsertionPoint insertionPoint, CSharpSelectionResult selectionResult, AnalyzerResult analyzeResult, ExtractMethodGenerationOptions options, CancellationToken cancellationToken)
163-
=> CSharpCodeGenerator.GenerateAsync(insertionPoint, selectionResult, analyzeResult, options, LocalFunction, cancellationToken);
173+
protected override Task<GeneratedCode> GenerateCodeAsync(InsertionPoint insertionPoint, SelectionResult selectionResult, AnalyzerResult analyzeResult, ExtractMethodGenerationOptions options, CancellationToken cancellationToken)
174+
{
175+
var codeGenerator = CSharpCodeGenerator.Create(selectionResult, analyzeResult, options, this.LocalFunction);
176+
return codeGenerator.GenerateAsync(insertionPoint, cancellationToken);
177+
}
164178

165179
protected override AbstractFormattingRule GetCustomFormattingRule(Document document)
166180
=> FormattingRule.Instance;

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,9 @@ internal abstract partial class CSharpSelectionResult
2424
private sealed class ExpressionResult(
2525
SemanticDocument document,
2626
SelectionType selectionType,
27-
TextSpan originalSpan,
2827
TextSpan finalSpan,
2928
bool selectionChanged) : CSharpSelectionResult(
30-
document, selectionType, originalSpan, finalSpan, selectionChanged)
29+
document, selectionType, finalSpan, selectionChanged)
3130
{
3231
public override bool ContainingScopeHasAsyncKeyword()
3332
=> false;

0 commit comments

Comments
 (0)