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
Prevent multiple return statements in a loop when replacing expressions #5030
Conversation
Codecov Report@@ Coverage Diff @@
## master #5030 +/- ##
==========================================
+ Coverage 89.15% 89.17% +0.02%
==========================================
Files 203 203
Lines 9823 9824 +1
Branches 2616 2615 -1
==========================================
+ Hits 8758 8761 +3
+ Misses 1065 1063 -2
Continue to review full report at Codecov.
|
Nice job! 👍 |
let uid = loop.getData("expressionReplacementReturnUid"); | ||
|
||
if (!uid) { | ||
let callee = this.get("callee"); |
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.
const
@@ -0,0 +1,9 @@ | |||
let a = do { | |||
while (p = p.parentPath) { |
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.
I'd feel better if there was a let p
before the let a
😃
uid = callee.scope.generateDeclaredUidIdentifier("ret"); | ||
callee.get("body").pushContainer("body", t.returnStatement(uid)); | ||
loop.setData("expressionReplacementReturnUid", uid); | ||
} |
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.
We should probably do
} else {
uid = t.identifier(uid.name);
}
or else we'll be inserting the same identifier node in multiple places in the AST.
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.
👍
One small comment, and this has a conflict, but otherwise looks good. |
edf5bdb
to
9c46fd6
Compare
Could definitely use some guidance on this, as I'm still digging through a lot of this :P