diff --git a/src/harness/unittests/extractConstants.ts b/src/harness/unittests/extractConstants.ts index d37f9b2036..179affd86c 100644 --- a/src/harness/unittests/extractConstants.ts +++ b/src/harness/unittests/extractConstants.ts @@ -91,13 +91,6 @@ namespace ts { // export const y = [#|j * j|]; // }`); - testExtractConstantFailed("extractConstant_BlockScopes_Dependencies", - `for (let i = 0; i < 10; i++) { - for (let j = 0; j < 10; j++) { - let x = [#|i + 1|]; - } -}`); - testExtractConstant("extractConstant_VariableList_const", `const a = 1, b = [#|a + 1|];`); @@ -110,10 +103,7 @@ namespace ts { `const /*About A*/a = 1, /*About B*/b = [#|a + 1|];`); - // NOTE: this test isn't normative - it just documents our sub-optimal behavior. - // `i` doesn't bind in the target scope (file-level), so the extraction is disallowed. - // TODO (17098): should probably allow extraction into the same scope - testExtractConstantFailed("extractConstant_BlockScopeMismatch", ` + testExtractConstant("extractConstant_BlockScopeMismatch", ` for (let i = 0; i < 10; i++) { for (let j = 0; j < 10; j++) { const x = [#|i + 1|]; @@ -205,7 +195,6 @@ const x = [#|2 + 1|]; const x = [#|2 + 1|]; `); - // NOTE: this test isn't normative - it just documents our sub-optimal behavior. testExtractConstant("extractConstant_PinnedCommentAndDocComment", ` /*! Copyright */ @@ -217,8 +206,4 @@ const x = [#|2 + 1|]; function testExtractConstant(caption: string, text: string) { testExtractSymbol(caption, text, "extractConstant", Diagnostics.Extract_constant); } - - function testExtractConstantFailed(caption: string, text: string) { - testExtractSymbolFailed(caption, text, Diagnostics.Extract_constant); - } } diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 549bf653e9..352ab91c6b 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -1349,7 +1349,10 @@ namespace ts.refactor.extractSymbol { for (let i = 0; i < scopes.length; i++) { if (!isReadonlyArray(targetRange.range)) { const scopeUsages = usagesPerScope[i]; - if (scopeUsages.usages.size > 0 || scopeUsages.typeParameterUsages.size > 0) { + // 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)); } } diff --git a/tests/baselines/reference/extractConstant/extractConstant_BlockScopeMismatch.js b/tests/baselines/reference/extractConstant/extractConstant_BlockScopeMismatch.js new file mode 100644 index 0000000000..64ec315b39 --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_BlockScopeMismatch.js @@ -0,0 +1,18 @@ +// ==ORIGINAL== + +for (let i = 0; i < 10; i++) { + for (let j = 0; j < 10; j++) { + const x = i + 1; + } +} + +// ==SCOPE::Extract to constant in global scope== + +for (let i = 0; i < 10; i++) { + for (let j = 0; j < 10; j++) { + const newLocal = i + 1; + + const x = /*RENAME*/newLocal; + } +} + \ No newline at end of file diff --git a/tests/baselines/reference/extractConstant/extractConstant_BlockScopeMismatch.ts b/tests/baselines/reference/extractConstant/extractConstant_BlockScopeMismatch.ts new file mode 100644 index 0000000000..64ec315b39 --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_BlockScopeMismatch.ts @@ -0,0 +1,18 @@ +// ==ORIGINAL== + +for (let i = 0; i < 10; i++) { + for (let j = 0; j < 10; j++) { + const x = i + 1; + } +} + +// ==SCOPE::Extract to constant in global scope== + +for (let i = 0; i < 10; i++) { + for (let j = 0; j < 10; j++) { + const newLocal = i + 1; + + const x = /*RENAME*/newLocal; + } +} + \ No newline at end of file