Assertion error with if-else-if inside class

[jeffreymorlan]

Type-checking the following (broken) program:

class C {
    if (a) {}
    else if (0) {}
}

results in an assertion error:

Error: Debug Failure. False expression: 
    at Object.assert (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:724:23)
    at reportImplementationExpectedError (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:18530:38)
    at checkFunctionOrConstructorSymbol (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:18580:25)
    at checkFunctionLikeDeclaration (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:18943:21)
    at checkMethodDeclaration (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:18251:13)
    at checkSourceElement (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:20413:28)
    at Object.forEach (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:90:30)
    at checkClassDeclaration (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:19663:16)
    at checkSourceElement (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:20477:28)
    at Object.forEach (/home/jeffrey/TypeScript-1.6.2/lib/tsc.js:90:30)

[mhegazy]

A PR would be appreciated.

[jeffreymorlan]

I don't know what the right fix to this is, but here's my analysis:

• The "if"s are getting parsed as MethodDeclarations. The first method node's .end is less than the second method node's .pos because of the intervening erroneous "else", which does not become a node.

• checkFunctionOrConstructSymbol loops over declarations of a symbol. If declarations are not "consecutive", it calls reportImplementationExpectedError on the earlier one:

  else if (!isExportSymbolInsideModule && previousDeclaration && previousDeclaration.parent === node.parent && previousDeclaration.end !== node.pos) {
      reportImplementationExpectedError(previousDeclaration);
  }

• reportImplementationExpectedError assumes that its node and its next sibling (found by ts.forEachChild) must be for different symbols (and thus differ in some way: kind, name, or static-ness). Because if they were for the same symbol, those would be "consecutive" declarations and not an error.

The problem is the two different definitions of what makes nodes "consecutive". Is it node.pos === prev.end, or being next in the ts.forEachChild iteration? cFOCS uses the former definition, while rIEE uses the latter. And with the current parser, they are not the same.

[mhegazy]

@vladima any thoughts?

[jeffreymorlan]

I've realized it would be inefficient for checkFunctionOrConstructorSymbol to walk the tree since it has to check for consecutiveness on every function/method overload, whether error or not, and having to scan the list of children for each means checking a class becomes potentially O(n²). So it's reportImplementationExpectedError that needs to change. PR

[DanielRosenwasser]

Thanks @jeffreymorlan!