switch on enum exhaustive for direct return but not for indirect return

[rogierschouten]

TypeScript Version: 2.1.4

Code

enum MyEnum {
	A,
	B
}

function thisGivesError(e: MyEnum): string {
	let s: string;
	switch (e) {
		case MyEnum.A: s = "it was A"; break;
		case MyEnum.B: s = "it was B"; break;
	}
	return s; // Variable 's' is used before being assigned
}

function good1(e: MyEnum): string {
	let s: string;
	switch (e) {
		case MyEnum.A: s = "it was A"; break;
		case MyEnum.B: s = "it was B"; break;
		default: s = "it was something else"; break;
	}
	return s; // ok
}

function good2(e: MyEnum): string {
	switch (e) {
		case MyEnum.A: return "it was A";
		case MyEnum.B: return "it was B";
	}
}

tsconfig.json:

{
	"compilerOptions": {
		"alwaysStrict": true,
		"declaration": true,
		"forceConsistentCasingInFileNames": false,
		"inlineSourceMap": true,
		"inlineSources": true,
		"module": "commonjs",
		"noFallthroughCasesInSwitch": true,
		"noImplicitAny": true,
		"noImplicitReturns": true,
		"noImplicitThis": true,
		"noLib": false,
		"noUnusedLocals": true,
		"noUnusedParameters": true,
		"outDir": "./dist",
		"preserveConstEnums": true,
		"pretty": true,
		"removeComments": false,
		"strictNullChecks": true,
		"suppressImplicitAnyIndexErrors": true,
		"target": "es5"
	},
}

Expected behavior:

Compiles

Actual behavior:

In the first function, returns 'Variable s is used before being assigned'. On the one hand, the compiler knows that the switch statment is exhaustive (see both functions below) and on the other hand it doesn't know that therefore the variable is assigned.

Might be related to #12771

[zpdDG4gta8XKpMCd]

it looks like that what you want is to blindly trust your contract, which is an imaginary thing that you put in your code, trying to describe the real world, hoping you got yourself covered from all unexpected things out there, i wish i had your confidence

in our realm we can't trust the contract, and this is how we go about it

function good1(e: MyEnum): string {
	let s: string;
	switch (e) {
		case MyEnum.A: s = "it was A"; break;
		case MyEnum.B: s = "it was B"; break;
		default: throw new Error('Unable to do good1. Unexpected runtime value that somehow slipped into my code and cannot be interpreted as MyEnum. Probably due to corrupt or updated server data contracts or something.');
	}
	return s; // ok
}

[rogierschouten]

I understand your point, and if you are right then I would expect a compiler error 'not all control paths return a value' in function good2.

My point is that therefore compiler behaviour is currently inconsistent.

And about the confidence part: please don't judge me on example code, and let's keep the tone of the discussion professional. I'm trying to help here.

[zpdDG4gta8XKpMCd]

good2 definitely looks like a bug

[rogierschouten]

I discussed this with my collegues, and it is not quite that clear whether the bug is in the error() function or the good2() function.

Consider what a programmer would do if good2 would start giving compile errors: you would add a default clause to the switch. With the result that you will no longer notice any problems when you add a member to the enum type. The compiler as it is implemented now helps you in refactoring your code.

The point that aleksey-bykov makes is that the caller could provide an undefined or null value. That depends on the use case: if the code is only called by other typescript code then this is generally not possible (assuming strict null checks). It depends on the use case how many run-time assertions on input values you are going to put into your method. The compiler, on the other hand, can only assume that the input arguments are according to the parameter types. It does so for most other type inference, control flow analysis and compile errors, so why not this one.

[zpdDG4gta8XKpMCd]

as i see it, there is only one question: is deafult case a valid and mandatory execution branch? good2 testifies it is not (as of now)

in my opinion it has to be mandatory

[krryan]

(I work with @Aleksey-Bykov and want to give some insight into what we do.)

Consider a function like function never(impossible: never, message: string): void { throw new Error(message); }. At compile-time, it requires you to pass in a never value, that is, ensure you already have exhausted all known-at-compile-time possibilities. We can add this to a default statement in a switch, providing we have exhausted those possibilities:

function good1(e: MyEnum): string {
	let s: string;
	switch (e) {
		case MyEnum.A: s = "it was A"; break;
		case MyEnum.B: s = "it was B"; break;
		default: never(e, 'Unable to do good1. Unexpected runtime value that somehow slipped into my code and cannot be interpreted as MyEnum. Probably due to corrupt or updated server data contracts or something.');
	}
	return s; // ok
}

Adding this to the default statement of a switch causes compile errors if the case statements of the switch does not exhaust all possibilities, since in such a case the value will not be never type. This covers the concern that @rogierschouten raises; if we add a new value to MyEnum, this switch now has a compile-time error. However, unlike simply not having a default statement, it also fails fast—an error is thrown immediately if some unexpected value appears during runtime that we did not account for at compile-time. That saves having an error caused by an undefined value that needs to be traced back to this function before you realize it happened because of an unexpected MyEnum value.

[sandersn]

Unfortunately, the compiler does not actually know the the first switch is exhaustive; exhaustiveness checking is syntactic, meaning that the only sure way to make it work is to have a default clause.

There is a special case for switch statements that are the last statement and whose case blocks all return. The checker will verify that such cases are exhaustive after binding is done. But all that special case does is remove the "return type does not include 'undefined'" error. And it's easily defeated; the following code still shows the error, even though the console.log is dead code:

function good2(e: MyEnum): string {
  switch (e) {
    case MyEnum.A: return "it was A";
    case MyEnum.B: return "it was B";
  }
  console.log("it was a mistake");
}

Since control flow and therefore exhaustiveness checking is built during binding based on syntactic structure, this is a basic architectural limitation.

[alamothe]

You will have to rewrite thisGivesError to use a function to get exhaustiveness

This guide: http://www.typescriptlang.org/docs/handbook/advanced-types.html explains really well how to get exhaustiveness (search for "Exhaustiveness checking")