From ab2e14f24ef387056bf02bef9d469be31a2f999e Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Thu, 19 Jan 2017 10:10:03 -0800 Subject: [PATCH] Addressing code review in previous PR. Combining below commits Address comment: fix spelling mistakes Address comment: calling `checkApplicableSignatureForJsxOpeningLikeElement` from inside `checkApplicableSignature` Address comment: fix spelling, rename function to be more consistent Address comment: minor fix indentation, fix function name isObjectLiteralPropertyDeclaration => isObjectLiteralElement Address PR: gotoDefinition return the last signature when there is an error in statelss function component Address PR: convert Foreach to for...of Address comment: fix type, inline code, clarify name of variables --- src/compiler/checker.ts | 49 ++++++++-------- src/services/completions.ts | 24 ++++---- src/services/goToDefinition.ts | 18 +++--- src/services/services.ts | 58 ++++++++----------- src/services/signatureHelp.ts | 4 +- .../tsxGoToDefinitionStatelessFunction2.ts | 6 +- 6 files changed, 73 insertions(+), 86 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3b007bcccc..aeaf1871cf 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11870,7 +11870,7 @@ namespace ts { * @param openingLikeElement a Jsx opening-like element * @return an anonymous type (similar to the one returned by checkObjectLiteral) in which its properties are attributes property. */ - function createJsxAttributesTypeFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement, filter?:(symbol: Symbol)=>boolean) { + function createJsxAttributesTypeFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement, filter?: (symbol: Symbol) => boolean) { const attributes = openingLikeElement.attributes; let attributesTable = createMap(); let spread: Type = emptyObjectType; @@ -11924,8 +11924,8 @@ namespace ts { attributesTable = createMap(); if (attributesArray) { forEach(attributesArray, (attr) => { - if (!filter || (filter && filter(attr))) { - attributesTable[attr.name] = attr; + if (!filter || filter(attr)) { + attributesTable.set(attr.name, attr); } }); } @@ -11963,7 +11963,7 @@ namespace ts { */ function checkJsxAttributesAssignableToTagnameAttributes(openingLikeElement: JsxOpeningLikeElement) { // The function involves following steps: - // 1. Figure out expected attributes type expected by resolving tag-name of the JSX opening-like element, tagetAttributesType. + // 1. Figure out expected attributes type expected by resolving tag-name of the JSX opening-like element, targetAttributesType. // During these steps, we will try to resolve the tag-name as intrinsic name, stateless function, stateful component (in the order) // 2. Solved Jsx attributes type given by users, sourceAttributesType, which is by resolving "attributes" property of the JSX opening-like element. // 3. Check if the two are assignable to each other @@ -11977,7 +11977,7 @@ namespace ts { // i.e
// attr1 and attr2 are treated as JSXAttributes attached in the JsxOpeningLikeElement as "attributes". They resolved to be sourceAttributesType. const sourceAttributesType = createJsxAttributesTypeFromAttributesProperty(openingLikeElement, - (attribute: Symbol) => { + attribute => { return isUnhyphenatedJsxName(attribute.name) || !!(getPropertyOfType(targetAttributesType, attribute.name)); }); @@ -13109,12 +13109,6 @@ namespace ts { * @param excludeArgument */ function checkApplicableSignatureForJsxOpeningLikeElement(node: JsxOpeningLikeElement, signature: Signature, relation: Map) { - const headMessage = Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1; - // Stateless function components can have maximum of three arguments: "props", "context", and "updater". - // However "context" and "updater" are implicit and can't be specify by users. Only the first parameter, props, - // can be specified by users through attributes property. - const paramType = getTypeAtPosition(signature, 0); - // JSX opening-like element has correct arity for stateless-function component if the one of the following condition is true: // 1. callIsIncomplete // 2. attributes property has same number of properties as the parameter object type. @@ -13126,6 +13120,11 @@ namespace ts { return true; } + const headMessage = Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1; + // Stateless function components can have maximum of three arguments: "props", "context", and "updater". + // However "context" and "updater" are implicit and can't be specify by users. Only the first parameter, props, + // can be specified by users through attributes property. + const paramType = getTypeAtPosition(signature, 0); const argType = checkExpressionWithContextualType(node.attributes, paramType, /*contextualMapper*/ undefined); const argProperties = getPropertiesOfType(argType); const paramProperties = getPropertiesOfType(paramType); @@ -13146,6 +13145,9 @@ namespace ts { } function checkApplicableSignature(node: CallLikeExpression, args: Expression[], signature: Signature, relation: Map, excludeArgument: boolean[], reportErrors: boolean) { + if (isJsxOpeningLikeElement(node)) { + return checkApplicableSignatureForJsxOpeningLikeElement(node, signature, relation); + } const thisType = getThisTypeOfSignature(signature); if (thisType && thisType !== voidType && node.kind !== SyntaxKind.NewExpression) { // If the called expression is not of the form `x.f` or `x["f"]`, then sourceType = voidType @@ -13620,12 +13622,10 @@ namespace ts { return result; } - // Do not report any error if we are doing so for stateless function component as such error will be error will be handle in "resolveCustomJsxElementAttributesType". if (isJsxOpeningOrSelfClosingElement) { - // If this is a type resolution session, e.g. Language Service, just return undefined as the language service can decide how to proceed with this failure. - // (see getDefinitionAtPosition which simply get the symbol and return the first declaration of the JSXopeningLikeElement node) - // otherwise, just return the latest signature candidate we try so far so that when we report an error we will get better error message. - return produceDiagnostics ? candidateForArgumentError : undefined; + // If there is not result, just return the last one we try as a candidate. + // We do not report any error here because any error will be handled in "resolveCustomJsxElementAttributesType". + return candidateForArgumentError; } // No signatures were applicable. Now report errors based on the last applicable signature with @@ -13726,10 +13726,7 @@ namespace ts { } candidate = getSignatureInstantiation(candidate, typeArgumentTypes); } - if (isJsxOpeningOrSelfClosingElement && !checkApplicableSignatureForJsxOpeningLikeElement(node, candidate, relation)) { - break; - } - else if (!isJsxOpeningOrSelfClosingElement && !checkApplicableSignature(node, args, candidate, relation, excludeArgument, /*reportErrors*/ false)) { + if (!checkApplicableSignature(node, args, candidate, relation, excludeArgument, /*reportErrors*/ false)) { break; } const index = excludeArgument ? indexOf(excludeArgument, true) : -1; @@ -14061,7 +14058,7 @@ namespace ts { } links.resolvedSignature = resolvingSignature; - let callSignature = resolvedStatelessJsxOpeningLikeElement(openingLikeElement, elementType, candidatesOutArray); + let callSignature = resolveStatelessJsxOpeningLikeElement(openingLikeElement, elementType, candidatesOutArray); if (!callSignature || callSignature === unknownSignature) { const callSignatures = elementType && getSignaturesOfType(elementType, SignatureKind.Call); callSignature = callSignatures[callSignatures.length - 1]; @@ -14083,14 +14080,14 @@ namespace ts { * @return a resolved signature if we can find function matching function signature through resolve call or a first signature in the list of functions. * otherwise return undefined if tag-name of the opening-like element doesn't have call signatures */ - function resolvedStatelessJsxOpeningLikeElement(openingLikeElement: JsxOpeningLikeElement, elementType: Type, candidatesOutArray: Signature[]): Signature { + function resolveStatelessJsxOpeningLikeElement(openingLikeElement: JsxOpeningLikeElement, elementType: Type, candidatesOutArray: Signature[]): Signature { if (elementType.flags & TypeFlags.Union) { const types = (elementType as UnionType).types; let result: Signature; - types.map(type => { + for (const type of types) { // This is mainly to fill in all the candidates if there is one. - result = result || resolvedStatelessJsxOpeningLikeElement(openingLikeElement, type, candidatesOutArray); - }); + result = result || resolveStatelessJsxOpeningLikeElement(openingLikeElement, type, candidatesOutArray); + } return result; } @@ -14117,7 +14114,7 @@ namespace ts { return resolveDecorator(node, candidatesOutArray); case SyntaxKind.JsxOpeningElement: case SyntaxKind.JsxSelfClosingElement: - return resolvedStatelessJsxOpeningLikeElement(node, checkExpression((node).tagName), candidatesOutArray); + return resolveStatelessJsxOpeningLikeElement(node, checkExpression((node).tagName), candidatesOutArray); } Debug.fail("Branch in 'resolveSignature' should be unreachable."); } diff --git a/src/services/completions.ts b/src/services/completions.ts index 294210c124..45e9da9fd0 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1381,11 +1381,11 @@ namespace ts.Completions { return parent; } else if (parent.kind === SyntaxKind.JsxAttribute) { - // Currently we parse JsxOpeninLikeElement as: - // JsxOpeninLikeElement + // Currently we parse JsxOpeningLikeElement as: + // JsxOpeningLikeElement // attributes: JsxAttributes // properties: NodeArray - return /*properties list*/parent./*attributes*/parent.parent as JsxOpeningLikeElement; + return parent.parent.parent as JsxOpeningLikeElement; } break; @@ -1394,11 +1394,11 @@ namespace ts.Completions { // whose parent is a JsxOpeningLikeElement case SyntaxKind.StringLiteral: if (parent && ((parent.kind === SyntaxKind.JsxAttribute) || (parent.kind === SyntaxKind.JsxSpreadAttribute))) { - // Currently we parse JsxOpeninLikeElement as: - // JsxOpeninLikeElement + // Currently we parse JsxOpeningLikeElement as: + // JsxOpeningLikeElement // attributes: JsxAttributes // properties: NodeArray - return /*properties list*/parent./*attributes*/parent.parent as JsxOpeningLikeElement; + return parent.parent.parent as JsxOpeningLikeElement; } break; @@ -1407,20 +1407,20 @@ namespace ts.Completions { if (parent && parent.kind === SyntaxKind.JsxExpression && parent.parent && parent.parent.kind === SyntaxKind.JsxAttribute) { - // Currently we parse JsxOpeninLikeElement as: - // JsxOpeninLikeElement + // Currently we parse JsxOpeningLikeElement as: + // JsxOpeningLikeElement // attributes: JsxAttributes // properties: NodeArray // each JsxAttribute can have initializer as JsxExpression - return /*JsxExpression*/parent./*JsxAttribute*/parent./*JsxAttributes*/parent.parent as JsxOpeningLikeElement; + return parent.parent.parent.parent as JsxOpeningLikeElement; } if (parent && parent.kind === SyntaxKind.JsxSpreadAttribute) { - // Currently we parse JsxOpeninLikeElement as: - // JsxOpeninLikeElement + // Currently we parse JsxOpeningLikeElement as: + // JsxOpeningLikeElement // attributes: JsxAttributes // properties: NodeArray - return /*properties list*/parent./*attributes*/parent.parent as JsxOpeningLikeElement; + return parent.parent.parent as JsxOpeningLikeElement; } break; diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index c8114e1809..c418c0e375 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -102,19 +102,19 @@ namespace ts.GoToDefinition { // object literal, lookup the property symbol in the contextual type, and use this for goto-definition. // For example // interface Props{ - // /first*/prop1: number + // /*first*/prop1: number // prop2: boolean // } // function Foo(arg: Props) {} // Foo( { pr/*1*/op1: 10, prop2: true }) - const container = getContainingObjectLiteralElement(node); - if (container) { - const contextualType = typeChecker.getContextualType(node.parent.parent as Expression); - if (contextualType) { - let result: DefinitionInfo[] = []; - forEach(getPropertySymbolsFromContextualType(typeChecker, container), contextualSymbol => { - result = result.concat(getDefinitionFromSymbol(typeChecker, contextualSymbol, node)); - }); + const element = getContainingObjectLiteralElement(node); + if (element) { + if (typeChecker.getContextualType(element.parent as Expression)) { + const result: DefinitionInfo[] = []; + const propertySymbols = getPropertySymbolsFromContextualType(typeChecker, element); + for (const propertySymbol of propertySymbols) { + result.push(...getDefinitionFromSymbol(typeChecker, propertySymbol, node)); + } return result; } } diff --git a/src/services/services.ts b/src/services/services.ts index 227e9a33ed..62673c01c9 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1990,9 +1990,10 @@ namespace ts { } } - function isObjectLiteralPropertyDeclaration(node: Node): node is ObjectLiteralElement { + function isObjectLiteralElement(node: Node): node is ObjectLiteralElement { switch (node.kind) { case SyntaxKind.JsxAttribute: + case SyntaxKind.JsxSpreadAttribute: case SyntaxKind.PropertyAssignment: case SyntaxKind.ShorthandPropertyAssignment: case SyntaxKind.MethodDeclaration: @@ -2003,18 +2004,6 @@ namespace ts { return false; } - function getNameFromObjectLiteralElement(node: ObjectLiteralElement) { - if (node.name.kind === SyntaxKind.ComputedPropertyName) { - const nameExpression = (node.name).expression; - // treat computed property names where expression is string/numeric literal as just string/numeric literal - if (isStringOrNumericLiteral(nameExpression)) { - return (nameExpression).text; - } - return undefined; - } - return (node.name).text; - } - /** * Returns the containing object literal property declaration given a possible name node, e.g. "a" in x = { "a": 1 } */ @@ -2024,11 +2013,11 @@ namespace ts { case SyntaxKind.StringLiteral: case SyntaxKind.NumericLiteral: if (node.parent.kind === SyntaxKind.ComputedPropertyName) { - return isObjectLiteralPropertyDeclaration(node.parent.parent) ? node.parent.parent : undefined; + return isObjectLiteralElement(node.parent.parent) ? node.parent.parent : undefined; } // intentionally fall through case SyntaxKind.Identifier: - return isObjectLiteralPropertyDeclaration(node.parent) && + return isObjectLiteralElement(node.parent) && (node.parent.parent.kind === SyntaxKind.ObjectLiteralExpression || node.parent.parent.kind === SyntaxKind.JsxAttributes) && (node.parent).name === node ? node.parent as ObjectLiteralElement : undefined; } @@ -2037,27 +2026,28 @@ namespace ts { /* @internal */ export function getPropertySymbolsFromContextualType(typeChecker: TypeChecker, node: ObjectLiteralElement): Symbol[] { - const objectLiteral = node.parent; - const contextualType = typeChecker.getContextualType(objectLiteral); - const name = getNameFromObjectLiteralElement(node); - if (name && contextualType) { - const result: Symbol[] = []; - const symbol = contextualType.getProperty(name); - if (symbol) { - result.push(symbol); - } - - if (contextualType.flags & TypeFlags.Union) { - forEach((contextualType).types, t => { - const symbol = t.getProperty(name); - if (symbol) { - result.push(symbol); - } - }); - } + const objectLiteral = node.parent; + const contextualType = typeChecker.getContextualType(objectLiteral); + const name = getTextOfPropertyName(node.name); + if (name && contextualType) { + const result: Symbol[] = []; + const symbol = contextualType.getProperty(name); + if (contextualType.flags & TypeFlags.Union) { + forEach((contextualType).types, t => { + const symbol = t.getProperty(name); + if (symbol) { + result.push(symbol); + } + }); return result; } - return undefined; + + if (symbol) { + result.push(symbol); + return result; + } + } + return undefined; } function isArgumentOfElementAccessExpression(node: Node) { diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 4bf40a9845..3b22fcef95 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -296,7 +296,7 @@ namespace ts.SignatureHelp { // findListItemInfo can return undefined if we are not in parent's argument list // or type argument list. This includes cases where the cursor is: - // - To the right of the closing parenthesize, non-substitution template, or template tail. + // - To the right of the closing parenthesis, non-substitution template, or template tail. // - Between the type arguments and the arguments (greater than token) // - On the target of the call (parent.func) // - On the 'new' keyword in a 'new' expression @@ -358,7 +358,7 @@ namespace ts.SignatureHelp { // This is not guarantee that JSX tag-name is resolved into stateless function component. (that is done in "getSignatureHelpItems") // i.e // export function MainButton(props: ButtonProps, context: any): JSX.Element { ... } - // ; verify.goToDefinition({ - firstTarget: "firstSource", - secondTarget: "firstSource", + firstTarget: "thirdSource", + secondTarget: "thirdSource", thirdTarget: "firstSource", fourthTarget: "firstSource", fivethTarget: "secondSource", - sixthTarget: "firstSource" + sixthTarget: "thirdSource" });