From 2e69a52b39452b3d119dec8158f52b15f707c686 Mon Sep 17 00:00:00 2001 From: Jack Williams Date: Sat, 17 Aug 2019 14:40:39 +0100 Subject: [PATCH] Add exhaustiveness checking for try,catch,re-throw. --- src/compiler/checker.ts | 22 ++- .../exhaustiveSwitchImplicitReturn.errors.txt | 62 ++++++- .../exhaustiveSwitchImplicitReturn.js | 113 ++++++++++++ .../exhaustiveSwitchImplicitReturn.symbols | 143 +++++++++++++++ .../exhaustiveSwitchImplicitReturn.types | 169 ++++++++++++++++++ .../exhaustiveSwitchImplicitReturn.ts | 57 ++++++ 6 files changed, 557 insertions(+), 9 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 62308cbfbbf78..bf21e1aef1b8a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -23648,15 +23648,21 @@ namespace ts { return eachTypeContainedIn(mapType(type, getRegularTypeOfLiteralType), switchTypes); } - function functionHasImplicitReturn(func: FunctionLikeDeclaration) { - if (!(func.flags & NodeFlags.HasImplicitReturn)) { - return false; - } + function isTotalStatement(statement: Statement): boolean { + return statement.kind === SyntaxKind.SwitchStatement && isExhaustiveSwitchStatement(statement) || + statement.kind === SyntaxKind.TryStatement && + ((statement).catchClause !== undefined) && + some((statement).catchClause!.block.statements, statement => statement.kind === SyntaxKind.ThrowStatement) && + some((statement).tryBlock.statements, isTotalStatement); + } - if (some((func.body).statements, statement => statement.kind === SyntaxKind.SwitchStatement && isExhaustiveSwitchStatement(statement))) { - return false; - } - return true; + function functionHasImplicitReturn(func: FunctionLikeDeclaration) { + // A function has an implicit return if its body contains no total statements. + // A total statement explicitly returns or diverges, with the following cases recognized: + // (1) an exhaustive switch statement. + // (2) a try-catch statement where the catch clause always + // throws and the try block contains a total statement. + return !!(func.flags & NodeFlags.HasImplicitReturn) && !some((func.body).statements, isTotalStatement); } /** NOTE: Return value of `[]` means a different thing than `undefined`. `[]` means func returns `void`, `undefined` means it returns `never`. */ diff --git a/tests/baselines/reference/exhaustiveSwitchImplicitReturn.errors.txt b/tests/baselines/reference/exhaustiveSwitchImplicitReturn.errors.txt index 43b5467f0d20f..e6c74edd3adf1 100644 --- a/tests/baselines/reference/exhaustiveSwitchImplicitReturn.errors.txt +++ b/tests/baselines/reference/exhaustiveSwitchImplicitReturn.errors.txt @@ -1,7 +1,8 @@ tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts(35,32): error TS7030: Not all code paths return a value. +tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts(82,28): error TS7030: Not all code paths return a value. -==== tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts (1 errors) ==== +==== tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts (2 errors) ==== function foo1(bar: "a"): number { switch(bar) { case "a": @@ -44,4 +45,63 @@ tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts(35,32): error TS7030: Not return 1; } } + + // Repro from #32905. + + enum Foo { + One, + Two, + Three + } + + function test2(type: Foo): number { + try { + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + return 0; + } + } catch (e) { + throw new Error('some error') + } + } + + function test3(type: Foo): number { + try { + console.log('some switch') + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + return 0; + } + } catch (e) { + console.log('some error') + throw new Error('some error') + } + } + + function test4(type: Foo): number { + ~~~~~~ +!!! error TS7030: Not all code paths return a value. + try { + console.log('some switch') + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + 0; + } + } catch (e) { + console.log('some error') + throw new Error('some error') + } + } \ No newline at end of file diff --git a/tests/baselines/reference/exhaustiveSwitchImplicitReturn.js b/tests/baselines/reference/exhaustiveSwitchImplicitReturn.js index 5f8bf348c5f40..3c177f5ca8c89 100644 --- a/tests/baselines/reference/exhaustiveSwitchImplicitReturn.js +++ b/tests/baselines/reference/exhaustiveSwitchImplicitReturn.js @@ -39,6 +39,63 @@ function foo5(bar: "a" | "b"): number { return 1; } } + +// Repro from #32905. + +enum Foo { + One, + Two, + Three +} + +function test2(type: Foo): number { + try { + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + return 0; + } + } catch (e) { + throw new Error('some error') + } +} + +function test3(type: Foo): number { + try { + console.log('some switch') + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + return 0; + } + } catch (e) { + console.log('some error') + throw new Error('some error') + } +} + +function test4(type: Foo): number { + try { + console.log('some switch') + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + 0; + } + } catch (e) { + console.log('some error') + throw new Error('some error') + } +} //// [exhaustiveSwitchImplicitReturn.js] @@ -75,3 +132,59 @@ function foo5(bar) { return 1; } } +// Repro from #32905. +var Foo; +(function (Foo) { + Foo[Foo["One"] = 0] = "One"; + Foo[Foo["Two"] = 1] = "Two"; + Foo[Foo["Three"] = 2] = "Three"; +})(Foo || (Foo = {})); +function test2(type) { + try { + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + return 0; + } + } + catch (e) { + throw new Error('some error'); + } +} +function test3(type) { + try { + console.log('some switch'); + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + return 0; + } + } + catch (e) { + console.log('some error'); + throw new Error('some error'); + } +} +function test4(type) { + try { + console.log('some switch'); + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + 0; + } + } + catch (e) { + console.log('some error'); + throw new Error('some error'); + } +} diff --git a/tests/baselines/reference/exhaustiveSwitchImplicitReturn.symbols b/tests/baselines/reference/exhaustiveSwitchImplicitReturn.symbols index d93a522836247..b4bd375f07b57 100644 --- a/tests/baselines/reference/exhaustiveSwitchImplicitReturn.symbols +++ b/tests/baselines/reference/exhaustiveSwitchImplicitReturn.symbols @@ -69,3 +69,146 @@ function foo5(bar: "a" | "b"): number { } } +// Repro from #32905. + +enum Foo { +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) + + One, +>One : Symbol(Foo.One, Decl(exhaustiveSwitchImplicitReturn.ts, 43, 10)) + + Two, +>Two : Symbol(Foo.Two, Decl(exhaustiveSwitchImplicitReturn.ts, 44, 8)) + + Three +>Three : Symbol(Foo.Three, Decl(exhaustiveSwitchImplicitReturn.ts, 45, 8)) +} + +function test2(type: Foo): number { +>test2 : Symbol(test2, Decl(exhaustiveSwitchImplicitReturn.ts, 47, 1)) +>type : Symbol(type, Decl(exhaustiveSwitchImplicitReturn.ts, 49, 15)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) + + try { + switch (type) { +>type : Symbol(type, Decl(exhaustiveSwitchImplicitReturn.ts, 49, 15)) + + case Foo.One: +>Foo.One : Symbol(Foo.One, Decl(exhaustiveSwitchImplicitReturn.ts, 43, 10)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>One : Symbol(Foo.One, Decl(exhaustiveSwitchImplicitReturn.ts, 43, 10)) + + return 0; + case Foo.Two: +>Foo.Two : Symbol(Foo.Two, Decl(exhaustiveSwitchImplicitReturn.ts, 44, 8)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>Two : Symbol(Foo.Two, Decl(exhaustiveSwitchImplicitReturn.ts, 44, 8)) + + return 0; + case Foo.Three: +>Foo.Three : Symbol(Foo.Three, Decl(exhaustiveSwitchImplicitReturn.ts, 45, 8)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>Three : Symbol(Foo.Three, Decl(exhaustiveSwitchImplicitReturn.ts, 45, 8)) + + return 0; + } + } catch (e) { +>e : Symbol(e, Decl(exhaustiveSwitchImplicitReturn.ts, 59, 13)) + + throw new Error('some error') +>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) + } +} + +function test3(type: Foo): number { +>test3 : Symbol(test3, Decl(exhaustiveSwitchImplicitReturn.ts, 62, 1)) +>type : Symbol(type, Decl(exhaustiveSwitchImplicitReturn.ts, 64, 15)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) + + try { + console.log('some switch') +>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) +>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) + + switch (type) { +>type : Symbol(type, Decl(exhaustiveSwitchImplicitReturn.ts, 64, 15)) + + case Foo.One: +>Foo.One : Symbol(Foo.One, Decl(exhaustiveSwitchImplicitReturn.ts, 43, 10)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>One : Symbol(Foo.One, Decl(exhaustiveSwitchImplicitReturn.ts, 43, 10)) + + return 0; + case Foo.Two: +>Foo.Two : Symbol(Foo.Two, Decl(exhaustiveSwitchImplicitReturn.ts, 44, 8)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>Two : Symbol(Foo.Two, Decl(exhaustiveSwitchImplicitReturn.ts, 44, 8)) + + return 0; + case Foo.Three: +>Foo.Three : Symbol(Foo.Three, Decl(exhaustiveSwitchImplicitReturn.ts, 45, 8)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>Three : Symbol(Foo.Three, Decl(exhaustiveSwitchImplicitReturn.ts, 45, 8)) + + return 0; + } + } catch (e) { +>e : Symbol(e, Decl(exhaustiveSwitchImplicitReturn.ts, 75, 13)) + + console.log('some error') +>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) +>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) + + throw new Error('some error') +>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) + } +} + +function test4(type: Foo): number { +>test4 : Symbol(test4, Decl(exhaustiveSwitchImplicitReturn.ts, 79, 1)) +>type : Symbol(type, Decl(exhaustiveSwitchImplicitReturn.ts, 81, 15)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) + + try { + console.log('some switch') +>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) +>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) + + switch (type) { +>type : Symbol(type, Decl(exhaustiveSwitchImplicitReturn.ts, 81, 15)) + + case Foo.One: +>Foo.One : Symbol(Foo.One, Decl(exhaustiveSwitchImplicitReturn.ts, 43, 10)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>One : Symbol(Foo.One, Decl(exhaustiveSwitchImplicitReturn.ts, 43, 10)) + + return 0; + case Foo.Two: +>Foo.Two : Symbol(Foo.Two, Decl(exhaustiveSwitchImplicitReturn.ts, 44, 8)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>Two : Symbol(Foo.Two, Decl(exhaustiveSwitchImplicitReturn.ts, 44, 8)) + + return 0; + case Foo.Three: +>Foo.Three : Symbol(Foo.Three, Decl(exhaustiveSwitchImplicitReturn.ts, 45, 8)) +>Foo : Symbol(Foo, Decl(exhaustiveSwitchImplicitReturn.ts, 39, 1)) +>Three : Symbol(Foo.Three, Decl(exhaustiveSwitchImplicitReturn.ts, 45, 8)) + + 0; + } + } catch (e) { +>e : Symbol(e, Decl(exhaustiveSwitchImplicitReturn.ts, 92, 13)) + + console.log('some error') +>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) +>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) + + throw new Error('some error') +>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) + } +} + diff --git a/tests/baselines/reference/exhaustiveSwitchImplicitReturn.types b/tests/baselines/reference/exhaustiveSwitchImplicitReturn.types index c868aa99b8f1d..2b567f9679067 100644 --- a/tests/baselines/reference/exhaustiveSwitchImplicitReturn.types +++ b/tests/baselines/reference/exhaustiveSwitchImplicitReturn.types @@ -85,3 +85,172 @@ function foo5(bar: "a" | "b"): number { } } +// Repro from #32905. + +enum Foo { +>Foo : Foo + + One, +>One : Foo.One + + Two, +>Two : Foo.Two + + Three +>Three : Foo.Three +} + +function test2(type: Foo): number { +>test2 : (type: Foo) => number +>type : Foo + + try { + switch (type) { +>type : Foo + + case Foo.One: +>Foo.One : Foo.One +>Foo : typeof Foo +>One : Foo.One + + return 0; +>0 : 0 + + case Foo.Two: +>Foo.Two : Foo.Two +>Foo : typeof Foo +>Two : Foo.Two + + return 0; +>0 : 0 + + case Foo.Three: +>Foo.Three : Foo.Three +>Foo : typeof Foo +>Three : Foo.Three + + return 0; +>0 : 0 + } + } catch (e) { +>e : any + + throw new Error('some error') +>new Error('some error') : Error +>Error : ErrorConstructor +>'some error' : "some error" + } +} + +function test3(type: Foo): number { +>test3 : (type: Foo) => number +>type : Foo + + try { + console.log('some switch') +>console.log('some switch') : void +>console.log : (message?: any, ...optionalParams: any[]) => void +>console : Console +>log : (message?: any, ...optionalParams: any[]) => void +>'some switch' : "some switch" + + switch (type) { +>type : Foo + + case Foo.One: +>Foo.One : Foo.One +>Foo : typeof Foo +>One : Foo.One + + return 0; +>0 : 0 + + case Foo.Two: +>Foo.Two : Foo.Two +>Foo : typeof Foo +>Two : Foo.Two + + return 0; +>0 : 0 + + case Foo.Three: +>Foo.Three : Foo.Three +>Foo : typeof Foo +>Three : Foo.Three + + return 0; +>0 : 0 + } + } catch (e) { +>e : any + + console.log('some error') +>console.log('some error') : void +>console.log : (message?: any, ...optionalParams: any[]) => void +>console : Console +>log : (message?: any, ...optionalParams: any[]) => void +>'some error' : "some error" + + throw new Error('some error') +>new Error('some error') : Error +>Error : ErrorConstructor +>'some error' : "some error" + } +} + +function test4(type: Foo): number { +>test4 : (type: Foo) => number +>type : Foo + + try { + console.log('some switch') +>console.log('some switch') : void +>console.log : (message?: any, ...optionalParams: any[]) => void +>console : Console +>log : (message?: any, ...optionalParams: any[]) => void +>'some switch' : "some switch" + + switch (type) { +>type : Foo + + case Foo.One: +>Foo.One : Foo.One +>Foo : typeof Foo +>One : Foo.One + + return 0; +>0 : 0 + + case Foo.Two: +>Foo.Two : Foo.Two +>Foo : typeof Foo +>Two : Foo.Two + + return 0; +>0 : 0 + + case Foo.Three: +>Foo.Three : Foo.Three +>Foo : typeof Foo +>Three : Foo.Three + + 0; +>0 : 0 + } + } catch (e) { +>e : any + + console.log('some error') +>console.log('some error') : void +>console.log : (message?: any, ...optionalParams: any[]) => void +>console : Console +>log : (message?: any, ...optionalParams: any[]) => void +>'some error' : "some error" + + throw new Error('some error') +>new Error('some error') : Error +>Error : ErrorConstructor +>'some error' : "some error" + } +} + diff --git a/tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts b/tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts index b87baffdaf95a..ea0799b597786 100644 --- a/tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts +++ b/tests/cases/compiler/exhaustiveSwitchImplicitReturn.ts @@ -40,3 +40,60 @@ function foo5(bar: "a" | "b"): number { return 1; } } + +// Repro from #32905. + +enum Foo { + One, + Two, + Three +} + +function test2(type: Foo): number { + try { + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + return 0; + } + } catch (e) { + throw new Error('some error') + } +} + +function test3(type: Foo): number { + try { + console.log('some switch') + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + return 0; + } + } catch (e) { + console.log('some error') + throw new Error('some error') + } +} + +function test4(type: Foo): number { + try { + console.log('some switch') + switch (type) { + case Foo.One: + return 0; + case Foo.Two: + return 0; + case Foo.Three: + 0; + } + } catch (e) { + console.log('some error') + throw new Error('some error') + } +}