Repository navigation
Type guard not working as expected in function overloading with union types #5826
Copy link
Copy link
Closed
Labels
BugA bug in TypeScriptA bug in TypeScriptBy DesignDeprecated - use "Working as Intended" or "Design Limitation" insteadDeprecated - use "Working as Intended" or "Design Limitation" insteadFix AvailableA PR has been opened for this issueA PR has been opened for this issue
Milestone
Description
class Square {
length : number;
constructor(length : number) {
this.length = length;
}
}
class Rectangle {
length : number;
width : number;
constructor(length : number, width : number) {
this.length = length;
this.width = width;
}
}
class Triangle {
base : number
height : number;
constructor(base : number, height : number) {
this.base = base;
this.height = height;
}
}
The following works as expected:
namespace function_overloading_with_any {
function calculateArea(shape : Square) : number; // overloaded signature
function calculateArea(shape : Rectangle) : number; // overloaded signature
function calculateArea(shape : Triangle) : number; // overloaded signature
function calculateArea(shape : any) : number // implementation signature
{
if(shape instanceof Square) {
return shape.length * shape.length;
}
else if(shape instanceof Rectangle) {
return shape.length * shape.width;
}
else if(shape instanceof Triangle) {
return (shape.base * shape.height) / 2;
}
}
var square = new Square(5);
var rectangle = new Rectangle(3, 5);
var triangle = new Triangle(3, 5);
calculateArea(square);
calculateArea(rectangle);
calculateArea(triangle);
}
However, when I use an union type instead of anyin the implementation signature:
namespace chapter02.function_overloading.with_union {
function calculateArea(shape : Square) : number;
function calculateArea(shape : Rectangle) : number;
function calculateArea(shape : Triangle) : number;
function calculateArea(shape : (Square | Rectangle | Triangle)) : number {
if(shape instanceof Square) {
return shape.length * shape.length;
}
else if(shape instanceof Rectangle) {
return shape.length * shape.width; // Error!
}
else if(shape instanceof Triangle) {
return (shape.base * shape.height) / 2;
}
}
var square = new Square(5);
var rectangle = new Rectangle(3, 5);
var triangle = new Triangle(3, 5);
calculateArea(square);
calculateArea(rectangle);
calculateArea(triangle);
}
I get a compilation error:
error TS2339: Property 'length' does not exist on type 'Triangle'.
error TS2339: Property 'width' does not exist on type 'Triangle'.
Looks like the type guard is not working as expected. I can use an if instead of if else to fix the error:
function calculateArea(shape : Square) : number;
function calculateArea(shape : Rectangle) : number;
function calculateArea(shape : Triangle) : number;
function calculateArea(shape : (Square | Rectangle | Triangle)) : number {
if(shape instanceof Square) {
return shape.length * shape.length;
}
if(shape instanceof Rectangle) {
return shape.length * shape.width; // OK!
}
if(shape instanceof Triangle) {
return (shape.base * shape.height) / 2;
}
}
I'm using TypeScript 1.8.0-dev.20151129 with gulp-typescript:
gulp.task('build', ['lint'], function() {
var tsProject = ts.createProject('tsconfig.json', {
typescript: require('typescript')
});
var compile = ts(tsProject);
return gulp.src("src/**/**.ts")
.pipe(compile)
.js.pipe(gulp.dest("src/"));
});
And the following tsconfig.json compilation:
{
"compilerOptions": {
"target": "es5",
"module": "umd",
"moduleResolution": "node",
"isolatedModules": false,
"jsx": "react",
"experimentalDecorators": true,
"emitDecoratorMetadata": true,
"declaration": false,
"noImplicitAny": false,
"removeComments": true,
"noLib": false,
"preserveConstEnums": true,
"suppressImplicitAnyIndexErrors": true
}
}
The source code that I used to discover this issue can be found here.
Thanks!
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
BugA bug in TypeScriptA bug in TypeScriptBy DesignDeprecated - use "Working as Intended" or "Design Limitation" insteadDeprecated - use "Working as Intended" or "Design Limitation" insteadFix AvailableA PR has been opened for this issueA PR has been opened for this issue
Activity
DanielRosenwasser commentedon Nov 30, 2015
I think this is happening because we try to eliminate all types in the union that are assignable to
Squarein the firstelse. That's very incorrect becauseRectangleis assignable toSquare, but it has nothing to do with its instance identity.weswigham commentedon Dec 1, 2015
I think this may be a regression - in the playground, there's no errors for this example.
weswigham commentedon Dec 2, 2015
I think I've figured out why this is a regression - when I swapped narrowing to work from outside in rather than inside out (lol) in #5442 so empty sets could correctly propagate down and compound/nested type guards would work as expected, at the same time I made it so behaviors like this became visible. We had always been narrowing with the logic/spec saying this should happen, but since we'd narrow by the
ifwithin theelse, then theelse, we'd narrowSquare | Rectangle | TriangletoRectangleand then just return that subtype again at the higher level (sinceRectanglewouldn't narrow any further - by old logic the expression!(instanceof Square)would have resulted in an intervening empty set which resulted in the type at that step just getting reused (Rectangle)).So fixing a set of other bugs exposed this one. :D
While the new behavior is a change, the behavior described in the post, as Daniel says, is by design. Since
Rectangleis aSquare,instanceof SquareremovesRectanglefrom the union, asinstanceoftype guards act the same way as user-defined type guards - structurally.This is actually problematic -
instanceofat runtime is highly nominal, it will actually only catch instances of the RHS constructor or its prototypical inheritors - structural similarity doesn't play into it, and because of our ignorance of this, we do inaccurate narrowing. We could add a nominal relationship to use withinstanceoffairly easily, but it is fairly against the philosophy of TS - we're actively trying to remove the only place where we've accidentally introduced nominality.So Anders Hejlsberg (@ahejlsberg), this behavior stays (since it's correct, according to our spec - it was incorrect before) unless we want to add a nominal relationship to use with
instanceof.weswigham commentedon Dec 3, 2015
Remo H. Jansen (@remojansen) as a workaround while we figure this out (or if it just sticks around), you can either brand your classes with dummy members (
_square: void,_rectangle: void,_triangle: void) so they're not structurally assignable or reorder yourif/else ifchains so that the more specific types (eg,Rectangle) are first.remojansen commentedon Dec 3, 2015
Wesley Wigham (@weswigham) 👍 thanks for that