Merge pull request #18831 from amcasey/AdditionSpecialCase

Eliminate special case for extracting from a binary operator chain

Author: amcasey

src/harness/unittests/extractFunctions.ts

@@ -109,16 +109,6 @@ namespace ts {
             return C.foo();|]
         }
     }
-}`);
-        testExtractFunction("extractFunction8",
-            `namespace A {
-    let x = 1;
-    namespace B {
-        function a() {
-            let a1 = 1;
-            return 1 + [#|a1 + x|] + 100;
-        }
-    }
 }`);
         testExtractFunction("extractFunction9",
             `namespace A {

src/harness/unittests/extractRanges.ts

@@ -152,18 +152,6 @@ namespace ts {
                     }
                 }
             `);
-            testExtractRange(`
-                function f() {
-                    return [$|1 + [#|2 + 3|]|];
-                    }
-                }
-            `);
-            testExtractRange(`
-                function f() {
-                    return [$|1 + 2 + [#|3 + 4|]|];
-                    }
-                }
-            `);
         });
 
         testExtractRangeFailed("extractRangeFailed1",
@@ -311,7 +299,18 @@ switch (x) {
         testExtractRangeFailed("extractRangeFailed9",
         `var x = ([#||]1 + 2);`,
         [
-            "Cannot extract empty range."
+            refactor.extractSymbol.Messages.CannotExtractEmpty.message
+        ]);
+
+        testExtractRangeFailed("extractRangeFailed10",
+        `
+            function f() {
+                return 1 + [#|2 + 3|];
+                }
+            }
+        `,
+        [
+            refactor.extractSymbol.Messages.CannotExtractRange.message
         ]);
 
         testExtractRangeFailed("extract-method-not-for-token-expression-statement", `[#|a|]`, [refactor.extractSymbol.Messages.CannotExtractIdentifier.message]);

src/services/refactors/extractSymbol.ts

@@ -201,9 +201,9 @@ namespace ts.refactor.extractSymbol {
 
         // Walk up starting from the the start position until we find a non-SourceFile node that subsumes the selected span.
         // This may fail (e.g. you select two statements in the root of a source file)
-        let start = getParentNodeInSpan(getTokenAtPosition(sourceFile, span.start, /*includeJsDocComment*/ false), sourceFile, span);
+        const start = getParentNodeInSpan(getTokenAtPosition(sourceFile, span.start, /*includeJsDocComment*/ false), sourceFile, span);
         // Do the same for the ending position
-        let end = getParentNodeInSpan(findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span)), sourceFile, span);
+        const end = getParentNodeInSpan(findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span)), sourceFile, span);
 
         const declarations: Symbol[] = [];
 
@@ -217,31 +217,10 @@ namespace ts.refactor.extractSymbol {
         }
 
         if (start.parent !== end.parent) {
-            // handle cases like 1 + [2 + 3] + 4
-            // user selection is marked with [].
-            // in this case 2 + 3 does not belong to the same tree node
-            // instead the shape of the tree looks like this:
-            //          +
-            //         / \
-            //        +   4
-            //       / \
-            //      +   3
-            //     / \
-            //    1   2
-            // in this case there is no such one node that covers ends of selection and is located inside the selection
-            // to handle this we check if both start and end of the selection belong to some binary operation
-            // and start node is parented by the parent of the end node
-            // if this is the case - expand the selection to the entire parent of end node (in this case it will be [1 + 2 + 3] + 4)
-            const startParent = skipParentheses(start.parent);
-            const endParent = skipParentheses(end.parent);
-            if (isBinaryExpression(startParent) && isBinaryExpression(endParent) && isNodeDescendantOf(startParent, endParent)) {
-                start = end = endParent;
-            }
-            else {
-                // start and end nodes belong to different subtrees
-                return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.CannotExtractRange)] };
-            }
+            // start and end nodes belong to different subtrees
+            return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.CannotExtractRange)] };
         }
+
         if (start !== end) {
             // start and end should be statements and parent should be either block or a source file
             if (!isBlockLike(start.parent)) {