Finish addMissingConst codefix for single variable and array literal assignments

Author: rFlorian

src/services/codefixes/addMissingConst.ts

@@ -9,43 +9,44 @@ 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)];
             }
         },
         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<Node>) {
+    function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, program: Program, fixedNodes?: NodeSet<Node>) {
         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);
         }
     }
 
     function applyChange(changeTracker: textChanges.ChangeTracker, initializer: Node, sourceFile: SourceFile, fixedNodes?: NodeSet<Node>) {
         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);
+    }
 }

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
         }
 

tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation1.ts

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

tests/cases/fourslash/codeFixAddMissingConstPreservingIndentation2.ts

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