infer from usage's unification uses multiple passes

[sandersn]

Previously, getTypeFromUsageContext of the infer-from-usage codefix would stop as soon an answer was found. Now it 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.

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. 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.

{} 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 inferences to the same parameter will replace the {} with a specific anonymous type. For example:

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

Should produce { x?, y?, z? } | undefined not {} | undefined.

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.

[ghost]

I had to stare at this for a while until I realized isEmptyObjectTypes returns true non-empty non-object types like 42 | {}. And mapType has nothing to do with mapped types, why on earth would I think that?

[ghost]

This can be done by combining inferFromCandidates with inferFromStructure. This also makes checker.mapType unnecessary because we can just keep the candidates as an array instead of creating a union type.

function getTypeFromUsageContext(usageContext: UsageContext, checker: TypeChecker): Type | undefined {
    return inferFromPrimitive(usageContext, checker) ||
        inferFromObject(usageContext, checker) ||
        inferFromStructure(usageContext, checker);
}

function inferFromStructure(usageContext: UsageContext, checker: TypeChecker): Type | undefined {
    let propertiesAreOptional = false;
    const candidates = mapDefined(usageContext.candidateTypes, t => {
        const type = checker.getBaseTypeOfLiteralType(t);
        if (checker.isEmptyObjectType(type)) {
            propertiesAreOptional = true;
            return undefined;
        }
        return type;
    });

    if (usageContext.numberIndexContext) {
        return checker.createArrayType(recur(usageContext.numberIndexContext));
    }
    else if (usageContext.properties || usageContext.callContexts || usageContext.constructContexts || usageContext.stringIndexContext) {
        const optionalFlag = propertiesAreOptional ? SymbolFlags.Optional : 0;
        const members = mapValues(usageContext.properties, (context, name) => {
            const symbol = checker.createSymbol(SymbolFlags.Property | optionalFlag, name);
            symbol.type = recur(context);
            return symbol;
        });
        const callSignatures = map(usageContext.callContexts, callContext => getSignatureFromCallContext(callContext, checker)) || emptyArray;
        const constructSignatures = map(usageContext.constructContexts, constructContext => getSignatureFromCallContext(constructContext, checker)) || emptyArray;
        const stringIndexInfo = usageContext.stringIndexContext && checker.createIndexInfo(recur(usageContext.stringIndexContext), /*isReadonly*/ false);

        const structuralInference = checker.createAnonymousType(/*symbol*/ undefined!, members, callSignatures, constructSignatures, stringIndexInfo, /*numberIndexInfo*/ undefined); // TODO: GH#18217
        return checker.getWidenedType(checker.getUnionType([...candidates, structuralInference]));
    }
    else {
        return candidates.length ? checker.getWidenedType(checker.getUnionType(candidates)) : undefined;
    }

    function recur(innerContext: UsageContext): Type {
        return getTypeFromUsageContext(innerContext, checker) || checker.getAnyType();
    }
}

function mapValues<T, U>(map: ReadonlyUnderscoreEscapedMap<T> | undefined, cb: (t: T, key: __String) => U): UnderscoreEscapedMap<U> {
    if (!map) return emptyMap as any as UnderscoreEscapedMap<U>;
    const res = createUnderscoreEscapedMap<U>();
    map.forEach((value, key) => res.set(key, cb(value, key)));
    return res;
}

[sandersn]

Yeah, I realized nearly the same thing on the walk home. I’ll improve it tomorrow morning.

[sandersn]

I think this is ready to look at again. I updated the PR description to reflect the new commits.

[ghost]

We can avoid some state in usageContext by computing flags at the end:

const inferences = [fallback, ...inferFromPrimitive(k, checker), ...inferFromContext(k, checker), ...inferFromObject(k, checker), ...inferFromStructure(k, checker)];
const hasNonVacuousType = inferences.some(type => !(type.flags & (TypeFlags.Any | TypeFlags.Void)));
const hasNonVacuousNonAnonymousType = inferences.some(type =>
    !(type.flags & (TypeFlags.Nullable | TypeFlags.Any | TypeFlags.Void)) && !(checker.getObjectFlags(type) & ObjectFlags.Anonymous));

[ghost]

Could use some tests of combining different object inferences.

[sandersn]

I added one, and now I'm taking a look at the user tests for more examples.

[sandersn]

That's how I had it, but at one point I had four variables, four calls to some and three parameters to isVacuousType. Things have calmed down since then, so I think your suggestion makes sense. However, I may end up re-making this change if more properties need to be passed around.