Skip to content

Commit 656b85e

Browse files
a-tarasyuka-tarasyuk
committed
fix(39836): allow type declaration/unknown type in catch arguments in JavaScript files
1 parent 621e0af commit 656b85e

11 files changed

+584
-13
lines changed

‎src/compiler/checker.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8741,9 +8741,11 @@ namespace ts {
87418741
// Handle catch clause variables
87428742
const declaration = symbol.valueDeclaration;
87438743
if (isCatchClauseVariableDeclarationOrBindingElement(declaration)) {
8744-
const decl = declaration as VariableDeclaration;
8745-
if (!decl.type) return anyType;
8746-
const type = getTypeOfNode(decl.type);
8744+
const typeNode = getEffectiveTypeAnnotationNode(declaration);
8745+
if (typeNode === undefined) {
8746+
return anyType;
8747+
}
8748+
const type = getTypeOfNode(typeNode);
87478749
// an errorType will make `checkTryStatement` issue an error
87488750
return isTypeAny(type) || type === unknownType ? type : errorType;
87498751
}
@@ -35601,10 +35603,11 @@ namespace ts {
3560135603
// Grammar checking
3560235604
if (catchClause.variableDeclaration) {
3560335605
const declaration = catchClause.variableDeclaration;
35604-
if (declaration.type) {
35605-
const type = getTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ false);
35606+
const typeNode = getEffectiveTypeAnnotationNode(getRootDeclaration(declaration));
35607+
if (typeNode) {
35608+
const type = getTypeFromTypeNode(typeNode);
3560635609
if (type && !(type.flags & TypeFlags.AnyOrUnknown)) {
35607-
grammarErrorOnFirstToken(declaration.type, Diagnostics.Catch_clause_variable_type_annotation_must_be_any_or_unknown_if_specified);
35610+
grammarErrorOnFirstToken(typeNode, Diagnostics.Catch_clause_variable_type_annotation_must_be_any_or_unknown_if_specified);
3560835611
}
3560935612
}
3561035613
else if (declaration.initializer) {

‎src/compiler/parser.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6253,6 +6253,7 @@ namespace ts {
62536253

62546254
function parseVariableDeclaration(allowExclamation?: boolean): VariableDeclaration {
62556255
const pos = getNodePos();
6256+
const hasJSDoc = hasPrecedingJSDocComment();
62566257
const name = parseIdentifierOrPattern(Diagnostics.Private_identifiers_are_not_allowed_in_variable_declarations);
62576258
let exclamationToken: ExclamationToken | undefined;
62586259
if (allowExclamation && name.kind === SyntaxKind.Identifier &&
@@ -6262,7 +6263,7 @@ namespace ts {
62626263
const type = parseTypeAnnotation();
62636264
const initializer = isInOrOfKeyword(token()) ? undefined : parseInitializer();
62646265
const node = factory.createVariableDeclaration(name, exclamationToken, type, initializer);
6265-
return finishNode(node, pos);
6266+
return withJSDoc(finishNode(node, pos), hasJSDoc);
62666267
}
62676268

62686269
function parseVariableDeclarationList(inForStatementInitializer: boolean): VariableDeclarationList {

‎src/compiler/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,7 @@ namespace ts {
877877
| FunctionDeclaration
878878
| ConstructorDeclaration
879879
| MethodDeclaration
880+
| VariableDeclaration
880881
| PropertyDeclaration
881882
| AccessorDeclaration
882883
| ClassLikeDeclaration
@@ -1238,7 +1239,7 @@ namespace ts {
12381239

12391240
export type BindingName = Identifier | BindingPattern;
12401241

1241-
export interface VariableDeclaration extends NamedDeclaration {
1242+
export interface VariableDeclaration extends NamedDeclaration, JSDocContainer {
12421243
readonly kind: SyntaxKind.VariableDeclaration;
12431244
readonly parent: VariableDeclarationList | CatchClause;
12441245
readonly name: BindingName; // Declared variable name

‎src/compiler/utilities.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1204,7 +1204,8 @@ namespace ts {
12041204
node.kind === SyntaxKind.TypeParameter ||
12051205
node.kind === SyntaxKind.FunctionExpression ||
12061206
node.kind === SyntaxKind.ArrowFunction ||
1207-
node.kind === SyntaxKind.ParenthesizedExpression) ?
1207+
node.kind === SyntaxKind.ParenthesizedExpression ||
1208+
node.kind === SyntaxKind.VariableDeclaration) ?
12081209
concatenate(getTrailingCommentRanges(text, node.pos), getLeadingCommentRanges(text, node.pos)) :
12091210
getLeadingCommentRanges(text, node.pos);
12101211
// True if the comment starts with '/**' but not if it is '/**/'

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ declare namespace ts {
558558
}
559559
export interface JSDocContainer {
560560
}
561-
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;
561+
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;
562562
export type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
563563
export type HasTypeArguments = CallExpression | NewExpression | TaggedTemplateExpression | JsxOpeningElement | JsxSelfClosingElement;
564564
export type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
@@ -686,7 +686,7 @@ declare namespace ts {
686686
readonly kind: SyntaxKind.ConstructSignature;
687687
}
688688
export type BindingName = Identifier | BindingPattern;
689-
export interface VariableDeclaration extends NamedDeclaration {
689+
export interface VariableDeclaration extends NamedDeclaration, JSDocContainer {
690690
readonly kind: SyntaxKind.VariableDeclaration;
691691
readonly parent: VariableDeclarationList | CatchClause;
692692
readonly name: BindingName;

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ declare namespace ts {
558558
}
559559
export interface JSDocContainer {
560560
}
561-
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;
561+
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;
562562
export type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
563563
export type HasTypeArguments = CallExpression | NewExpression | TaggedTemplateExpression | JsxOpeningElement | JsxSelfClosingElement;
564564
export type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
@@ -686,7 +686,7 @@ declare namespace ts {
686686
readonly kind: SyntaxKind.ConstructSignature;
687687
}
688688
export type BindingName = Identifier | BindingPattern;
689-
export interface VariableDeclaration extends NamedDeclaration {
689+
export interface VariableDeclaration extends NamedDeclaration, JSDocContainer {
690690
readonly kind: SyntaxKind.VariableDeclaration;
691691
readonly parent: VariableDeclarationList | CatchClause;
692692
readonly name: BindingName;

‎tests/baselines/reference/jsdocCatchClauseWithTypeAnnotation.errors.txt

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
tests/cases/conformance/jsdoc/foo.js(20,54): error TS2339: Property 'foo' does not exist on type 'unknown'.
2+
tests/cases/conformance/jsdoc/foo.js(21,54): error TS2339: Property 'foo' does not exist on type 'unknown'.
3+
tests/cases/conformance/jsdoc/foo.js(22,31): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
4+
tests/cases/conformance/jsdoc/foo.js(23,31): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
5+
tests/cases/conformance/jsdoc/foo.js(35,7): error TS2492: Cannot redeclare identifier 'err' in catch clause.
6+
tests/cases/conformance/jsdoc/foo.js(48,31): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
7+
tests/cases/conformance/jsdoc/foo.js(49,31): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
8+
9+
10+
==== tests/cases/conformance/jsdoc/foo.js (7 errors) ====
11+
/**
12+
* @typedef {any} Any
13+
*/
14+
15+
/**
16+
* @typedef {unknown} Unknown
17+
*/
18+
19+
function fn() {
20+
try { } catch (x) { } // should be OK
21+
try { } catch (/** @type {any} */ err) { } // should be OK
22+
try { } catch (/** @type {Any} */ err) { } // should be OK
23+
try { } catch (/** @type {unknown} */ err) { } // should be OK
24+
try { } catch (/** @type {Unknown} */ err) { } // should be OK
25+
try { } catch (err) { err.foo; } // should be OK
26+
try { } catch (/** @type {any} */ err) { err.foo; } // should be OK
27+
try { } catch (/** @type {Any} */ err) { err.foo; } // should be OK
28+
try { } catch (/** @type {unknown} */ err) { console.log(err); } // should be OK
29+
try { } catch (/** @type {Unknown} */ err) { console.log(err); } // should be OK
30+
try { } catch (/** @type {unknown} */ err) { err.foo; } // error in the body
31+
~~~
32+
!!! error TS2339: Property 'foo' does not exist on type 'unknown'.
33+
try { } catch (/** @type {Unknown} */ err) { err.foo; } // error in the body
34+
~~~
35+
!!! error TS2339: Property 'foo' does not exist on type 'unknown'.
36+
try { } catch (/** @type {Error} */ err) { } // error in the type
37+
~~~~~
38+
!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
39+
try { } catch (/** @type {object} */ err) { } // error in the type
40+
~~~~~~
41+
!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
42+
43+
try { console.log(); }
44+
// @ts-ignore
45+
catch (/** @type {number} */ err) { // e should not be a `number`
46+
console.log(err.toLowerCase());
47+
}
48+
49+
// minor bug: shows that the `catch` argument is skipped when checking scope
50+
try { }
51+
catch (err) {
52+
/** @type {string} */
53+
let err;
54+
~~~
55+
!!! error TS2492: Cannot redeclare identifier 'err' in catch clause.
56+
}
57+
try { }
58+
catch (err) {
59+
/** @type {boolean} */
60+
var err;
61+
}
62+
63+
try { } catch ({ x }) { } // should be OK
64+
try { } catch (/** @type {any} */ { x }) { x.foo; } // should be OK
65+
try { } catch (/** @type {Any} */ { x }) { x.foo;} // should be OK
66+
try { } catch (/** @type {unknown} */ { x }) { console.log(x); } // should be OK
67+
try { } catch (/** @type {Unknown} */ { x }) { console.log(x); } // should be OK
68+
try { } catch (/** @type {Error} */ { x }) { } // error in the type
69+
~~~~~
70+
!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
71+
try { } catch (/** @type {object} */ { x }) { } // error in the type
72+
~~~~~~
73+
!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
74+
}
75+

‎tests/baselines/reference/jsdocCatchClauseWithTypeAnnotation.js

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
//// [foo.js]
2+
/**
3+
* @typedef {any} Any
4+
*/
5+
6+
/**
7+
* @typedef {unknown} Unknown
8+
*/
9+
10+
function fn() {
11+
try { } catch (x) { } // should be OK
12+
try { } catch (/** @type {any} */ err) { } // should be OK
13+
try { } catch (/** @type {Any} */ err) { } // should be OK
14+
try { } catch (/** @type {unknown} */ err) { } // should be OK
15+
try { } catch (/** @type {Unknown} */ err) { } // should be OK
16+
try { } catch (err) { err.foo; } // should be OK
17+
try { } catch (/** @type {any} */ err) { err.foo; } // should be OK
18+
try { } catch (/** @type {Any} */ err) { err.foo; } // should be OK
19+
try { } catch (/** @type {unknown} */ err) { console.log(err); } // should be OK
20+
try { } catch (/** @type {Unknown} */ err) { console.log(err); } // should be OK
21+
try { } catch (/** @type {unknown} */ err) { err.foo; } // error in the body
22+
try { } catch (/** @type {Unknown} */ err) { err.foo; } // error in the body
23+
try { } catch (/** @type {Error} */ err) { } // error in the type
24+
try { } catch (/** @type {object} */ err) { } // error in the type
25+
26+
try { console.log(); }
27+
// @ts-ignore
28+
catch (/** @type {number} */ err) { // e should not be a `number`
29+
console.log(err.toLowerCase());
30+
}
31+
32+
// minor bug: shows that the `catch` argument is skipped when checking scope
33+
try { }
34+
catch (err) {
35+
/** @type {string} */
36+
let err;
37+
}
38+
try { }
39+
catch (err) {
40+
/** @type {boolean} */
41+
var err;
42+
}
43+
44+
try { } catch ({ x }) { } // should be OK
45+
try { } catch (/** @type {any} */ { x }) { x.foo; } // should be OK
46+
try { } catch (/** @type {Any} */ { x }) { x.foo;} // should be OK
47+
try { } catch (/** @type {unknown} */ { x }) { console.log(x); } // should be OK
48+
try { } catch (/** @type {Unknown} */ { x }) { console.log(x); } // should be OK
49+
try { } catch (/** @type {Error} */ { x }) { } // error in the type
50+
try { } catch (/** @type {object} */ { x }) { } // error in the type
51+
}
52+
53+
54+
//// [foo.js]
55+
/**
56+
* @typedef {any} Any
57+
*/
58+
/**
59+
* @typedef {unknown} Unknown
60+
*/
61+
function fn() {
62+
try { }
63+
catch (x) { } // should be OK
64+
try { }
65+
catch ( /** @type {any} */err) { } // should be OK
66+
try { }
67+
catch ( /** @type {Any} */err) { } // should be OK
68+
try { }
69+
catch ( /** @type {unknown} */err) { } // should be OK
70+
try { }
71+
catch ( /** @type {Unknown} */err) { } // should be OK
72+
try { }
73+
catch (err) {
74+
err.foo;
75+
} // should be OK
76+
try { }
77+
catch ( /** @type {any} */err) {
78+
err.foo;
79+
} // should be OK
80+
try { }
81+
catch ( /** @type {Any} */err) {
82+
err.foo;
83+
} // should be OK
84+
try { }
85+
catch ( /** @type {unknown} */err) {
86+
console.log(err);
87+
} // should be OK
88+
try { }
89+
catch ( /** @type {Unknown} */err) {
90+
console.log(err);
91+
} // should be OK
92+
try { }
93+
catch ( /** @type {unknown} */err) {
94+
err.foo;
95+
} // error in the body
96+
try { }
97+
catch ( /** @type {Unknown} */err) {
98+
err.foo;
99+
} // error in the body
100+
try { }
101+
catch ( /** @type {Error} */err) { } // error in the type
102+
try { }
103+
catch ( /** @type {object} */err) { } // error in the type
104+
try {
105+
console.log();
106+
}
107+
// @ts-ignore
108+
catch ( /** @type {number} */err) { // e should not be a `number`
109+
console.log(err.toLowerCase());
110+
}
111+
// minor bug: shows that the `catch` argument is skipped when checking scope
112+
try { }
113+
catch (err) {
114+
/** @type {string} */
115+
let err;
116+
}
117+
try { }
118+
catch (err) {
119+
/** @type {boolean} */
120+
var err;
121+
}
122+
try { }
123+
catch ({ x }) { } // should be OK
124+
try { }
125+
catch ( /** @type {any} */{ x }) {
126+
x.foo;
127+
} // should be OK
128+
try { }
129+
catch ( /** @type {Any} */{ x }) {
130+
x.foo;
131+
} // should be OK
132+
try { }
133+
catch ( /** @type {unknown} */{ x }) {
134+
console.log(x);
135+
} // should be OK
136+
try { }
137+
catch ( /** @type {Unknown} */{ x }) {
138+
console.log(x);
139+
} // should be OK
140+
try { }
141+
catch ( /** @type {Error} */{ x }) { } // error in the type
142+
try { }
143+
catch ( /** @type {object} */{ x }) { } // error in the type
144+
}

0 commit comments

Comments
 (0)