Respond to more comments

Arthur Ozga · Arthur Ozga · commit c4ddc5bffad8 · 2017-03-22T12:54:51.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -2191,43 +2191,44 @@ namespace ts {
             return result;
         }
 
-        function createTypeParameterDeclarationFromType(type: TypeParameter, enclosingDeclaration: Node): TypeParameterDeclaration {
+        function typeParameterToDeclaration(type: TypeParameter, enclosingDeclaration?: Node): TypeParameterDeclaration {
             if (!(type && type.symbol && type.flags & TypeFlags.TypeParameter)) {
                 return undefined;
             }
 
             const constraint = typeToTypeNode(getConstraintFromTypeParameter(type), enclosingDeclaration);
             const defaultParameter = typeToTypeNode(getDefaultFromTypeParameter(type), enclosingDeclaration);
 
+            // TODO: use method internal to typeToTypeNode.
             const name = symbolToString(type.symbol);
             return createTypeParameterDeclaration(name, constraint, defaultParameter);
         }
 
-        function createParameterDeclarationFromSymbol(parameterSymbol: Symbol, enclosingDeclaration: Node): ParameterDeclaration {
+        function symbolToParameterDeclaration(parameterSymbol: Symbol, enclosingDeclaration?: Node): ParameterDeclaration {
             const parameterDeclaration = parameterSymbol.declarations[0] as ParameterDeclaration;
             const parameterType = getTypeOfSymbol(parameterSymbol);
             const parameterTypeNode = typeToTypeNode(parameterType, enclosingDeclaration);
             // TODO: how should we clone members/modifiers?
             // TODO: check initializer accessibility correctly.
             const parameterNode = createParameter(
-                parameterDeclaration.decorators && parameterDeclaration.decorators.map(getSynthesizedDeepClone),
-                parameterDeclaration.modifiers && parameterDeclaration.modifiers.map(getSynthesizedDeepClone),
+                parameterDeclaration.decorators,
+                parameterDeclaration.modifiers,
                 parameterDeclaration.dotDotDotToken && createToken(SyntaxKind.DotDotDotToken),
-                getSynthesizedDeepClone(parameterDeclaration.name),
+                parameterDeclaration.name,
                 parameterDeclaration.questionToken && createToken(SyntaxKind.QuestionToken),
                 parameterTypeNode,
-                parameterDeclaration.initializer && getSynthesizedDeepClone(parameterDeclaration.initializer));
+                parameterDeclaration.initializer);
             return parameterNode;
         }
 
-        function signatureToSignatureDeclaration(signature: Signature, kind: SyntaxKind, enclosingDeclaration: Node): SignatureDeclaration {
-            const typeParameters = signature.typeParameters && signature.typeParameters.map(parameter => createTypeParameterDeclarationFromType(parameter, enclosingDeclaration));
-            const parameters = signature.parameters.map(parameter => createParameterDeclarationFromSymbol(parameter, enclosingDeclaration));
-            const type = createTypeNodeExceptAny(getReturnTypeOfSignature(signature));
+        function signatureToSignatureDeclaration(signature: Signature, kind: SyntaxKind, enclosingDeclaration?: Node): SignatureDeclaration {
+            const typeParameters = signature.typeParameters && signature.typeParameters.map(parameter => typeParameterToDeclaration(parameter, enclosingDeclaration));
+            const parameters = signature.parameters.map(parameter => symbolToParameterDeclaration(parameter, enclosingDeclaration));
+            const type = typeToTypeNodeExceptAny(getReturnTypeOfSignature(signature));
             
             return createSignatureDeclaration(kind, typeParameters, parameters, type);
 
-            function createTypeNodeExceptAny(type: Type): TypeNode | undefined {
+            function typeToTypeNodeExceptAny(type: Type): TypeNode | undefined {
                 const typeNode = typeToTypeNode(type, enclosingDeclaration);
                 return typeNode && typeNode.kind !== SyntaxKind.AnyKeyword ? typeNode : undefined;
             }
@@ -2236,17 +2237,17 @@ namespace ts {
 
         // function typeToDisplayParts
 
-        function typeToTypeNode(type: Type, enclosingDeclaration: Node, returnNodeOnError?: boolean): TypeNode {
+        function typeToTypeNode(type: Type, enclosingDeclaration?: Node, returnNodeOnError?: boolean): TypeNode {
             let encounteredError = false;
             let inObjectTypeLiteral = false;
             let checkAlias = true;
             let symbolStack: Symbol[] = undefined;
 
-            const result = createTypeNodeWorker(type);
+            const result = typeToTypeNodeWorker(type);
             // returnNodeOnError = true; // TODO: unset.
             return encounteredError && !returnNodeOnError ? undefined: result;
 
-            function createTypeNodeWorker(type: Type): TypeNode {
+            function typeToTypeNodeWorker(type: Type): TypeNode {
                 if (!type) {
                     encounteredError = true;
                     return undefined;
@@ -2308,7 +2309,7 @@ namespace ts {
 
                 if (objectFlags & ObjectFlags.Reference) {
                     Debug.assert(!!(type.flags & TypeFlags.Object));
-                    return createTypeReferenceNodeFromType(<TypeReference>type);
+                    return typeReferenceToTypeReferenceNode(<TypeReference>type);
                 }
                 if (objectFlags & ObjectFlags.ClassOrInterface) {
                     Debug.assert(!!(type.flags & TypeFlags.Object));
@@ -2347,25 +2348,25 @@ namespace ts {
 
                 if (type.flags & TypeFlags.Index) {
                     const indexedType = (<IndexType>type).type;
-                    const indexTypeNode = createTypeNodeWorker(indexedType);
+                    const indexTypeNode = typeToTypeNodeWorker(indexedType);
                     return createTypeOperatorNode(indexTypeNode);
                 }
                 if (type.flags & TypeFlags.IndexedAccess) {
-                    const objectTypeNode = createTypeNodeWorker((<IndexedAccessType>type).objectType);
-                    const indexTypeNode = createTypeNodeWorker((<IndexedAccessType>type).indexType);
+                    const objectTypeNode = typeToTypeNodeWorker((<IndexedAccessType>type).objectType);
+                    const indexTypeNode = typeToTypeNodeWorker((<IndexedAccessType>type).indexType);
                     return createIndexedAccessTypeNode(objectTypeNode, indexTypeNode);
                 }
 
                 Debug.fail("Should be unreachable.");
 
                 function mapToTypeNodeArray(types: Type[]): NodeArray<TypeNode> {
-                    return types && asNodeArray(types.map(createTypeNodeWorker) as TypeNode[]);
+                    return types && asNodeArray(types.map(typeToTypeNodeWorker) as TypeNode[]);
                 }
 
                 function createMappedTypeNodeFromType(type: MappedType) {
                     Debug.assert(!!(type.flags & TypeFlags.Object));
                     const typeParameter = getTypeParameterFromMappedType(<MappedType>type);
-                    const typeParameterNode = createTypeParameterDeclarationFromType(typeParameter, enclosingDeclaration);
+                    const typeParameterNode = typeParameterToDeclaration(typeParameter, enclosingDeclaration);
 
                     const templateTypeNode = typeToTypeNode(getTemplateTypeFromMappedType(<MappedType>type), enclosingDeclaration);
                     const readonlyToken = (<MappedType>type).declaration && (<MappedType>type).declaration.readonlyToken ? createToken(SyntaxKind.ReadonlyKeyword) : undefined;
@@ -2464,10 +2465,10 @@ namespace ts {
                     }
                 }
 
-                function createTypeReferenceNodeFromType(type: TypeReference) {
+                function typeReferenceToTypeReferenceNode(type: TypeReference) {
                     const typeArguments: Type[] = type.typeArguments || emptyArray;
                     if (type.target === globalArrayType) {
-                        const elementType = createTypeNodeWorker(typeArguments[0]);
+                        const elementType = typeToTypeNodeWorker(typeArguments[0]);
                         return createArrayTypeNode(elementType);
                     }
                     else if (type.target.objectFlags & ObjectFlags.Tuple) {
@@ -2489,21 +2490,20 @@ namespace ts {
                                 // When type parameters are their own type arguments for the whole group (i.e. we have
                                 // the default outer type arguments), we don't show the group.
                                 if (!rangeEquals(outerTypeParameters, typeArguments, start, i)) {
-                                    const name = createNameFromSymbol(parent);
-                                    const qualifiedNamePart = name;
+                                    const qualifiedNamePart = createNameFromSymbol(parent, /*mustBeIdentifier*/ true);
                                     if (!qualifiedName) {
-                                        qualifiedName = createQualifiedName(qualifiedNamePart, /*right*/undefined);
+                                        qualifiedName = createQualifiedName(qualifiedNamePart, /*right*/ undefined);
                                     }
                                     else {
                                         Debug.assert(!qualifiedName.right);
                                         qualifiedName.right = qualifiedNamePart;
-                                        qualifiedName = createQualifiedName(qualifiedName, /*right*/undefined);
+                                        qualifiedName = createQualifiedName(qualifiedName, /*right*/ undefined);
                                     }
                                 }
                             }
                         }
                         let entityName: EntityName = undefined;
-                        const nameIdentifier = createNameFromSymbol(type.symbol);
+                        const nameIdentifier = createNameFromSymbol(type.symbol, /*mustBeIdentifier*/ true);
                         if (qualifiedName) {
                             Debug.assert(!qualifiedName.right);
                             qualifiedName.right = nameIdentifier;
@@ -2544,7 +2544,7 @@ namespace ts {
                         if (!oldDeclaration) {
                             return;
                         }
-                        const propertyName = getSynthesizedDeepClone(oldDeclaration.name);
+                        const propertyName = oldDeclaration.name;
                         const optionalToken = propertySymbol.flags & SymbolFlags.Optional ? createToken(SyntaxKind.QuestionToken) : undefined;;
                         if (propertySymbol.flags & (SymbolFlags.Function | SymbolFlags.Method) && !getPropertiesOfObjectType(propertyType).length) {
                             const signatures = getSignaturesOfType(propertyType, SignatureKind.Call);
@@ -2559,16 +2559,16 @@ namespace ts {
                             typeElements.push(createPropertySignature(
                                 propertyName,
                                 optionalToken,
-                                createTypeNodeWorker(propertyType),
+                                typeToTypeNodeWorker(propertyType),
                                /*initializer*/undefined));
                         }
                     }
                     return typeElements.length ? typeElements : undefined;
                 }
 
-                function createNameFromSymbol(symbol: Symbol): Identifier;
-                function createNameFromSymbol(symbol: Symbol): EntityName;
-                function createNameFromSymbol(symbol: Symbol): EntityName {
+                function createNameFromSymbol(symbol: Symbol, mustBeIdentifier: true): Identifier;
+                function createNameFromSymbol(symbol: Symbol, mustBeIdentifier?: false): EntityName;
+                function createNameFromSymbol(symbol: Symbol, mustBeIdentifier: boolean | undefined): EntityName {
                     let parentSymbol: Symbol;
                     let meaning: SymbolFlags;
 
@@ -2586,8 +2586,12 @@ namespace ts {
                     }
 
                     parentSymbol = undefined;
-                    const result = createEntityNameFromSymbolChain(chain, chain.length - 1);
-                    return result;
+                    if(mustBeIdentifier && chain.length !== 1) {
+                        encounteredError = true;
+                        // TODO: failing to get an identifier when we expect one generates an unprintable node.
+                        // Should error handling be more severe?
+                    }
+                    return createEntityNameFromSymbolChain(chain, chain.length - 1);
 
                     function createEntityNameFromSymbolChain(chain: Symbol[], index: number): EntityName {
                         Debug.assert(chain && 0 <= index && index < chain.length);
diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts
@@ -67,15 +67,6 @@ namespace ts {
         return clone;
     }
 
-    /* @internal */
-    export function getSynthesizedDeepClone<T extends Node>(node: T | undefined): T {
-        if (node === undefined) {
-            return undefined;
-        }
-        const clone = visitEachChild(node, getSynthesizedDeepClone, nullTransformationContext);
-        return clone === node ? getSynthesizedClone(node) : clone;
-    }
-
     // Literals
 
     export function createLiteral(value: string): StringLiteral;
@@ -227,9 +218,7 @@ namespace ts {
 
     // Type Elements
 
-    export function createConstructSignature() {
-        throw new Error("not implemented.");
-    }
+    // TODO: add signatures
 
     // Types
 
@@ -251,7 +240,7 @@ namespace ts {
 
     export function createTypeReferenceNode(typeName: string | EntityName, typeArguments: NodeArray<TypeNode> | undefined) {
         const typeReference = createSynthesizedNode(SyntaxKind.TypeReference) as TypeReferenceNode;
-        typeReference.typeName = isQualifiedName(<EntityName>typeName) ? <QualifiedName>typeName : asName(<string | Identifier>typeName);
+        typeReference.typeName = asName(typeName);
         typeReference.typeArguments = typeArguments;
         return typeReference;
     }
@@ -388,7 +377,7 @@ namespace ts {
             : node;
     }
 
-    // TODO: ask if we should have multiple implementations. Some T's can't have question token.
+    // TODO: Split according to AST nodes.
     export function createSignatureDeclaration<T extends SignatureDeclaration>(kind: SyntaxKind, typeParameters: TypeParameterDeclaration[] | undefined, parameters: ParameterDeclaration[], type: TypeNode | undefined): T;
     export function createSignatureDeclaration<T extends SignatureDeclaration & TypeElement>(kind: SyntaxKind, typeParameters: TypeParameterDeclaration[] | undefined, parameters: ParameterDeclaration[], type: TypeNode | undefined, name: string | PropertyName, questionToken: QuestionToken | undefined): T;
     export function createSignatureDeclaration<T extends SignatureDeclaration & TypeElement>(kind: SyntaxKind, typeParameters: TypeParameterDeclaration[] | undefined, parameters: ParameterDeclaration[], type: TypeNode | undefined, name?: string | PropertyName, questionToken?: QuestionToken): T {
@@ -2030,7 +2019,8 @@ namespace ts {
     function asName(name: string | Identifier): Identifier;
     function asName(name: string | BindingName): BindingName;
     function asName(name: string | PropertyName): PropertyName;
-    function asName(name: string | Identifier | BindingName | PropertyName) {
+    function asName(name: string | EntityName): EntityName;
+    function asName(name: string | Identifier | BindingName | PropertyName | QualifiedName) {
         return typeof name === "string" ? createIdentifier(name) : name;
     }
 
diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts
@@ -31,7 +31,7 @@ namespace ts.codefix {
             return undefined;
         }
 
-        let typeNode: TypeNode = createKeywordTypeNode(SyntaxKind.AnyKeyword);
+        let typeNode: TypeNode;
 
         if (token.parent.parent.kind === SyntaxKind.BinaryExpression) {
             const binaryExpression = token.parent.parent as BinaryExpression;
@@ -41,6 +41,8 @@ namespace ts.codefix {
             typeNode = checker.typeToTypeNode(widenedType, classDeclaration) || typeNode;
         }
 
+        typeNode = typeNode ? typeNode : createKeywordTypeNode(SyntaxKind.AnyKeyword);
+
         const openBrace = getOpenBraceOfClassLike(classDeclaration, sourceFile);
 
         const property = createProperty(
diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts
@@ -62,7 +62,7 @@ namespace ts.codefix {
         }
 
         const declaration = declarations[0] as Declaration;
-        const name = declaration.name ? getSynthesizedDeepClone(declaration.name) as PropertyName : undefined;
+        const name = <PropertyName>declaration.name;
         const visibilityModifier = createVisibilityModifier(getModifierFlags(declaration));
         const modifiers = visibilityModifier ? createNodeArray([visibilityModifier]) : undefined;
         const type = checker.getWidenedType(checker.getTypeOfSymbolAtLocation(symbol, enclosingDeclaration));
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
@@ -275,8 +275,8 @@ namespace ts.textChanges {
 
         /**
          * This function should be used to insert nodes in lists when nodes  don't carry separators as the part of the node range,
-         * i.e. arguments in arguments lists, parameters in parameter lists etc. Statements or class elements are different in sense that 
-         * for them separators are treated as the part of the node.
+         * i.e. arguments in arguments lists, parameters in parameter lists etc.
+         * Separators are treated as part of the node for statements and class elements.
          */
         public insertNodeInListAfter(sourceFile: SourceFile, after: Node, newNode: Node) {
             const containingList = formatting.SmartIndenter.getContainingList(after, sourceFile);
@@ -484,7 +484,7 @@ namespace ts.textChanges {
         private static normalize(changes: Change[]) {
             // order changes by start position
             const normalized = stableSort(changes, (a, b) => a.range.pos - b.range.pos);
-            // verify that end position of the change is less than start position of the next change
+            // verify that change intervals to not overlap, except possible at end points.
             for (let i = 0; i < normalized.length - 2; i++) {
                 Debug.assert(normalized[i].range.end <= normalized[i + 1].range.pos);
             }

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ namespace ts.codefix {`
`31`	`31`	`return undefined;`
`32`	`32`	`}`
`33`	`33`
`34`		`- let typeNode: TypeNode = createKeywordTypeNode(SyntaxKind.AnyKeyword);`
	`34`	`+ let typeNode: TypeNode;`
`35`	`35`
`36`	`36`	`if (token.parent.parent.kind === SyntaxKind.BinaryExpression) {`
`37`	`37`	`const binaryExpression = token.parent.parent as BinaryExpression;`
`@@ -41,6 +41,8 @@ namespace ts.codefix {`
`41`	`41`	`typeNode = checker.typeToTypeNode(widenedType, classDeclaration) \|\| typeNode;`
`42`	`42`	`}`
`43`	`43`
	`44`	`+ typeNode = typeNode ? typeNode : createKeywordTypeNode(SyntaxKind.AnyKeyword);`
	`45`	`+`
`44`	`46`	`const openBrace = getOpenBraceOfClassLike(classDeclaration, sourceFile);`
`45`	`47`
`46`	`48`	`const property = createProperty(`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ namespace ts.codefix {`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`const declaration = declarations[0] as Declaration;`
`65`		`- const name = declaration.name ? getSynthesizedDeepClone(declaration.name) as PropertyName : undefined;`
	`65`	`+ const name = <PropertyName>declaration.name;`
`66`	`66`	`const visibilityModifier = createVisibilityModifier(getModifierFlags(declaration));`
`67`	`67`	`const modifiers = visibilityModifier ? createNodeArray([visibilityModifier]) : undefined;`
`68`	`68`	`const type = checker.getWidenedType(checker.getTypeOfSymbolAtLocation(symbol, enclosingDeclaration));`