Cleanup Extract method code (part 3) #76587
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -177,8 +177,8 @@ protected static async Task TestSelectionAsync( | |||
| Assert.True(status.Succeeded); | |||
| } | |||
|
|
|||
| if (status.Succeeded && result.SelectionChanged) | |||
| Assert.Equal(namedSpans["r"].Single(), result.FinalSpan); | |||
| if (status.Succeeded && namedSpans.TryGetValue("r", out var revisedSpans)) | |||
There was a problem hiding this comment.
these tests were busted. they only ran if the code set this boolean. meaning if the code didn't set the oolean, the test might still specify these spans, but not validate tehm. i've at least updated it to always validate if present.
| @@ -25,9 +25,8 @@ internal abstract partial class CSharpSelectionResult | |||
| private sealed class ExpressionResult( | |||
| SemanticDocument document, | |||
| SelectionType selectionType, | |||
| TextSpan finalSpan, | |||
| bool selectionChanged) : CSharpSelectionResult( | |||
There was a problem hiding this comment.
remove this valiue. it was there to just signal data to tests, but iddn't affect behavior in any way.
| @@ -7,7 +7,6 @@ namespace Microsoft.CodeAnalysis.ExtractMethod; | |||
| internal enum DeclarationBehavior | |||
| { | |||
| None, | |||
| Delete, | |||
There was a problem hiding this comment.
completely unread. removed since i'm trying to wrap my head around all these values and what tjhey do.
|
|
||
| public IEnumerable<VariableInfo> GetVariablesToMoveIntoMethodDefinition(CancellationToken cancellationToken) | ||
| => Variables.Where(v => v.GetDeclarationBehavior(cancellationToken) == DeclarationBehavior.MoveIn); | ||
|
|
||
| public IEnumerable<VariableInfo> GetVariablesToMoveOutToCallSite(CancellationToken cancellationToken) |
| status = TryCheckVariableType(semanticModel, context, analyzeResult.GetVariablesToSplitOrMoveIntoMethodDefinition(cancellationToken), status); | ||
| status = TryCheckVariableType(semanticModel, context, analyzeResult.MethodParameters, status); | ||
| status = TryCheckVariableType(semanticModel, context, analyzeResult.GetVariablesToMoveOutToCallSite(cancellationToken), status); | ||
| status = TryCheckVariableType(semanticModel, context, analyzeResult.GetVariablesToSplitOrMoveOutToCallSite(cancellationToken), status); |
There was a problem hiding this comment.
this code was incredibly wacky. it seemed to be trying to slice the analyzeResult.Variables into all abutting subsets, to then check to see if particular disallowed types were used. But it could just access analyzeResult.Variables insetad.
|
@JoeRobich @ToddGrun hopefully last cleanup pass PR. |
Followup to #76585