Skip to content

Object union member functions that return never are incorrectly narrowed #56425

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

Open
kalkronline opened this issue Nov 16, 2023 · 6 comments
Open
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@kalkronline
Copy link

kalkronline commented Nov 16, 2023

πŸ”Ž Search Terms

"never", "class union", "narrowing unions", "never union"

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about never

⏯ Playground Link

Playground Link

πŸ’» Code

class Foo {
    defoo(): never {
        throw new Error("foo error")
    }
}

class Bar {
    defoo() { }
}

type Baz = Foo | Bar;

function defooer(baz: Baz) {
    baz.defoo() 
    // type = void
    // aka (never | void)

    return baz
    // type = Baz
    // should narrow to Bar
}

πŸ™ Actual behavior

baz doesn't get narrowed after calling .defoo(), even though it's impossible for any value after that point to be Foo. This seems to happen because the type of Baz["defoo"] is equal to Foo["defoo"] | Bar["defoo"] or () => never | void.

πŸ™‚ Expected behavior

baz should narrow to Bar after calling .defoo() because the return type of Foo.defoo is never.

Additional information about the issue

No response

@kalkronline kalkronline changed the title Object union member functions don't narrow if they return never Object union member functions that return never are incorrectly narrowed Nov 16, 2023
@fatcerberus
Copy link

But without the catch to know where to pick up the flow, for all we the know the program is terminated anyway.

I think that was the point - the program will terminate if baz is a Foo, but not if it's a Bar, so TS should be able to narrow it to the latter. In fact, with the try-catch there, the function will not terminate (because the exception is caught inside it), and therefore baz could still be a Foo afterwards.

@andrewbranch
Copy link
Member

Possibly related to #49771, but not sure that fully covers the use case here

@andrewbranch andrewbranch added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Nov 16, 2023
@fatcerberus
Copy link

never has a very well-defined type theoretical meaning - it’s the bottom type. It is by definition an empty set, which means any code which claims to have a value of this type is unreachable. If it is not, you are dealing with an unsoundness.

A function which claims to return never is saying that it won’t ever return normally, because, by the definition of never, there’s no way it can.

I think maybe a throws [type] is needed as an available return type

That would be typed/checked exceptions which is #56365 (Ryan needs to update the description there, methinks).

@craigphicks
Copy link

craigphicks commented Nov 16, 2023

@fatcerberus

Does flow need to track the try blocks or not?
In the case a return type throws [optional type], it is obvious that is does.
If it doesn't, lets not place that burden on the TypeScript flow processing.

E.g., a return type of never could mean "program terminated" or "this case won't happen, so it won't even throw". No need for the flow processing to track an exception in those cases. The latter could the return type of one of a set of overloads, for example.

@kalkronline
Copy link
Author

kalkronline commented Nov 17, 2023

@craigphicks

Narrowing in try blocks is applied only if the catch block terminates, this is how it works now:

// returns number | undefined
function destring(value: string | number) {
    try {
        if (typeof value == 'string') {
            throw new Error("string_error");
        }
        // value: number
    } catch {
        // value: string | number
        return undefined;
    }

    // value: number
    return value
}

It would work the same way here, whether an exception was thrown or not wouldn't matter.

// returns Bar | undefined
function try_defoo(baz: Baz) {
    try {
        baz.defoo();
        // baz: Bar
    } catch (e) {
        // baz: Baz
        return undefined;
    }

    // baz: Bar
    return baz;
}

@craigphicks
Copy link

Reflecting the never/throws return type back onto the flow of the base object is definitely a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants