Implicitly typed parameters of function properties of generic parameters break autocompletion

[yusufkandemir]

Bug Report

🔎 Search Terms

generic parameter autocomplete
extends autocomplete
parameter breaks autocomplete
implicit parameter breaks autocomplete

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about (almost?) everything
• I was unable to test this on prior versions because TS v3.3.3 acts function parameters as implicitly any, while 3.5.1 and higher versions(the ones on the playground) correctly get the type as a string.

⏯ Playground Link

Playground link with relevant code

💻 Code

// (Simplified)
interface StepDefinition {
  title: string;
  enabled: true | ((foo: string) => boolean);
}

// To avoid TS from narrowing the keys to 'string' and definitions to just StepDefinition
function defineSteps<T extends Record<keyof T, StepDefinition>>(steps: T) {
  return steps;
}

const STEPS = defineSteps({
  test: {
    title: 'Test',
    enabled: true,
  },
  bar: {
    title: 'Foo Bar',
    enabled: (foo: string) => foo.length > 0, // When this is enabled, baz: { ... } gets auto-completion
    // enabled: () => 'foo'.length > 0, // When this is enabled, baz: { ... } gets auto-completion
    // enabled: (str) => str.length > 0 // When this is enabled, baz: { ... } does NOT get auto-completion
  },
  //baz: {
    
    // enabled: () => [].concat([ 1 ]).length > 0, // When this is enabled, auto-completion here still works
    // enabled: (foo) => !!foo, // When this is enabled, auto-completion here stops working
  //}
});

// The code below is for context
export type Steps = typeof STEPS;

// The aim is getting the step names and other types while also having IntelliSense when defining the steps
export type StepNames = keyof Steps;
export type ConditionalSteps = {
  [S in keyof Steps]: Steps[S]['enabled'] extends true ? never : S;
}[keyof Steps];

// ...

🙁 Actual behavior

No autocomplete inside defineSteps({ ... }) after defining a function with an implicitly typed parameter in any of enabled properties. TS can't even get the parameter type implicitly on v3.3.3, so my guess is some stuff got improved which helped to infer the type, but there is something slightly wrong under the carpet, which causes this.

🙂 Expected behavior

Getting autocomplete inside defineSteps({ ... }), even when I define a function with an implicitly typed parameter.

[RyanCavanaugh]

Note for future investigators: The magic concept at play here is the "context-sensitive" expression

// enabled: (str) => str.length > 0

which is somehow defeating our logic that gets completions in the face of an incomplete generic call

[Andarist]

There are different angles to this issue. I'm not sure if it's worth fixing this but perhaps a fix to this would improve some other, more compelling, scenarios.

First of all, this signature doesn't make much sense to me. The type parameter is defined as T extends Record<keyof T, StepDefinition> but I can't find any reason to actually use such a constraint. If I'm not mistaken this is essentially the same as T extends Record<PropertyKey, StepDefinition> and that works just OK when it comes to property completions within that incomplete object.

While investigating this case we can learn that, in a case without context-sensitive functions, completions are provided based on the signature computed by getCandidateForOverloadFailure. The computed signature is (steps: Record<"test" | "bar" | "baz", StepDefinition>): Record<"test" | "bar" | "baz", StepDefinition>.

So why do context-sensitive functions break this? They make all objects that contain them (recursively, up to the root object) non-inferrable. So when getCandidateForOverloadFailure gets called no inference candidate can be gathered for T. This makes getInferredType return the T's constraint while instantiating T within it to... unknown. keyof unknown is just never so the whole instantiatedConstraint is Record<never, StepDefinition> and the returned candidate for the overload failure is (steps: Record<never, StepDefinition>): Record<never, StepDefinition>.

That is later used to obtain the getApparentTypeOfContextualType for the object in which we want to get completions and getTypeOfPropertyOfContextualType just can't return anything useful. It's essentially trying to do Record<never, StepDefinition>["baz"] and that's an invalid indexed access.

So what could potentially be done about this?

• how non-inferrable types could be reworked, it's essentially an implementation detail and this could be changed at anytime. I don't think that computing Record<never, StepDefinition> here for the constraint is the best that can be done. It's rather tricky to improve the situation this way though and that's definitely over my head
• when requesting completions you could allow this indexed access - the worst case scenario is that some completions would get returned to the user when they shouldn't. That's rather unlikely but either way - it's usually better to give some completions than none.
• typeChecker.getContextualType (maybe only when called with ContextFlags.Completions) could also, somehow, collect contextual types used within checkExpressionWithContextualType that happens while choosing regular overloads. The current problem is that the contextual type is obtained from the resolved signature that failed to resolve correctly. I'm not sure how that would actually play out with overloads and stuff but potentially it actually would be the improvement because right now overloads are sort of ignored in completions requests that use typeChecker.getContextualType. Some completions requests (for "direct" arguments) are using checker.getResolvedSignatureForStringLiteralCompletions today and that call doesn't ignore~ overloads. So in a sense, there is already a discrepancy between what completions might be returned, and it's solely based on the positions within the function's argument.

[Andarist]

I'm thinking about this more and more and I have a hunch that there might be something in that third idea. I imagine that each checkExpressionWithContextualType in inferTypeArguments within a completion request could create a new stack of contextual types and pushCachedContextualType could push into it (without popping). Then completions request could query all of those stacks (1 stack per 1 explored overload), find the closest contextual type there, and compute its own contextual type based on the object path from that closes contextual type to the node that the completion request is interested in.

cc @andrewbranch maybe you'd be interested in this... am I completely delusional here or does this sound plausible? 😅

[LukeAbby]

I believe @Conviley ran into the same issue. This is my (mostly) minimal reproduction of their issue:

declare function makeRequest<
  QueryParam
>(
  getFn: (client: { prop: number }) => QueryParam,
  params: NoInfer<QueryParam>,
): void;

// Hovering over `makeRequest` shows that `QueryParam` is inferred as `unknown` and `params` is obstensibly typed as `unknown` which is incongruous with this call is an error.
makeRequest(
    (client) => client,
    // Using autocomplete here suggests no property keys.
    {},
// ^ Argument of type '{}' is not assignable to parameter of type '{ prop: number; }'.
);

makeRequest(
    (client: { prop: number }) => client,
    // Using autocomplete here now suggests keys.
    {},
);

@Andarist maybe you have a better understanding of whether internally this is the same thing or not?

[Andarist]

My intuition is this is different - even if only because this issue focuses so much on multi-step~ thing within a single argument object. That's usually a tell-tale that the issue is about intra-expression inferences. I think it might be connected to your issue - but fixing yours might be way simpler than fixing this one. At least that's my intuition.

What happens here is that it fails to infer a correct signature here so it goes for getCandidateForOverloadFailure and then for inferSignatureInstantiationForOverloadFailure. That, in turn, infers type arguments again with checkMode | CheckMode.SkipContextSensitive | CheckMode.SkipGenericFunctions. This example has a context-sensitive function at the first position and it can't infer from its resolved return type because that happens in the "second inference pass" but that's prevented altogether by CheckMode.SkipContextSensitive.

I already played with removing this flag from there on like 2 occasions. While I don't remember exactly why I gave up... I do remember that it didn't break that much in the existing test suite. There were some hurdles there though and it didn't seem important enough to be bothered with it. I think I was mainly looking into fixing this weird discrepancy between the reported error and the inferred result that we can see here (unknown).

Fixing this for completions would be rather easy because the compiler could behave differently when resolving signatures for that - but fundamentally it would be better to try fixing the above again.

[mirobo]

I wonder if the issue described here is the same "issue" as the following (I don't think it is.. but it seems just a bit confusing).

When I explicitly add a type to the return function, then type checking works as I'd like. But when the type is defined after the variable definition, then TS only checks if the type is compatible. I'd also like to know whether the inferred type from the anonymous function has some properties, that don't exist in the type FooBar.

type Signal<T> = (() => T);

interface FooBar {
    foo: string;
}


const typeCheckWithObject: FooBar = {
    foo: 'bar',
    // Good: Error is shown (Object literal may only specify known properties, and 'nonExistingProperty' does not exist in type 'FooBar'.)
    nonExistingProperty: ''
};

const typeCheckWithExplicitFunctionReturnType = ((): FooBar => ({
    foo: 'bar',
    // Good: same error is shown
    nonExistingProperty: ''
}))();

const typeCheckWithFunctionCall: FooBar = (() => ({
    foo: 'bar',
    // Not ideal: No error is shown, because types are compatible
    nonExistingProperty: ''
}))();

[Andarist]

This is a different and it's a duplicate of #59586