Skip to content

Commit

Permalink
Make catch clause checking consistent with variable declarations (#52240
Browse files Browse the repository at this point in the history
)
  • Loading branch information
jakebailey authored Jan 19, 2023
1 parent ecaf6d9 commit b7f619d
Show file tree
Hide file tree
Showing 30 changed files with 1,719 additions and 42 deletions.
28 changes: 15 additions & 13 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10219,6 +10219,15 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

// Use type from type annotation if one is present
const declaredType = tryGetTypeFromEffectiveTypeNode(declaration);
if (isCatchClauseVariableDeclarationOrBindingElement(declaration)) {
if (declaredType) {
// If the catch clause is explicitly annotated with any or unknown, accept it, otherwise error.
return isTypeAny(declaredType) || declaredType === unknownType ? declaredType : errorType;
}
// If the catch clause is not explicitly annotated, treat it as though it were explicitly
// annotated with unknown or any, depending on useUnknownInCatchVariables.
return useUnknownInCatchVariables ? unknownType : anyType;
}
if (declaredType) {
return addOptionality(declaredType, isProperty, isOptional);
}
Expand Down Expand Up @@ -10877,18 +10886,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
members.set("exports" as __String, result);
return createAnonymousType(symbol, members, emptyArray, emptyArray, emptyArray);
}
// Handle catch clause variables
Debug.assertIsDefined(symbol.valueDeclaration);
const declaration = symbol.valueDeclaration;
if (isCatchClauseVariableDeclarationOrBindingElement(declaration)) {
const typeNode = getEffectiveTypeAnnotationNode(declaration);
if (typeNode === undefined) {
return useUnknownInCatchVariables ? unknownType : anyType;
}
const type = getTypeOfNode(typeNode);
// an errorType will make `checkTryStatement` issue an error
return isTypeAny(type) || type === unknownType ? type : errorType;
}
// Handle export default expressions
if (isSourceFile(declaration) && isJsonSourceFile(declaration)) {
if (!declaration.statements.length) {
Expand Down Expand Up @@ -27079,6 +27078,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
isFunctionLike(node) && !getImmediatelyInvokedFunctionExpression(node) ||
node.kind === SyntaxKind.ModuleBlock ||
node.kind === SyntaxKind.SourceFile ||
node.kind === SyntaxKind.CatchClause ||
node.kind === SyntaxKind.PropertyDeclaration)!;
}

Expand Down Expand Up @@ -27451,6 +27451,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const isParameter = getRootDeclaration(declaration).kind === SyntaxKind.Parameter;
const declarationContainer = getControlFlowContainer(declaration);
let flowContainer = getControlFlowContainer(node);
const isCatch = flowContainer.kind === SyntaxKind.CatchClause;
const isOuterVariable = flowContainer !== declarationContainer;
const isSpreadDestructuringAssignmentTarget = node.parent && node.parent.parent && isSpreadAssignment(node.parent) && isDestructuringAssignmentTarget(node.parent.parent);
const isModuleExports = symbol.flags & SymbolFlags.ModuleExports;
Expand All @@ -27465,7 +27466,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// We only look for uninitialized variables in strict null checking mode, and only when we can analyze
// the entire control flow graph from the variable's declaration (i.e. when the flow container and
// declaration container are the same).
const assumeInitialized = isParameter || isAlias || isOuterVariable || isSpreadDestructuringAssignmentTarget || isModuleExports || isSameScopedBindingElement(node, declaration) ||
const assumeInitialized = isParameter || isCatch || isAlias || isOuterVariable || isSpreadDestructuringAssignmentTarget || isModuleExports || isSameScopedBindingElement(node, declaration) ||
type !== autoType && type !== autoArrayType && (!strictNullChecks || (type.flags & (TypeFlags.AnyOrUnknown | TypeFlags.Void)) !== 0 ||
isInTypeQuery(node) || isInAmbientOrTypeNode(node) || node.parent.kind === SyntaxKind.ExportSpecifier) ||
node.parent.kind === SyntaxKind.NonNullExpression ||
Expand Down Expand Up @@ -41230,9 +41231,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// Grammar checking
if (catchClause.variableDeclaration) {
const declaration = catchClause.variableDeclaration;
const typeNode = getEffectiveTypeAnnotationNode(getRootDeclaration(declaration));
checkVariableLikeDeclaration(declaration);
const typeNode = getEffectiveTypeAnnotationNode(declaration);
if (typeNode) {
const type = getTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ false, CheckMode.Normal);
const type = getTypeFromTypeNode(typeNode);
if (type && !(type.flags & TypeFlags.AnyOrUnknown)) {
grammarErrorOnFirstToken(typeNode, Diagnostics.Catch_clause_variable_type_annotation_must_be_any_or_unknown_if_specified);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.t
tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts(20,23): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts(29,29): error TS2492: Cannot redeclare identifier 'x' in catch clause.
tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts(30,29): error TS2403: Subsequent variable declarations must have the same type. Variable 'x' must be of type 'boolean', but here has type 'string'.
tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts(36,22): error TS2339: Property 'x' does not exist on type '{}'.
tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts(37,22): error TS2339: Property 'x' does not exist on type '{}'.
tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts(38,27): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts(39,27): error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.


==== tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts (8 errors) ====
==== tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.ts (10 errors) ====
type any1 = any;
type unknown1 = unknown;

Expand Down Expand Up @@ -57,8 +59,12 @@ tests/cases/conformance/statements/tryStatements/catchClauseWithTypeAnnotation.t
try { } catch ({ x }) { } // should be OK
try { } catch ({ x }: any) { x.foo; } // should be OK
try { } catch ({ x }: any1) { x.foo;} // should be OK
try { } catch ({ x }: unknown) { console.log(x); } // should be OK
try { } catch ({ x }: unknown1) { console.log(x); } // should be OK
try { } catch ({ x }: unknown) { console.log(x); } // error in the destructure
~
!!! error TS2339: Property 'x' does not exist on type '{}'.
try { } catch ({ x }: unknown1) { console.log(x); } // error in the destructure
~
!!! error TS2339: Property 'x' does not exist on type '{}'.
try { } catch ({ x }: object) { } // error in the type
~~~~~~
!!! error TS1196: Catch clause variable type annotation must be 'any' or 'unknown' if specified.
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/catchClauseWithTypeAnnotation.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ function fn(x: boolean) {
try { } catch ({ x }) { } // should be OK
try { } catch ({ x }: any) { x.foo; } // should be OK
try { } catch ({ x }: any1) { x.foo;} // should be OK
try { } catch ({ x }: unknown) { console.log(x); } // should be OK
try { } catch ({ x }: unknown1) { console.log(x); } // should be OK
try { } catch ({ x }: unknown) { console.log(x); } // error in the destructure
try { } catch ({ x }: unknown1) { console.log(x); } // error in the destructure
try { } catch ({ x }: object) { } // error in the type
try { } catch ({ x }: Error) { } // error in the type
}
Expand Down Expand Up @@ -124,12 +124,12 @@ function fn(x) {
catch (_d) {
var x_5 = _d.x;
console.log(x_5);
} // should be OK
} // error in the destructure
try { }
catch (_e) {
var x_6 = _e.x;
console.log(x_6);
} // should be OK
} // error in the destructure
try { }
catch (_f) {
var x_7 = _f.x;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ function fn(x: boolean) {
>any1 : Symbol(any1, Decl(catchClauseWithTypeAnnotation.ts, 0, 0))
>x : Symbol(x, Decl(catchClauseWithTypeAnnotation.ts, 34, 20))

try { } catch ({ x }: unknown) { console.log(x); } // should be OK
try { } catch ({ x }: unknown) { console.log(x); } // error in the destructure
>x : Symbol(x, Decl(catchClauseWithTypeAnnotation.ts, 35, 20))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>x : Symbol(x, Decl(catchClauseWithTypeAnnotation.ts, 35, 20))

try { } catch ({ x }: unknown1) { console.log(x); } // should be OK
try { } catch ({ x }: unknown1) { console.log(x); } // error in the destructure
>x : Symbol(x, Decl(catchClauseWithTypeAnnotation.ts, 36, 20))
>unknown1 : Symbol(unknown1, Decl(catchClauseWithTypeAnnotation.ts, 0, 16))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/catchClauseWithTypeAnnotation.types
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,15 @@ function fn(x: boolean) {
>x : any
>foo : any

try { } catch ({ x }: unknown) { console.log(x); } // should be OK
try { } catch ({ x }: unknown) { console.log(x); } // error in the destructure
>x : any
>console.log(x) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>x : any

try { } catch ({ x }: unknown1) { console.log(x); } // should be OK
try { } catch ({ x }: unknown1) { console.log(x); } // error in the destructure
>x : any
>console.log(x) : void
>console.log : (...data: any[]) => void
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
tests/cases/compiler/destructureCatchClause.ts(26,17): error TS2339: Property 'x' does not exist on type '{}'.
tests/cases/compiler/destructureCatchClause.ts(27,15): error TS2461: Type 'unknown' is not an array type.
tests/cases/compiler/destructureCatchClause.ts(29,17): error TS2339: Property 'a' does not exist on type '{}'.
tests/cases/compiler/destructureCatchClause.ts(30,17): error TS2339: Property 'a' does not exist on type '{}'.
tests/cases/compiler/destructureCatchClause.ts(32,15): error TS2461: Type 'unknown' is not an array type.
tests/cases/compiler/destructureCatchClause.ts(33,15): error TS2461: Type 'unknown' is not an array type.
tests/cases/compiler/destructureCatchClause.ts(35,17): error TS2339: Property 'a' does not exist on type '{}'.


==== tests/cases/compiler/destructureCatchClause.ts (7 errors) ====
// These are okay with useUnknownInCatchVariables=false, but not okay with useUnknownInCatchVariables=true.
try {} catch ({ x }) { x }
try {} catch ([ x ]) { x }

try {} catch ({ a: { x } }) { x }
try {} catch ({ a: [ x ] }) { x }

try {} catch ([{ x }]) { x }
try {} catch ([[ x ]]) { x }

try {} catch ({ a: { b: { c: { x }} }}) { x }


try {} catch ({ x }: any) { x }
try {} catch ([ x ]: any) { x }

try {} catch ({ a: { x } }: any) { x }
try {} catch ({ a: [ x ] }: any) { x }

try {} catch ([{ x }]: any) { x }
try {} catch ([[ x ]]: any) { x }

try {} catch ({ a: { b: { c: { x }} }}: any) { x }


try {} catch ({ x }: unknown) { x }
~
!!! error TS2339: Property 'x' does not exist on type '{}'.
try {} catch ([ x ]: unknown) { x }
~~~~~
!!! error TS2461: Type 'unknown' is not an array type.

try {} catch ({ a: { x } }: unknown) { x }
~
!!! error TS2339: Property 'a' does not exist on type '{}'.
try {} catch ({ a: [ x ] }: unknown) { x }
~
!!! error TS2339: Property 'a' does not exist on type '{}'.

try {} catch ([{ x }]: unknown) { x }
~~~~~~~
!!! error TS2461: Type 'unknown' is not an array type.
try {} catch ([[ x ]]: unknown) { x }
~~~~~~~
!!! error TS2461: Type 'unknown' is not an array type.

try {} catch ({ a: { b: { c: { x }} }}: unknown) { x }
~
!!! error TS2339: Property 'a' does not exist on type '{}'.

Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
//// [destructureCatchClause.ts]
// These are okay with useUnknownInCatchVariables=false, but not okay with useUnknownInCatchVariables=true.
try {} catch ({ x }) { x }
try {} catch ([ x ]) { x }

try {} catch ({ a: { x } }) { x }
try {} catch ({ a: [ x ] }) { x }

try {} catch ([{ x }]) { x }
try {} catch ([[ x ]]) { x }

try {} catch ({ a: { b: { c: { x }} }}) { x }


try {} catch ({ x }: any) { x }
try {} catch ([ x ]: any) { x }

try {} catch ({ a: { x } }: any) { x }
try {} catch ({ a: [ x ] }: any) { x }

try {} catch ([{ x }]: any) { x }
try {} catch ([[ x ]]: any) { x }

try {} catch ({ a: { b: { c: { x }} }}: any) { x }


try {} catch ({ x }: unknown) { x }
try {} catch ([ x ]: unknown) { x }

try {} catch ({ a: { x } }: unknown) { x }
try {} catch ({ a: [ x ] }: unknown) { x }

try {} catch ([{ x }]: unknown) { x }
try {} catch ([[ x ]]: unknown) { x }

try {} catch ({ a: { b: { c: { x }} }}: unknown) { x }


//// [destructureCatchClause.js]
// These are okay with useUnknownInCatchVariables=false, but not okay with useUnknownInCatchVariables=true.
try { }
catch (_a) {
var x = _a.x;
x;
}
try { }
catch (_b) {
var x = _b[0];
x;
}
try { }
catch (_c) {
var x = _c.a.x;
x;
}
try { }
catch (_d) {
var x = _d.a[0];
x;
}
try { }
catch (_e) {
var x = _e[0].x;
x;
}
try { }
catch (_f) {
var x = _f[0][0];
x;
}
try { }
catch (_g) {
var x = _g.a.b.c.x;
x;
}
try { }
catch (_h) {
var x = _h.x;
x;
}
try { }
catch (_j) {
var x = _j[0];
x;
}
try { }
catch (_k) {
var x = _k.a.x;
x;
}
try { }
catch (_l) {
var x = _l.a[0];
x;
}
try { }
catch (_m) {
var x = _m[0].x;
x;
}
try { }
catch (_o) {
var x = _o[0][0];
x;
}
try { }
catch (_p) {
var x = _p.a.b.c.x;
x;
}
try { }
catch (_q) {
var x = _q.x;
x;
}
try { }
catch (_r) {
var x = _r[0];
x;
}
try { }
catch (_s) {
var x = _s.a.x;
x;
}
try { }
catch (_t) {
var x = _t.a[0];
x;
}
try { }
catch (_u) {
var x = _u[0].x;
x;
}
try { }
catch (_v) {
var x = _v[0][0];
x;
}
try { }
catch (_w) {
var x = _w.a.b.c.x;
x;
}
Loading

0 comments on commit b7f619d

Please sign in to comment.