Skip to content

Type guards on variable break refinements on its properties #18840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ivogabe opened this issue Sep 29, 2017 · 2 comments
Closed

Type guards on variable break refinements on its properties #18840

ivogabe opened this issue Sep 29, 2017 · 2 comments
Assignees
Labels
Fixed A PR has been merged for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@ivogabe
Copy link
Contributor

ivogabe commented Sep 29, 2017

TypeScript Version: 2.5.2 & nightly (2.6.0-dev.20170929)

Code

interface A { type: "a", foo: string };
interface B { type: "b", foo: string | number };

function f(x: A | B) {
	if (typeof x.foo === "number") return;

	// x.foo: string

	let y = x.type === "a" ? 1 : 2;

	x.foo.substring(1, 2);
}

Expected behavior:
Given that the first line of the body of f checks whether x.foo is a string, the last line of the body should not cause an error.

Actual behavior:
The compiler shows an error on the last line of the body of f, since the type of x.foo is (wrongly) resolved to string | number instead of string. The type guard on the first line of f is not considered.

file.ts(11,8): error TS2339: Property 'substring' does not exist on type 'string | number'.

I think that this happens because the compiler only searches for property type guards (on x.foo in this case) until the last refinement on the related variable (in this case x). It would be better if it would search until the last assignment, as type guards on x should not break any refinement on properties.

Note that the correct behavior can be found when commenting the line that declares y.

@olydis
Copy link

olydis commented Nov 21, 2017

@ivogabe I'd argue that the behavior is actually correct, given that evaluating x.type can easily invalidate the knowledge that x.foo: string.

Example:

class EvilB implements B {
    public get type(): "b" {
        this.foo = 42; // <- oh no!
        return "b";
    }
    public foo: string | number = "s";
}

If the compiler accepted your function f, then f(new EvilB()) will cause a runtime error at x.foo.substring since x.foo: number.

One does not simply assume that property reads are side-effects-free 😉

@ahejlsberg
Copy link
Member

This issue was fixed by #36114.

@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed A PR has been merged for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants