infer from usage's unification uses multiple passes (#28244)

* infer from usage's unification uses multiple passes

Previously, the unification step of infer-from-usage codefix would stop
as soon an answer was found. Now it continues if the result is
*incomplete*, with the idea that later passes may provide a better
inference.

Currently, an *incomplete* inference is

1. The type any.
2. The empty object type `{}` or a union or intersection that contains
`{}`.

In the checker, any takes priority over other types since it basically
shuts down type checking. For type inference, however, any is one of the least
useful inferences.

`{}` is not a good inference for a similar reason; as a parameter
inference, it doesn't tell the caller much about what is expected, and
it doesn't allow the function author to use an object as expected. But
currently it's inferred whenever there's an initialisation with the
value `{}`. With this change, subsequent property assignments to the
same parameter will replace the `{}` with a specific anonymous type. For
example:

```js
function C(config) {
  if (config === undefined) config = {};
  this.x = config.x;
  this.y = config.y;
  this.z = config.z;
}
```

* Unify all passes of inference from usage

In the previous commit, I changed inference from usage to continue
inference if a the result was *incomplete*. This commit now runs all 4
inference passes and combines them in a unification step. Currently the
unification step is simple, it:

1. Gathers all inferences in a list.
2. Makes properties of anonymous types optional if there is an empty
object in the inference list.
3. Removes *vacuous* inferences.
4. Combines the type in a union.

An inference is *vacuous* if it:

1. Is any or void, when a non-any, non-void type is also inferred.
2. Is the empty object type, when an object type that is not empty is
also inferred.
3. Is an anonymous type, when a non-nullable, non-any, non-void,
non-anonymous type is also inferred.

I think I might eventually want a concept of priorities, like the
compiler's type parameter inference, but I don't have enough examples to
be sure yet.

Eventually, unification should have an additional step that examines the
whole inference list to see if its contents are collectively
meaningless. A good example is `null | undefined`, which is not useful.

* Remove isNumberOrString

* Unify anonymous types

@Andy-MS pointed out that my empty object code was a special case of
merging all anonymous types from an inference and making properties
optional that are not in all the anonymous type. So I did that instead.

* Use getTypeOfSymbolAtLocation instead of Symbol.type!

* Unify parameter call-site inferences too

Because they still have a separate code path, they didn't use the new
unification code.

Also some cleanup from PR comments.

* Add object type unification test

Also remove dead code.

* Only use fallback if no inferences were found

Instead of relying on the unification code to remove the fallback.

Author: sandersn

src/compiler/checker.ts

@@ -301,6 +301,7 @@ namespace ts {
             getESSymbolType: () => esSymbolType,
             getNeverType: () => neverType,
             isSymbolAccessible,
+            getObjectFlags,
             isArrayLikeType,
             isTypeInvalidDueToUnionDiscriminant,
             getAllPossiblePropertiesOfTypes,

src/compiler/types.ts

@@ -3173,6 +3173,8 @@ namespace ts {
         /* @internal */ getTypeCount(): number;
 
         /* @internal */ isArrayLikeType(type: Type): boolean;
+        /* @internal */ getObjectFlags(type: Type): ObjectFlags;
+
         /**
          * True if `contextualType` should not be considered for completions because
          * e.g. it specifies `kind: "a"` and obj has `kind: "b"`.

src/services/codefixes/inferFromUsage.ts

@@ -369,7 +369,9 @@ namespace ts.codefix {
         interface UsageContext {
             isNumber?: boolean;
             isString?: boolean;
-            isNumberOrString?: boolean;
+            hasNonVacuousType?: boolean;
+            hasNonVacuousNonAnonymousType?: boolean;
+
             candidateTypes?: Type[];
             properties?: UnderscoreEscapedMap<UsageContext>;
             callContexts?: CallContext[];
@@ -384,7 +386,7 @@ namespace ts.codefix {
                 cancellationToken.throwIfCancellationRequested();
                 inferTypeFromContext(reference, checker, usageContext);
             }
-            return getTypeFromUsageContext(usageContext, checker) || checker.getAnyType();
+            return unifyFromContext(inferFromContext(usageContext, checker), checker);
         }
 
         export function inferTypeForParametersFromReferences(references: ReadonlyArray<Identifier>, declaration: FunctionLikeDeclaration, program: Program, cancellationToken: CancellationToken): ParameterInference[] | undefined {
@@ -411,6 +413,7 @@ namespace ts.codefix {
                 for (const callContext of callContexts) {
                     if (callContext.argumentTypes.length <= parameterIndex) {
                         isOptional = isInJSFile(declaration);
+                        types.push(checker.getUndefinedType());
                         continue;
                     }
 
@@ -423,14 +426,10 @@ namespace ts.codefix {
                         types.push(checker.getBaseTypeOfLiteralType(callContext.argumentTypes[parameterIndex]));
                     }
                 }
-
-                let type = types.length && checker.getWidenedType(checker.getUnionType(types, UnionReduction.Subtype));
-                if ((!type || type.flags & TypeFlags.Any) && isIdentifier(parameter.name)) {
+                let type = unifyFromContext(types, checker);
+                if (type.flags & TypeFlags.Any && isIdentifier(parameter.name)) {
                     type = inferTypeForVariableFromUsage(parameter.name, program, cancellationToken);
                 }
-                if (!type) {
-                    type = checker.getAnyType();
-                }
                 return {
                     type: isRest ? checker.createArrayType(type) : type,
                     isOptional: isOptional && !isRest,
@@ -504,7 +503,8 @@ namespace ts.codefix {
                     break;
 
                 case SyntaxKind.PlusToken:
-                    usageContext.isNumberOrString = true;
+                    usageContext.isNumber = true;
+                    usageContext.isString = true;
                     break;
 
                 // case SyntaxKind.ExclamationToken:
@@ -575,7 +575,8 @@ namespace ts.codefix {
                         usageContext.isString = true;
                     }
                     else {
-                        usageContext.isNumberOrString = true;
+                        usageContext.isNumber = true;
+                        usageContext.isString = true;
                     }
                     break;
 
@@ -649,7 +650,8 @@ namespace ts.codefix {
 
         function inferTypeFromPropertyElementExpressionContext(parent: ElementAccessExpression, node: Expression, checker: TypeChecker, usageContext: UsageContext): void {
             if (node === parent.argumentExpression) {
-                usageContext.isNumberOrString = true;
+                usageContext.isNumber = true;
+                usageContext.isString = true;
                 return;
             }
             else {
@@ -665,29 +667,83 @@ namespace ts.codefix {
             }
         }
 
-        function getTypeFromUsageContext(usageContext: UsageContext, checker: TypeChecker): Type | undefined {
-            if (usageContext.isNumberOrString && !usageContext.isNumber && !usageContext.isString) {
-                return checker.getUnionType([checker.getNumberType(), checker.getStringType()]);
+        function unifyFromContext(inferences: ReadonlyArray<Type>, checker: TypeChecker, fallback = checker.getAnyType()): Type {
+            if (!inferences.length) return fallback;
+            const hasNonVacuousType = inferences.some(i => !(i.flags & (TypeFlags.Any | TypeFlags.Void)));
+            const hasNonVacuousNonAnonymousType = inferences.some(
+                i => !(i.flags & (TypeFlags.Nullable | TypeFlags.Any | TypeFlags.Void)) && !(checker.getObjectFlags(i) & ObjectFlags.Anonymous));
+            const anons = inferences.filter(i => checker.getObjectFlags(i) & ObjectFlags.Anonymous) as AnonymousType[];
+            const good = [];
+            if (!hasNonVacuousNonAnonymousType && anons.length) {
+                good.push(unifyAnonymousTypes(anons, checker));
             }
-            else if (usageContext.isNumber) {
-                return checker.getNumberType();
+            good.push(...inferences.filter(i => !(checker.getObjectFlags(i) & ObjectFlags.Anonymous) && !(hasNonVacuousType && i.flags & (TypeFlags.Any | TypeFlags.Void))));
+            return checker.getWidenedType(checker.getUnionType(good));
+        }
+
+        function unifyAnonymousTypes(anons: AnonymousType[], checker: TypeChecker) {
+            if (anons.length === 1) {
+                return anons[0];
             }
-            else if (usageContext.isString) {
-                return checker.getStringType();
+            const calls = [];
+            const constructs = [];
+            const stringIndices = [];
+            const numberIndices = [];
+            let stringIndexReadonly = false;
+            let numberIndexReadonly = false;
+            const props = createMultiMap<Type>();
+            for (const anon of anons) {
+                for (const p of checker.getPropertiesOfType(anon)) {
+                    props.add(p.name, checker.getTypeOfSymbolAtLocation(p, p.valueDeclaration));
+                }
+                calls.push(...checker.getSignaturesOfType(anon, SignatureKind.Call));
+                constructs.push(...checker.getSignaturesOfType(anon, SignatureKind.Construct));
+                if (anon.stringIndexInfo) {
+                    stringIndices.push(anon.stringIndexInfo.type);
+                    stringIndexReadonly = stringIndexReadonly || anon.stringIndexInfo.isReadonly;
+                }
+                if (anon.numberIndexInfo) {
+                    numberIndices.push(anon.numberIndexInfo.type);
+                    numberIndexReadonly = numberIndexReadonly || anon.numberIndexInfo.isReadonly;
+                }
             }
-            else if (usageContext.candidateTypes) {
-                return checker.getWidenedType(checker.getUnionType(usageContext.candidateTypes.map(t => checker.getBaseTypeOfLiteralType(t)), UnionReduction.Subtype));
+            const members = mapEntries(props, (name, types) => {
+                const isOptional = types.length < anons.length ? SymbolFlags.Optional : 0;
+                const s = checker.createSymbol(SymbolFlags.Property | isOptional, name as __String);
+                s.type = checker.getUnionType(types);
+                return [name, s];
+            });
+            return checker.createAnonymousType(
+                anons[0].symbol,
+                members as UnderscoreEscapedMap<TransientSymbol>,
+                calls,
+                constructs,
+                stringIndices.length ? checker.createIndexInfo(checker.getUnionType(stringIndices), stringIndexReadonly) : undefined,
+                numberIndices.length ? checker.createIndexInfo(checker.getUnionType(numberIndices), numberIndexReadonly) : undefined);
+        }
+
+        function inferFromContext(usageContext: UsageContext, checker: TypeChecker) {
+            const types = [];
+            if (usageContext.isNumber) {
+                types.push(checker.getNumberType());
             }
-            else if (usageContext.properties && hasCallContext(usageContext.properties.get("then" as __String))) {
+            if (usageContext.isString) {
+                types.push(checker.getStringType());
+            }
+
+            types.push(...(usageContext.candidateTypes || []).map(t => checker.getBaseTypeOfLiteralType(t)));
+
+            if (usageContext.properties && hasCallContext(usageContext.properties.get("then" as __String))) {
                 const paramType = getParameterTypeFromCallContexts(0, usageContext.properties.get("then" as __String)!.callContexts!, /*isRestParameter*/ false, checker)!; // TODO: GH#18217
                 const types = paramType.getCallSignatures().map(c => c.getReturnType());
-                return checker.createPromiseType(types.length ? checker.getUnionType(types, UnionReduction.Subtype) : checker.getAnyType());
+                types.push(checker.createPromiseType(types.length ? checker.getUnionType(types, UnionReduction.Subtype) : checker.getAnyType()));
             }
             else if (usageContext.properties && hasCallContext(usageContext.properties.get("push" as __String))) {
-                return checker.createArrayType(getParameterTypeFromCallContexts(0, usageContext.properties.get("push" as __String)!.callContexts!, /*isRestParameter*/ false, checker)!);
+                types.push(checker.createArrayType(getParameterTypeFromCallContexts(0, usageContext.properties.get("push" as __String)!.callContexts!, /*isRestParameter*/ false, checker)!));
             }
-            else if (usageContext.numberIndexContext) {
-                return checker.createArrayType(recur(usageContext.numberIndexContext));
+
+            if (usageContext.numberIndexContext) {
+                return [checker.createArrayType(recur(usageContext.numberIndexContext))];
             }
             else if (usageContext.properties || usageContext.callContexts || usageContext.constructContexts || usageContext.stringIndexContext) {
                 const members = createUnderscoreEscapedMap<Symbol>();
@@ -719,14 +775,12 @@ namespace ts.codefix {
                     stringIndexInfo = checker.createIndexInfo(recur(usageContext.stringIndexContext), /*isReadonly*/ false);
                 }
 
-                return checker.createAnonymousType(/*symbol*/ undefined!, members, callSignatures, constructSignatures, stringIndexInfo, /*numberIndexInfo*/ undefined); // TODO: GH#18217
-            }
-            else {
-                return undefined;
+                types.push(checker.createAnonymousType(/*symbol*/ undefined!, members, callSignatures, constructSignatures, stringIndexInfo, /*numberIndexInfo*/ undefined)); // TODO: GH#18217
             }
+            return types;
 
             function recur(innerContext: UsageContext): Type {
-                return getTypeFromUsageContext(innerContext, checker) || checker.getAnyType();
+                return unifyFromContext(inferFromContext(innerContext, checker), checker);
             }
         }
 
@@ -759,7 +813,7 @@ namespace ts.codefix {
                 symbol.type = checker.getWidenedType(checker.getBaseTypeOfLiteralType(callContext.argumentTypes[i]));
                 parameters.push(symbol);
             }
-            const returnType = getTypeFromUsageContext(callContext.returnType, checker) || checker.getVoidType();
+            const returnType = unifyFromContext(inferFromContext(callContext.returnType, checker), checker, checker.getVoidType());
             // TODO: GH#18217
             return checker.createSignature(/*declaration*/ undefined!, /*typeParameters*/ undefined, /*thisParameter*/ undefined, parameters, returnType, /*typePredicate*/ undefined, callContext.argumentTypes.length, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false);
         }

tests/cases/fourslash/codeFixInferFromUsageEmptyTypePriority.ts

@@ -0,0 +1,13 @@
+/// <reference path='fourslash.ts' />
+// @strict: true
+// based on acorn, translated to TS
+
+////function TokenType([|label, conf |]) {
+////  if ( conf === void 0 ) conf = {};
+////
+////  var l = label;
+////  var keyword = conf.keyword;
+////  var beforeExpr = !!conf.beforeExpr;
+////};
+
+verify.rangeAfterCodeFix("label: any, conf: { keyword?: any; beforeExpr?: any; } | undefined",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 0);

tests/cases/fourslash/codeFixInferFromUsageMemberJS.ts

@@ -27,10 +27,10 @@ verify.codeFixAll({
     constructor() {
         /**
          * this is fine
-         * @type {undefined}
+         * @type {number[] | undefined}
          */
         this.p = undefined;
-        /** @type {undefined} */
+        /** @type {number[] | undefined} */
         this.q = undefined
     }
     method() {

tests/cases/fourslash/codeFixInferFromUsageUnifyAnonymousType.ts

@@ -0,0 +1,19 @@
+/// <reference path='fourslash.ts' />
+// @strict: true
+// based on acorn, translated to TS
+
+////function kw([|name, options |]) {
+////  if ( options === void 0 ) options = {};
+////
+////  options.keyword = name;
+////  return keywords$1[name] = new TokenType(name, options)
+////}
+////kw("1")
+////kw("2", { startsExpr: true })
+////kw("3", { beforeExpr: false })
+////kw("4", { isLoop: false })
+////kw("5", { beforeExpr: true, startsExpr: true })
+////kw("6", { beforeExpr: true, prefix: true, startsExpr: true })
+
+
+verify.rangeAfterCodeFix("name: string, options: { startsExpr?: boolean; beforeExpr?: boolean; isLoop?: boolean; prefix?: boolean; } | undefined",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 0);