Handle extraction ranges including case clause expressions

amcasey · amcasey · commit 2f3b06a3cd5c · 2018-01-23T11:09:27.000-08:00
(mostly by rejecting them) Fixes microsoft#20559
diff --git 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]);
     });
 }
diff --git 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 } };
         }
 

Original file line number	Diff line number	Diff line change
`@@ -235,6 +235,17 @@ namespace ts.refactor.extractSymbol {`
`235`	`235`	`break;`
`236`	`236`	`}`
`237`	`237`	`}`
	`238`	`+`
	`239`	`+ if (!statements.length) {`
	`240`	`+ // https://github.com/Microsoft/TypeScript/issues/20559`
	`241`	+ // Ranges like [\|case 1: break;\|] will fail to populate `statements` because
	`242`	+ // they will never find `start` in `start.parent.statements`.
	`243`	`+ // Consider: We could support ranges like [\|case 1:\|] by refining them to just`
	`244`	`+ // the expression.`
	`245`	`+ Debug.assert(isCaseClause(start.parent) && span.start < start.parent.expression.end);`
	`246`	`+ return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractRange)] };`
	`247`	`+ }`
	`248`	`+`
`238`	`249`	`return { targetRange: { range: statements, facts: rangeFacts, declarations } };`
`239`	`250`	`}`
`240`	`251`