microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.29k stars 12.39k forks source link

--noImplicitAny codefixes infer anonymous object types despite appropriate interfaces in (or out of) scope #28991

Open JoshuaKGoldberg opened 5 years ago

JoshuaKGoldberg commented 5 years ago

TypeScript Version: 3.3.0-dev.20181212

Search Terms: noimplicitany infer code fix codefix quick suggested

Code

interface IBox {
    x: number;
    y: number;
}

const shiftBox = (box) => {
    box.x += 1;
    box.y += 1;
}

const box = {
    x: 0,
    y: 0,
};

shiftBox(box);

Expected behavior:

--noImplicitAny suggested fix on the box parameter:

const shiftBox = (box: IBox) =>

Actual behavior:

const shiftBox = (box: { x: any; y: any; }) => {

Related Issues: #13243 (parent tracking quick fixes) and #15114 (discussion on inference difficulties)

If there is an interface available that can satisfy an inferred type in a --noImplicitAny code fix, can we use that? Perhaps with an ordering of possibilities like:

  1. Interfaces already available in the file, by how small they are
  2. User-defined interfaces that could be imported, by how distant the file is
  3. Module types already imported in a user file, by how distant the nearest import is

...where, if multiple interfaces could satisfy the best possibility of those three, we choose the one with the fewest fields?

In code bases that don't explicitly type when unnecessary (e.g. : IBox for variables), I'm finding the --noImplicitAny fixes to be a bit useless for anything other than primitives.

sandersn commented 5 years ago

Non-primitive inference is indeed a known hole in inferFromUsage.

I haven't published a roadmap for the inferFromUsage codefix, but I prototyped a fix for this last year. At least, I defined the way to choose the "closest type". The part I did not do was create a reliable way to find types that were in scope from a particular definition. The closest-type definition also didn't take scopes into account.

JoshuaKGoldberg commented 2 years ago

Coming back to this issue, in general I've observed it to be a common annoyance for folks trying to convert React apps to TypeScript. It blocks them from being able to infer types for parameters in common patterns like this one:

import React from 'react';

export const MyComponent = () => {
    // Expected:
    //   event: React.FormEvent<HTMLFormElement>
    // Actual:
    //   event: { preventDefault: () => void }
    const submitHandler = (event) => {
        event.preventDefault();
    };

    return <form onSubmit={submitHandler} />;
};

@sandersn is that code you prototyped 3/4 years ago still available & something you could post here?

@RyanCavanaugh is there any particular feedback this issue is still waiting on? I don't see any reason to block it from coming in.

sandersn commented 2 years ago

Well, I found this unrelated gem, and I did lots of related prototyping in:

The latter branch is the same as the former but has one more commit, which is very large because I forked most of the services layer for some reason.

That's all I found, though. Those branches are earlier than the experiment I'm thinking of, I'm pretty sure. In theory the only thing that's missing is some way to access the list of types that get created during checking, since the code to determine if a type is "close enough" to an inferred object type already exists.