Automated Migration for Breaking Changes to the Type System

mheiber commented 5 years ago

Search Terms

codemod, migration, upgrade

Suggestion

semi-automated migration when there are breaking changes in the type system

One way of implementing this would be via codemods (could use the TS Compiler API) that do trivial update tasks as described in https://github.com/microsoft/TypeScript/issues/33272

Use Cases

Would make it easier for users to upgrade to the latest TypeScript
Would make it easier for the TS team to upgrade TypeScript
Might enable TS to evolve faster, as the pain from these "breaking" changes will be smaller

The changes described in https://github.com/microsoft/TypeScript/issues/33272, for example, were mostly trivial, and ideally would not need much human intervention.

Type-only changes are especially amenable to this kind of treatment, because no one's app will misbehave at runtime if there is a bug in the transformation.

I say 'semi-automated' because in cases where the tool can't figure out the problem, the tool could yield to human judgment. Tooling for this doesn't have to be perfect, but a small amount of automation could go a long way.

Examples

example.ts

const myPromise: Promise<{}> = dontCarePromise();

tsc --upgrade --from=3.5 --to=3.6 example.ts

const myPromise: Promise<unknown> = dontCarePromise();

The example is from https://github.com/microsoft/TypeScript/issues/33272.

In a case where the conversion would lead to a new type error, I'd expect the tool to bail out, explain the situation, and let the human decide what to do.

Checklist

My suggestion meets these guidelines:

[x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
[x] This wouldn't change the runtime behavior of existing JavaScript code
[x] This could be implemented without emitting different JS based on the types of the expressions
[x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
[x] This feature would agree with the rest of TypeScript's Design Goals.

orta commented 5 years ago

This is cool, I'd also like to see the same on a per-flag basis, e.g. tsc --upgrade-flag strictFunctionTypes example.ts

weswigham commented 5 years ago

If we think it's possible and useful, we do build quickfixes for the editor for these kinds of things (though we still don't have a tool to run fixes from the command line @DanielRosenwasser ). We just don't always bother - the unknown default change only needed around 20 packages on DT updated (each in one or two places, and actually pointing out a bug of unhandled null/undefined in a few cases), so didn't seem all that bad.

jack-williams commented 5 years ago

To throw this out there: I think #13195 would be something that needs automated migration.

Though, there are multiple other blocking features associated with that issue.

mheiber commented 5 years ago

Should I close this issue, given that this sort of thing is in the works already per Wes' comment? https://github.com/microsoft/TypeScript/issues/33345#issuecomment-530123819

weswigham commented 5 years ago

I mean, I'd leave that to @DanielRosenwasser, as he's been looking for a reason to build out the quickfixes-on-the-commandline feature for forever.

RyanCavanaugh commented 5 years ago

Broadly speaking there are two distinct categories of things to consider here:

Breaks for which the root cause is tractably identifiable by the type system (Type I)
Breaks for which the root cause is not tractably identifiable by the type system (Type II)

An example Type I break would be strictClassPropertyInitialization - by the construction of the error itself, it's trivial for TS to identify that "I am issuing an error because of a rule that was added in a breaking manner".

An example Type II break would be the Set constructor change identified in #33272 - this was caused by a subtle change in inference rules, and the exact error occurring later in the code isn't something TS could plausibly trace back to a change in behavior.

We can create Quick Fixes for Type I errors, and typically do. These also tend to be very well-documented and well-thought-out changes because they're typically the "We are breaking you on purpose" kind of breaking change. Many Type I breaks are also associated with a commandline flag to turn them off.

For a Type II error, even if we could figure out what happened, often there's no "quick fix" because it depends on what the code was trying to do in the first place, and the change that does need to occur might be very lexically far away from where the first actual type error is issued.

We can provide tooling all day long for Type I breaks, but I would argue that this is a bit of a low-value activity. These breaks are often not particularly painful, are signaled well in advance, and usually have a simple commandline fix or can be bulk-addressed by a sufficiently clever RegExp. Adopting something like Facebook's codemod tool for offering a CLI experience for these fixes is something we might look in to.

Type II breaks are vexing. Often we don't even really know we're making them - the filter(Boolean) break again from #33272 is an example. And even if we did fully realize this was going to be a break, it's not clear what we'd do about it. There's no obvious quickfix to issue for "This expression is going to change to an arguably more-accurate type in a way that might break downstream code that makes certain other assumptions".

For example, let's say you wrote some code

const arr = [1, 2, 3, "four", "five"];
const arr2 = arr.filter(x => +x === x);
arr2.push("six");

Maybe in the future, TS gets smarter and says +x === x is a type-guarding expression for x is number, and also gets smarter by identifying that array.filter(type guard function) returns a more-specific array type. At first glance, this isn't a breaking change... but it is: the arr2.push("ok") line is now an error because arr.filter returned number[] instead of (string | number)[].

In even the most trivial example, the error is very causally disconnected from the original change, and it's not even clear what the right "fix" to issue is.

What it seems like you want for Type II errors is just a tool that runs tsc, takes the errors, and inserts either assertions or // @ts-ignore comments depending. For example, I'd probably want one of two outputs depending on error density:

const arr = [1, 2, 3, "four", "five"];
const arr2 = arr.filter(x => +x === x);
// @ts-ignore Error 'cannot convert "six" to 'number'" introduced in TS 4.2
arr2.push("six");

vs

const arr = [1, 2, 3, "four", "five"];
// TODO: Multiple errors involving 'arr2' introduced in TS 4.2
const arr2: any = arr.filter(x => +x === x);
arr2.push("six");
arr2.push("seven");
arr2.push("eight");

DanielRosenwasser commented 5 years ago

We can provide tooling all day long for Type I breaks, but I would argue that this is a bit of a low-value activity. These breaks are often not particularly painful, are signaled well in advance, and usually have a simple commandline fix or can be bulk-addressed by a sufficiently clever RegExp.

I feel that quick fixes, more often than not, are often fairly low-effort and help educate users more about the breaks themselves and how to fix them. So I don't believe it's low-value, and even if it is, the effort isn't unreasonable in the first place.

microsoft / TypeScript