Return diagnostics instead of throwing errors#769
Return diagnostics instead of throwing errors#769Perryvw merged 49 commits intoTypeScriptToLua:masterfrom
Conversation
ark120202
force-pushed
the
diagnostics
branch
from
2af391e to
0306f28
Compare
ark120202
force-pushed
the
diagnostics
branch
from
c511364 to
ef22253
Compare
This reverts commit 9312a0c.
| return transformIdentifier(context, name); | ||
| } else if (ts.isBindingElement(name) && ts.isIdentifier(name.name)) { | ||
| } else if (ts.isBindingElement(name)) { | ||
| // TODO: It should always be true when called from `transformVariableDeclaration`, |
There was a problem hiding this comment.
so what is the action TODO here? Handle the potential failure?
There was a problem hiding this comment.
The assertion below is invalid in the case of for (const [[x]] of string.gmatch('', '')) {}
There was a problem hiding this comment.
That does not really answer my question: What is the action we have to do here?
There was a problem hiding this comment.
Well, handle it somehow - assert is meant to never fail under the expected conditions. I haven't looked much into it, but in case of transformVariableDeclaration it's only used in the optimization transform, so maybe the same thing should be done in the other case
| statement: ts.ForOfStatement, | ||
| block: lua.Block | ||
| ): lua.Statement { | ||
| let valueVariable: lua.Identifier; |
There was a problem hiding this comment.
Would have preferred this ForIn refactor to have been in its own PR...
There was a problem hiding this comment.
Just found the easiest way to resolve TODOs ¯\_(ツ)_/¯
It was already changed quite a lot for diagnostics anyway: master...9eaa4ad#diff-198b7c2ac4e943adf558f34c21ac4143
| return transformIdentifier(context, name); | ||
| } else if (ts.isBindingElement(name) && ts.isIdentifier(name.name)) { | ||
| } else if (ts.isBindingElement(name)) { | ||
| // TODO: It should always be true when called from `transformVariableDeclaration`, |
There was a problem hiding this comment.
That does not really answer my question: What is the action we have to do here?
2 participants
Resolves #412, closes #567.