Instance methods on inferred type instances which return never don't trigger unreachable code

[claycoleman]

Bug Report

🔎 Search Terms

return never instance method unreachable code

🕗 Version & Regression Information

Saw this first on version 4.4.4, confirmed still exists in 4.7.4

⏯ Playground Link

https://www.typescriptlang.org/play?#code/GYVwdgxgLglg9mABFAFgJzgdwM4AoCUAXImAKYBupaiA3gFCKPLpYmmaICiaGaBdAXzp1QkWAmSlsUAKpg0pAIYQUigEYAbUgEYCtBk1QYcBANwHGFxAHprybImAbFAcweoYDxQ-ALlqzVIrCARsOC0AOg04F1wAIk9EXyUVdS0AQjj8QWFciGdsBwAVKSgAYQKHeiZmYzwiNkpqaprDFg4yDm5efhqhIToQsGlEEukK72wASWGoRUhSRABeNg4x8sr+EXBoeCQoUrk-VMCAJj0WxnWJwpnpeYhSCKMsevMrW0QAPh+f+0QAFQAgAmcCkJDgUCBjmcLmYiW8SXkKQCWmCoXCT2isTi2BQcBAGmBiDUi2S-jSi0I8WyAiAA

💻 Code

function throws(): never {
    throw new Error()
}

function testUnreachable1() {
    throws();
    
    // ts flags this as unreachable
    console.log("is unreachable!")
}




class TestClass {
    throws(): never {
        throw new Error()
    }
}

const TestClassInstance = new TestClass()

function testUnreachable2() {
    TestClassInstance.throws();

    // >>>>> ts **does not** flag this as unreachable
    console.log("should be unreachable :(")
}

🙁 Actual behavior

Unlike a stand-alone function, instance methods which return never don't mark the following lines in the block as unreachable code

🙂 Expected behavior

Similar to a stand-alone function, instance methods which return never should mark the following lines in the block as unreachable code

[fatcerberus]

Workaround:

const TestClassInstance: TestClass = new TestClass()

This is because, per #32695,

A function call is analyzed as an assertion call or never-returning call [only] when:

1. the call occurs as a top-level expression statement, and
2. the call specifies a single identifier or a dotted sequence of identifiers for the function name, and
3. each identifier in the function name references an entity with an explicit type, and
4. the function name resolves to a function type with an asserts return type or an explicit never return type annotation.

Your code fails condition 3 because TestClassInstance's type is inferred.

[claycoleman]

thanks @fatcerberus ! still seems a bit strange but the workaround is great

[holtwick]

Thanks for the workaround @fatcerberus. I see it like @claycoleman. The whole thing does not feel right and makes the development of e.g. a logger that is supposed to support assert very impractical. It would be nice if this could be adapted, if it does not fundamentally contradict the logic of the function.

I use it here https://github.com/holtwick/zeed/blob/95e544199e1ceb2ffd8235f0c5d79bc6beadf9b1/src/common/log-base.ts#L70

[fatcerberus]

@holtwick It's not really intentional; it's a design limitation arising from the fact that control-flow analysis happens before type inference. At the time the call is analyzed, if it has no type annotation, TS has no way to know it should create a control-flow node for it (and can't create a node for every single call because that would tank compile times). It's not ideal, but my understanding from past discussions around this limitation is that it's been implemented this way because doing otherwise would introduce circularities (i.e. type narrowing depends on inference, which in turn depends on the results of narrowing, etc. ad infinitum)

Per #32695:

This particular rule exists so that control flow analysis of potential assertion calls doesn't circularly trigger further analysis.

Fixing it would essentially require a ground-up rewrite of the internal type inference/CFA machinery in the compiler.

[holtwick]

I see, thanks for the details @fatcerberus Then it might be an idea to alternatively point to the workaround in the compiler's error message or otherwise suggest explicit type declarations to the developer?

[fatcerberus]

There's no direct error from this though; it's just a type narrowing operation that didn't happen and/or unreachable code that wasn't detected as unreachable. There may be errors further down the line resulting from that, but at that point it's just another "this is the wrong type" error; there's no context to know the error arises from a missed assertion call (since the compiler didn't even know it was an assert at the time--and if it could figure that out after the fact, this limitation wouldn't exist).

There probably should be a FAQ entry for this though, since people hit it reasonably often and it is indeed unintuitive.