Skip to content

Fix default property assigned prototype #40836

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

Merged
merged 4 commits into from
Sep 30, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 61 additions & 19 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ namespace ts {
},
getAugmentedPropertiesOfType,
getRootSymbols,
getSymbolOfExpando,
getContextualType: (nodeIn: Expression, contextFlags?: ContextFlags) => {
const node = getParseTreeNode(nodeIn, isExpression);
if (!node) {
Expand Down Expand Up @@ -8728,9 +8729,9 @@ namespace ts {
let links = getSymbolLinks(symbol);
const originalLinks = links;
if (!links.type) {
const jsDeclaration = symbol.valueDeclaration && getDeclarationOfExpando(symbol.valueDeclaration);
if (jsDeclaration) {
const merged = mergeJSSymbols(symbol, getSymbolOfNode(jsDeclaration));
const expando = symbol.valueDeclaration && getSymbolOfExpando(symbol.valueDeclaration, /*allowDeclaration*/ false);
if (expando) {
const merged = mergeJSSymbols(symbol, expando);
if (merged) {
// note:we overwrite links because we just cloned the symbol
symbol = links = merged;
Expand Down Expand Up @@ -24828,7 +24829,7 @@ namespace ts {
const exprType = checkJsxAttribute(attributeDecl, checkMode);
objectFlags |= getObjectFlags(exprType) & ObjectFlags.PropagatingFlags;

const attributeSymbol = createSymbol(SymbolFlags.Property | SymbolFlags.Transient | member.flags, member.escapedName);
Copy link
Member Author

Choose a reason for hiding this comment

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

this is a drive-by fix; createSymbol always adds Transient.

const attributeSymbol = createSymbol(SymbolFlags.Property | member.flags, member.escapedName);
attributeSymbol.declarations = member.declarations;
attributeSymbol.parent = member.parent;
if (member.valueDeclaration) {
Expand Down Expand Up @@ -24887,7 +24888,7 @@ namespace ts {
const contextualType = getApparentTypeOfContextualType(openingLikeElement.attributes);
const childrenContextualType = contextualType && getTypeOfPropertyOfContextualType(contextualType, jsxChildrenPropertyName);
// If there are children in the body of JSX element, create dummy attribute "children" with the union of children types so that it will pass the attribute checking process
const childrenPropSymbol = createSymbol(SymbolFlags.Property | SymbolFlags.Transient, jsxChildrenPropertyName);
const childrenPropSymbol = createSymbol(SymbolFlags.Property, jsxChildrenPropertyName);
childrenPropSymbol.type = childrenTypes.length === 1 ? childrenTypes[0] :
childrenContextualType && forEachType(childrenContextualType, isTupleLikeType) ? createTupleType(childrenTypes) :
createArrayType(getUnionType(childrenTypes));
Expand Down Expand Up @@ -28076,15 +28077,59 @@ namespace ts {
}

function getAssignedClassSymbol(decl: Declaration): Symbol | undefined {
const assignmentSymbol = decl && decl.parent &&
(isFunctionDeclaration(decl) && getSymbolOfNode(decl) ||
isBinaryExpression(decl.parent) && getSymbolOfNode(decl.parent.left) ||
isVariableDeclaration(decl.parent) && getSymbolOfNode(decl.parent));
const prototype = assignmentSymbol && assignmentSymbol.exports && assignmentSymbol.exports.get("prototype" as __String);
const init = prototype && prototype.valueDeclaration && getAssignedJSPrototype(prototype.valueDeclaration);
const assignmentSymbol = decl && getSymbolOfExpando(decl, /*allowDeclaration*/ true);
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the actual fix

const prototype = assignmentSymbol?.exports?.get("prototype" as __String);
const init = prototype?.valueDeclaration && getAssignedJSPrototype(prototype.valueDeclaration);
return init ? getSymbolOfNode(init) : undefined;
}

function getSymbolOfExpando(node: Node, allowDeclaration: boolean): Symbol | undefined {
Copy link
Member Author

Choose a reason for hiding this comment

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

changes from the move:

  1. Return a Symbol instead of a Node, since all callers immediately do that anyway. This is why I moved it into the checker.
  2. Add a parameter allowDeclaration that allows node to be either the expando or its declaration. The parameter disables the checks that assert that node is the expando initialiser.
  3. Add a FunctionDeclaration case when allowDeclaration is true.

if (!node.parent) {
return undefined;
}
let name: Expression | BindingName | undefined;
let decl: Node | undefined;
if (isVariableDeclaration(node.parent) && node.parent.initializer === node) {
if (!isInJSFile(node) && !isVarConst(node.parent)) {
return undefined;
}
name = node.parent.name;
decl = node.parent;
}
else if (isBinaryExpression(node.parent)) {
const parentNode = node.parent;
const parentNodeOperator = node.parent.operatorToken.kind;
if (parentNodeOperator === SyntaxKind.EqualsToken && (allowDeclaration || parentNode.right === node)) {
name = parentNode.left;
decl = name;
}
else if (parentNodeOperator === SyntaxKind.BarBarToken || parentNodeOperator === SyntaxKind.QuestionQuestionToken) {
if (isVariableDeclaration(parentNode.parent) && parentNode.parent.initializer === parentNode) {
name = parentNode.parent.name;
decl = parentNode.parent;
}
else if (isBinaryExpression(parentNode.parent) && parentNode.parent.operatorToken.kind === SyntaxKind.EqualsToken && (allowDeclaration || parentNode.parent.right === parentNode)) {
name = parentNode.parent.left;
decl = name;
}

if (!name || !isBindableStaticNameExpression(name) || !isSameEntityName(name, parentNode.left)) {
return undefined;
}
}
}
else if (allowDeclaration && isFunctionDeclaration(node)) {
name = node.name;
decl = node;
}

if (!decl || !name || (!allowDeclaration && !getExpandoInitializer(node, isPrototypeAccess(name)))) {
return undefined;
}
return getSymbolOfNode(decl);
}


function getAssignedJSPrototype(node: Node) {
if (!node.parent) {
return false;
Expand Down Expand Up @@ -28161,14 +28206,11 @@ namespace ts {
}

if (isInJSFile(node)) {
const decl = getDeclarationOfExpando(node);
if (decl) {
const jsSymbol = getSymbolOfNode(decl);
if (jsSymbol?.exports?.size) {
const jsAssignmentType = createAnonymousType(jsSymbol, jsSymbol.exports, emptyArray, emptyArray, undefined, undefined);
jsAssignmentType.objectFlags |= ObjectFlags.JSLiteral;
return getIntersectionType([returnType, jsAssignmentType]);
}
const jsSymbol = getSymbolOfExpando(node, /*allowDeclaration*/ false);
if (jsSymbol?.exports?.size) {
const jsAssignmentType = createAnonymousType(jsSymbol, jsSymbol.exports, emptyArray, emptyArray, undefined, undefined);
jsAssignmentType.objectFlags |= ObjectFlags.JSLiteral;
return getIntersectionType([returnType, jsAssignmentType]);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4004,6 +4004,7 @@ namespace ts {
getAugmentedPropertiesOfType(type: Type): Symbol[];

getRootSymbols(symbol: Symbol): readonly Symbol[];
getSymbolOfExpando(node: Node, allowDeclaration: boolean): Symbol | undefined;
getContextualType(node: Expression): Type | undefined;
/* @internal */ getContextualType(node: Expression, contextFlags?: ContextFlags): Type | undefined; // eslint-disable-line @typescript-eslint/unified-signatures
/* @internal */ getContextualTypeForObjectLiteralElement(element: ObjectLiteralElementLike): Type | undefined;
Expand Down
44 changes: 1 addition & 43 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1937,48 +1937,6 @@ namespace ts {
return getSourceTextOfNodeFromSourceFile(sourceFile, str).charCodeAt(0) === CharacterCodes.doubleQuote;
}

export function getDeclarationOfExpando(node: Node): Node | undefined {
if (!node.parent) {
return undefined;
}
let name: Expression | BindingName | undefined;
let decl: Node | undefined;
if (isVariableDeclaration(node.parent) && node.parent.initializer === node) {
if (!isInJSFile(node) && !isVarConst(node.parent)) {
return undefined;
}
name = node.parent.name;
decl = node.parent;
}
else if (isBinaryExpression(node.parent)) {
const parentNode = node.parent;
const parentNodeOperator = node.parent.operatorToken.kind;
if (parentNodeOperator === SyntaxKind.EqualsToken && parentNode.right === node) {
name = parentNode.left;
decl = name;
}
else if (parentNodeOperator === SyntaxKind.BarBarToken || parentNodeOperator === SyntaxKind.QuestionQuestionToken) {
if (isVariableDeclaration(parentNode.parent) && parentNode.parent.initializer === parentNode) {
name = parentNode.parent.name;
decl = parentNode.parent;
}
else if (isBinaryExpression(parentNode.parent) && parentNode.parent.operatorToken.kind === SyntaxKind.EqualsToken && parentNode.parent.right === parentNode) {
name = parentNode.parent.left;
decl = name;
}

if (!name || !isBindableStaticNameExpression(name) || !isSameEntityName(name, parentNode.left)) {
return undefined;
}
}
}

if (!name || !getExpandoInitializer(node, isPrototypeAccess(name))) {
return undefined;
}
return decl;
}

export function isAssignmentDeclaration(decl: Declaration) {
return isBinaryExpression(decl) || isAccessExpression(decl) || isIdentifier(decl) || isCallExpression(decl);
}
Expand Down Expand Up @@ -2098,7 +2056,7 @@ namespace ts {
* var min = window.min || {}
* my.app = self.my.app || class { }
*/
function isSameEntityName(name: Expression, initializer: Expression): boolean {
export function isSameEntityName(name: Expression, initializer: Expression): boolean {
if (isPropertyNameLiteral(name) && isPropertyNameLiteral(initializer)) {
return getTextOfIdentifierOrLiteral(name) === getTextOfIdentifierOrLiteral(initializer);
}
Expand Down
7 changes: 3 additions & 4 deletions src/services/suggestionDiagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace ts {

function check(node: Node) {
if (isJsFile) {
if (canBeConvertedToClass(node)) {
if (canBeConvertedToClass(node, checker)) {
diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration));
}
}
Expand Down Expand Up @@ -190,14 +190,13 @@ namespace ts {
return `${exp.pos.toString()}:${exp.end.toString()}`;
}

function canBeConvertedToClass(node: Node): boolean {
function canBeConvertedToClass(node: Node, checker: TypeChecker): boolean {
if (node.kind === SyntaxKind.FunctionExpression) {
if (isVariableDeclaration(node.parent) && node.symbol.members?.size) {
return true;
}

const decl = getDeclarationOfExpando(node);
const symbol = decl?.symbol;
const symbol = checker.getSymbolOfExpando(node, /*allowDeclaration*/ false);
return !!(symbol && (symbol.exports?.size || symbol.members?.size));
}

Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2207,6 +2207,7 @@ declare namespace ts {
getFullyQualifiedName(symbol: Symbol): string;
getAugmentedPropertiesOfType(type: Type): Symbol[];
getRootSymbols(symbol: Symbol): readonly Symbol[];
getSymbolOfExpando(node: Node, allowDeclaration: boolean): Symbol | undefined;
getContextualType(node: Expression): Type | undefined;
/**
* returns unknownSignature in the case of an error.
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2207,6 +2207,7 @@ declare namespace ts {
getFullyQualifiedName(symbol: Symbol): string;
getAugmentedPropertiesOfType(type: Type): Symbol[];
getRootSymbols(symbol: Symbol): readonly Symbol[];
getSymbolOfExpando(node: Node, allowDeclaration: boolean): Symbol | undefined;
getContextualType(node: Expression): Type | undefined;
/**
* returns unknownSignature in the case of an error.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
=== tests/cases/conformance/salsa/bug39167.js ===
var test = {};
>test : Symbol(test, Decl(bug39167.js, 0, 3), Decl(bug39167.js, 0, 14), Decl(bug39167.js, 2, 18))

test.K = test.K ||
>test.K : Symbol(test.K, Decl(bug39167.js, 0, 14), Decl(bug39167.js, 4, 5))
>test : Symbol(test, Decl(bug39167.js, 0, 3), Decl(bug39167.js, 0, 14), Decl(bug39167.js, 2, 18))
>K : Symbol(test.K, Decl(bug39167.js, 0, 14), Decl(bug39167.js, 4, 5))
>test.K : Symbol(test.K, Decl(bug39167.js, 0, 14), Decl(bug39167.js, 4, 5))
>test : Symbol(test, Decl(bug39167.js, 0, 3), Decl(bug39167.js, 0, 14), Decl(bug39167.js, 2, 18))
>K : Symbol(test.K, Decl(bug39167.js, 0, 14), Decl(bug39167.js, 4, 5))

function () {}

test.K.prototype = {
>test.K.prototype : Symbol(test.K.prototype, Decl(bug39167.js, 2, 18))
>test.K : Symbol(test.K, Decl(bug39167.js, 0, 14), Decl(bug39167.js, 4, 5))
>test : Symbol(test, Decl(bug39167.js, 0, 3), Decl(bug39167.js, 0, 14), Decl(bug39167.js, 2, 18))
>K : Symbol(test.K, Decl(bug39167.js, 0, 14), Decl(bug39167.js, 4, 5))
>prototype : Symbol(test.K.prototype, Decl(bug39167.js, 2, 18))

add() {}
>add : Symbol(add, Decl(bug39167.js, 4, 20))

};

new test.K().add;
>new test.K().add : Symbol(add, Decl(bug39167.js, 4, 20))
>test.K : Symbol(test.K, Decl(bug39167.js, 0, 14), Decl(bug39167.js, 4, 5))
>test : Symbol(test, Decl(bug39167.js, 0, 3), Decl(bug39167.js, 0, 14), Decl(bug39167.js, 2, 18))
>K : Symbol(test.K, Decl(bug39167.js, 0, 14), Decl(bug39167.js, 4, 5))
>add : Symbol(add, Decl(bug39167.js, 4, 20))

Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
=== tests/cases/conformance/salsa/bug39167.js ===
var test = {};
>test : typeof test
>{} : {}

test.K = test.K ||
>test.K = test.K || function () {} : typeof K
>test.K : typeof K
>test : typeof test
>K : typeof K
>test.K || function () {} : typeof K
>test.K : typeof K
>test : typeof test
>K : typeof K

function () {}
>function () {} : typeof K

test.K.prototype = {
>test.K.prototype = { add() {}} : { add(): void; }
>test.K.prototype : { add(): void; }
>test.K : typeof K
>test : typeof test
>K : typeof K
>prototype : { add(): void; }
>{ add() {}} : { add(): void; }

add() {}
>add : () => void

};

new test.K().add;
>new test.K().add : () => void
>new test.K() : K
>test.K : typeof K
>test : typeof test
>K : typeof K
>add : () => void

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @noEmit: true
// @allowJs: true
// @checkJs: true
// @Filename: bug39167.js
var test = {};
test.K = test.K ||
function () {}

test.K.prototype = {
add() {}
};

new test.K().add;