Skip to content

Commit bcc93f2

Browse files
committed
Loosen restriction on usages in innermost scope
Now that we're smarter about where we declare the extracted local, we can ignore usage problems in the innermost script - it'll always have access to the same symbols as the extracted expression.
1 parent c08308a commit bcc93f2

File tree

4 files changed

+41
-17
lines changed

4 files changed

+41
-17
lines changed

src/harness/unittests/extractConstants.ts

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,6 @@ namespace ts {
9191
// export const y = [#|j * j|];
9292
// }`);
9393

94-
testExtractConstantFailed("extractConstant_BlockScopes_Dependencies",
95-
`for (let i = 0; i < 10; i++) {
96-
for (let j = 0; j < 10; j++) {
97-
let x = [#|i + 1|];
98-
}
99-
}`);
100-
10194
testExtractConstant("extractConstant_VariableList_const",
10295
`const a = 1, b = [#|a + 1|];`);
10396

@@ -110,10 +103,7 @@ namespace ts {
110103
`const /*About A*/a = 1,
111104
/*About B*/b = [#|a + 1|];`);
112105

113-
// NOTE: this test isn't normative - it just documents our sub-optimal behavior.
114-
// `i` doesn't bind in the target scope (file-level), so the extraction is disallowed.
115-
// TODO (17098): should probably allow extraction into the same scope
116-
testExtractConstantFailed("extractConstant_BlockScopeMismatch", `
106+
testExtractConstant("extractConstant_BlockScopeMismatch", `
117107
for (let i = 0; i < 10; i++) {
118108
for (let j = 0; j < 10; j++) {
119109
const x = [#|i + 1|];
@@ -205,7 +195,6 @@ const x = [#|2 + 1|];
205195
const x = [#|2 + 1|];
206196
`);
207197

208-
// NOTE: this test isn't normative - it just documents our sub-optimal behavior.
209198
testExtractConstant("extractConstant_PinnedCommentAndDocComment", `
210199
/*! Copyright */
211200
@@ -217,8 +206,4 @@ const x = [#|2 + 1|];
217206
function testExtractConstant(caption: string, text: string) {
218207
testExtractSymbol(caption, text, "extractConstant", Diagnostics.Extract_constant);
219208
}
220-
221-
function testExtractConstantFailed(caption: string, text: string) {
222-
testExtractSymbolFailed(caption, text, Diagnostics.Extract_constant);
223-
}
224209
}

src/services/refactors/extractSymbol.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1349,7 +1349,10 @@ namespace ts.refactor.extractSymbol {
13491349
for (let i = 0; i < scopes.length; i++) {
13501350
if (!isReadonlyArray(targetRange.range)) {
13511351
const scopeUsages = usagesPerScope[i];
1352-
if (scopeUsages.usages.size > 0 || scopeUsages.typeParameterUsages.size > 0) {
1352+
// Special case: in the innermost scope, all usages are available.
1353+
// (The computed value reflects the value at the top-level of the scope, but the
1354+
// local will actually be declared at the same level as the extracted expression).
1355+
if (i > 0 && (scopeUsages.usages.size > 0 || scopeUsages.typeParameterUsages.size > 0)) {
13531356
constantErrorsPerScope[i].push(createDiagnosticForNode(targetRange.range, Messages.CannotAccessVariablesFromNestedScopes));
13541357
}
13551358
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// ==ORIGINAL==
2+
3+
for (let i = 0; i < 10; i++) {
4+
for (let j = 0; j < 10; j++) {
5+
const x = i + 1;
6+
}
7+
}
8+
9+
// ==SCOPE::Extract to constant in global scope==
10+
11+
for (let i = 0; i < 10; i++) {
12+
for (let j = 0; j < 10; j++) {
13+
const newLocal = i + 1;
14+
15+
const x = /*RENAME*/newLocal;
16+
}
17+
}
18+
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// ==ORIGINAL==
2+
3+
for (let i = 0; i < 10; i++) {
4+
for (let j = 0; j < 10; j++) {
5+
const x = i + 1;
6+
}
7+
}
8+
9+
// ==SCOPE::Extract to constant in global scope==
10+
11+
for (let i = 0; i < 10; i++) {
12+
for (let j = 0; j < 10; j++) {
13+
const newLocal = i + 1;
14+
15+
const x = /*RENAME*/newLocal;
16+
}
17+
}
18+

0 commit comments

Comments
 (0)