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