From ddd7f244c13e008dbb8a1c2c3b73431e4fb8941d Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 5 Jan 2020 18:38:33 -0500 Subject: [PATCH 1/5] Prefer a likely literal over anonymous type in --noImplicitAny codefixes Before trying to make an anonymous type for a type's usage, we'll first check if there is exactly one builtin primitive the usage is assignable to, and use it if so. Right now that's only `number` and `string` because `boolean` has no distinguishable members. A couple of implementation details: * `tryInsertTypeAnnotation` needed to know to insert a type _after_ a node's `exclamationToken` if it exists * This code area was written before `??` :wink: --- src/services/codefixes/inferFromUsage.ts | 25 +++++++++++++------ src/services/textChanges.ts | 2 +- .../codeFixInferFromUsageParameterLiteral.ts | 10 ++++++++ .../codeFixInferFromUsageVariableLiteral.ts | 9 +++++++ 4 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 tests/cases/fourslash/codeFixInferFromUsageParameterLiteral.ts create mode 100644 tests/cases/fourslash/codeFixInferFromUsageVariableLiteral.ts diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 5718f592cb82a..3d756f55579e3 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -457,9 +457,12 @@ namespace ts.codefix { Array: t => checker.createArrayType(t), Promise: t => checker.createPromiseType(t), }; - const builtins = [ + const builtinPrimitives = [ checker.getStringType(), checker.getNumberType(), + ]; + const builtins = [ + ...builtinPrimitives, checker.createArrayType(checker.getAnyType()), checker.createPromiseType(checker.getAnyType()), ]; @@ -967,11 +970,8 @@ namespace ts.codefix { if (usage.numberIndex) { types.push(checker.createArrayType(combineFromUsage(usage.numberIndex))); } - if (usage.properties && usage.properties.size - || usage.calls && usage.calls.length - || usage.constructs && usage.constructs.length - || usage.stringIndex) { - types.push(inferStructuralType(usage)); + if (usage.properties?.size || usage.calls?.length || usage.constructs?.length || usage.stringIndex) { + types.push(inferLiteralOrStructuralType(usage)); } types.push(...(usage.candidateTypes || []).map(t => checker.getBaseTypeOfLiteralType(t))); @@ -980,7 +980,12 @@ namespace ts.codefix { return types; } - function inferStructuralType(usage: Usage) { + function inferLiteralOrStructuralType(usage: Usage) { + const matchingBuiltins = builtinPrimitives.filter(builtin => allPropertiesAreAssignableToUsage(builtin, usage)); + if (matchingBuiltins.length === 1) { + return matchingBuiltins[0]; + } + const members = createUnderscoreEscapedMap(); if (usage.properties) { usage.properties.forEach((u, name) => { @@ -1016,7 +1021,11 @@ namespace ts.codefix { return !sigs.length || !checker.isTypeAssignableTo(source, getFunctionFromCalls(propUsage.calls)); } else { - return !checker.isTypeAssignableTo(source, combineFromUsage(propUsage)); + const combinedPropUsage = combineFromUsage(propUsage); + if (combinedPropUsage.flags & TypeFlags.Void) { + return false; + } + return !checker.isTypeAssignableTo(source, combinedPropUsage); } }); } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 07a75d884d9a2..c23281e0ff9a7 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -406,7 +406,7 @@ namespace ts.textChanges { } } else { - endNode = node.kind !== SyntaxKind.VariableDeclaration && node.questionToken ? node.questionToken : node.name; + endNode = (node.kind === SyntaxKind.VariableDeclaration ? node.exclamationToken : node.questionToken) ?? node.name; } this.insertNodeAt(sourceFile, endNode.end, type, { prefix: ": " }); diff --git a/tests/cases/fourslash/codeFixInferFromUsageParameterLiteral.ts b/tests/cases/fourslash/codeFixInferFromUsageParameterLiteral.ts new file mode 100644 index 0000000000000..1661063598299 --- /dev/null +++ b/tests/cases/fourslash/codeFixInferFromUsageParameterLiteral.ts @@ -0,0 +1,10 @@ +/// + +// @noImplicitAny: true +//// function foo([|text |]) { +//// text.length; +//// text.indexOf("z"); +//// text.charAt(0); +//// } + +verify.rangeAfterCodeFix("text: string", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0); diff --git a/tests/cases/fourslash/codeFixInferFromUsageVariableLiteral.ts b/tests/cases/fourslash/codeFixInferFromUsageVariableLiteral.ts new file mode 100644 index 0000000000000..2641b00963601 --- /dev/null +++ b/tests/cases/fourslash/codeFixInferFromUsageVariableLiteral.ts @@ -0,0 +1,9 @@ +/// + +// @noImplicitAny: true +//// let [|text! |]; +//// text.length; +//// text.indexOf("z"); +//// text.charAt(0); + +verify.rangeAfterCodeFix("text!: string", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0); From e33cc595578fb51ac402d1483130be410eb8cc21 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 19 Apr 2020 02:54:28 -0400 Subject: [PATCH 2/5] Used unknown/any instead of void when applicable --- src/compiler/checker.ts | 1 + src/compiler/types.ts | 1 + src/services/codefixes/inferFromUsage.ts | 30 +++++++------------ .../codeFixInferFromUsagePropertyAccess.ts | 2 +- .../codeFixInferFromUsagePropertyAccessJS.ts | 4 +-- 5 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 065fc617857c1..94f3921993f80 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -544,6 +544,7 @@ namespace ts { createSymbol, createIndexInfo, getAnyType: () => anyType, + getUnknownType: () => unknownType, getStringType: () => stringType, getNumberType: () => numberType, createPromiseType, diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 12aae7b465cc8..fba6a3929354c 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3491,6 +3491,7 @@ namespace ts { getDefaultFromTypeParameter(type: Type): Type | undefined; /* @internal */ getAnyType(): Type; + /* @internal */ getUnknownType(): Type; /* @internal */ getStringType(): Type; /* @internal */ getNumberType(): Type; /* @internal */ getBooleanType(): Type; diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 3d756f55579e3..2ef76822328f0 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -457,12 +457,9 @@ namespace ts.codefix { Array: t => checker.createArrayType(t), Promise: t => checker.createPromiseType(t), }; - const builtinPrimitives = [ + const builtins = [ checker.getStringType(), checker.getNumberType(), - ]; - const builtins = [ - ...builtinPrimitives, checker.createArrayType(checker.getAnyType()), checker.createPromiseType(checker.getAnyType()), ]; @@ -613,7 +610,7 @@ namespace ts.codefix { switch (node.parent.kind) { case SyntaxKind.ExpressionStatement: - addCandidateType(usage, checker.getVoidType()); + inferTypeFromExpressionStatement(node, usage); break; case SyntaxKind.PostfixUnaryExpression: usage.isNumber = true; @@ -671,6 +668,10 @@ namespace ts.codefix { } } + function inferTypeFromExpressionStatement(node: Expression, usage: Usage): void { + addCandidateType(usage, isPropertyAccessExpression(node) ? checker.getAnyType() : checker.getVoidType()); + } + function inferTypeFromPrefixUnaryExpression(node: PrefixUnaryExpression, usage: Usage): void { switch (node.operator) { case SyntaxKind.PlusPlusToken: @@ -898,8 +899,8 @@ namespace ts.codefix { low: t => t === stringNumber }, { - high: t => !(t.flags & (TypeFlags.Any | TypeFlags.Void)), - low: t => !!(t.flags & (TypeFlags.Any | TypeFlags.Void)) + high: t => !(t.flags & (TypeFlags.Any | TypeFlags.Unknown | TypeFlags.Void)), + low: t => !!(t.flags & (TypeFlags.Any | TypeFlags.Unknown | TypeFlags.Void)) }, { high: t => !(t.flags & (TypeFlags.Nullable | TypeFlags.Any | TypeFlags.Void)) && !(getObjectFlags(t) & ObjectFlags.Anonymous), @@ -971,7 +972,7 @@ namespace ts.codefix { types.push(checker.createArrayType(combineFromUsage(usage.numberIndex))); } if (usage.properties?.size || usage.calls?.length || usage.constructs?.length || usage.stringIndex) { - types.push(inferLiteralOrStructuralType(usage)); + types.push(inferStructuralType(usage)); } types.push(...(usage.candidateTypes || []).map(t => checker.getBaseTypeOfLiteralType(t))); @@ -980,12 +981,7 @@ namespace ts.codefix { return types; } - function inferLiteralOrStructuralType(usage: Usage) { - const matchingBuiltins = builtinPrimitives.filter(builtin => allPropertiesAreAssignableToUsage(builtin, usage)); - if (matchingBuiltins.length === 1) { - return matchingBuiltins[0]; - } - + function inferStructuralType(usage: Usage) { const members = createUnderscoreEscapedMap(); if (usage.properties) { usage.properties.forEach((u, name) => { @@ -1021,11 +1017,7 @@ namespace ts.codefix { return !sigs.length || !checker.isTypeAssignableTo(source, getFunctionFromCalls(propUsage.calls)); } else { - const combinedPropUsage = combineFromUsage(propUsage); - if (combinedPropUsage.flags & TypeFlags.Void) { - return false; - } - return !checker.isTypeAssignableTo(source, combinedPropUsage); + return !checker.isTypeAssignableTo(source, combineFromUsage(propUsage)); } }); } diff --git a/tests/cases/fourslash/codeFixInferFromUsagePropertyAccess.ts b/tests/cases/fourslash/codeFixInferFromUsagePropertyAccess.ts index 44d2e2d7b1c25..41112b2409eff 100644 --- a/tests/cases/fourslash/codeFixInferFromUsagePropertyAccess.ts +++ b/tests/cases/fourslash/codeFixInferFromUsagePropertyAccess.ts @@ -12,4 +12,4 @@ //// return x.y.z ////} -verify.rangeAfterCodeFix("a: { b: { c: void; }; }, m: { n: () => number; }, x: { y: { z: number[]; }; }", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0); +verify.rangeAfterCodeFix("a: { b: { c: any; }; }, m: { n: () => number; }, x: { y: { z: number[]; }; }", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0); diff --git a/tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts b/tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts index 357b1a991ba83..0322c78b4be54 100644 --- a/tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts +++ b/tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts @@ -20,8 +20,8 @@ verify.codeFix({ description: "Infer parameter types from usage", index: 0, newFileContent: -`/** - * @param {{ b: { c: void; }; }} a + `/** + * @param {{ b: { c: any; }; }} a * @param {{ n: () => number; }} m * @param {{ y: { z: number[]; }; }} x */ From 76fd88af49af2cb381fd2b5d2fdea7eb24c76443 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 19 Apr 2020 03:13:07 -0400 Subject: [PATCH 3/5] Fix little whitespace change in tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts --- tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts b/tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts index 0322c78b4be54..ed9d4b33b377c 100644 --- a/tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts +++ b/tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts @@ -20,7 +20,7 @@ verify.codeFix({ description: "Infer parameter types from usage", index: 0, newFileContent: - `/** +`/** * @param {{ b: { c: any; }; }} a * @param {{ n: () => number; }} m * @param {{ y: { z: number[]; }; }} x From 18fdff5fc0b4c21e392b8fb3ca3b0e548fd65149 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 19 Apr 2020 11:49:58 -0400 Subject: [PATCH 4/5] Undid some now-unnecessary unknown additions --- src/compiler/checker.ts | 1 - src/compiler/types.ts | 1 - src/services/codefixes/inferFromUsage.ts | 4 ++-- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 94f3921993f80..065fc617857c1 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -544,7 +544,6 @@ namespace ts { createSymbol, createIndexInfo, getAnyType: () => anyType, - getUnknownType: () => unknownType, getStringType: () => stringType, getNumberType: () => numberType, createPromiseType, diff --git a/src/compiler/types.ts b/src/compiler/types.ts index fba6a3929354c..12aae7b465cc8 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3491,7 +3491,6 @@ namespace ts { getDefaultFromTypeParameter(type: Type): Type | undefined; /* @internal */ getAnyType(): Type; - /* @internal */ getUnknownType(): Type; /* @internal */ getStringType(): Type; /* @internal */ getNumberType(): Type; /* @internal */ getBooleanType(): Type; diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 2ef76822328f0..1ace3a58107c0 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -899,8 +899,8 @@ namespace ts.codefix { low: t => t === stringNumber }, { - high: t => !(t.flags & (TypeFlags.Any | TypeFlags.Unknown | TypeFlags.Void)), - low: t => !!(t.flags & (TypeFlags.Any | TypeFlags.Unknown | TypeFlags.Void)) + high: t => !(t.flags & (TypeFlags.Any | TypeFlags.Void)), + low: t => !!(t.flags & (TypeFlags.Any | TypeFlags.Void)) }, { high: t => !(t.flags & (TypeFlags.Nullable | TypeFlags.Any | TypeFlags.Void)) && !(getObjectFlags(t) & ObjectFlags.Anonymous), From 02c2426d9dd607ee6eb4330f54074aff1293daa5 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 22 Apr 2020 14:01:26 -0400 Subject: [PATCH 5/5] Took advice on restricting void to just call expressions --- src/services/codefixes/inferFromUsage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 1ace3a58107c0..3726517cc60b4 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -669,7 +669,7 @@ namespace ts.codefix { } function inferTypeFromExpressionStatement(node: Expression, usage: Usage): void { - addCandidateType(usage, isPropertyAccessExpression(node) ? checker.getAnyType() : checker.getVoidType()); + addCandidateType(usage, isCallExpression(node) ? checker.getVoidType() : checker.getAnyType()); } function inferTypeFromPrefixUnaryExpression(node: PrefixUnaryExpression, usage: Usage): void {