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.

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

Fix 'as const'-like behavior in JSDoc type cast #45464

Merged
merged 1 commit into from
Aug 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 24 additions & 12 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8388,12 +8388,12 @@ namespace ts {
}

function isNullOrUndefined(node: Expression) {
const expr = skipParentheses(node);
const expr = skipParentheses(node, /*excludeJSDocTypeAssertions*/ true);
return expr.kind === SyntaxKind.NullKeyword || expr.kind === SyntaxKind.Identifier && getResolvedSymbol(expr as Identifier) === undefinedSymbol;
}

function isEmptyArrayLiteral(node: Expression) {
const expr = skipParentheses(node);
const expr = skipParentheses(node, /*excludeJSDocTypeAssertions*/ true);
return expr.kind === SyntaxKind.ArrayLiteralExpression && (expr as ArrayLiteralExpression).elements.length === 0;
}

Expand Down Expand Up @@ -22956,7 +22956,7 @@ namespace ts {
}

function isFalseExpression(expr: Expression): boolean {
const node = skipParentheses(expr);
const node = skipParentheses(expr, /*excludeJSDocTypeAssertions*/ true);
return node.kind === SyntaxKind.FalseKeyword || node.kind === SyntaxKind.BinaryExpression && (
(node as BinaryExpression).operatorToken.kind === SyntaxKind.AmpersandAmpersandToken && (isFalseExpression((node as BinaryExpression).left) || isFalseExpression((node as BinaryExpression).right)) ||
(node as BinaryExpression).operatorToken.kind === SyntaxKind.BarBarToken && isFalseExpression((node as BinaryExpression).left) && isFalseExpression((node as BinaryExpression).right));
Expand Down Expand Up @@ -23278,7 +23278,7 @@ namespace ts {
}

function narrowTypeByAssertion(type: Type, expr: Expression): Type {
const node = skipParentheses(expr);
const node = skipParentheses(expr, /*excludeJSDocTypeAssertions*/ true);
if (node.kind === SyntaxKind.FalseKeyword) {
return unreachableNeverType;
}
Expand Down Expand Up @@ -25858,7 +25858,9 @@ namespace ts {
case SyntaxKind.ParenthesizedExpression: {
// Like in `checkParenthesizedExpression`, an `/** @type {xyz} */` comment before a parenthesized expression acts as a type cast.
const tag = isInJSFile(parent) ? getJSDocTypeTag(parent) : undefined;
return tag ? getTypeFromTypeNode(tag.typeExpression.type) : getContextualType(parent as ParenthesizedExpression, contextFlags);
return !tag ? getContextualType(parent as ParenthesizedExpression, contextFlags) :
Copy link
Member Author

Choose a reason for hiding this comment

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

I inverted the order here so that we have one condition per branch (i.e., !a ? x : b ? y : z) instead of nested conditions (i.e., a ? b ? y : x : z).

isJSDocTypeTag(tag) && isConstTypeReference(tag.typeExpression.type) ? tryFindWhenConstTypeReference(parent as ParenthesizedExpression) :
getTypeFromTypeNode(tag.typeExpression.type);
}
case SyntaxKind.NonNullExpression:
return getContextualType(parent as NonNullExpression, contextFlags);
Expand Down Expand Up @@ -32768,8 +32770,10 @@ namespace ts {
}

function isTypeAssertion(node: Expression) {
node = skipParentheses(node);
return node.kind === SyntaxKind.TypeAssertionExpression || node.kind === SyntaxKind.AsExpression;
node = skipParentheses(node, /*excludeJSDocTypeAssertions*/ true);
return node.kind === SyntaxKind.TypeAssertionExpression ||
node.kind === SyntaxKind.AsExpression ||
isJSDocTypeAssertion(node);
}

function checkDeclarationInitializer(declaration: HasExpressionInitializer, contextualType?: Type | undefined) {
Expand Down Expand Up @@ -32844,6 +32848,7 @@ namespace ts {
function isConstContext(node: Expression): boolean {
const parent = node.parent;
return isAssertionExpression(parent) && isConstTypeReference(parent.type) ||
isJSDocTypeAssertion(parent) && isConstTypeReference(getJSDocTypeAssertionType(parent)) ||
(isParenthesizedExpression(parent) || isArrayLiteralExpression(parent) || isSpreadElement(parent)) && isConstContext(parent) ||
(isPropertyAssignment(parent) || isShorthandPropertyAssignment(parent) || isTemplateSpan(parent)) && isConstContext(parent.parent);
}
Expand Down Expand Up @@ -33056,7 +33061,14 @@ namespace ts {
}

function getQuickTypeOfExpression(node: Expression) {
const expr = skipParentheses(node);
let expr = skipParentheses(node, /*excludeJSDocTypeAssertions*/ true);
if (isJSDocTypeAssertion(expr)) {
const type = getJSDocTypeAssertionType(expr);
if (!isConstTypeReference(type)) {
return getTypeFromTypeNode(type);
}
}
expr = skipParentheses(node);
// Optimize for the common case of a call to a function with a single non-generic call
// signature where we can just fetch the return type without checking the arguments.
if (isCallExpression(expr) && expr.expression.kind !== SyntaxKind.SuperKeyword && !isRequireCall(expr, /*checkArgumentIsStringLiteralLike*/ true) && !isSymbolOrSymbolForCall(expr)) {
Expand Down Expand Up @@ -33143,9 +33155,9 @@ namespace ts {
}

function checkParenthesizedExpression(node: ParenthesizedExpression, checkMode?: CheckMode): Type {
const tag = isInJSFile(node) ? getJSDocTypeTag(node) : undefined;
if (tag) {
return checkAssertionWorker(tag.typeExpression.type, tag.typeExpression.type, node.expression, checkMode);
if (isJSDocTypeAssertion(node)) {
const type = getJSDocTypeAssertionType(node);
return checkAssertionWorker(type, type, node.expression, checkMode);
}
return checkExpression(node.expression, checkMode);
}
Expand Down Expand Up @@ -36093,7 +36105,7 @@ namespace ts {
if (getFalsyFlags(type)) return;

const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
if (isPropertyAccessExpression(location) && isAssertionExpression(skipParentheses(location.expression))) {
if (isPropertyAccessExpression(location) && isTypeAssertion(location.expression)) {
return;
}

Expand Down
15 changes: 15 additions & 0 deletions src/compiler/factory/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,24 @@ namespace ts {
node.kind === SyntaxKind.CommaListExpression;
}

export function isJSDocTypeAssertion(node: Node): node is JSDocTypeAssertion {
return isParenthesizedExpression(node)
&& isInJSFile(node)
&& !!getJSDocTypeTag(node);
}

export function getJSDocTypeAssertionType(node: JSDocTypeAssertion) {
const type = getJSDocType(node);
Debug.assertIsDefined(type);
return type;
}

export function isOuterExpression(node: Node, kinds = OuterExpressionKinds.All): node is OuterExpression {
switch (node.kind) {
case SyntaxKind.ParenthesizedExpression:
if (kinds & OuterExpressionKinds.ExcludeJSDocTypeAssertion && isJSDocTypeAssertion(node)) {
return false;
}
return (kinds & OuterExpressionKinds.Parentheses) !== 0;
case SyntaxKind.TypeAssertionExpression:
case SyntaxKind.AsExpression:
Expand Down
9 changes: 8 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2253,6 +2253,11 @@ namespace ts {
readonly expression: Expression;
}

/* @internal */
export interface JSDocTypeAssertion extends ParenthesizedExpression {
readonly _jsDocTypeAssertionBrand: never;
}

export interface ArrayLiteralExpression extends PrimaryExpression {
readonly kind: SyntaxKind.ArrayLiteralExpression;
readonly elements: NodeArray<Expression>;
Expand Down Expand Up @@ -6889,7 +6894,9 @@ namespace ts {
PartiallyEmittedExpressions = 1 << 3,

Assertions = TypeAssertions | NonNullAssertions,
All = Parentheses | Assertions | PartiallyEmittedExpressions
All = Parentheses | Assertions | PartiallyEmittedExpressions,

ExcludeJSDocTypeAssertion = 1 << 4,
}

/* @internal */
Expand Down
35 changes: 29 additions & 6 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2634,13 +2634,13 @@ namespace ts {
let result: (JSDoc | JSDocTag)[] | undefined;
// Pull parameter comments from declaring function as well
if (isVariableLike(hostNode) && hasInitializer(hostNode) && hasJSDocNodes(hostNode.initializer!)) {
result = append(result, last((hostNode.initializer as HasJSDoc).jsDoc!));
result = addRange(result, filterOwnedJSDocTags(hostNode, last((hostNode.initializer as HasJSDoc).jsDoc!)));
}

let node: Node | undefined = hostNode;
while (node && node.parent) {
if (hasJSDocNodes(node)) {
result = append(result, last(node.jsDoc!));
result = addRange(result, filterOwnedJSDocTags(hostNode, last(node.jsDoc!)));
}

if (node.kind === SyntaxKind.Parameter) {
Expand All @@ -2656,6 +2656,26 @@ namespace ts {
return result || emptyArray;
}

function filterOwnedJSDocTags(hostNode: Node, jsDoc: JSDoc | JSDocTag) {
if (isJSDoc(jsDoc)) {
const ownedTags = filter(jsDoc.tags, tag => ownsJSDocTag(hostNode, tag));
return jsDoc.tags === ownedTags ? [jsDoc] : ownedTags;
sandersn marked this conversation as resolved.
Show resolved Hide resolved
}
return ownsJSDocTag(hostNode, jsDoc) ? [jsDoc] : undefined;
}

/**
* Determines whether a host node owns a jsDoc tag. A `@type` tag attached to a
* a ParenthesizedExpression belongs only to the ParenthesizedExpression.
*/
function ownsJSDocTag(hostNode: Node, tag: JSDocTag) {
return !isJSDocTypeTag(tag)
|| !tag.parent
|| !isJSDoc(tag.parent)
|| !isParenthesizedExpression(tag.parent.parent)
|| tag.parent.parent === hostNode;
}

export function getNextJSDocCommentLocation(node: Node) {
const parent = node.parent;
if (parent.kind === SyntaxKind.PropertyAssignment ||
Expand Down Expand Up @@ -2899,10 +2919,13 @@ namespace ts {
return [child, node];
}

export function skipParentheses(node: Expression): Expression;
export function skipParentheses(node: Node): Node;
export function skipParentheses(node: Node): Node {
return skipOuterExpressions(node, OuterExpressionKinds.Parentheses);
export function skipParentheses(node: Expression, excludeJSDocTypeAssertions?: boolean): Expression;
export function skipParentheses(node: Node, excludeJSDocTypeAssertions?: boolean): Node;
export function skipParentheses(node: Node, excludeJSDocTypeAssertions?: boolean): Node {
const flags = excludeJSDocTypeAssertions ?
OuterExpressionKinds.Parentheses | OuterExpressionKinds.ExcludeJSDocTypeAssertion :
OuterExpressionKinds.Parentheses;
return skipOuterExpressions(node, flags);
}

// a node is delete target iff. it is PropertyAccessExpression/ElementAccessExpression with parentheses skipped
Expand Down
3 changes: 2 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3218,7 +3218,8 @@ declare namespace ts {
NonNullAssertions = 4,
PartiallyEmittedExpressions = 8,
Assertions = 6,
All = 15
All = 15,
ExcludeJSDocTypeAssertion = 16
}
export type TypeOfTag = "undefined" | "number" | "bigint" | "boolean" | "string" | "symbol" | "object" | "function";
export interface NodeFactory {
Expand Down
3 changes: 2 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3218,7 +3218,8 @@ declare namespace ts {
NonNullAssertions = 4,
PartiallyEmittedExpressions = 8,
Assertions = 6,
All = 15
All = 15,
ExcludeJSDocTypeAssertion = 16
}
export type TypeOfTag = "undefined" | "number" | "bigint" | "boolean" | "string" | "symbol" | "object" | "function";
export interface NodeFactory {
Expand Down
5 changes: 4 additions & 1 deletion tests/baselines/reference/jsdocTypeTagCast.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,7 @@ tests/cases/conformance/jsdoc/b.js(67,8): error TS2454: Variable 'numOrStr' is u
}



var asConst1 = /** @type {const} */(1);
var asConst2 = /** @type {const} */({
x: 1
});
9 changes: 8 additions & 1 deletion tests/baselines/reference/jsdocTypeTagCast.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ if(/** @type {numOrStr is string} */(numOrStr === undefined)) { // Error
}



var asConst1 = /** @type {const} */(1);
var asConst2 = /** @type {const} */({
x: 1
});

//// [a.js]
var W;
Expand Down Expand Up @@ -154,3 +157,7 @@ var str;
if ( /** @type {numOrStr is string} */(numOrStr === undefined)) { // Error
str = numOrStr; // Error, no narrowing occurred
}
var asConst1 = /** @type {const} */ (1);
var asConst2 = /** @type {const} */ ({
x: 1
});
9 changes: 9 additions & 0 deletions tests/baselines/reference/jsdocTypeTagCast.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,13 @@ if(/** @type {numOrStr is string} */(numOrStr === undefined)) { // Error
}


var asConst1 = /** @type {const} */(1);
>asConst1 : Symbol(asConst1, Decl(b.js, 70, 3))

var asConst2 = /** @type {const} */({
>asConst2 : Symbol(asConst2, Decl(b.js, 71, 3))

x: 1
>x : Symbol(x, Decl(b.js, 71, 37))

});
14 changes: 14 additions & 0 deletions tests/baselines/reference/jsdocTypeTagCast.types
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,18 @@ if(/** @type {numOrStr is string} */(numOrStr === undefined)) { // Error
}


var asConst1 = /** @type {const} */(1);
>asConst1 : 1
>(1) : 1
>1 : 1

var asConst2 = /** @type {const} */({
>asConst2 : { readonly x: 1; }
>({ x: 1}) : { readonly x: 1; }
>{ x: 1} : { readonly x: 1; }

x: 1
>x : 1
>1 : 1

});
4 changes: 4 additions & 0 deletions tests/cases/conformance/jsdoc/jsdocTypeTagCast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,7 @@ if(/** @type {numOrStr is string} */(numOrStr === undefined)) { // Error
}


var asConst1 = /** @type {const} */(1);
var asConst2 = /** @type {const} */({
x: 1
});