From 1e6a5f3d0b053bd84b4d6d15abc20a13523fb276 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Thu, 1 Nov 2018 19:58:10 +0100 Subject: [PATCH 01/14] Add codefix for 'Cannot find name' diagnostic --- src/compiler/diagnosticMessages.json | 8 +++++++ .../codefixes/addMissingConstInForLoop.ts | 23 +++++++++++++++++++ src/services/tsconfig.json | 1 + src/services/utilities.ts | 2 +- .../codeFixAddMissingConstInForLoop1.ts | 8 +++++++ .../codeFixAddMissingConstInForLoop2.ts | 12 ++++++++++ .../codeFixAddMissingConstInForLoop3.ts | 8 +++++++ .../codeFixAddMissingConstInForLoop4.ts | 8 +++++++ 8 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 src/services/codefixes/addMissingConstInForLoop.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoop2.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index e3f8e1a7ad5..85ad60b9850 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4799,5 +4799,13 @@ "Add names to all parameters without names": { "category": "Message", "code": 95073 + }, + "Add const modifier to unresolved variable": { + "category": "Message", + "code": 95074 + }, + "Add const modifiers to all unresolved variables": { + "category": "Message", + "code": 95075 } } diff --git a/src/services/codefixes/addMissingConstInForLoop.ts b/src/services/codefixes/addMissingConstInForLoop.ts new file mode 100644 index 00000000000..cd4507729de --- /dev/null +++ b/src/services/codefixes/addMissingConstInForLoop.ts @@ -0,0 +1,23 @@ +/* @internal */ +namespace ts.codefix { + const fixId = "addMissingConstInForLoop"; + const errorCodes = [Diagnostics.Cannot_find_name_0.code]; + registerCodeFix({ + errorCodes, + getCodeActions: (context) => { + const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span.start)); + return [createCodeFixAction(fixId, changes, Diagnostics.Add_const_modifier_to_unresolved_variable, fixId, Diagnostics.Add_const_modifiers_to_all_unresolved_variables)]; + }, + fixIds: [fixId], + getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag.start)), + }); + + function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) { + const token = getTokenAtPosition(sourceFile, pos); + // fails on 'for ([x, y] of [[1,2]]) {}' when called for y + // since findPrecedingMatchingToken does not return the open paren here after iterating over another identifier (x) + const openParenToken = findPrecedingMatchingToken(token, SyntaxKind.OpenParenToken, sourceFile); + Debug.assert(!!openParenToken, "openParenToken must be defined"); + changeTracker.insertNodeAt(sourceFile, openParenToken.getEnd(), createToken(SyntaxKind.ConstKeyword), { suffix: " " }); + } +} diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 15044416f84..12191f491ab 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -44,6 +44,7 @@ "codeFixProvider.ts", "refactorProvider.ts", "codefixes/addConvertToUnknownForNonOverlappingTypes.ts", + "codefixes/addMissingConstInForLoop.ts", "codefixes/addMissingInvocationForDecorator.ts", "codefixes/addNameToNamelessParameter.ts", "codefixes/annotateWithTypeFromJSDoc.ts", diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 22e6b31b55c..7b340099a74 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -936,7 +936,7 @@ namespace ts { return undefined; } token = preceding; - + if (token.kind === matchingTokenKind) { if (remainingMatchingTokens === 0) { return token; diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts new file mode 100644 index 00000000000..be35538b59b --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts @@ -0,0 +1,8 @@ +/// + +////[|for (x of []) {}|] + +verify.codeFix({ + description: "Add const modifier to unresolved variable", + newRangeContent: "for (const x of []) {}" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoop2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoop2.ts new file mode 100644 index 00000000000..ce79b68a769 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoop2.ts @@ -0,0 +1,12 @@ +/// + +////[|for (x of []) {}|] +////[|for (y of []) {}|] + +verify.codeFixAll({ + fixId: "addMissingConstInForLoop", + fixAllDescription: "Add const modifiers to all unresolved variables", + newFileContent: +`for (const x of []) {} +for (const y of []) {}` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts new file mode 100644 index 00000000000..388ab0b5503 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts @@ -0,0 +1,8 @@ +/// + +////[|for ([x] of [[1,2]]) {}|] + +verify.codeFix({ + description: "Add const modifier to unresolved variable", + newRangeContent: "for (const [x] of [[1,2]]) {}" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts new file mode 100644 index 00000000000..4e57f453cfd --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts @@ -0,0 +1,8 @@ +/// + +////[|for ([x, y] of [[1,2]]) {}|] + +verify.codeFix({ + description: "Add const modifier to unresolved variable", + newRangeContent: "for (const [x, y] of [[1,2]]) {}" +}); From 2dae10f6ba8db1c155a09d5158b92e339b86b840 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Thu, 1 Nov 2018 20:05:57 +0100 Subject: [PATCH 02/14] Revert whitespace change --- src/services/utilities.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 7b340099a74..22e6b31b55c 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -936,7 +936,7 @@ namespace ts { return undefined; } token = preceding; - + if (token.kind === matchingTokenKind) { if (remainingMatchingTokens === 0) { return token; From 70fee3fb367f99eff87d356bdd7005e7f71bb3ba Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Fri, 2 Nov 2018 01:32:12 +0100 Subject: [PATCH 03/14] Review changes and adjustment of fourslash tests --- src/compiler/diagnosticMessages.json | 4 +- .../codefixes/addMissingConstInForLoop.ts | 44 ++++++++++++++++--- src/services/textChanges.ts | 3 ++ .../codeFixAddMissingConstInForInLoop1.ts | 8 ++++ .../codeFixAddMissingConstInForInLoop2.ts | 12 +++++ .../codeFixAddMissingConstInForLoop1.ts | 8 ---- .../codeFixAddMissingConstInForLoop3.ts | 8 ---- .../codeFixAddMissingConstInForLoop4.ts | 8 ---- ...ngConstInForLoopWithArrayDestructuring1.ts | 8 ++++ ...ngConstInForLoopWithArrayDestructuring2.ts | 12 +++++ ...gConstInForLoopWithObjectDestructuring1.ts | 8 ++++ ...gConstInForLoopWithObjectDestructuring2.ts | 12 +++++ .../codeFixAddMissingConstInForOfLoop1.ts | 8 ++++ ... => codeFixAddMissingConstInForOfLoop2.ts} | 6 +-- .../fourslash/codeFixAwaitInSyncFunction4.ts | 2 +- .../fourslash/codeFixAwaitInSyncFunction7.ts | 4 +- ...deFixAwaitShouldNotCrashIfNotInFunction.ts | 1 + ...sImplementInterfaceIndexSignaturesNoFix.ts | 2 +- ...xUnusedIdentifier_destructure_allUnused.ts | 2 +- ...sedIdentifier_destructure_allUnused_all.ts | 6 +-- ...sedIdentifier_destructure_allUnused_for.ts | 4 +- ...Identifier_destructure_allUnused_nested.ts | 4 +- .../incompleteFunctionCallCodefix3.ts | 4 +- 23 files changed, 129 insertions(+), 49 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForInLoop1.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts delete mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts delete mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts delete mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring1.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring1.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstInForOfLoop1.ts rename tests/cases/fourslash/{codeFixAddMissingConstInForLoop2.ts => codeFixAddMissingConstInForOfLoop2.ts} (55%) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 85ad60b9850..cc6e8922c64 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4800,11 +4800,11 @@ "category": "Message", "code": 95073 }, - "Add const modifier to unresolved variable": { + "Add 'const' to unresolved variable": { "category": "Message", "code": 95074 }, - "Add const modifiers to all unresolved variables": { + "Add 'const' to all unresolved variables": { "category": "Message", "code": 95075 } diff --git a/src/services/codefixes/addMissingConstInForLoop.ts b/src/services/codefixes/addMissingConstInForLoop.ts index cd4507729de..d1534c7020f 100644 --- a/src/services/codefixes/addMissingConstInForLoop.ts +++ b/src/services/codefixes/addMissingConstInForLoop.ts @@ -6,18 +6,48 @@ namespace ts.codefix { errorCodes, getCodeActions: (context) => { const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span.start)); - return [createCodeFixAction(fixId, changes, Diagnostics.Add_const_modifier_to_unresolved_variable, fixId, Diagnostics.Add_const_modifiers_to_all_unresolved_variables)]; + if (changes) { + return [createCodeFixAction(fixId, changes, Diagnostics.Add_const_to_unresolved_variable, fixId, Diagnostics.Add_const_to_all_unresolved_variables)]; + } }, fixIds: [fixId], getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag.start)), }); function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) { - const token = getTokenAtPosition(sourceFile, pos); - // fails on 'for ([x, y] of [[1,2]]) {}' when called for y - // since findPrecedingMatchingToken does not return the open paren here after iterating over another identifier (x) - const openParenToken = findPrecedingMatchingToken(token, SyntaxKind.OpenParenToken, sourceFile); - Debug.assert(!!openParenToken, "openParenToken must be defined"); - changeTracker.insertNodeAt(sourceFile, openParenToken.getEnd(), createToken(SyntaxKind.ConstKeyword), { suffix: " " }); + const forInitializer = findAncestor(getTokenAtPosition(sourceFile, pos), node => + isForInOrOfStatement(node.parent) ? node.parent.initializer === node + : isPossiblyPartOfDestructuring(node) ? false : "quit"); + if (!forInitializer) return; + if (alreadyContainsConstCodeFixForInitializer(changeTracker, forInitializer, sourceFile)) return; + changeTracker.insertNodeBefore(sourceFile, forInitializer, createToken(SyntaxKind.ConstKeyword)); + } + + function isPossiblyPartOfDestructuring(node: Node): boolean { + switch (node.kind) { + case SyntaxKind.Identifier: + case SyntaxKind.ArrayLiteralExpression: + case SyntaxKind.ObjectLiteralExpression: + case SyntaxKind.PropertyAssignment: + case SyntaxKind.ShorthandPropertyAssignment: + return true; + default: + return false; + } + } + + function alreadyContainsConstCodeFixForInitializer(changeTracker: textChanges.ChangeTracker, forInitializer: Node, sourceFile: SourceFile): boolean { + return changeTracker.getChanges().some(change => { + const textChanges = change.textChanges; + if (!textChanges) return false; + return textChanges.some(textChange => { + if (textChange.newText !== "const ") return false; + const changeStart = textChange.span.start; + const changeEnd = changeStart + textChange.span.length; + const initStart = forInitializer.getStart(sourceFile); + const initEnd = forInitializer.getEnd(); + return initStart <= changeEnd && changeStart <= initEnd; + }); + }); } } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 9c773f73f43..86076ecb877 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -403,6 +403,9 @@ namespace ts.textChanges { else if (isStringLiteral(before) && isImportDeclaration(before.parent) || isNamedImports(before)) { return { suffix: ", " }; } + else if (isForInitializer(before)) { + return { suffix: " " }; + } return Debug.failBadSyntaxKind(before); // We haven't handled this kind of node yet -- add it } diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForInLoop1.ts b/tests/cases/fourslash/codeFixAddMissingConstInForInLoop1.ts new file mode 100644 index 00000000000..76b752465a9 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForInLoop1.ts @@ -0,0 +1,8 @@ +/// + +////for (x in []) {} + +verify.codeFix({ + description: "Add 'const' to unresolved variable", + newFileContent: "for (const x in []) {}" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts new file mode 100644 index 00000000000..d101f384044 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts @@ -0,0 +1,12 @@ +/// + +////for (x in []) {} +////for (y in []) {} + +verify.codeFixAll({ + fixId: "addMissingConstInForLoop", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: +`for (const x in []) {} +for (const y in []) {}` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts deleted file mode 100644 index be35538b59b..00000000000 --- a/tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts +++ /dev/null @@ -1,8 +0,0 @@ -/// - -////[|for (x of []) {}|] - -verify.codeFix({ - description: "Add const modifier to unresolved variable", - newRangeContent: "for (const x of []) {}" -}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts deleted file mode 100644 index 388ab0b5503..00000000000 --- a/tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts +++ /dev/null @@ -1,8 +0,0 @@ -/// - -////[|for ([x] of [[1,2]]) {}|] - -verify.codeFix({ - description: "Add const modifier to unresolved variable", - newRangeContent: "for (const [x] of [[1,2]]) {}" -}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts deleted file mode 100644 index 4e57f453cfd..00000000000 --- a/tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts +++ /dev/null @@ -1,8 +0,0 @@ -/// - -////[|for ([x, y] of [[1,2]]) {}|] - -verify.codeFix({ - description: "Add const modifier to unresolved variable", - newRangeContent: "for (const [x, y] of [[1,2]]) {}" -}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring1.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring1.ts new file mode 100644 index 00000000000..791615f5ea0 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring1.ts @@ -0,0 +1,8 @@ +/// + +////for ([x] of [[1,2]]) {} + +verify.codeFix({ + description: "Add 'const' to unresolved variable", + newFileContent: "for (const [x] of [[1,2]]) {}" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts new file mode 100644 index 00000000000..734b53ed4fb --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts @@ -0,0 +1,12 @@ +/// + +////for ([x, y] of [[1,2]]) {} +////for ([x] of [[1,2]]) {} + +verify.codeFixAll({ + fixId: "addMissingConstInForLoop", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: +`for (const [x, y] of [[1,2]]) {} +for (const [x] of [[1,2]]) {}` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring1.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring1.ts new file mode 100644 index 00000000000..f926091da80 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring1.ts @@ -0,0 +1,8 @@ +/// + +////for ({ x } of [{ x: 0 }]) { } + +verify.codeFix({ + description: "Add 'const' to unresolved variable", + newFileContent: "for (const { x } of [{ x: 0 }]) { }" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts new file mode 100644 index 00000000000..82cd2db6dc2 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts @@ -0,0 +1,12 @@ +/// + +////for ({ x, y } of [{ x: 0, y: 1 }]) { } +////for ({ x } of [{ x: 0 }]) { } + +verify.codeFixAll({ + fixId: "addMissingConstInForLoop", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: +`for (const { x, y } of [{ x: 0, y: 1 }]) { } +for (const { x } of [{ x: 0 }]) { }` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop1.ts b/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop1.ts new file mode 100644 index 00000000000..49efe2bc9a6 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop1.ts @@ -0,0 +1,8 @@ +/// + +////for (x of []) {} + +verify.codeFix({ + description: "Add 'const' to unresolved variable", + newFileContent: "for (const x of []) {}" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoop2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop2.ts similarity index 55% rename from tests/cases/fourslash/codeFixAddMissingConstInForLoop2.ts rename to tests/cases/fourslash/codeFixAddMissingConstInForOfLoop2.ts index ce79b68a769..3dbab65de32 100644 --- a/tests/cases/fourslash/codeFixAddMissingConstInForLoop2.ts +++ b/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop2.ts @@ -1,11 +1,11 @@ /// -////[|for (x of []) {}|] -////[|for (y of []) {}|] +////for (x of []) {} +////for (y of []) {} verify.codeFixAll({ fixId: "addMissingConstInForLoop", - fixAllDescription: "Add const modifiers to all unresolved variables", + fixAllDescription: "Add 'const' to all unresolved variables", newFileContent: `for (const x of []) {} for (const y of []) {}` diff --git a/tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts b/tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts index dd123e25d0b..24506bb39cb 100644 --- a/tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts +++ b/tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts @@ -1,7 +1,7 @@ /// ////class Foo { -//// constructor { +//// constructor() { //// await Promise.resolve(); //// } ////} diff --git a/tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts b/tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts index a467e9ee0ce..1d058fe473b 100644 --- a/tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts +++ b/tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts @@ -1,7 +1,7 @@ /// ////function f() { -//// for await (const x of g()) { +//// for await (const x of []) { //// console.log(x); //// } ////} @@ -10,7 +10,7 @@ verify.codeFix({ description: "Add async modifier to containing function", newFileContent: `async function f() { - for await (const x of g()) { + for await (const x of []) { console.log(x); } }`, diff --git a/tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts b/tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts index 1ce5c771ed4..4d916fe25b6 100644 --- a/tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts +++ b/tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts @@ -1,5 +1,6 @@ /// +////async function a() {} ////await a verify.not.codeFixAvailable(); diff --git a/tests/cases/fourslash/codeFixClassImplementInterfaceIndexSignaturesNoFix.ts b/tests/cases/fourslash/codeFixClassImplementInterfaceIndexSignaturesNoFix.ts index 07896b4cdb3..39470dfdd50 100644 --- a/tests/cases/fourslash/codeFixClassImplementInterfaceIndexSignaturesNoFix.ts +++ b/tests/cases/fourslash/codeFixClassImplementInterfaceIndexSignaturesNoFix.ts @@ -4,7 +4,7 @@ //// [x: string, y: number]: number; //// } //// -//// class C implements I {[| |]} +//// class C implements I4 {[| |]} verify.not.codeFixAvailable(); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts index 4f3f9a3e61e..97556092ade 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts @@ -4,7 +4,7 @@ // @noUnusedParameters: true ////export {}; -////const { x, y } = o; +////const { x, y } = [{}]; verify.codeFix({ description: "Remove destructuring", diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts index c9d4f62f2db..066ba07c4f4 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts @@ -3,8 +3,8 @@ // @noUnusedLocals: true // @noUnusedParameters: true -////const { x, y } = o; -////const { a, b } = o; +////const { x, y } = [{}]; +////const { a, b } = [{}]; ////a; ////export function f({ a, b }, { x, y }) { //// a; @@ -14,7 +14,7 @@ verify.codeFixAll({ fixId: "unusedIdentifier_delete", fixAllDescription: "Delete all unused declarations", newFileContent: -`const { a } = o; +`const { a } = [{}]; a; export function f({ a }) { a; diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_for.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_for.ts index 27103357d3d..ea94e5275aa 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_for.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_for.ts @@ -3,10 +3,10 @@ // @noUnusedLocals: true // @noUnusedParameters: true -////for (const { x } of o) {} +////for (const { x } of [{}]) {} verify.codeFix({ description: "Remove destructuring", newFileContent: -`for (const {} of o) {}`, +`for (const {} of [{}]) {}`, }); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts index 73fca113928..dc1ae26b3d0 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts @@ -4,11 +4,11 @@ // @noUnusedParameters: true ////export {}; -////const { x: { a, b } } = o; +////const { x: { a, b } } = [{}]; verify.codeFix({ description: "Remove destructuring", newFileContent: `export {}; -const { } = o;`, +const { } = [{}];`, }); diff --git a/tests/cases/fourslash/incompleteFunctionCallCodefix3.ts b/tests/cases/fourslash/incompleteFunctionCallCodefix3.ts index 29072c7c855..db2e82f94bf 100644 --- a/tests/cases/fourslash/incompleteFunctionCallCodefix3.ts +++ b/tests/cases/fourslash/incompleteFunctionCallCodefix3.ts @@ -3,4 +3,6 @@ // @noImplicitAny: true //// function ...q) {}} f(10); -verify.not.codeFixAvailable(); +verify.not.codeFixAvailable([ + { "description": "Infer parameter types from usage" } +]); From 94118b64c5bf082e7bdaa06a71046fbe921d61b5 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Fri, 2 Nov 2018 01:47:50 +0100 Subject: [PATCH 04/14] Properly check if a codefix is needed --- src/services/codefixes/addMissingConstInForLoop.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/codefixes/addMissingConstInForLoop.ts b/src/services/codefixes/addMissingConstInForLoop.ts index d1534c7020f..3caea2222fa 100644 --- a/src/services/codefixes/addMissingConstInForLoop.ts +++ b/src/services/codefixes/addMissingConstInForLoop.ts @@ -6,7 +6,7 @@ namespace ts.codefix { errorCodes, getCodeActions: (context) => { const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span.start)); - if (changes) { + if (changes.length > 0) { return [createCodeFixAction(fixId, changes, Diagnostics.Add_const_to_unresolved_variable, fixId, Diagnostics.Add_const_to_all_unresolved_variables)]; } }, From 1fd2371a3731811223e205f61ee1b27ff7b08a5b Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Sat, 3 Nov 2018 00:00:12 +0100 Subject: [PATCH 05/14] Fixed codeFixAll behavior and adjusted changed fourslash tests --- .../codefixes/addMissingConstInForLoop.ts | 28 ++++++------------- ...xUnusedIdentifier_destructure_allUnused.ts | 2 +- ...sedIdentifier_destructure_allUnused_all.ts | 6 ++-- ...Identifier_destructure_allUnused_nested.ts | 4 +-- .../incompleteFunctionCallCodefix3.ts | 4 +-- 5 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/services/codefixes/addMissingConstInForLoop.ts b/src/services/codefixes/addMissingConstInForLoop.ts index 3caea2222fa..d2add652e9a 100644 --- a/src/services/codefixes/addMissingConstInForLoop.ts +++ b/src/services/codefixes/addMissingConstInForLoop.ts @@ -2,6 +2,7 @@ namespace ts.codefix { const fixId = "addMissingConstInForLoop"; const errorCodes = [Diagnostics.Cannot_find_name_0.code]; + registerCodeFix({ errorCodes, getCodeActions: (context) => { @@ -11,16 +12,20 @@ namespace ts.codefix { } }, fixIds: [fixId], - getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag.start)), + getAllCodeActions: context => { + const fixedNodes = new NodeSet(); + return codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag.start, fixedNodes)); + }, }); - function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number) { + function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, fixedNodes?: NodeSet) { const forInitializer = findAncestor(getTokenAtPosition(sourceFile, pos), node => isForInOrOfStatement(node.parent) ? node.parent.initializer === node : isPossiblyPartOfDestructuring(node) ? false : "quit"); if (!forInitializer) return; - if (alreadyContainsConstCodeFixForInitializer(changeTracker, forInitializer, sourceFile)) return; - changeTracker.insertNodeBefore(sourceFile, forInitializer, createToken(SyntaxKind.ConstKeyword)); + if (!fixedNodes || fixedNodes.tryAdd(forInitializer)) { + changeTracker.insertNodeBefore(sourceFile, forInitializer, createToken(SyntaxKind.ConstKeyword)); + } } function isPossiblyPartOfDestructuring(node: Node): boolean { @@ -35,19 +40,4 @@ namespace ts.codefix { return false; } } - - function alreadyContainsConstCodeFixForInitializer(changeTracker: textChanges.ChangeTracker, forInitializer: Node, sourceFile: SourceFile): boolean { - return changeTracker.getChanges().some(change => { - const textChanges = change.textChanges; - if (!textChanges) return false; - return textChanges.some(textChange => { - if (textChange.newText !== "const ") return false; - const changeStart = textChange.span.start; - const changeEnd = changeStart + textChange.span.length; - const initStart = forInitializer.getStart(sourceFile); - const initEnd = forInitializer.getEnd(); - return initStart <= changeEnd && changeStart <= initEnd; - }); - }); - } } diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts index 97556092ade..6f2dc76d0ae 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts @@ -4,7 +4,7 @@ // @noUnusedParameters: true ////export {}; -////const { x, y } = [{}]; +////const { x, y } = {}; verify.codeFix({ description: "Remove destructuring", diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts index 066ba07c4f4..176f108c517 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts @@ -3,8 +3,8 @@ // @noUnusedLocals: true // @noUnusedParameters: true -////const { x, y } = [{}]; -////const { a, b } = [{}]; +////const { x, y } = {}; +////const { a, b } = {}; ////a; ////export function f({ a, b }, { x, y }) { //// a; @@ -14,7 +14,7 @@ verify.codeFixAll({ fixId: "unusedIdentifier_delete", fixAllDescription: "Delete all unused declarations", newFileContent: -`const { a } = [{}]; +`const { a } = {}; a; export function f({ a }) { a; diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts index dc1ae26b3d0..03a8ff826c7 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts @@ -4,11 +4,11 @@ // @noUnusedParameters: true ////export {}; -////const { x: { a, b } } = [{}]; +////const { x: { a, b } } = {{}}; verify.codeFix({ description: "Remove destructuring", newFileContent: `export {}; -const { } = [{}];`, +const { } = {{}};`, }); diff --git a/tests/cases/fourslash/incompleteFunctionCallCodefix3.ts b/tests/cases/fourslash/incompleteFunctionCallCodefix3.ts index db2e82f94bf..29072c7c855 100644 --- a/tests/cases/fourslash/incompleteFunctionCallCodefix3.ts +++ b/tests/cases/fourslash/incompleteFunctionCallCodefix3.ts @@ -3,6 +3,4 @@ // @noImplicitAny: true //// function ...q) {}} f(10); -verify.not.codeFixAvailable([ - { "description": "Infer parameter types from usage" } -]); +verify.not.codeFixAvailable(); From 8987e56e410ae8839c681cb7018f84fddb609f49 Mon Sep 17 00:00:00 2001 From: rflorian Date: Thu, 9 May 2019 00:30:55 +0200 Subject: [PATCH 06/14] Fix error from master merge --- src/compiler/diagnosticMessages.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index bf70e22ef6f..2db71a6d266 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4974,7 +4974,6 @@ "category": "Message", "code": 95079 }, - "Add 'const' to unresolved variable": { "category": "Message", "code": 95080 @@ -4982,8 +4981,8 @@ "Add 'const' to all unresolved variables": { "category": "Message", "code": 95081 - - "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer." :{ + }, + "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", "code": 18004 }, From e395e49a8fcc33b894933542c4d5aeff0f6e2332 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Thu, 9 May 2019 01:17:31 +0200 Subject: [PATCH 07/14] Add secondary error code to relevant error code list in addMissingConstInForLoop codefix --- src/services/codefixes/addMissingConstInForLoop.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/services/codefixes/addMissingConstInForLoop.ts b/src/services/codefixes/addMissingConstInForLoop.ts index d2add652e9a..433fa45f458 100644 --- a/src/services/codefixes/addMissingConstInForLoop.ts +++ b/src/services/codefixes/addMissingConstInForLoop.ts @@ -1,7 +1,10 @@ /* @internal */ namespace ts.codefix { const fixId = "addMissingConstInForLoop"; - const errorCodes = [Diagnostics.Cannot_find_name_0.code]; + const errorCodes = [ + Diagnostics.Cannot_find_name_0.code, + Diagnostics.No_value_exists_in_scope_for_the_shorthand_property_0_Either_declare_one_or_provide_an_initializer.code + ]; registerCodeFix({ errorCodes, From e0a685da4f2a60f092be60069e6c4d823b44abe7 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Thu, 9 May 2019 01:34:21 +0200 Subject: [PATCH 08/14] Fix whitespace issue in codefix file --- src/services/codefixes/addMissingConstInForLoop.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/codefixes/addMissingConstInForLoop.ts b/src/services/codefixes/addMissingConstInForLoop.ts index 433fa45f458..9110fb31a10 100644 --- a/src/services/codefixes/addMissingConstInForLoop.ts +++ b/src/services/codefixes/addMissingConstInForLoop.ts @@ -2,7 +2,7 @@ namespace ts.codefix { const fixId = "addMissingConstInForLoop"; const errorCodes = [ - Diagnostics.Cannot_find_name_0.code, + Diagnostics.Cannot_find_name_0.code, Diagnostics.No_value_exists_in_scope_for_the_shorthand_property_0_Either_declare_one_or_provide_an_initializer.code ]; From 41ebeec05756abb164566a34296ff380c9b44cf9 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Tue, 2 Jul 2019 00:36:44 +0200 Subject: [PATCH 09/14] Revert unrelated fourslash test changes --- tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts | 2 +- tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts | 4 ++-- .../fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts | 1 - .../codeFixClassImplementInterfaceIndexSignaturesNoFix.ts | 2 +- .../codeFixUnusedIdentifier_destructure_allUnused.ts | 2 +- .../codeFixUnusedIdentifier_destructure_allUnused_all.ts | 6 +++--- .../codeFixUnusedIdentifier_destructure_allUnused_for.ts | 4 ++-- .../codeFixUnusedIdentifier_destructure_allUnused_nested.ts | 4 ++-- 8 files changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts b/tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts index 24506bb39cb..dd123e25d0b 100644 --- a/tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts +++ b/tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts @@ -1,7 +1,7 @@ /// ////class Foo { -//// constructor() { +//// constructor { //// await Promise.resolve(); //// } ////} diff --git a/tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts b/tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts index 1d058fe473b..a467e9ee0ce 100644 --- a/tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts +++ b/tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts @@ -1,7 +1,7 @@ /// ////function f() { -//// for await (const x of []) { +//// for await (const x of g()) { //// console.log(x); //// } ////} @@ -10,7 +10,7 @@ verify.codeFix({ description: "Add async modifier to containing function", newFileContent: `async function f() { - for await (const x of []) { + for await (const x of g()) { console.log(x); } }`, diff --git a/tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts b/tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts index 4d916fe25b6..1ce5c771ed4 100644 --- a/tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts +++ b/tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts @@ -1,6 +1,5 @@ /// -////async function a() {} ////await a verify.not.codeFixAvailable(); diff --git a/tests/cases/fourslash/codeFixClassImplementInterfaceIndexSignaturesNoFix.ts b/tests/cases/fourslash/codeFixClassImplementInterfaceIndexSignaturesNoFix.ts index 39470dfdd50..07896b4cdb3 100644 --- a/tests/cases/fourslash/codeFixClassImplementInterfaceIndexSignaturesNoFix.ts +++ b/tests/cases/fourslash/codeFixClassImplementInterfaceIndexSignaturesNoFix.ts @@ -4,7 +4,7 @@ //// [x: string, y: number]: number; //// } //// -//// class C implements I4 {[| |]} +//// class C implements I {[| |]} verify.not.codeFixAvailable(); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts index 6f2dc76d0ae..4f3f9a3e61e 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused.ts @@ -4,7 +4,7 @@ // @noUnusedParameters: true ////export {}; -////const { x, y } = {}; +////const { x, y } = o; verify.codeFix({ description: "Remove destructuring", diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts index 176f108c517..c9d4f62f2db 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_all.ts @@ -3,8 +3,8 @@ // @noUnusedLocals: true // @noUnusedParameters: true -////const { x, y } = {}; -////const { a, b } = {}; +////const { x, y } = o; +////const { a, b } = o; ////a; ////export function f({ a, b }, { x, y }) { //// a; @@ -14,7 +14,7 @@ verify.codeFixAll({ fixId: "unusedIdentifier_delete", fixAllDescription: "Delete all unused declarations", newFileContent: -`const { a } = {}; +`const { a } = o; a; export function f({ a }) { a; diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_for.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_for.ts index ea94e5275aa..27103357d3d 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_for.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_for.ts @@ -3,10 +3,10 @@ // @noUnusedLocals: true // @noUnusedParameters: true -////for (const { x } of [{}]) {} +////for (const { x } of o) {} verify.codeFix({ description: "Remove destructuring", newFileContent: -`for (const {} of [{}]) {}`, +`for (const {} of o) {}`, }); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts index 03a8ff826c7..73fca113928 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_destructure_allUnused_nested.ts @@ -4,11 +4,11 @@ // @noUnusedParameters: true ////export {}; -////const { x: { a, b } } = {{}}; +////const { x: { a, b } } = o; verify.codeFix({ description: "Remove destructuring", newFileContent: `export {}; -const { } = {{}};`, +const { } = o;`, }); From 7d08f172d8006c5aa328adcf39b10f3003efebf3 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Sun, 7 Jul 2019 13:56:34 +0200 Subject: [PATCH 10/14] Added fourslash tests for standalone and array initialization cases and started implementing them --- ...ngConstInForLoop.ts => addMissingConst.ts} | 28 +++++++++++++++---- src/services/tsconfig.json | 2 +- .../codeFixAddMissingConstInForInLoop2.ts | 2 +- ...ngConstInForLoopWithArrayDestructuring2.ts | 2 +- ...gConstInForLoopWithObjectDestructuring2.ts | 2 +- .../codeFixAddMissingConstInForOfLoop2.ts | 2 +- ...FixAddMissingConstToArrayDestructuring1.ts | 8 ++++++ ...FixAddMissingConstToArrayDestructuring2.ts | 9 ++++++ ...FixAddMissingConstToArrayDestructuring3.ts | 6 ++++ ...xAddMissingConstToStandaloneIdentifier1.ts | 8 ++++++ ...xAddMissingConstToStandaloneIdentifier2.ts | 5 ++++ 11 files changed, 64 insertions(+), 10 deletions(-) rename src/services/codefixes/{addMissingConstInForLoop.ts => addMissingConst.ts} (55%) create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring1.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring2.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring3.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier1.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier2.ts diff --git a/src/services/codefixes/addMissingConstInForLoop.ts b/src/services/codefixes/addMissingConst.ts similarity index 55% rename from src/services/codefixes/addMissingConstInForLoop.ts rename to src/services/codefixes/addMissingConst.ts index 9110fb31a10..857d7fb4282 100644 --- a/src/services/codefixes/addMissingConstInForLoop.ts +++ b/src/services/codefixes/addMissingConst.ts @@ -1,6 +1,6 @@ /* @internal */ namespace ts.codefix { - const fixId = "addMissingConstInForLoop"; + const fixId = "addMissingConst"; const errorCodes = [ Diagnostics.Cannot_find_name_0.code, Diagnostics.No_value_exists_in_scope_for_the_shorthand_property_0_Either_declare_one_or_provide_an_initializer.code @@ -22,12 +22,30 @@ namespace ts.codefix { }); function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, fixedNodes?: NodeSet) { - const forInitializer = findAncestor(getTokenAtPosition(sourceFile, pos), node => + const token = getTokenAtPosition(sourceFile, pos); + + const forInitializer = findAncestor(token, node => isForInOrOfStatement(node.parent) ? node.parent.initializer === node : isPossiblyPartOfDestructuring(node) ? false : "quit"); - if (!forInitializer) return; - if (!fixedNodes || fixedNodes.tryAdd(forInitializer)) { - changeTracker.insertNodeBefore(sourceFile, forInitializer, createToken(SyntaxKind.ConstKeyword)); + if (forInitializer) return applyChange(changeTracker, forInitializer, sourceFile, fixedNodes); + + const parent = token.parent; + const standaloneInitializer = isExpressionStatement(parent.parent); + if (standaloneInitializer) return applyChange(changeTracker, parent, sourceFile, fixedNodes); + + const arrayLiteralInitializer = isArrayLiteralExpression(token.parent); + if (arrayLiteralInitializer) { + const availableIdentifiers: string[] = []; // TODO: where to get/gather this information from? + const noIdentifiersDeclared = parent.forEachChild(node => availableIdentifiers.indexOf(node.getFullText()) < 0); + if (!noIdentifiersDeclared) return; + + return applyChange(changeTracker, parent, sourceFile, fixedNodes); + } + } + + function applyChange(changeTracker: textChanges.ChangeTracker, initializer: Node, sourceFile: SourceFile, fixedNodes?: NodeSet) { + if (!fixedNodes || fixedNodes.tryAdd(initializer)) { + changeTracker.insertNodeBefore(sourceFile, initializer, createToken(SyntaxKind.ConstKeyword)); } } diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index b48f27cf1c6..5e6de0f15ca 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -45,7 +45,7 @@ "codeFixProvider.ts", "refactorProvider.ts", "codefixes/addConvertToUnknownForNonOverlappingTypes.ts", - "codefixes/addMissingConstInForLoop.ts", + "codefixes/addMissingConst.ts", "codefixes/addMissingInvocationForDecorator.ts", "codefixes/addNameToNamelessParameter.ts", "codefixes/annotateWithTypeFromJSDoc.ts", diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts index d101f384044..03fb12f5d7e 100644 --- a/tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts +++ b/tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts @@ -4,7 +4,7 @@ ////for (y in []) {} verify.codeFixAll({ - fixId: "addMissingConstInForLoop", + fixId: "addMissingConst", fixAllDescription: "Add 'const' to all unresolved variables", newFileContent: `for (const x in []) {} diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts index 734b53ed4fb..c0f8b733aec 100644 --- a/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts @@ -4,7 +4,7 @@ ////for ([x] of [[1,2]]) {} verify.codeFixAll({ - fixId: "addMissingConstInForLoop", + fixId: "addMissingConst", fixAllDescription: "Add 'const' to all unresolved variables", newFileContent: `for (const [x, y] of [[1,2]]) {} diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts index 82cd2db6dc2..aa8ec294972 100644 --- a/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts +++ b/tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts @@ -4,7 +4,7 @@ ////for ({ x } of [{ x: 0 }]) { } verify.codeFixAll({ - fixId: "addMissingConstInForLoop", + fixId: "addMissingConst", fixAllDescription: "Add 'const' to all unresolved variables", newFileContent: `for (const { x, y } of [{ x: 0, y: 1 }]) { } diff --git a/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop2.ts b/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop2.ts index 3dbab65de32..24f8b48dfc9 100644 --- a/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop2.ts +++ b/tests/cases/fourslash/codeFixAddMissingConstInForOfLoop2.ts @@ -4,7 +4,7 @@ ////for (y of []) {} verify.codeFixAll({ - fixId: "addMissingConstInForLoop", + fixId: "addMissingConst", fixAllDescription: "Add 'const' to all unresolved variables", newFileContent: `for (const x of []) {} diff --git a/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring1.ts b/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring1.ts new file mode 100644 index 00000000000..8e40466c3ba --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring1.ts @@ -0,0 +1,8 @@ +/// + +////[x] = [0]; + +verify.codeFix({ + description: "Add 'const' to unresolved variable", + newFileContent: "const [x] = [0];" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring2.ts b/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring2.ts new file mode 100644 index 00000000000..57a89076cb2 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring2.ts @@ -0,0 +1,9 @@ +/// + +////[x, y] = [0, 1]; + +verify.codeFixAll({ + fixId: "addMissingConst", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: "const [x, y] = [0, 1];" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring3.ts b/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring3.ts new file mode 100644 index 00000000000..4099e1fe473 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring3.ts @@ -0,0 +1,6 @@ +/// + +////let x: any; +////[x, y] = [0, 1]; + +verify.not.codeFixAvailable(); diff --git a/tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier1.ts b/tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier1.ts new file mode 100644 index 00000000000..0a603dced30 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier1.ts @@ -0,0 +1,8 @@ +/// + +////x = 0; + +verify.codeFix({ + description: "Add 'const' to unresolved variable", + newFileContent: "const x = 0;" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier2.ts b/tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier2.ts new file mode 100644 index 00000000000..aa08f4e9196 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier2.ts @@ -0,0 +1,5 @@ +/// + +////x = 0, y = 0; + +verify.not.codeFixAvailable(); From 384669a1cefa5b1d68e07da1d016e506380b2b85 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Tue, 9 Jul 2019 01:56:50 +0200 Subject: [PATCH 11/14] Finish addMissingConst codefix for single variable and array literal assignments --- src/services/codefixes/addMissingConst.ts | 37 ++++++++++++------- src/services/textChanges.ts | 3 -- ...ixAddMissingConstPreservingIndentation1.ts | 19 ++++++++++ ...ixAddMissingConstPreservingIndentation2.ts | 19 ++++++++++ 4 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation1.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation2.ts diff --git a/src/services/codefixes/addMissingConst.ts b/src/services/codefixes/addMissingConst.ts index 857d7fb4282..fa353cc5b83 100644 --- a/src/services/codefixes/addMissingConst.ts +++ b/src/services/codefixes/addMissingConst.ts @@ -9,7 +9,7 @@ namespace ts.codefix { registerCodeFix({ errorCodes, getCodeActions: (context) => { - const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span.start)); + const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span.start, context.program)); if (changes.length > 0) { return [createCodeFixAction(fixId, changes, Diagnostics.Add_const_to_unresolved_variable, fixId, Diagnostics.Add_const_to_all_unresolved_variables)]; } @@ -17,27 +17,28 @@ namespace ts.codefix { fixIds: [fixId], getAllCodeActions: context => { const fixedNodes = new NodeSet(); - return codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag.start, fixedNodes)); + return codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag.start, context.program, fixedNodes)); }, }); - function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, fixedNodes?: NodeSet) { + function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, program: Program, fixedNodes?: NodeSet) { const token = getTokenAtPosition(sourceFile, pos); - const forInitializer = findAncestor(token, node => - isForInOrOfStatement(node.parent) ? node.parent.initializer === node - : isPossiblyPartOfDestructuring(node) ? false : "quit"); + isForInOrOfStatement(node.parent) ? node.parent.initializer === node : + isPossiblyPartOfDestructuring(node) ? false : "quit" + ); if (forInitializer) return applyChange(changeTracker, forInitializer, sourceFile, fixedNodes); const parent = token.parent; - const standaloneInitializer = isExpressionStatement(parent.parent); - if (standaloneInitializer) return applyChange(changeTracker, parent, sourceFile, fixedNodes); + if (isBinaryExpression(parent) && isExpressionStatement(parent.parent)) { + return applyChange(changeTracker, token, sourceFile, fixedNodes); + } - const arrayLiteralInitializer = isArrayLiteralExpression(token.parent); - if (arrayLiteralInitializer) { - const availableIdentifiers: string[] = []; // TODO: where to get/gather this information from? - const noIdentifiersDeclared = parent.forEachChild(node => availableIdentifiers.indexOf(node.getFullText()) < 0); - if (!noIdentifiersDeclared) return; + if (isArrayLiteralExpression(parent)) { + const checker = program.getTypeChecker(); + if (!every(parent.elements, element => arrayElementCouldBeVariableDeclaration(element, checker))) { + return; + } return applyChange(changeTracker, parent, sourceFile, fixedNodes); } @@ -45,7 +46,7 @@ namespace ts.codefix { function applyChange(changeTracker: textChanges.ChangeTracker, initializer: Node, sourceFile: SourceFile, fixedNodes?: NodeSet) { if (!fixedNodes || fixedNodes.tryAdd(initializer)) { - changeTracker.insertNodeBefore(sourceFile, initializer, createToken(SyntaxKind.ConstKeyword)); + changeTracker.insertModifierBefore(sourceFile, SyntaxKind.ConstKeyword, initializer); } } @@ -61,4 +62,12 @@ namespace ts.codefix { return false; } } + + function arrayElementCouldBeVariableDeclaration(expression: Expression, checker: TypeChecker) { + const identifier = + isIdentifier(expression) ? expression : + isAssignmentExpression(expression, /*excludeCompoundAssignment*/ true) && isIdentifier(expression.left) ? expression.left : + undefined; + return !!identifier && !checker.getSymbolAtLocation(identifier); + } } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 52dd9a2b9a6..f6af03aa2c5 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -425,9 +425,6 @@ namespace ts.textChanges { else if (isStringLiteral(before) && isImportDeclaration(before.parent) || isNamedImports(before)) { return { suffix: ", " }; } - else if (isForInitializer(before)) { - return { suffix: " " }; - } return Debug.failBadSyntaxKind(before); // We haven't handled this kind of node yet -- add it } diff --git a/tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation1.ts b/tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation1.ts new file mode 100644 index 00000000000..10818a621bb --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation1.ts @@ -0,0 +1,19 @@ +/// + +////a = () => { +//// x = 0; +//// [y] = [1]; +//// weirdlyIndented = 2; +////}; +////b = 3; + +verify.codeFixAll({ + fixId: "addMissingConst", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: `const a = () => { + const x = 0; + const [y] = [1]; + const weirdlyIndented = 2; +}; +const b = 3;` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation2.ts b/tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation2.ts new file mode 100644 index 00000000000..1411dc357e1 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation2.ts @@ -0,0 +1,19 @@ +/// + +////a = () => { +//// for (x in []) { +//// y = 0; +//// } +////}; +////b = 3; + +verify.codeFixAll({ + fixId: "addMissingConst", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: `const a = () => { + for (const x in []) { + const y = 0; + } +}; +const b = 3;` +}); From 1de7881141d8ef7556331f23d7d9f315a062cc53 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Tue, 9 Jul 2019 02:07:54 +0200 Subject: [PATCH 12/14] Add negative test case for addMissingConst codeFix with unexpected array elements --- .../fourslash/codeFixAddMissingConstToArrayDestructuring4.ts | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring4.ts diff --git a/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring4.ts b/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring4.ts new file mode 100644 index 00000000000..2c077e4c149 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToArrayDestructuring4.ts @@ -0,0 +1,5 @@ +/// + +////[x, y()] = [0, () => 1]; + +verify.not.codeFixAvailable(); From f273448925f5dd40136df414ecdc7fad56614d96 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Wed, 10 Jul 2019 02:11:02 +0200 Subject: [PATCH 13/14] Added addMissingConst codefix for comma separated initializers --- src/services/codefixes/addMissingConst.ts | 38 ++++++++++++++++++- ...issingConstToCommaSeparatedInitializer1.ts | 9 +++++ ...issingConstToCommaSeparatedInitializer2.ts | 11 ++++++ ...issingConstToCommaSeparatedInitializer3.ts | 15 ++++++++ ...ssingConstToCommaSeparatedInitializer4.ts} | 1 + 5 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer1.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer2.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer3.ts rename tests/cases/fourslash/{codeFixAddMissingConstToStandaloneIdentifier2.ts => codeFixAddMissingConstToCommaSeparatedInitializer4.ts} (80%) diff --git a/src/services/codefixes/addMissingConst.ts b/src/services/codefixes/addMissingConst.ts index fa353cc5b83..10dba5d9444 100644 --- a/src/services/codefixes/addMissingConst.ts +++ b/src/services/codefixes/addMissingConst.ts @@ -42,6 +42,19 @@ namespace ts.codefix { return applyChange(changeTracker, parent, sourceFile, fixedNodes); } + + const commaExpression = findAncestor(token, node => + isExpressionStatement(node.parent) ? true : + isPossiblyPartOfCommaSeperatedInitializer(node) ? false : "quit" + ); + if (commaExpression) { + const checker = program.getTypeChecker(); + if (!expressionCouldBeVariableDeclaration(commaExpression, checker)) { + return; + } + + return applyChange(changeTracker, commaExpression, sourceFile, fixedNodes); + } } function applyChange(changeTracker: textChanges.ChangeTracker, initializer: Node, sourceFile: SourceFile, fixedNodes?: NodeSet) { @@ -63,11 +76,34 @@ namespace ts.codefix { } } - function arrayElementCouldBeVariableDeclaration(expression: Expression, checker: TypeChecker) { + function arrayElementCouldBeVariableDeclaration(expression: Expression, checker: TypeChecker): boolean { const identifier = isIdentifier(expression) ? expression : isAssignmentExpression(expression, /*excludeCompoundAssignment*/ true) && isIdentifier(expression.left) ? expression.left : undefined; return !!identifier && !checker.getSymbolAtLocation(identifier); } + + function isPossiblyPartOfCommaSeperatedInitializer(node: Node): boolean { + switch (node.kind) { + case SyntaxKind.Identifier: + case SyntaxKind.BinaryExpression: + case SyntaxKind.CommaToken: + return true; + default: + return false; + } + } + + function expressionCouldBeVariableDeclaration(expression: Node, checker: TypeChecker): boolean { + if (!isBinaryExpression(expression)) { + return false; + } + + if (expression.operatorToken.kind === SyntaxKind.CommaToken) { + return every([expression.left, expression.right], expression => expressionCouldBeVariableDeclaration(expression, checker)); + } + + return isIdentifier(expression.left) && !checker.getSymbolAtLocation(expression.left); + } } diff --git a/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer1.ts b/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer1.ts new file mode 100644 index 00000000000..c5bb309698d --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer1.ts @@ -0,0 +1,9 @@ +/// + +////x = 0, y = 0; + +verify.codeFixAll({ + fixId: "addMissingConst", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: "const x = 0, y = 0;" +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer2.ts b/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer2.ts new file mode 100644 index 00000000000..1a60b16489d --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer2.ts @@ -0,0 +1,11 @@ +/// + +////x = 0, +////y = 0; + +verify.codeFixAll({ + fixId: "addMissingConst", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: `const x = 0, +y = 0;` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer3.ts b/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer3.ts new file mode 100644 index 00000000000..d39388af335 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer3.ts @@ -0,0 +1,15 @@ +/// + +////function f() { return 42; } +////x = 0, y = f() +//// +////, z = 0; + +verify.codeFixAll({ + fixId: "addMissingConst", + fixAllDescription: "Add 'const' to all unresolved variables", + newFileContent: `function f() { return 42; } +const x = 0, y = f() + +, z = 0;` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier2.ts b/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer4.ts similarity index 80% rename from tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier2.ts rename to tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer4.ts index aa08f4e9196..263ed33231a 100644 --- a/tests/cases/fourslash/codeFixAddMissingConstToStandaloneIdentifier2.ts +++ b/tests/cases/fourslash/codeFixAddMissingConstToCommaSeparatedInitializer4.ts @@ -1,5 +1,6 @@ /// +////let y: any; ////x = 0, y = 0; verify.not.codeFixAvailable(); From 196db5bbcb6043c08526fc1821aa9e16f82c8a12 Mon Sep 17 00:00:00 2001 From: Florian Regensburger Date: Wed, 10 Jul 2019 02:35:40 +0200 Subject: [PATCH 14/14] Fixed lint error --- src/services/codefixes/addMissingConst.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/codefixes/addMissingConst.ts b/src/services/codefixes/addMissingConst.ts index 10dba5d9444..37739d01eee 100644 --- a/src/services/codefixes/addMissingConst.ts +++ b/src/services/codefixes/addMissingConst.ts @@ -99,7 +99,7 @@ namespace ts.codefix { if (!isBinaryExpression(expression)) { return false; } - + if (expression.operatorToken.kind === SyntaxKind.CommaToken) { return every([expression.left, expression.right], expression => expressionCouldBeVariableDeclaration(expression, checker)); }