Review changes and adjustment of fourslash tests

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
     }

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 = <Node>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;
+            });
+        });
     }
 }

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
         }
 

tests/cases/fourslash/codeFixAddMissingConstInForInLoop1.ts

@@ -0,0 +1,8 @@
+/// <reference path='fourslash.ts' />
+
+////for (x in []) {}
+
+verify.codeFix({
+    description: "Add 'const' to unresolved variable",
+    newFileContent: "for (const x in []) {}"
+});

tests/cases/fourslash/codeFixAddMissingConstInForInLoop2.ts

@@ -0,0 +1,12 @@
+/// <reference path='fourslash.ts' />
+
+////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 []) {}`
+});

tests/cases/fourslash/codeFixAddMissingConstInForLoop1.ts

@@ -1,8 +0,0 @@
-/// <reference path='fourslash.ts' />
-
-////[|for (x of []) {}|]
-
-verify.codeFix({
-    description: "Add const modifier to unresolved variable",
-    newRangeContent: "for (const x of []) {}"
-});

tests/cases/fourslash/codeFixAddMissingConstInForLoop3.ts

@@ -1,8 +0,0 @@
-/// <reference path='fourslash.ts' />
-
-////[|for ([x] of [[1,2]]) {}|]
-
-verify.codeFix({
-    description: "Add const modifier to unresolved variable",
-    newRangeContent: "for (const [x] of [[1,2]]) {}"
-});

tests/cases/fourslash/codeFixAddMissingConstInForLoop4.ts

@@ -1,8 +0,0 @@
-/// <reference path='fourslash.ts' />
-
-////[|for ([x, y] of [[1,2]]) {}|]
-
-verify.codeFix({
-    description: "Add const modifier to unresolved variable",
-    newRangeContent: "for (const [x, y] of [[1,2]]) {}"
-});

tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring1.ts

@@ -0,0 +1,8 @@
+/// <reference path='fourslash.ts' />
+
+////for ([x] of [[1,2]]) {}
+
+verify.codeFix({
+    description: "Add 'const' to unresolved variable",
+    newFileContent: "for (const [x] of [[1,2]]) {}"
+});

tests/cases/fourslash/codeFixAddMissingConstInForLoopWithArrayDestructuring2.ts

@@ -0,0 +1,12 @@
+/// <reference path='fourslash.ts' />
+
+////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]]) {}`
+});

tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring1.ts

@@ -0,0 +1,8 @@
+/// <reference path='fourslash.ts' />
+
+////for ({ x } of [{ x: 0 }]) { }
+
+verify.codeFix({
+    description: "Add 'const' to unresolved variable",
+    newFileContent: "for (const { x } of [{ x: 0 }]) { }"
+});

tests/cases/fourslash/codeFixAddMissingConstInForLoopWithObjectDestructuring2.ts

@@ -0,0 +1,12 @@
+/// <reference path='fourslash.ts' />
+
+////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 }]) { }`
+});

tests/cases/fourslash/codeFixAddMissingConstInForOfLoop1.ts

@@ -0,0 +1,8 @@
+/// <reference path='fourslash.ts' />
+
+////for (x of []) {}
+
+verify.codeFix({
+    description: "Add 'const' to unresolved variable",
+    newFileContent: "for (const x of []) {}"
+});

tests/cases/fourslash/codeFixAddMissingConstInForOfLoop2.ts

@@ -1,11 +1,11 @@
 /// <reference path='fourslash.ts' />
 
-////[|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 []) {}`

tests/cases/fourslash/codeFixAwaitInSyncFunction4.ts

@@ -1,7 +1,7 @@
 /// <reference path='fourslash.ts' />
 
 ////class Foo {
-////    constructor {
+////    constructor() {
 ////        await Promise.resolve();
 ////    }
 ////}

tests/cases/fourslash/codeFixAwaitInSyncFunction7.ts

@@ -1,7 +1,7 @@
 /// <reference path='fourslash.ts' />
 
 ////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);
     }
 }`,

tests/cases/fourslash/codeFixAwaitShouldNotCrashIfNotInFunction.ts

@@ -1,5 +1,6 @@
 /// <reference path='fourslash.ts' />
 
+////async function a() {}
 ////await a
 
 verify.not.codeFixAvailable();

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();
 

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",

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;

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 [{}]) {}`,
 });

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 { } = [{}];`,
 });

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" }
+]);