fix(39836): allow type declaration/unknown type in catch arguments in…

… JavaScript files
microsoft · Jan 17, 2021 · 43c907c · 43c907c
1 parent b5bab16
commit 43c907c
Show file tree

Hide file tree

Showing 12 changed files with 585 additions and 14 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -8741,9 +8741,11 @@ namespace ts {
             // Handle catch clause variables
             const declaration = symbol.valueDeclaration;
             if (isCatchClauseVariableDeclarationOrBindingElement(declaration)) {
-                const decl = declaration as VariableDeclaration;
-                if (!decl.type) return anyType;
-                const type = getTypeOfNode(decl.type);
+                const typeNode = getEffectiveTypeAnnotationNode(declaration);
+                if (typeNode === undefined) {
+                    return anyType;
+                }
+                const type = getTypeOfNode(typeNode);
                 // an errorType will make `checkTryStatement` issue an error
                 return isTypeAny(type) || type === unknownType ? type : errorType;
             }
@@ -35601,10 +35603,11 @@ namespace ts {
                 // Grammar checking
                 if (catchClause.variableDeclaration) {
                     const declaration = catchClause.variableDeclaration;
-                    if (declaration.type) {
-                        const type = getTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ false);
+                    const typeNode = getEffectiveTypeAnnotationNode(getRootDeclaration(declaration));
+                    if (typeNode) {
+                        const type = getTypeFromTypeNode(typeNode);
                         if (type && !(type.flags & TypeFlags.AnyOrUnknown)) {
-                            grammarErrorOnFirstToken(declaration.type, Diagnostics.Catch_clause_variable_type_annotation_must_be_any_or_unknown_if_specified);
+                            grammarErrorOnFirstToken(typeNode, Diagnostics.Catch_clause_variable_type_annotation_must_be_any_or_unknown_if_specified);
                         }
                     }
                     else if (declaration.initializer) {

diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -6253,6 +6253,7 @@ namespace ts {
 
         function parseVariableDeclaration(allowExclamation?: boolean): VariableDeclaration {
             const pos = getNodePos();
+            const hasJSDoc = hasPrecedingJSDocComment();
             const name = parseIdentifierOrPattern(Diagnostics.Private_identifiers_are_not_allowed_in_variable_declarations);
             let exclamationToken: ExclamationToken | undefined;
             if (allowExclamation && name.kind === SyntaxKind.Identifier &&
@@ -6262,7 +6263,7 @@ namespace ts {
             const type = parseTypeAnnotation();
             const initializer = isInOrOfKeyword(token()) ? undefined : parseInitializer();
             const node = factory.createVariableDeclaration(name, exclamationToken, type, initializer);
-            return finishNode(node, pos);
+            return withJSDoc(finishNode(node, pos), hasJSDoc);
         }
 
         function parseVariableDeclarationList(inForStatementInitializer: boolean): VariableDeclarationList {

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -877,6 +877,7 @@ namespace ts {
         | FunctionDeclaration
         | ConstructorDeclaration
         | MethodDeclaration
+        | VariableDeclaration
         | PropertyDeclaration
         | AccessorDeclaration
         | ClassLikeDeclaration
@@ -1238,7 +1239,7 @@ namespace ts {
 
     export type BindingName = Identifier | BindingPattern;
 
-    export interface VariableDeclaration extends NamedDeclaration {
+    export interface VariableDeclaration extends NamedDeclaration, JSDocContainer {
         readonly kind: SyntaxKind.VariableDeclaration;
         readonly parent: VariableDeclarationList | CatchClause;
         readonly name: BindingName;                    // Declared variable name

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -1204,7 +1204,8 @@ namespace ts {
             node.kind === SyntaxKind.TypeParameter ||
             node.kind === SyntaxKind.FunctionExpression ||
             node.kind === SyntaxKind.ArrowFunction ||
-            node.kind === SyntaxKind.ParenthesizedExpression) ?
+            node.kind === SyntaxKind.ParenthesizedExpression ||
+            node.kind === SyntaxKind.VariableDeclaration) ?
             concatenate(getTrailingCommentRanges(text, node.pos), getLeadingCommentRanges(text, node.pos)) :
             getLeadingCommentRanges(text, node.pos);
         // True if the comment starts with '/**' but not if it is '/**/'

diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -558,7 +558,7 @@ declare namespace ts {
     }
     export interface JSDocContainer {
     }
-    export type HasJSDoc = ParameterDeclaration | CallSignatureDeclaration | ConstructSignatureDeclaration | MethodSignature | PropertySignature | ArrowFunction | ParenthesizedExpression | SpreadAssignment | ShorthandPropertyAssignment | PropertyAssignment | FunctionExpression | LabeledStatement | ExpressionStatement | VariableStatement | FunctionDeclaration | ConstructorDeclaration | MethodDeclaration | PropertyDeclaration | AccessorDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | EnumMember | EnumDeclaration | ModuleDeclaration | ImportEqualsDeclaration | ImportDeclaration | NamespaceExportDeclaration | ExportAssignment | IndexSignatureDeclaration | FunctionTypeNode | ConstructorTypeNode | JSDocFunctionType | ExportDeclaration | NamedTupleMember | EndOfFileToken;
+    export type HasJSDoc = ParameterDeclaration | CallSignatureDeclaration | ConstructSignatureDeclaration | MethodSignature | PropertySignature | ArrowFunction | ParenthesizedExpression | SpreadAssignment | ShorthandPropertyAssignment | PropertyAssignment | FunctionExpression | LabeledStatement | ExpressionStatement | VariableStatement | FunctionDeclaration | ConstructorDeclaration | MethodDeclaration | VariableDeclaration | PropertyDeclaration | AccessorDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | EnumMember | EnumDeclaration | ModuleDeclaration | ImportEqualsDeclaration | ImportDeclaration | NamespaceExportDeclaration | ExportAssignment | IndexSignatureDeclaration | FunctionTypeNode | ConstructorTypeNode | JSDocFunctionType | ExportDeclaration | NamedTupleMember | EndOfFileToken;
     export type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
     export type HasTypeArguments = CallExpression | NewExpression | TaggedTemplateExpression | JsxOpeningElement | JsxSelfClosingElement;
     export type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
@@ -686,7 +686,7 @@ declare namespace ts {
         readonly kind: SyntaxKind.ConstructSignature;
     }
     export type BindingName = Identifier | BindingPattern;
-    export interface VariableDeclaration extends NamedDeclaration {
+    export interface VariableDeclaration extends NamedDeclaration, JSDocContainer {
         readonly kind: SyntaxKind.VariableDeclaration;
         readonly parent: VariableDeclarationList | CatchClause;
         readonly name: BindingName;

diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -558,7 +558,7 @@ declare namespace ts {
     }
     export interface JSDocContainer {
     }
-    export type HasJSDoc = ParameterDeclaration | CallSignatureDeclaration | ConstructSignatureDeclaration | MethodSignature | PropertySignature | ArrowFunction | ParenthesizedExpression | SpreadAssignment | ShorthandPropertyAssignment | PropertyAssignment | FunctionExpression | LabeledStatement | ExpressionStatement | VariableStatement | FunctionDeclaration | ConstructorDeclaration | MethodDeclaration | PropertyDeclaration | AccessorDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | EnumMember | EnumDeclaration | ModuleDeclaration | ImportEqualsDeclaration | ImportDeclaration | NamespaceExportDeclaration | ExportAssignment | IndexSignatureDeclaration | FunctionTypeNode | ConstructorTypeNode | JSDocFunctionType | ExportDeclaration | NamedTupleMember | EndOfFileToken;
+    export type HasJSDoc = ParameterDeclaration | CallSignatureDeclaration | ConstructSignatureDeclaration | MethodSignature | PropertySignature | ArrowFunction | ParenthesizedExpression | SpreadAssignment | ShorthandPropertyAssignment | PropertyAssignment | FunctionExpression | LabeledStatement | ExpressionStatement | VariableStatement | FunctionDeclaration | ConstructorDeclaration | MethodDeclaration | VariableDeclaration | PropertyDeclaration | AccessorDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | EnumMember | EnumDeclaration | ModuleDeclaration | ImportEqualsDeclaration | ImportDeclaration | NamespaceExportDeclaration | ExportAssignment | IndexSignatureDeclaration | FunctionTypeNode | ConstructorTypeNode | JSDocFunctionType | ExportDeclaration | NamedTupleMember | EndOfFileToken;
     export type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
     export type HasTypeArguments = CallExpression | NewExpression | TaggedTemplateExpression | JsxOpeningElement | JsxSelfClosingElement;
     export type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
@@ -686,7 +686,7 @@ declare namespace ts {
         readonly kind: SyntaxKind.ConstructSignature;
     }
     export type BindingName = Identifier | BindingPattern;
-    export interface VariableDeclaration extends NamedDeclaration {
+    export interface VariableDeclaration extends NamedDeclaration, JSDocContainer {
         readonly kind: SyntaxKind.VariableDeclaration;
         readonly parent: VariableDeclarationList | CatchClause;
         readonly name: BindingName;

diff --git a/tests/baselines/reference/jsdocCatchClauseWithTypeAnnotation.errors.txt b/tests/baselines/reference/jsdocCatchClauseWithTypeAnnotation.errors.txt
@@ -0,0 +1,75 @@
+tests/cases/conformance/jsdoc/foo.js(20,54): error TS2339: Property 'foo' does not exist on type 'unknown'.
+tests/cases/conformance/jsdoc/foo.js(21,54): error TS2339: Property 'foo' does not exist on type 'unknown'.
+tests/cases/conformance/jsdoc/foo.js(22,31): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
+tests/cases/conformance/jsdoc/foo.js(23,31): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
+tests/cases/conformance/jsdoc/foo.js(35,7): error TS2492: Cannot redeclare identifier 'err' in catch clause.
+tests/cases/conformance/jsdoc/foo.js(48,31): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
+tests/cases/conformance/jsdoc/foo.js(49,31): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
+
+
+==== tests/cases/conformance/jsdoc/foo.js (7 errors) ====
+    /**
+     * @typedef {any} Any
+     */
+
+    /**
+     * @typedef {unknown} Unknown
+     */
+
+    function fn() {
+        try { } catch (x) { }                                            // should be OK
+        try { } catch (/** @type {any} */ err) { }                       // should be OK
+        try { } catch (/** @type {Any} */ err) { }                       // should be OK
+        try { } catch (/** @type {unknown} */ err) { }                   // should be OK
+        try { } catch (/** @type {Unknown} */ err) { }                   // should be OK
+        try { } catch (err) { err.foo; }                                 // should be OK
+        try { } catch (/** @type {any} */ err) { err.foo; }              // should be OK
+        try { } catch (/** @type {Any} */ err) { err.foo; }              // should be OK
+        try { } catch (/** @type {unknown} */ err) { console.log(err); } // should be OK
+        try { } catch (/** @type {Unknown} */ err) { console.log(err); } // should be OK
+        try { } catch (/** @type {unknown} */ err) { err.foo; }          // error in the body
+                                                         ~~~
+!!! error TS2339: Property 'foo' does not exist on type 'unknown'.
+        try { } catch (/** @type {Unknown} */ err) { err.foo; }          // error in the body
+                                                         ~~~
+!!! error TS2339: Property 'foo' does not exist on type 'unknown'.
+        try { } catch (/** @type {Error} */ err) { }                     // error in the type
+                                  ~~~~~
+!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
+        try { } catch (/** @type {object} */ err) { }                    // error in the type
+                                  ~~~~~~
+!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
+
+        try { console.log(); }
+        // @ts-ignore
+        catch (/** @type {number} */ err) { // e should not be a `number`
+            console.log(err.toLowerCase());
+        }
+
+    	// minor bug: shows that the `catch` argument is skipped when checking scope
+    	try { }
+    	catch (err) {
+    		/** @type {string} */
+    		let err;
+    		    ~~~
+!!! error TS2492: Cannot redeclare identifier 'err' in catch clause.
+    	}
+    	try { }
+    	catch (err) {
+    		/** @type {boolean} */
+    		var err;
+    	}
+
+        try { } catch ({ x }) { }                                        // should be OK
+        try { } catch (/** @type {any} */ { x }) { x.foo; }              // should be OK
+        try { } catch (/** @type {Any} */ { x }) { x.foo;}               // should be OK
+        try { } catch (/** @type {unknown} */ { x }) { console.log(x); } // should be OK
+        try { } catch (/** @type {Unknown} */ { x }) { console.log(x); } // should be OK
+        try { } catch (/** @type {Error} */ { x }) { }                   // error in the type
+                                  ~~~~~
+!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
+        try { } catch (/** @type {object} */ { x }) { }                  // error in the type
+                                  ~~~~~~
+!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
+    }
+
diff --git a/tests/baselines/reference/jsdocCatchClauseWithTypeAnnotation.js b/tests/baselines/reference/jsdocCatchClauseWithTypeAnnotation.js
@@ -0,0 +1,144 @@
+//// [foo.js]
+/**
+ * @typedef {any} Any
+ */
+
+/**
+ * @typedef {unknown} Unknown
+ */
+
+function fn() {
+    try { } catch (x) { }                                            // should be OK
+    try { } catch (/** @type {any} */ err) { }                       // should be OK
+    try { } catch (/** @type {Any} */ err) { }                       // should be OK
+    try { } catch (/** @type {unknown} */ err) { }                   // should be OK
+    try { } catch (/** @type {Unknown} */ err) { }                   // should be OK
+    try { } catch (err) { err.foo; }                                 // should be OK
+    try { } catch (/** @type {any} */ err) { err.foo; }              // should be OK
+    try { } catch (/** @type {Any} */ err) { err.foo; }              // should be OK
+    try { } catch (/** @type {unknown} */ err) { console.log(err); } // should be OK
+    try { } catch (/** @type {Unknown} */ err) { console.log(err); } // should be OK
+    try { } catch (/** @type {unknown} */ err) { err.foo; }          // error in the body
+    try { } catch (/** @type {Unknown} */ err) { err.foo; }          // error in the body
+    try { } catch (/** @type {Error} */ err) { }                     // error in the type
+    try { } catch (/** @type {object} */ err) { }                    // error in the type
+
+    try { console.log(); }
+    // @ts-ignore
+    catch (/** @type {number} */ err) { // e should not be a `number`
+        console.log(err.toLowerCase());
+    }
+
+	// minor bug: shows that the `catch` argument is skipped when checking scope
+	try { }
+	catch (err) {
+		/** @type {string} */
+		let err;
+	}
+	try { }
+	catch (err) {
+		/** @type {boolean} */
+		var err;
+	}
+
+    try { } catch ({ x }) { }                                        // should be OK
+    try { } catch (/** @type {any} */ { x }) { x.foo; }              // should be OK
+    try { } catch (/** @type {Any} */ { x }) { x.foo;}               // should be OK
+    try { } catch (/** @type {unknown} */ { x }) { console.log(x); } // should be OK
+    try { } catch (/** @type {Unknown} */ { x }) { console.log(x); } // should be OK
+    try { } catch (/** @type {Error} */ { x }) { }                   // error in the type
+    try { } catch (/** @type {object} */ { x }) { }                  // error in the type
+}
+
+
+//// [foo.js]
+/**
+ * @typedef {any} Any
+ */
+/**
+ * @typedef {unknown} Unknown
+ */
+function fn() {
+    try { }
+    catch (x) { } // should be OK
+    try { }
+    catch ( /** @type {any} */err) { } // should be OK
+    try { }
+    catch ( /** @type {Any} */err) { } // should be OK
+    try { }
+    catch ( /** @type {unknown} */err) { } // should be OK
+    try { }
+    catch ( /** @type {Unknown} */err) { } // should be OK
+    try { }
+    catch (err) {
+        err.foo;
+    } // should be OK
+    try { }
+    catch ( /** @type {any} */err) {
+        err.foo;
+    } // should be OK
+    try { }
+    catch ( /** @type {Any} */err) {
+        err.foo;
+    } // should be OK
+    try { }
+    catch ( /** @type {unknown} */err) {
+        console.log(err);
+    } // should be OK
+    try { }
+    catch ( /** @type {Unknown} */err) {
+        console.log(err);
+    } // should be OK
+    try { }
+    catch ( /** @type {unknown} */err) {
+        err.foo;
+    } // error in the body
+    try { }
+    catch ( /** @type {Unknown} */err) {
+        err.foo;
+    } // error in the body
+    try { }
+    catch ( /** @type {Error} */err) { } // error in the type
+    try { }
+    catch ( /** @type {object} */err) { } // error in the type
+    try {
+        console.log();
+    }
+    // @ts-ignore
+    catch ( /** @type {number} */err) { // e should not be a `number`
+        console.log(err.toLowerCase());
+    }
+    // minor bug: shows that the `catch` argument is skipped when checking scope
+    try { }
+    catch (err) {
+        /** @type {string} */
+        let err;
+    }
+    try { }
+    catch (err) {
+        /** @type {boolean} */
+        var err;
+    }
+    try { }
+    catch ({ x }) { } // should be OK
+    try { }
+    catch ( /** @type {any} */{ x }) {
+        x.foo;
+    } // should be OK
+    try { }
+    catch ( /** @type {Any} */{ x }) {
+        x.foo;
+    } // should be OK
+    try { }
+    catch ( /** @type {unknown} */{ x }) {
+        console.log(x);
+    } // should be OK
+    try { }
+    catch ( /** @type {Unknown} */{ x }) {
+        console.log(x);
+    } // should be OK
+    try { }
+    catch ( /** @type {Error} */{ x }) { } // error in the type
+    try { }
+    catch ( /** @type {object} */{ x }) { } // error in the type
+}