microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.1k stars 12.5k forks source link

A type can be named undefined #57573

Closed dragomirtitian closed 8 months ago

dragomirtitian commented 8 months ago

🔎 Search Terms

type named undefined

🕗 Version & Regression Information

⏯ Playground Link

Playground Link

💻 Code

namespace ns {
    export type undefined = "";
    export function x(p: undefined): undefined { // global undefined

    }
}

function x(p: ns.undefined) { // undefined from the namespace

}

🙁 Actual behavior

Inside the namespace undefined is the global undefined type. We can access the type if we qualify it with the namespace

🙂 Expected behavior

undefined should not be allowed as a type name. (null is not)

Additional information about the issue

No response

fatcerberus commented 8 months ago

undefined should not be allowed as a type name. (null is not)

Amusingly, this matches runtime behavior: null is a keyword, while undefined is not--it's a global variable that can be freely shadowed, and prior to ES5 you could actually mutate the global one (which is why void 0 became popular).

dragomirtitian commented 8 months ago

@fatcerberus It does partially match that. Except in ES5 if you defined undefined it would use that in the context. This allows you to define it but then ignores it.

We could allow it. But it'd rather ban it 😅

RyanCavanaugh commented 8 months ago

I can't imagine anyone doing this on purpose except to be evil... banning seems like the right call

jakebailey commented 8 months ago

On second look, what is the problem here per se? Whenever you write undefined in type space, you always get the real undefined, even inside the namespace. It never emits "wrong", because that undefined (even when inferred) always points to the global undefined.

If you do this outside of a namespace, it will say no, but inside it's just a symbol that can only be accessed from the namespace, no other way, so this doesn't seem harmful, just... weird?

jakebailey commented 8 months ago

Hm, no, it won't say no outside of a namespace. Just error when it's also a global. https://www.typescriptlang.org/play?#code/KYDwDg9gTgLgBDAnmYcCuA7AJsAZgSw2CzgF44AiCgbgFgAoIA

dragomirtitian commented 8 months ago

On second look, what is the problem here per se? Whenever you write undefined in type space, you always get the real undefined, even inside the namespace. It never emits "wrong", because that undefined (even when inferred) always points to the global undefined.

If you do this outside of a namespace, it will say no, but inside it's just a symbol that can only be accessed from the namespace, no other way, so this doesn't seem harmful, just... weird?

But it feels very strange that you have a local type named undefined, which regular scoping rules would dictate would take precedence over the global undefined and yet it does not.

Erroring on undefined would also make it similar to what happens for other keyword types such as number Playground Link

jakebailey commented 8 months ago

Ah, there's the error I was looking for. Thanks.

RyanCavanaugh commented 8 months ago
export type undefined = "";
let m: undefined = "";
// error^

If it's not going to work, we shouldn't let you do it

reverofevil commented 4 months ago

Related #43215

Did anyone check this time there are no other such identifiers? Is there anything like identifier contingency plan, such as refactoring the parser to support precisely defined thought-through identifier classes?

I suspect this might either not be fixed, or will break again soon.

jakebailey commented 4 months ago

Did anyone check this time there are no other such identifiers?

Yes: https://github.com/microsoft/TypeScript/pull/57575#issuecomment-1969694635

jakebailey commented 4 months ago

Is there anything like identifier contingency plan, such as refactoring the parser to support precisely defined thought-through identifier classes?

I suspect this might either not be fixed, or will break again soon.

undefined is the special case as it's not actually a keyword, but a predeclared identifier in JS but a keyword in type space, which is why this wasn't caught.