Add type annotations to all tokenizaton-related code #1709

wants to merge 1 commit into


None yet

4 participants

ariya commented Jan 3, 2017

Fix #1705

codecov-io commented Jan 3, 2017 edited

Current coverage is 100% (diff: 100%)

Merging #1709 into master will not change coverage

@@           master   #1709   diff @@
  Files           1       1          
  Lines        4004    4005     +1   
  Methods         0       0          
  Messages        0       0          
  Branches      701     699     -2   
+ Hits         4004    4005     +1   
  Misses          0       0          
  Partials        0       0          

Powered by Codecov. Last update 0cdc8e0...c67a81f


Looks good. Approving, but I had a question about the Unreachable line...

@@ -256,9 +266,12 @@ export class JSXParser extends Parser {
+ /* istanbul ignore next */
+ throw new Error('Unreachable');
mikesherov Jan 3, 2017 Member

why is this line needed?

ariya Jan 4, 2017 Contributor

Without the line, the TypeScript compiler will complain: error TS2366: Function lacks ending return statement and return type does not include 'undefined'.

@DanielRosenwasser Could this be another problem with the flow analysis? throwUnexpectedToken() does nothing but to throw an exception, thus the compiler should not treat this function as terminated with an undefined return value.

DanielRosenwasser Jan 4, 2017 edited

You could instead write return this.scanner.throwUnexpectedToken(), which I assume returns never.

ariya Jan 4, 2017 Contributor

@DanielRosenwasser Somehow it still does not work. I think it's because this.scanner.throwUnexpectedToken() does not directly throw, it calls this.scanner.errorHandler.throwError(...) which does throw.

Strangely, if I inline the content of this.scanner.throwUnexpectedToken() here, then the compiler is quite happy.

DanielRosenwasser Jan 4, 2017

If you add a type annotation to throwError of type never, and then have throwUnexpectedToken actually return the call to throwError, you should get it working.

ariya Jan 4, 2017 Contributor

@DanielRosenwasser That does the trick! Thank you so much 👍

@ariya ariya Add type annotations to all tokenization-related code
Fix #1705
@ariya ariya added a commit that closed this pull request Jan 4, 2017
@ariya ariya Add type annotations to all tokenization-related code
Fix #1705
Closes gh-1709
@ariya ariya closed this in 11bb115 Jan 4, 2017
@ariya ariya deleted the ariya:type-annotation branch Jan 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment