Methods that always throw aren't always recognized as a return from a function

[yagoldin]

TypeScript Version: 3.9.2

Search Terms:
throw, return
Code

class Logger {
    throwError() : never {
        throw Error();
    }
}

class Foo {
    logger: Logger;

    constructor() {
        this.logger = new Logger();
    }

    // does not compile
    fail(x : number) : number {
        if (x < 0) {
            return x;
        } else {
            const logger = new Logger();
            logger.throwError();
        }
    }

    // does compile
    success1(x : number) : number {
        if (x < 0) {
            return x;
        } else {
            this.logger.throwError();
        }
    }

    // does compile
    success2(x: number, logger: Logger) : number {
        if (x < 0) {
            return x;
        } else {
            logger.throwError();
        }
    }
}

Expected behavior:
In all of these cases, the code should compile because logger.throwError() will always throw an error, therefore returning from the current function.

Actual behavior:
However, for the fail method, you get the compiler error

Function lacks ending return statement and return type does not include 'undefined'.(2366)

I'm not sure why the difference between having the logger as a field/parameter vs. a local variable causes some cases to compile but some not.

Playground Link:
https://www.typescriptlang.org/play/#code/MYGwhgzhAEAyD2BzRBTATtA3gWAFDQOgBcALNeAdwFE1y0AKASmgC5oA7FAN3Sz0IHEylaDTpMA3P0IBfPHNx5QkGADF48PvkIgkqNGwTJ0UxdoLB47CETQBXYEXgNmOc4NIBLCADpdxjABeDhQKOD10SWkCBWjoAHp46AATeBQYdngiaEsAWwAHTxAUOIAzMCL6AA9WDjtcgCN0ZjZ2eqaMN0FCT1LoaugAHmgABlc47oI0FCI7NHZoKtNJ6BloFBAIFC0VgUtrbP99aGDOMKN9KPcVo-QfUnJqWmcrlYUBWPdElLSYPMLinEIA5gOkIABGAatdrNWptRq8LqTXr9GrDMY7XZTGZzBZLCaydabbZIrFeXy3ND3YRPcSMZaTd6yPBxb6pdI5eAFIoldzA4CgqAAJmq0IRaAANNBKYYImgWnVxZjuiiBujxtdJtNZvNFgzumsNltlbtKdTHmIXvSCTE4goZEA

Related Issues:

[DanielRosenwasser]

I'm embarrassed because the workaround is so silly, but it will work if you annotate logger.

- const logger = new Logger();
+ const logger: Logger = new Logger();

The issues are explained a bit on #34573.

When we tried to start recognizing never-returning function calls, we tried to warn users that we couldn't recognize this the circularities that might occur in the control flow graph in #33622; however, we realized that it would break too much code using never-returning functions.

Maybe @ahejlsberg can correct me if I got anything wrong.

[ahejlsberg]

@DanielRosenwasser Your explanation is right, but the whole issue is obviously pretty subtle. In #33622 it was possible for us to add errors when assertion functions are called in a manner than doesn't allow control flow analysis because assertion functions were a new feature, but we couldn't do it for never-returning functions because it broke existing code.