Repository: phpactor/phpactor (php)

Issue:
Generate method does not detect static contexts
That setup:
```php
	public static function setUpBeforeClass(): void
	{
		<>
		... some code ...
		<>
	}
```
should generate a static method, not an instance one.

Pull request:
Making extractMethod from static function also generate static function
Closes #1285

Patch:
diff --git a/lib/CodeTransform/Adapter/WorseReflection/Refactor/WorseExtractMethod.php b/lib/CodeTransform/Adapter/WorseReflection/Refactor/WorseExtractMethod.php
index b51a658a..8a103a7b 100644
--- a/lib/CodeTransform/Adapter/WorseReflection/Refactor/WorseExtractMethod.php
+++ b/lib/CodeTransform/Adapter/WorseReflection/Refactor/WorseExtractMethod.php
@@ -131,6 +131,10 @@ class WorseExtractMethod implements ExtractMethod
 
         $locals = $this->scopeLocalVariables($source, $offsetStart, $offsetEnd);
 
+        if ($reflectionMethod->isStatic()) {
+            $methodBuilder->static();
+        }
+
         $parameterVariables = $this->parameterVariables($locals->lessThan($offsetStart), $selection, $offsetStart);
         $args = $this->addParametersAndGetArgs($parameterVariables, $methodBuilder, $builder);
 


Test patch:
diff --git a/lib/CodeTransform/Tests/Adapter/WorseReflection/Refactor/WorseExtractMethodTest.php b/lib/CodeTransform/Tests/Adapter/WorseReflection/Refactor/WorseExtractMethodTest.php
index bdc80988..30af3f01 100644
--- a/lib/CodeTransform/Tests/Adapter/WorseReflection/Refactor/WorseExtractMethodTest.php
+++ b/lib/CodeTransform/Tests/Adapter/WorseReflection/Refactor/WorseExtractMethodTest.php
@@ -8,21 +8,17 @@ use Phpactor\CodeTransform\Domain\SourceCode;
 use Phpactor\WorseReflection\Core\SourceCode as WorseSourceCode;
 use Phpactor\CodeBuilder\Adapter\WorseReflection\WorseBuilderFactory;
 use Exception;
+use Generator;
 
 class WorseExtractMethodTest extends WorseTestCase
 {
     /**
      * @dataProvider provideExtractMethod
      */
-    public function testExtractMethod(string $test, string $name, ?string $expectedExceptionMessage = null): void
+    public function testExtractMethod(string $test): void
     {
         [$source, $expected, $offsetStart, $offsetEnd] = $this->sourceExpectedAndOffset(__DIR__ . '/fixtures/' . $test);
 
-        if ($expectedExceptionMessage) {
-            $this->expectException(Exception::class);
-            $this->expectExceptionMessage($expectedExceptionMessage);
-        }
-
         $worseSourceCode = WorseSourceCode::fromPathAndString('file:///source', $source);
         $reflector = $this->reflectorForWorkspace($worseSourceCode);
 
@@ -30,7 +26,7 @@ class WorseExtractMethodTest extends WorseTestCase
         $extractMethod = new WorseExtractMethod($reflector, $factory, $this->updater());
 
         $sourceCode = SourceCode::fromStringAndPath($source, 'file:///source');
-        $textDocumentEdits = $extractMethod->extractMethod($sourceCode, $offsetStart, $offsetEnd, $name);
+        $textDocumentEdits = $extractMethod->extractMethod($sourceCode, $offsetStart, $offsetEnd, 'newMethod');
 
         $transformed = SourceCode::fromStringAndPath(
             (string) $textDocumentEdits->textEdits()->apply($sourceCode),
@@ -39,154 +35,69 @@ class WorseExtractMethodTest extends WorseTestCase
         $this->assertEquals(trim($expected), trim($transformed));
     }
 
-    public function provideExtractMethod(): array
+    /** @return Generator<array<string>> */
+    public function provideExtractMethod(): Generator
+    {
+        yield 'no free variables' => ['extractMethod1.test'];
+        yield 'free variable' => ['extractMethod2.test'];
+        yield 'free variables' => ['extractMethod3.test'];
+        yield 'namespaced' => ['extractMethod4.test'];
+        yield 'duplicated vars' => ['extractMethod5.test'];
+        yield 'return value and assignment' => ['extractMethod6.test'];
+        yield 'multiple return value and assignment' => ['extractMethod7.test'];
+        yield 'multiple return value with incoming variables' => ['extractMethod8.test'];
+        yield 'multiple return value boundaries' => ['extractMethod10.test'];
+        yield 'tail variables are taken from scope' => ['extractMethod11.test'];
+        yield 'replacement indentation is preserved' => ['extractMethod12.test'];
+        yield 'only considers selection content for return vars' => ['extractMethod13.test'];
+        yield 'return mutated primative' => ['extractMethod14.test'];
+        yield 'imports classes' => ['extractMethod15.test'];
+        yield 'adds return type for scalar' => ['extractMethod16.test'];
+        yield 'adds return type for nullable scalar' => ['extractMethod16A.test'];
+        yield 'adds return type and import for nullable class' => ['extractMethod16B.test'];
+        yield 'adds return type for class' => ['extractMethod17.test'];
+        yield 'extracts expression to method' => ['extractMethod18.test'];
+        yield 'extracts assignment expression to method' => ['extractMethod19.test'];
+        yield 'extracts assignment expression with unknown return type' => ['extractMethod20.test'];
+        yield 'extract expression and adds short return type for class' => ['extractMethod21.test'];
+        yield 'return if extracted code has a return' => ['extractMethod22.test'];
+        yield 'adds method to declaring class' => ['extractMethod23.test'];
+        yield 'empty text selection' => ['extractMethod27.test'];
+        yield 'nullable argument' => ['extractMethod29.test'];
+        yield 'ignore scoped variables: catch clause' => ['extractMethod30.test'];
+        yield 'ignore scoped variables: anonymous function' => ['extractMethod31.test'];
+        yield 'union argument' => ['extractMethod32.test'];
+        yield 'extract method from trait' => ['extractMethod33.test'];
+        yield 'extract static method' => ['extractMethod34.test'];
+    }
+
+    /**
+     * @dataProvider provideExtractMethodFromDifferentScopes
+     */
+    public function testExtractingMethodsFromDifferentScopes(string $test): void
+    {
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Cannot extract method. Check if start and end statements are in different scopes.');
+
+        $this->testExtractMethod($test);
+    }
+
+    /** @return Generator<array<string>> */
+    public function provideExtractMethodFromDifferentScopes(): Generator
     {
-        return [
-            'no free variables' => [
-                'extractMethod1.test',
-                'newMethod'
-            ],
-            'free variable' => [
-                'extractMethod2.test',
-                'newMethod'
-            ],
-            'free variables' => [
-                'extractMethod3.test',
-                'newMethod'
-            ],
-            'namespaced' => [
-                'extractMethod4.test',
-                'newMethod'
-            ],
-            'duplicated vars' => [
-                'extractMethod5.test',
-                'newMethod'
-            ],
-            'return value and assignment' => [
-                'extractMethod6.test',
-                'newMethod'
-            ],
-            'multiple return value and assignment' => [
-                'extractMethod7.test',
-                'newMethod'
-            ],
-            'multiple return value with incoming variables' => [
-                'extractMethod8.test',
-                'newMethod'
-            ],
-            'multiple return value boundaries' => [
-                'extractMethod10.test',
-                'newMethod',
-            ],
-            'method exists' => [
-                'extractMethod9.test',
-                'newMethod',
-                'Class "extractMethod" already has method "newMethod"'
-            ],
-            'tail variables are taken from scope' => [
-                'extractMethod11.test',
-                'newMethod',
-            ],
-            'replacement indentation is preserved' => [
-                'extractMethod12.test',
-                'newMethod',
-            ],
-            'only considers selection content for return vars' => [
-                'extractMethod13.test',
-                'newMethod',
-            ],
-            'return mutated primative' => [
-                'extractMethod14.test',
-                'newMethod',
-            ],
-            'imports classes' => [
-                'extractMethod15.test',
-                'newMethod',
-            ],
-            'adds return type for scalar' => [
-                'extractMethod16.test',
-                'newMethod',
-            ],
-            'adds return type for nullable scalar' => [
-                'extractMethod16A.test',
-                'newMethod',
-            ],
-            'adds return type and import for nullable class' => [
-                'extractMethod16B.test',
-                'newMethod',
-            ],
-            'adds return type for class' => [
-                'extractMethod17.test',
-                'newMethod',
-            ],
-            'extracts expression to method' => [
-                'extractMethod18.test',
-                'newMethod',
-            ],
-            'extracts assignment expression to method' => [
-                'extractMethod19.test',
-                'newMethod',
-            ],
-            'extracts assignment expression with unknown return type' => [
-                'extractMethod20.test',
-                'newMethod',
-            ],
-            'extract expression and adds short return type for class' => [
-                'extractMethod21.test',
-                'newMethod',
-            ],
-            'return if extracted code has a return' => [
-                'extractMethod22.test',
-                'newMethod',
-            ],
-            'adds method to declaring class' => [
-                'extractMethod23.test',
-                'newMethod',
-            ],
-            'different scopes 1' => [
-                'extractMethod24.test',
-                'newMethod',
-                'Cannot extract method. Check if start and end statements are in different scopes.'
-            ],
-            'different scopes 2' => [
-                'extractMethod25.test',
-                'newMethod',
-                'Cannot extract method. Check if start and end statements are in different scopes.'
-            ],
-            'different scopes 3' => [
-                'extractMethod26.test',
-                'newMethod',
-                'Cannot extract method. Check if start and end statements are in different scopes.'
-            ],
-            'empty text selection' => [
-                'extractMethod27.test',
-                'newMethod',
-            ],
-            'empty selection 2' => [
-                'extractMethod28.test',
-                'newMethod',
-                'Cannot extract method. Check if start and end statements are in different scopes.'
-            ],
-            'nullable argument' => [
-                'extractMethod29.test',
-                'newMethod',
-            ],
-            'ignore scoped variables: catch clause' => [
-                'extractMethod30.test',
-                'newMethod',
-            ],
-            'ignore scoped variables: anonymous function' => [
-                'extractMethod31.test',
-                'newMethod',
-            ],
-            'union argument' => [
-                'extractMethod32.test',
-                'newMethod',
-            ],
-            'extract method from trait' => [
-                'extractMethod33.test',
-                'newMethod'
-            ],
+        yield['extractMethod24.test'];
+        yield['extractMethod25.test'];
+        yield['extractMethod26.test'];
+        yield 'empty selection 2' => [
+            'extractMethod28.test',
         ];
     }
+
+    public function testExtractMethodThatExists(): void
+    {
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Class "extractMethod" already has method "newMethod"');
+
+        $this->testExtractMethod('extractMethod_methodExists.test');
+    }
 }
diff --git a/lib/CodeTransform/Tests/Adapter/WorseReflection/Refactor/fixtures/extractMethod34.test b/lib/CodeTransform/Tests/Adapter/WorseReflection/Refactor/fixtures/extractMethod34.test
new file mode 100644
index 00000000..3db778e7
--- /dev/null
+++ b/lib/CodeTransform/Tests/Adapter/WorseReflection/Refactor/fixtures/extractMethod34.test
@@ -0,0 +1,26 @@
+// File: source
+<?php
+
+trait One {
+    public static function hello(?string $foobar) {
+        $union = $foobar;
+        <>$car = $union;<>
+        return $car;
+    }
+}
+// File: expected
+<?php
+
+trait One {
+    public static function hello(?string $foobar) {
+        $union = $foobar;
+        $car = $this->newMethod($union);
+        return $car;
+    }
+
+    private static function newMethod(?string $union): ?string
+    {
+        $car = $union;
+        return $car;
+    }
+}