Skip to content

Replace errorType return with Debug fail in checkExpressionWorker #54117

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Replace errorType return with Debug fail in checkExpressionWorker #54117

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d7dabe8
Replace errorType return with Debug fail in checkExpressionWorker
weswigham May 3, 2023
d36de35
Add `ImportKeyword` to checkExpressionWorker even though its not flag…
weswigham May 3, 2023
fc0ae95
Add a couple more non-explicit entries to checkExpressionWorker
weswigham May 3, 2023
7dfe497
Make invalid syntax uses of ImportKeyword expressions the errorType
weswigham May 3, 2023
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
12 changes: 10 additions & 2 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ import {
isImportDeclaration,
isImportEqualsDeclaration,
isImportKeyword,
isImportMeta,
isImportOrExportSpecifier,
isImportSpecifier,
isImportTypeNode,
Expand Down Expand Up @@ -37954,9 +37955,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
case SyntaxKind.JsxAttributes:
return checkJsxAttributes(node as JsxAttributes, checkMode);
case SyntaxKind.JsxOpeningElement:
Debug.fail("Shouldn't ever directly check a JsxOpeningElement");
return Debug.fail("Shouldn't ever directly check a JsxOpeningElement");
case SyntaxKind.ImportKeyword:
return isImportCall(node.parent) || isImportMeta(node.parent) ? anyType : errorType; // LHS of ImportCall or ImportMeta or invalid expression TODO: Should this have a better type?
case SyntaxKind.JsxNamespacedName:
return errorType; // FIXME: Remove when JsxNamespacedName is no longer used as an expression
case SyntaxKind.MissingDeclaration:
return errorType; // invalid decorated expression
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rbuckton should this be an expression? It's not flagged by isExpressionNode, but is definitely assigned to expression-expecting node positions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have two different ways to test for expressions, so we may need to verify both are correct:

  • isExpression in utilitiesPublic.ts — Tests whether a Node is a subtype of Expression. Doesn't care about context (i.e., no .parent access). Designed for use with transformers (where .parent may not be set).
  • isExpressionNode in _utilities.ts — Tests whether a Node is used in an expression position. Highly contextual (performs .parent lookups). Used by the checker, though I'd love to find a way to retire it at some point.

default:
return Debug.failBadSyntaxKind(node, "Unhandled expression node kind in checkExpressionWorker");
}
return errorType;
}

// DECLARATION AND STATEMENT TYPE CHECKING
Expand Down