Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(39836): unknown type declaration for catch argument doesn't work in js #42392

Merged
merged 1 commit into from
Mar 5, 2021
Merged

fix(39836): unknown type declaration for catch argument doesn't work in js #42392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8847,9 +8847,11 @@ namespace ts {
Debug.assertIsDefined(symbol.valueDeclaration);
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;
}
Expand Down Expand Up @@ -36044,10 +36046,11 @@ namespace ts {
// Grammar checking
if (catchClause.variableDeclaration) {
const declaration = catchClause.variableDeclaration;
if (declaration.type) {
const typeNode = getEffectiveTypeAnnotationNode(getRootDeclaration(declaration));
if (typeNode) {
const type = getTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ false);
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) {
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6279,6 +6279,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 &&
Expand All @@ -6288,7 +6289,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 {
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,7 @@ namespace ts {
| FunctionDeclaration
| ConstructorDeclaration
| MethodDeclaration
| VariableDeclaration
| PropertyDeclaration
| AccessorDeclaration
| ClassLikeDeclaration
Expand Down Expand Up @@ -1237,7 +1238,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
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 '/**/'
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -687,7 +687,7 @@ declare namespace ts {
readonly kind: SyntaxKind.ConstructSignature;
}
export type BindingName = Identifier | BindingPattern;
export interface VariableDeclaration extends NamedDeclaration {
export interface VariableDeclaration extends NamedDeclaration, JSDocContainer {
a-tarasyuk marked this conversation as resolved.
Show resolved Hide resolved
readonly kind: SyntaxKind.VariableDeclaration;
readonly parent: VariableDeclarationList | CatchClause;
readonly name: BindingName;
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -687,7 +687,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;
Expand Down
75 changes: 75 additions & 0 deletions tests/baselines/reference/jsdocCatchClauseWithTypeAnnotation.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
}

144 changes: 144 additions & 0 deletions tests/baselines/reference/jsdocCatchClauseWithTypeAnnotation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
Loading