Merge pull request #21370 from amcasey/GH20559

Handle case clause corner cases in extract symbol

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;
+}
 `);
     });
 

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]);
     });
 }

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;

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;
+}

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;
+}