From 7f347025de951042ec4543eaede107721b775687 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 23 Jan 2018 11:07:59 -0800 Subject: [PATCH 1/2] Handle extraction ranges including case clause expressions (mostly by rejecting them) Fixes #20559 --- src/harness/unittests/extractRanges.ts | 46 +++++++++++++++++++++++++ src/services/refactors/extractSymbol.ts | 11 ++++++ 2 files changed, 57 insertions(+) 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..efdf9b233a9 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 } }; } From f7f81254d3499f84c35229ab8dfe77fa6e27fa4c Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 24 Jan 2018 12:46:13 -0800 Subject: [PATCH 2/2] Remove incorrect assert --- src/harness/unittests/extractRanges.ts | 6 ++++++ src/services/refactors/extractSymbol.ts | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/harness/unittests/extractRanges.ts b/src/harness/unittests/extractRanges.ts index 2810f323abf..00fbf334b38 100644 --- a/src/harness/unittests/extractRanges.ts +++ b/src/harness/unittests/extractRanges.ts @@ -411,6 +411,12 @@ switch (x) { refactor.extractSymbol.Messages.cannotExtractRange.message ]); + testExtractRangeFailed("extractRangeFailed18", + `[#|{ 1;|] }`, + [ + 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 efdf9b233a9..3f79a0755e1 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -242,7 +242,6 @@ namespace ts.refactor.extractSymbol { // 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)] }; }