Merge pull request #18931 from amcasey/ExtractConstantThis

Allow Extract Constant into enclosing scope in spite of RangeFacts.UsesThis

Author: amcasey

src/harness/unittests/extractConstants.ts

@@ -230,6 +230,30 @@ function f(): void { }
         testExtractConstantFailed("extractConstant_Never", `
 function f(): never { }
 [#|f();|]`);
+
+        testExtractConstant("extractConstant_This_Constructor", `
+class C {
+    constructor() {
+        [#|this.m2()|];
+    }
+    m2() { return 1; }
+}`);
+
+        testExtractConstant("extractConstant_This_Method", `
+class C {
+    m1() {
+        [#|this.m2()|];
+    }
+    m2() { return 1; }
+}`);
+
+        testExtractConstant("extractConstant_This_Property", `
+namespace N { // Force this test to be TS-only
+    class C {
+        x = 1;
+        y = [#|this.x|];
+    }
+}`);
     });
 
     function testExtractConstant(caption: string, text: string) {

src/services/refactors/extractSymbol.ts

@@ -476,7 +476,10 @@ namespace ts.refactor.extractSymbol {
             // if range uses this as keyword or as type inside the class then it can only be extracted to a method of the containing class
             const containingClass = getContainingClass(current);
             if (containingClass) {
-                return [containingClass];
+                const containingFunction = findAncestor(current, isFunctionLikeDeclaration);
+                return containingFunction
+                    ? [containingFunction, containingClass]
+                    : [containingClass];
             }
         }
 

tests/baselines/reference/extractConstant/extractConstant_This_Constructor.js

@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+
+class C {
+    constructor() {
+        /*[#|*/this.m2()/*|]*/;
+    }
+    m2() { return 1; }
+}
+// ==SCOPE::Extract to constant in enclosing scope==
+
+class C {
+    constructor() {
+        const /*RENAME*/newLocal = this.m2();
+    }
+    m2() { return 1; }
+}

tests/baselines/reference/extractConstant/extractConstant_This_Constructor.ts

@@ -0,0 +1,26 @@
+// ==ORIGINAL==
+
+class C {
+    constructor() {
+        /*[#|*/this.m2()/*|]*/;
+    }
+    m2() { return 1; }
+}
+// ==SCOPE::Extract to constant in enclosing scope==
+
+class C {
+    constructor() {
+        const /*RENAME*/newLocal = this.m2();
+    }
+    m2() { return 1; }
+}
+// ==SCOPE::Extract to readonly field in class 'C'==
+
+class C {
+    private readonly newProperty = this.m2();
+
+    constructor() {
+        this./*RENAME*/newProperty;
+    }
+    m2() { return 1; }
+}

tests/baselines/reference/extractConstant/extractConstant_This_Method.js

@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+
+class C {
+    m1() {
+        /*[#|*/this.m2()/*|]*/;
+    }
+    m2() { return 1; }
+}
+// ==SCOPE::Extract to constant in enclosing scope==
+
+class C {
+    m1() {
+        const /*RENAME*/newLocal = this.m2();
+    }
+    m2() { return 1; }
+}

tests/baselines/reference/extractConstant/extractConstant_This_Method.ts

@@ -0,0 +1,26 @@
+// ==ORIGINAL==
+
+class C {
+    m1() {
+        /*[#|*/this.m2()/*|]*/;
+    }
+    m2() { return 1; }
+}
+// ==SCOPE::Extract to constant in enclosing scope==
+
+class C {
+    m1() {
+        const /*RENAME*/newLocal = this.m2();
+    }
+    m2() { return 1; }
+}
+// ==SCOPE::Extract to readonly field in class 'C'==
+
+class C {
+    private readonly newProperty = this.m2();
+
+    m1() {
+        this./*RENAME*/newProperty;
+    }
+    m2() { return 1; }
+}

tests/baselines/reference/extractConstant/extractConstant_This_Property.ts

@@ -0,0 +1,18 @@
+// ==ORIGINAL==
+
+namespace N { // Force this test to be TS-only
+    class C {
+        x = 1;
+        y = /*[#|*/this.x/*|]*/;
+    }
+}
+// ==SCOPE::Extract to readonly field in class 'C'==
+
+namespace N { // Force this test to be TS-only
+    class C {
+        x = 1;
+        private readonly newProperty = this.x;
+
+        y = this./*RENAME*/newProperty;
+    }
+}

tests/cases/fourslash/extract-method20.ts

@@ -10,5 +10,6 @@
 //// }
 
 goTo.select('a', 'b')
-verify.refactorAvailable('Extract Symbol', 'function_scope_0');
-verify.not.refactorAvailable('Extract Symbol', 'function_scope_1');
+verify.not.refactorAvailable('Extract Symbol', 'function_scope_0');
+verify.refactorAvailable('Extract Symbol', 'function_scope_1');
+verify.not.refactorAvailable('Extract Symbol', 'function_scope_2');