diff --git a/src/harness/unittests/extractConstants.ts b/src/harness/unittests/extractConstants.ts index 71b550cba8f..9dfb1264a70 100644 --- a/src/harness/unittests/extractConstants.ts +++ b/src/harness/unittests/extractConstants.ts @@ -273,6 +273,13 @@ let i: I = [#|{ a: 1 }|]; const myObj: { member(x: number, y: string): void } = { member: [#|(x, y) => x + y|], } +`); + + testExtractConstant("extractConstant_CaseClauseExpression", ` +switch (1) { + case [#|1|]: + break; +} `); }); diff --git a/src/harness/unittests/extractRanges.ts b/src/harness/unittests/extractRanges.ts index 493c9639c3d..2810f323abf 100644 --- a/src/harness/unittests/extractRanges.ts +++ b/src/harness/unittests/extractRanges.ts @@ -365,6 +365,52 @@ switch (x) { refactor.extractSymbol.Messages.cannotExtractRange.message ]); + testExtractRangeFailed("extractRangeFailed14", + ` + switch(1) { + case [#|1: + break;|] + } + `, + [ + refactor.extractSymbol.Messages.cannotExtractRange.message + ]); + + testExtractRangeFailed("extractRangeFailed15", + ` + switch(1) { + case [#|1: + break|]; + } + `, + [ + refactor.extractSymbol.Messages.cannotExtractRange.message + ]); + + // Documentation only - it would be nice if the result were [$|1|] + testExtractRangeFailed("extractRangeFailed16", + ` + switch(1) { + [#|case 1|]: + break; + } + `, + [ + refactor.extractSymbol.Messages.cannotExtractRange.message + ]); + + // Documentation only - it would be nice if the result were [$|1|] + testExtractRangeFailed("extractRangeFailed17", + ` + switch(1) { + [#|case 1:|] + break; + } + `, + [ + refactor.extractSymbol.Messages.cannotExtractRange.message + ]); + testExtractRangeFailed("extract-method-not-for-token-expression-statement", `[#|a|]`, [refactor.extractSymbol.Messages.cannotExtractIdentifier.message]); }); } \ No newline at end of file diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 479c387fc65..e8a2186aa8b 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -235,6 +235,17 @@ namespace ts.refactor.extractSymbol { break; } } + + if (!statements.length) { + // https://github.com/Microsoft/TypeScript/issues/20559 + // Ranges like [|case 1: break;|] will fail to populate `statements` because + // they will never find `start` in `start.parent.statements`. + // Consider: We could support ranges like [|case 1:|] by refining them to just + // the expression. + Debug.assert(isCaseClause(start.parent) && span.start < start.parent.expression.end); + return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractRange)] }; + } + return { targetRange: { range: statements, facts: rangeFacts, declarations } }; } @@ -1321,6 +1332,13 @@ namespace ts.refactor.extractSymbol { } prevStatement = statement; } + + if (!prevStatement && isCaseClause(curr)) { + // We must have been in the expression of the case clause. + Debug.assert(isSwitchStatement(curr.parent.parent)); + return curr.parent.parent; + } + // There must be at least one statement since we started in one. Debug.assert(prevStatement !== undefined); return prevStatement; diff --git a/tests/baselines/reference/extractConstant/extractConstant_CaseClauseExpression.js b/tests/baselines/reference/extractConstant/extractConstant_CaseClauseExpression.js new file mode 100644 index 00000000000..0031ecdf0f9 --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_CaseClauseExpression.js @@ -0,0 +1,13 @@ +// ==ORIGINAL== + +switch (1) { + case /*[#|*/1/*|]*/: + break; +} + +// ==SCOPE::Extract to constant in enclosing scope== +const newLocal = 1; +switch (1) { + case /*RENAME*/newLocal: + break; +} diff --git a/tests/baselines/reference/extractConstant/extractConstant_CaseClauseExpression.ts b/tests/baselines/reference/extractConstant/extractConstant_CaseClauseExpression.ts new file mode 100644 index 00000000000..0031ecdf0f9 --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_CaseClauseExpression.ts @@ -0,0 +1,13 @@ +// ==ORIGINAL== + +switch (1) { + case /*[#|*/1/*|]*/: + break; +} + +// ==SCOPE::Extract to constant in enclosing scope== +const newLocal = 1; +switch (1) { + case /*RENAME*/newLocal: + break; +}