For completions of union, exclude types with methods (microsoft#18124)

Andy · web-flow · commit 89eb06e47534 · 2017-09-14T12:37:38.000-07:00
For completions of union, exclude arrays
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -225,7 +225,8 @@ namespace ts {
                 return tryFindAmbientModule(moduleName, /*withAugmentations*/ false);
             },
             getApparentType,
-            getAllPossiblePropertiesOfType,
+            isArrayLikeType,
+            getAllPossiblePropertiesOfTypes,
             getSuggestionForNonexistentProperty: (node, type) => unescapeLeadingUnderscores(getSuggestionForNonexistentProperty(node, type)),
             getSuggestionForNonexistentSymbol: (location, name, meaning) => unescapeLeadingUnderscores(getSuggestionForNonexistentSymbol(location, escapeLeadingUnderscores(name), meaning)),
             getBaseConstraintOfType,
@@ -5925,25 +5926,21 @@ namespace ts {
                 getPropertiesOfObjectType(type);
         }
 
-        function getAllPossiblePropertiesOfType(type: Type): Symbol[] {
-            if (type.flags & TypeFlags.Union) {
-                const props = createSymbolTable();
-                for (const memberType of (type as UnionType).types) {
-                    if (memberType.flags & TypeFlags.Primitive) {
-                        continue;
-                    }
+        function getAllPossiblePropertiesOfTypes(types: Type[]): Symbol[] {
+            const unionType = getUnionType(types);
+            if (!(unionType.flags & TypeFlags.Union)) {
+                return getPropertiesOfType(unionType);
+            }
 
-                    for (const { escapedName } of getPropertiesOfType(memberType)) {
-                        if (!props.has(escapedName)) {
-                            props.set(escapedName, createUnionOrIntersectionProperty(type as UnionType, escapedName));
-                        }
+            const props = createSymbolTable();
+            for (const memberType of types) {
+                for (const { escapedName } of getPropertiesOfType(memberType)) {
+                    if (!props.has(escapedName)) {
+                        props.set(escapedName, createUnionOrIntersectionProperty(unionType as UnionType, escapedName));
                     }
                 }
-                return arrayFrom(props.values());
-            }
-            else {
-                return getPropertiesOfType(type);
             }
+            return arrayFrom(props.values());
         }
 
         function getConstraintOfType(type: TypeVariable | UnionOrIntersectionType): Type {
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -2703,7 +2703,8 @@ namespace ts {
          * So for `{ a } | { b }`, this will include both `a` and `b`.
          * Does not include properties of primitive types.
          */
-        /* @internal */ getAllPossiblePropertiesOfType(type: Type): Symbol[];
+        /* @internal */ isArrayLikeType(type: Type): boolean;
+        /* @internal */ getAllPossiblePropertiesOfTypes(type: ReadonlyArray<Type>): Symbol[];
         /* @internal */ resolveName(name: string, location: Node, meaning: SymbolFlags): Symbol | undefined;
         /* @internal */ getJsxNamespace(): string;
     }
diff --git a/src/services/completions.ts b/src/services/completions.ts
@@ -978,7 +978,7 @@ namespace ts.Completions {
                 isNewIdentifierLocation = true;
                 const typeForObject = typeChecker.getContextualType(<ObjectLiteralExpression>objectLikeContainer);
                 if (!typeForObject) return false;
-                typeMembers = typeChecker.getAllPossiblePropertiesOfType(typeForObject);
+                typeMembers = getPropertiesForCompletion(typeForObject, typeChecker);
                 existingMembers = (<ObjectLiteralExpression>objectLikeContainer).properties;
             }
             else {
@@ -1766,4 +1766,20 @@ namespace ts.Completions {
                 return node.parent;
         }
     }
+
+    /**
+     * Gets all properties on a type, but if that type is a union of several types,
+     * tries to only include those types which declare properties, not methods.
+     * This ensures that we don't try providing completions for all the methods on e.g. Array.
+     */
+    function getPropertiesForCompletion(type: Type, checker: TypeChecker): Symbol[] {
+        if (!(type.flags & TypeFlags.Union)) {
+            return checker.getPropertiesOfType(type);
+        }
+
+        const { types } = type as UnionType;
+        const filteredTypes = types.filter(memberType => !(memberType.flags & TypeFlags.Primitive || checker.isArrayLikeType(memberType)));
+        // If there are no property-only types, just provide completions for every type as usual.
+        return checker.getAllPossiblePropertiesOfTypes(filteredTypes);
+    }
 }
diff --git a/tests/cases/fourslash/completionListOfUnion.ts b/tests/cases/fourslash/completionListOfUnion.ts
@@ -16,5 +16,5 @@ verify.completionListContains("b", "(property) b: number | boolean");
 verify.completionListContains("c", "(property) c: string");
 
 goTo.marker("f");
-verify.completionListContains("a", "(property) a: number");
+verify.completionListContains("a", "(property) I.a: number");
 // Also contains array members
diff --git a/tests/cases/fourslash/completionsUnion.ts b/tests/cases/fourslash/completionsUnion.ts
@@ -0,0 +1,8 @@
+/// <reference path="fourslash.ts" />
+
+////interface I { x: number; }
+////interface Many<T> extends ReadonlyArray<T> { extra: number; }
+////const x: I | I[] | Many<string> = { /**/ };
+
+// We specifically filter out any array-like types.
+verify.completionsAt("", ["x"]);
