Infer typedef variable declaration to never
#36454
Conversation
Typedefs have two forms: specify a name in the `@typedef` declaration or use the name of the variable declaration it is attached to. Since `@typedef` types should only live in the type space, any reference to a variable that has a `@typedef` attached to is illegal. As such, assign it the `never` type if the `@typedef` itself does not specify a name. Fixes microsoft#36375
There was a problem hiding this comment.
A couple of comments on the code.
Also, I don't know how valuable this is. Its value is based on the number of closure projects that are going to migrate directly to "noImplicitAny": true. Any idea how many that would be? (I assume that they're all inside google.)
| // The typedef is attached to a variable declaration, instead of specifying the name in the typedef itself | ||
| // This means that the variable declared references the typedef and should not be used in regular control | ||
| // flow of the program, only in jsdoc type references. | ||
| if (jsDocTypeDefTag && !jsDocTypeDefTag.name && isVariableDeclaration(declaration)) { |
There was a problem hiding this comment.
probably need to check that there is no initialiser too
There was a problem hiding this comment.
oh, and isInJSFile, no exports, not a binding pattern, not a declare (which is illegal in js, but might show up anyway).
| if (jsDocTypeDefTag && !jsDocTypeDefTag.name && isVariableDeclaration(declaration)) { | ||
| return neverType; | ||
| } | ||
|
|
||
| if ((noImplicitAny || isInJSFile(declaration)) && |
There was a problem hiding this comment.
Actually, maybe the new code should be moved inside here.
|
Yesterday, I realized that I will process your code review tomorrow, thanks for taking a look! |
|
I'm not crazy about the use of type |
|
@ahejlsberg the intent is to prevent using the type alias variable as an assignment source, so But probably the correct fix is to give an error whenever the variable is referenced in a value position and then suppress the implicit any error. Then the type can remain Edit: Since closure verifies that there is no initialiser, I wouldn't bother making Typescript give an error; just treat it like a normal declaration: /** @typedef {number} */
var Typename = 12; // value references to Typename should be fine |
You mean assignment target, yes? If you give it type What exactly are we trying to prevent? Reads of the variable, writes to the variable, or both? |
|
Yep, I got it backwards. We're trying to prevent both reads and writes of the variable since it's only there so Closure can have a name for the associated type alias. I think an error on value-space usage of the variable is better than changing the type, plus suppression of the implicit any error. |
|
Yes, I think that's the right path forward. |
Typedefs have two forms: specify a name in the
@typedefdeclaration oruse the name of the variable declaration it is attached to. Since
@typedeftypes should only live in the type space, any reference to avariable that has a
@typedefattached to is illegal. As such, assignit the
nevertype if the@typedefitself does not specify a name.Fixes #36375