Throw a syntax error for a declare function with a body#12054
Throw a syntax error for a declare function with a body#12054nicolo-ribaudo merged 5 commits intobabel:mainfrom
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/28585/ |
|
May need to check ancestry for declare namespace n {
function foo() {}
}is also invalid. |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f2ff434:
|
|
@JLHwung I've fixed it so can you review? |
| type: string, | ||
| isMethod?: boolean = false, | ||
| // eslint-disable-next-line no-unused-vars -- this is used in /plugins/typescript | ||
| isDeclare?: boolean = false, |
There was a problem hiding this comment.
Not a big fan of modifying the base parser for this, but it does seem like it might be complicated to overload or wrap a bunch of functions in the plugin. Curious what the rest of the team thinks.
|
Maybe we can add a Boolean |
|
If |
|
Yes, I think it would help (I haven't checked the code yet), if we set it before parsing the params. |
8d0569d to
c242660
Compare
|
I addressed review comments (3471de1) and add test for |
| if ( | ||
| // $FlowIgnore | ||
| node.declare | ||
| ) { | ||
| this.finishNode(node, bodilessType); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Question: Why do we need to special-case this, rather than just parsing the (invalid) function body?
There was a problem hiding this comment.
What does special-case mean?
There was a problem hiding this comment.
I don't know if it's the same as what you're pointing out, I noticed a my mistake. I should have used super.parseFunctionBodyAndFinish.
EDIT: I've fixed at f2ff434
There was a problem hiding this comment.
I think you can remove this whole if, since parseFunctionBodyAndFinish would already be done anyway at the end of the function.
There was a problem hiding this comment.
I got it. But I think we need this if to use bodilessType as the function node type.
* Install babel/parser 7.12 * Add tests for babel/babel#12161 * Add test for babel/babel#12076 * Add test for babel/babel#12085 * Add test for babel/babel#12108 * Add test for babel/babel#12120 * Add test for babel/babel#12054 * Add test for babel/babel#12061 * Add test babel/babel#12093 * Add test for babel/babel#12065 * Add test for babel/babel#12111 * Add test for babel/babel#12072 * Switch syntax-module-attributes to syntax-import-assertion * Support "String import/export specifier" * Remove tests for module-attributes * Add changelog * Update to 7.12.3 * Fix by linter * Fix by spellchecker * Add tests for module attributes to errors * Add error test for module string name with import * Remove TSTypeCastExpression * Add tests for funny import-assertions * Update snapshots| * Add more tests
Sorry for creating PR for an issue that hasn't been triaged. If this isn't needed, please close.