Skip to content

Prefer a likely literal over anonymous type in --noImplicitAny codefixes #36015

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 22, 2020
Merged

Prefer a likely literal over anonymous type in --noImplicitAny codefixes #36015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ddd7f24
Prefer a likely literal over anonymous type in --noImplicitAny codefixes
JoshuaKGoldberg Jan 5, 2020
e33cc59
Used unknown/any instead of void when applicable
JoshuaKGoldberg Apr 19, 2020
76fd88a
Fix little whitespace change in tests/cases/fourslash/codeFixInferFro…
Apr 19, 2020
18fdff5
Undid some now-unnecessary unknown additions
JoshuaKGoldberg Apr 19, 2020
17fb22f
Merge branch 'infer-literal-if-single' of https://github.com/joshuakg…
JoshuaKGoldberg Apr 19, 2020
02c2426
Took advice on restricting void to just call expressions
JoshuaKGoldberg Apr 22, 2020
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
11 changes: 6 additions & 5 deletions src/services/codefixes/inferFromUsage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,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;
Expand Down Expand Up @@ -668,6 +668,10 @@ namespace ts.codefix {
}
}

function inferTypeFromExpressionStatement(node: Expression, usage: Usage): void {
addCandidateType(usage, isCallExpression(node) ? checker.getVoidType() : checker.getAnyType());
}

function inferTypeFromPrefixUnaryExpression(node: PrefixUnaryExpression, usage: Usage): void {
switch (node.operator) {
case SyntaxKind.PlusPlusToken:
Expand Down Expand Up @@ -967,10 +971,7 @@ 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) {
if (usage.properties?.size || usage.calls?.length || usage.constructs?.length || usage.stringIndex) {
types.push(inferStructuralType(usage));
}

Expand Down
2 changes: 1 addition & 1 deletion src/services/textChanges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: ": " });
Expand Down
10 changes: 10 additions & 0 deletions tests/cases/fourslash/codeFixInferFromUsageParameterLiteral.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// function foo([|text |]) {
//// text.length;
//// text.indexOf("z");
//// text.charAt(0);
//// }

verify.rangeAfterCodeFix("text: string", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);
2 changes: 1 addition & 1 deletion tests/cases/fourslash/codeFixInferFromUsagePropertyAccess.ts
View file
Open in desktop
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: 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks like an unintended positive bug fix, right? Other places (e.g. codeFixInferFromCallInAssignment) infer any for variable types that would otherwise be describable as unknown.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what you mean by "unintended positive". void was wrong and any is right as far as I can tell.

I think, in most positions, inferring any vs unknown is a matter of preference -- how strict do you want the result to be? Right now I assume that people want pretty loose results, since I expect people will use this after converting from .js to .ts, and before trying to get rid of any explicit any.

We could discuss adding another fix to the codefix, one that defaults to unknown when possible. I think it would be a decent idea, but hard to explain in the space of a codefix title.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes I think we're on the same page - I meant that the change from void to any is positive, and I didn't initially intend it as part of this PR's changes. 👍

2 changes: 1 addition & 1 deletion tests/cases/fourslash/codeFixInferFromUsagePropertyAccessJS.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ verify.codeFix({
index: 0,
newFileContent:
`/**
* @param {{ b: { c: void; }; }} a
* @param {{ b: { c: any; }; }} a
* @param {{ n: () => number; }} m
* @param {{ y: { z: number[]; }; }} x
*/
Expand Down
9 changes: 9 additions & 0 deletions tests/cases/fourslash/codeFixInferFromUsageVariableLiteral.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/// <reference path='fourslash.ts' />

// @noImplicitAny: true
//// let [|text! |];
//// text.length;
//// text.indexOf("z");
//// text.charAt(0);

verify.rangeAfterCodeFix("text!: string", /*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, /*index*/0);