Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infer from usage by similarity to builtin types #33263

Merged
merged 19 commits into from
Sep 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,9 @@ namespace ts {
},
getApparentType,
getUnionType,
isTypeAssignableTo: (source, target) => {
return isTypeAssignableTo(source, target);
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even @internal, some people are going to be ecstatic that this finally found its way into our API surface and will therefore finally be callable by people using the API (even if via cast).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhhhh should I be worried about exposing too much? I feeling like assignability is a State Secret or something.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really interested in the reason why it should be marked as @internal. Don't get me wrong, I'm one of those people ecstatic that this found it's way to the API, now I can ditch my custom typescript version from my library which exposed this. I'm just super curious about the reasoning behind this decision :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effectively this function now has to cross conceptual boundaries in TypeScript. Going from being useful internally only inside checker to being used inside the TSServices, which meant it needed to be exposed as a public API on the checker internally within our codebase,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but why the @internal annotation? It looks like the method is still kinda private?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% convinced that arbitrary uses of this function are safe and won't corrupt compiler state. Marking it @internal allows me to use it inside Typescript where I know how it's being used. Using it outside is an any cast away, but that way it's obvious that it's not technically supported.

createAnonymousType,
createSignature,
createSymbol,
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3289,6 +3289,7 @@ namespace ts {
/* @internal */ getElementTypeOfArrayType(arrayType: Type): Type | undefined;
/* @internal */ createPromiseType(type: Type): Type;

/* @internal */ isTypeAssignableTo(source: Type, target: Type): boolean;
/* @internal */ createAnonymousType(symbol: Symbol, members: SymbolTable, callSignatures: Signature[], constructSignatures: Signature[], stringIndexInfo: IndexInfo | undefined, numberIndexInfo: IndexInfo | undefined): Type;
/* @internal */ createSignature(
declaration: SignatureDeclaration,
Expand Down
314 changes: 222 additions & 92 deletions src/services/codefixes/inferFromUsage.ts

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions tests/cases/fourslash/codeFixInferFromCallInAssignment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function inferAny( [| app |] ) {
//// const result = app.use('hi')
//// return result
//// }

verify.rangeAfterCodeFix("app: { use: (arg0: string) => any; }");
8 changes: 8 additions & 0 deletions tests/cases/fourslash/codeFixInferFromExpressionStatement.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function inferVoid( [| app |] ) {
//// app.use('hi')
//// }

verify.rangeAfterCodeFix("app: { use: (arg0: string) => void; }");
4 changes: 2 additions & 2 deletions tests/cases/fourslash/codeFixInferFromFunctionUsage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

// @noImplicitAny: true
////function wrap( [| arr |] ) {
//// arr.sort(function (a: number, b: number) { return a < b ? -1 : 1 })
//// arr.other(function (a: number, b: number) { return a < b ? -1 : 1 })
//// }

// https://github.com/Microsoft/TypeScript/issues/29330
verify.rangeAfterCodeFix("arr: { sort: (arg0: (a: number, b: number) => 1 | -1) => void; }");
verify.rangeAfterCodeFix("arr: { other: (arg0: (a: number, b: number) => 1 | -1) => void; }");
10 changes: 10 additions & 0 deletions tests/cases/fourslash/codeFixInferFromPrimitiveUsage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function wrap( [| s |] ) {
//// return s.length + s.indexOf('hi')
//// }

// https://github.com/Microsoft/TypeScript/issues/29330
verify.rangeAfterCodeFix("s: string | string[]");

9 changes: 9 additions & 0 deletions tests/cases/fourslash/codeFixInferFromUsageAddition.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function foo([|a, m |]) {
//// return a + m
//// }

verify.rangeAfterCodeFix("a: any, m: any", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);

14 changes: 14 additions & 0 deletions tests/cases/fourslash/codeFixInferFromUsageArray.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function foo([|p, a, b, c, d, e |]) {
//// var x: string = a.pop()
//// b.reverse()
//// var rr: boolean[] = c.reverse()
//// d.some(t => t > 1); // can't infer from callbacks right now
//// var y = e.concat(12); // can't infer from overloaded functions right now
//// return p.push(12)
//// }

verify.rangeAfterCodeFix("p: number[], a: string[], b: any[], c: boolean[], d: any[], e: any[]", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);

2 changes: 1 addition & 1 deletion tests/cases/fourslash/codeFixInferFromUsageCallBodyBoth.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// <reference path='fourslash.ts' />

////class C {
////
//// p = 2
////}
////var c = new C()
////function f([|x, y |]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ verify.codeFix({
index: 0,
newFileContent:
`/**
* @param {(arg0: any) => void} callback
* @param {(arg0: any) => any} callback
*/
function coll(callback /*, name1, name2, ... */) {
return callback(this);
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@
//// }


verify.rangeAfterCodeFix("props: { isLoading: any; update: (arg0: any) => void; }",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 0);
verify.rangeAfterCodeFix("props: { isLoading: any; update: (arg0: any) => any; }",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 0);
10 changes: 10 additions & 0 deletions tests/cases/fourslash/codeFixInferFromUsageLiteralTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function foo([|a, m |]) {
//// a = 'hi'
//// m = 1
//// }

verify.rangeAfterCodeFix("a: string, m: number", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);

9 changes: 9 additions & 0 deletions tests/cases/fourslash/codeFixInferFromUsagePromise.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function foo([|p |]) {
//// return p.then((x: string[]) => x[0])
//// }

verify.rangeAfterCodeFix("p: Promise<string[]>", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);

Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
//// return x.y.z
////}

verify.rangeAfterCodeFix("a: { b: { c: any; }; }, m: { n: () => number; }, x: { y: { z: number[]; }; }", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);
verify.rangeAfterCodeFix("a: { b: { c: void; }; }, m: { n: () => number; }, x: { y: { z: number[]; }; }", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ verify.codeFix({
index: 0,
newFileContent:
`/**
* @param {{ b: { c: any; }; }} a
* @param {{ b: { c: void; }; }} a
* @param {{ n: () => number; }} m
* @param {{ y: { z: number[]; }; }} x
*/
Expand Down
12 changes: 12 additions & 0 deletions tests/cases/fourslash/codeFixInferFromUsageString.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function foo([|p, a, b |]) {
//// var x
//// p.charAt(x)
//// a.charAt(0)
//// b.concat('hi')
//// }

verify.rangeAfterCodeFix("p: string, a: string, b: string | any[]", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);