microsoft / TypeScript

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

Report evolving arrays / evolving variables differently in QuickInfo #54414

Open RyanCavanaugh opened 1 year ago

RyanCavanaugh commented 1 year ago

Suggestion

πŸ” Search Terms

evolving array any quickinfo tooltip

βœ… Viability Checklist

My suggestion meets these guidelines:

⭐ Suggestion

When hovering over an evolving array, we should show the type as something special so that people don't think it's actually any and get mad/confused about it

Proposed Instead of

const arr = [];
arr
// ?^ any[]

show

const arr = [];
arr
// ?^ (evolving array)

Same for

let a;

πŸ“ƒ Motivating Example

We have many reports both external and internal of people thinking that an evolving array is of type any. This also appears as bug reports where people think that noImplicitAny has a bug in it because it's not reporting one

Recently https://twitter.com/mattpocockuk/status/1662103198910300161

43752

45369

πŸ’» Use Cases

Prevent confusion

Andarist commented 1 year ago

I feel like this shouldn't be limited to evolving arrays - an auto type~ should get a similar treatment (TS playground):

let foo
//  ^? let foo: any

foo = ''
foo
// ^? let foo: string

if (Math.random()) {
   foo = 4
}

foo
// ^? let foo: string | number
fatcerberus commented 1 year ago

In the past, I've had to tell people "it's not really any[] but a special evolving type" in issues here enough times that I'm 100% behind surfacing this in type hints.

ecyrbe commented 1 year ago

maybe add something more powerfull and allow to declare an evolving type ?

let evolvingArray: evolving[] = [];
//        ^? type evolvingArray = evolving[]
let evolvingAny: evolving;
//        ^? type evolvingAny = evolving
evolvingAny = '';
// ^? type evolvingAny = string

even better with an utility type

let evolvingArray: Evolving[] = []; // it's an utility type where Evolving<T = unknown> by default
//        ^? type evolvingArray = Evolving<unknown>[]
let evolvingAny: Evolving;
//        ^? type evolvingAny = Evolving<unknown>
evolvingAny = '';
// ^? type evolvingAny = Evolving<string> // it's still evolving !
mattpocock commented 1 year ago

Yes please! Also having this terminology (or at least the behaviour) documented somewhere would be great.

fatcerberus commented 1 year ago

@mattpocock It’s not ideal but the release notes are considered part of the documentation. With that in mind, the behavior is documented here: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-1.html#improved-any-inference

Andarist commented 1 year ago

This is surprisingly harder to implement than I have anticipated. The problem is in how things are wired together today.

The best options to handle this I have:

eliasm307 commented 1 year ago

I have recently encountered the confusion around this area and fell into a rabbit hole which led me here. Although its interesting knowing about some of the terminology and how the TS compiler does things under the hood, I wonder if exposing this to users is the best way to reduce confusion.

If you tell users that a type is evolving the first question will likely be "what does that mean?" then they need to find out what TS means by this and in some cases another reaction is "how can I extend/control it?" which can lead to other requests like the previous comment about making evolving type utils etc. Generally, I don't know what extra benefit this will bring users to know this information.

Proposal

What if this stays as an internal implementation detail and the behaviour is just documented as a feature of noImplicitAny e.g. "It will enhance control flow analysis and updates some types based on usage". Then the IDE instead of using any as a placeholder for when a variable's type can evolve, it just shows the type representing what the value of the variable could be at that point which is what users really want to know tbh.

I've made an example of what I mean in this playground and what I think the types should show for it to be more intuitive (at least to me).

An aspect that makes this more confusing is that the evolving types show any or some specific type for the same variable depending on what kind of usage of that variable you are looking at, even if the variable has not been modified (ie the underlying value or possible values of that variable will not have changed). I can understand trying to represent that "anything" can be assigned to the variable, however, this fact is already covered mostly as the quick info shows it as either a let or a const etc variable so anyone with basic JS knowledge should understand its re-assignable ie a let variable shows: image and a const variable shows: image I know this doesn't cover evolving arrays as they can evolve from a const variable (I assume) but I think its less surprising/confusing for a user writing the code to see its type change as they assign different things to an array rather than seeing the array as any sometimes.

Most of the confusion with this is the use of any for the evolving type placeholder as for some it looks like something is wrong (if they dont look at the usages after assignments etc) and I feel just showing the current type (ie the type shown when the evolving type variable is used) at all times would be less confusing. I dont know how difficult this is to do practically however since the logic already exists to determine the current type of an evolving type at a specific point exists, why not just always use this for quick info?

Basically, I think the types should represent how JS works and what the actual value of a variable is or might be at specific points in the code. I'm not sure if adding new TS specific terminology about how it does this etc on top of the JS rules and terminology is beneficial and whether this makes TS easier to pick up for new users.

jakub-g commented 11 months ago

From this and previous tickets, I understand that "evolving arrays" is a feature-not-bug, and there are backward compatibility concerns.

While disabling evolving arrays completely would be unhelpful, and result in additional boilerplate which can be inferred by compiler, I think it would be interesting to have some middle ground.

For example, would it be feasible / interesting to only allow the not type-annotated arrays to accept a single type?

Let's say, a compiler flag like strictEvolvingArrays, which means that the array can only evolve to fit one type:

let ok = []
ok.push(1)
ok.push(2)
// OK: inferred to be number[]

let fail = []
fail.push(1)
fail.push('2')
// ERROR: inferred to be (number|string)[], not allowed by `strictEvolvingArrays`

Would a feature like this make sense? I think it would considerably improve type safety of arrays.

RyanCavanaugh commented 11 months ago

Let's say, a compiler flag like strictEvolvingArrays, which means that the array can only evolve to fit one type:

Once you stare deep enough into the abyss, this becomes nonsensical. Arguably your own example breaks the rule - 1 and 2 are different types, after all. There are also cases to think about like

let ok = []
ok.push(someDiv)
ok.push(someSpan)
ok.push(someHtmlElement)

Is the someSpan push illegal because span and div are different? Seems like it. But both are subtypes of HTMLElement, so the final type we would compute for this array would make the push legal. But then maybe this code becomes legal?

```ts
let ok = []
ok.push(someHtmlElement)
ok.push(someDiv)
ok.push(someSpan)

which doesn't really make sense; the order of the array can't plausibly affect its correctness. Worse, if you remove the ok.push(someHtmlElement), then what? An array with fewer elements has become more heterogenous? That doesn't make sense either; if some array A has a subset of array B's elements, then it's implausible that A has "more type mismatches" than B.