diff --git a/src/harness/unittests/extractConstants.ts b/src/harness/unittests/extractConstants.ts index 28109a6fa4d..63a150856f5 100644 --- a/src/harness/unittests/extractConstants.ts +++ b/src/harness/unittests/extractConstants.ts @@ -33,6 +33,20 @@ namespace ts { testExtractConstant("extractConstant_ExpressionStatementExpression", `[#|"hello"|];`); + testExtractConstant("extractConstant_ExpressionStatementInNestedScope", ` +let i = 0; +function F() { + [#|i++|]; +} + `); + + testExtractConstant("extractConstant_ExpressionStatementConsumesLocal", ` +function F() { + let i = 0; + [#|i++|]; +} + `); + testExtractConstant("extractConstant_BlockScopes_NoDependencies", `for (let i = 0; i < 10; i++) { for (let j = 0; j < 10; j++) { diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 00cb09412bd..d12e678dcec 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -911,12 +911,13 @@ namespace ts.refactor.extractSymbol { const localReference = createIdentifier(localNameText); changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference); } - else if (node.parent.kind === SyntaxKind.ExpressionStatement) { - // If the parent is an expression statement, replace the statement with the declaration. + else if (node.parent.kind === SyntaxKind.ExpressionStatement && scope === findAncestor(node, isScope)) { + // If the parent is an expression statement and the target scope is the immediately enclosing one, + // replace the statement with the declaration. const newVariableStatement = createVariableStatement( /*modifiers*/ undefined, createVariableDeclarationList([newVariableDeclaration], NodeFlags.Const)); - changeTracker.replaceNodeWithNodes(context.file, node.parent, [newVariableStatement], { nodeSeparator: context.newLineCharacter }); + changeTracker.replaceRange(context.file, { pos: node.parent.getStart(), end: node.parent.end }, newVariableStatement); } else { const newVariableStatement = createVariableStatement( @@ -940,8 +941,14 @@ namespace ts.refactor.extractSymbol { } // Consume - const localReference = createIdentifier(localNameText); - changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference); + if (node.parent.kind === SyntaxKind.ExpressionStatement) { + // If the parent is an expression statement, delete it. + changeTracker.deleteRange(context.file, { pos: node.parent.getStart(), end: node.parent.end }); + } + else { + const localReference = createIdentifier(localNameText); + changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference); + } } } @@ -1344,14 +1351,13 @@ namespace ts.refactor.extractSymbol { } for (let i = 0; i < scopes.length; i++) { - if (!isReadonlyArray(targetRange.range)) { - const scopeUsages = usagesPerScope[i]; - // Special case: in the innermost scope, all usages are available. - // (The computed value reflects the value at the top-level of the scope, but the - // local will actually be declared at the same level as the extracted expression). - if (i > 0 && (scopeUsages.usages.size > 0 || scopeUsages.typeParameterUsages.size > 0)) { - constantErrorsPerScope[i].push(createDiagnosticForNode(targetRange.range, Messages.CannotAccessVariablesFromNestedScopes)); - } + const scopeUsages = usagesPerScope[i]; + // Special case: in the innermost scope, all usages are available. + // (The computed value reflects the value at the top-level of the scope, but the + // local will actually be declared at the same level as the extracted expression). + if (i > 0 && (scopeUsages.usages.size > 0 || scopeUsages.typeParameterUsages.size > 0)) { + const errorNode = isReadonlyArray(targetRange.range) ? targetRange.range[0] : targetRange.range; + constantErrorsPerScope[i].push(createDiagnosticForNode(errorNode, Messages.CannotAccessVariablesFromNestedScopes)); } let hasWrite = false; diff --git a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementConsumesLocal.js b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementConsumesLocal.js new file mode 100644 index 00000000000..b57da0d8e8c --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementConsumesLocal.js @@ -0,0 +1,14 @@ +// ==ORIGINAL== + +function F() { + let i = 0; + i++; +} + +// ==SCOPE::Extract to constant in enclosing scope== + +function F() { + let i = 0; + const /*RENAME*/newLocal = i++; +} + \ No newline at end of file diff --git a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementConsumesLocal.ts b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementConsumesLocal.ts new file mode 100644 index 00000000000..b57da0d8e8c --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementConsumesLocal.ts @@ -0,0 +1,14 @@ +// ==ORIGINAL== + +function F() { + let i = 0; + i++; +} + +// ==SCOPE::Extract to constant in enclosing scope== + +function F() { + let i = 0; + const /*RENAME*/newLocal = i++; +} + \ No newline at end of file diff --git a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.js b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.js new file mode 100644 index 00000000000..f76af9e9390 --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.js @@ -0,0 +1,23 @@ +// ==ORIGINAL== + +let i = 0; +function F() { + i++; +} + +// ==SCOPE::Extract to constant in enclosing scope== + +let i = 0; +function F() { + const /*RENAME*/newLocal = i++; +} + +// ==SCOPE::Extract to constant in global scope== + +let i = 0; +const /*RENAME*/newLocal = i++; + +function F() { + +} + \ No newline at end of file diff --git a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.ts b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.ts new file mode 100644 index 00000000000..f76af9e9390 --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.ts @@ -0,0 +1,23 @@ +// ==ORIGINAL== + +let i = 0; +function F() { + i++; +} + +// ==SCOPE::Extract to constant in enclosing scope== + +let i = 0; +function F() { + const /*RENAME*/newLocal = i++; +} + +// ==SCOPE::Extract to constant in global scope== + +let i = 0; +const /*RENAME*/newLocal = i++; + +function F() { + +} + \ No newline at end of file