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
[SR-5739] Refactoring action to collapse nested if statement #48309
Comments
Comment by Maksym Grebenets (JIRA) Hi there. I was really interested in getting into basics of Swift development by looking at this ticket. But then I later noticed that it's been already assigned. I hope it wouldn't cause any misunderstanding, I'm not really trying to steal the thunder here 🙂 So I did some work and so far I was able to walk AST and collect enough information to decide whether refactoring is applicable, for example: struct NestedIfStatementsInfo {
IfStmt *FirstIfStmt = nullptr;
llvm::SmallVector<StmtCondition, 4> Conditions;
BraceStmt *LastThenStmt = nullptr;
bool isApplicable() { return FirstIfStmt && Conditions.size() > 1 && LastThenStmt; }
StmtCondition makeCombinedStmtCondition() {
llvm::SmallVector<StmtConditionElement, 4> AllStmtConditionElements;
std::for_each(Conditions.begin(), Conditions.end(), [&](StmtCondition SC) {
AllStmtConditionElements.append(SC.begin(), SC.end());
});
return StmtCondition(AllStmtConditionElements);
}
}; Here FirstIfStmt is the "root" if-statement, or in other words the first in a sequence of collapsible if-statements. Conditions is list of conditions for all collapsible if-statements. LastThenStmt is the actual "body" sitting inside all those collapsible ifs: // Pseudocode
FirstIfStmt(Condition1) {
SecondIfStmt(Condition2) {
N-thIfStmt(ConditionN) {
LastThenStmt
}
}
}
// After refactoring
NewCondition = Condition1 + Condition2 + ... + ConditionN
NewIfStmt(NewCondition) {
LastThenStmt
} This gives me enough info to decide if refactoring is applicable and should be enough to perform the refactoring action. As a matter of fact, I have code that successfully passes "Is refactoring kind applicable" tests for a bunch of sample code. The tough (for me) question I have is "How to perform such a refactoring?". Does it have to be manipulations with text directly or is there a way to modify AST by using setCond() and setThenStmt()? I've tried something like Result.FirstIfStmt->setThenStmt(Result.LastThenStmt);
Result.FirstIfStmt->setCond(Result.makeCombinedStmtCondition()); Not sure that 1st line is even safe, I'm changing ThenStmt pointer to point to LastThenStmt. Is such operation even correct and will it actually cause changes in AST? Even if I could work out the way to update AST, it still has to be written back to the EditConsumer in some way, right? Any help or advice would be really appreciated! |
Comment by Will T Ellis (JIRA) I tried modifying the AST, but that doesn't seem to be the right approach. Instead, I use information from the AST to create new text and replace the original range of text in the EditConsumer. You can find my work-in-progress PR here. |
Hey mgrebenets (JIRA User), thank you for your interest in Swift development! Your logic about when this refactoring is applicable makes sense to me. For the code modification part, as willtellis (JIRA User) suggested, we can only manipulate text to realize it. The long term goal is to use Swift's new syntax library to enable tree transformation, but this is still work in progress. |
Comment by Maksym Grebenets (JIRA) Thanks willtellis (JIRA User) and @nkcsgexi That was helpful. I've looked at your work in progress willtellis (JIRA User) and learned tons 🙂 I had no idea this action could actually be implemented as cursor-based refactoring. I also totally missed the fact we can just "copy-paste" code from original editor context.. Thanks. |
Comment by Maksym Grebenets (JIRA) Hi @nkcsgexi. Not sure if this is the best place to ask, but I have lots of questions on building and trying out the toolchain in Xcode. I've read your post here as well as Brian's post [here|https://modocache.io/getting-started-with-swift-development,] but there still a lot of unknowns for a Swift Dark Side beginner like me... There are question like
I mean this is probably not the best place to ask questions like this. There isn't Comments section on Swift.org blog either. Is there a "designated" place then? |
Hi mgrebenets (JIRA User), All these questions are valuable and i'm sure you are not the only one who are interested in getting the answer. Instead of discussing this in the issue thread, could you email these questions to swift-dev@swift.org? Xi |
Comment by Will T Ellis (JIRA) |
Attachment: Download
Additional Detail from JIRA
md5: 117ec8e1f9cb632e9ad9dd725519d643
Issue Description:
Implement action to change code from:
to
The action could also apply for if-let statements.
The text was updated successfully, but these errors were encountered: