-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extract method cleanup #76550
base: main
Are you sure you want to change the base?
Extract method cleanup #76550
Conversation
|
||
public bool IsExtractMethodOnSingleStatement() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to SelectionInfo.GetSelectionType. This should be computed once, not over and oer again.
{ | ||
return false; | ||
} | ||
if (!this.ContainsValidSelection) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pulled out common logic from VB and C#.
static bool CanMergeExistingSpineWithCurrent(ISyntaxFacts syntaxFacts, T existing, T current) | ||
=> syntaxFacts.AreStatementsInSameContainer(existing, current); | ||
} | ||
protected (TStatementSyntax firstStatement, TStatementSyntax lastStatement)? GetStatementRangeContainingSpan( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for this method to be generic, it was always being passed the TStatementSyntax type.
} | ||
|
||
return (firstStatement, lastStatement); | ||
|
||
static bool CanMergeExistingSpineWithCurrent(ISyntaxFacts syntaxFacts, T existing, T current) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inlined this method. it no longer uses syntaxFacts, it ust calls into an abstract method on this type.
return (firstStatement, lastStatement); | ||
} | ||
|
||
protected sealed class SelectionInfo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extracted to file.
if (token1.Parent == null || token2.Parent == null) | ||
{ | ||
return null; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these can't happen in real code. so just changed to GetrequiredParent() to ensure that.
Cleanup work in the extract method codebase before doign some more major feature work. I've documented int eh PR what changes i'm making. Should be reviewed with whitespace off.