Skip to content
This repository has been archived by the owner on Mar 6, 2022. It is now read-only.

Extract method LSP code action #41

Closed
wants to merge 4 commits into from
Closed

Extract method LSP code action #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
557c45a
Extract method
BladeMF May 8, 2021
0ed7075
Extract method tests
BladeMF May 8, 2021
f05bc5c
Fix wrong service id
BladeMF May 11, 2021
258404e
Change the extract method kind to use the proper base kind (https://m…
BladeMF May 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions lib/LanguageServerCodeTransform/CodeAction/ExtractMethodProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

namespace Phpactor\Extension\LanguageServerCodeTransform\CodeAction;

use Amp\Promise;
use Phpactor\CodeTransform\Domain\Refactor\ExtractMethod;
use Phpactor\CodeTransform\Domain\SourceCode;
use Phpactor\Extension\LanguageServerBridge\Converter\PositionConverter;
use Phpactor\Extension\LanguageServerCodeTransform\LspCommand\ExtractMethodCommand;
use Phpactor\LanguageServerProtocol\CodeAction;
use Phpactor\LanguageServerProtocol\Command;
use Phpactor\LanguageServerProtocol\Range;
use Phpactor\LanguageServerProtocol\TextDocumentItem;
use Phpactor\LanguageServer\Core\CodeAction\CodeActionProvider;
use function Amp\call;

class ExtractMethodProvider implements CodeActionProvider
{
public const KIND = 'refactor.extract.method';

/**
* @var ExtractMethod
*/
private $extractMethod;

public function __construct(ExtractMethod $extractMethod)
{
$this->extractMethod = $extractMethod;
}
/**
* {@inheritDoc}
*/
public function kinds(): array
{
return [
self::KIND
];
}
public function provideActionsFor(TextDocumentItem $textDocument, Range $range): Promise
{
return call(function () use ($textDocument, $range) {
if (!$this->extractMethod->canExtractMethod(
SourceCode::fromStringAndPath($textDocument->text, $textDocument->uri),
PositionConverter::positionToByteOffset($range->start, $textDocument->text)->toInt(),
PositionConverter::positionToByteOffset($range->end, $textDocument->text)->toInt()
)) {
return [];
}

return [
CodeAction::fromArray([
'title' => 'Extract method',
'kind' => self::KIND,
'diagnostics' => [],
'command' => new Command(
'Extract method',
ExtractMethodCommand::NAME,
[
$textDocument->uri,
PositionConverter::positionToByteOffset($range->start, $textDocument->text)->toInt(),
PositionConverter::positionToByteOffset($range->end, $textDocument->text)->toInt()
]
)
])
];
});
}
}
23 changes: 23 additions & 0 deletions lib/LanguageServerCodeTransform/LanguageServerCodeTransformExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Phpactor\CodeTransform\Domain\Helper\MissingMethodFinder;
use Phpactor\CodeTransform\Domain\Helper\UnresolvableClassNameFinder;
use Phpactor\CodeTransform\Domain\Refactor\ExtractMethod;
use Phpactor\CodeTransform\Domain\Refactor\GenerateMethod;
use Phpactor\CodeTransform\Domain\Refactor\ImportName;
use Phpactor\Container\Container;
Expand All @@ -13,10 +14,12 @@
use Phpactor\Extension\CodeTransform\CodeTransformExtension;
use Phpactor\Extension\LanguageServerBridge\Converter\TextEditConverter;
use Phpactor\Extension\LanguageServerCodeTransform\CodeAction\CreateClassProvider;
use Phpactor\Extension\LanguageServerCodeTransform\CodeAction\ExtractMethodProvider;
use Phpactor\Extension\LanguageServerCodeTransform\CodeAction\GenerateMethodProvider;
use Phpactor\Extension\LanguageServerCodeTransform\CodeAction\ImportNameProvider;
use Phpactor\Extension\LanguageServerCodeTransform\CodeAction\TransformerCodeActionPovider;
use Phpactor\Extension\LanguageServerCodeTransform\LspCommand\CreateClassCommand;
use Phpactor\Extension\LanguageServerCodeTransform\LspCommand\ExtractMethodCommand;
use Phpactor\Extension\LanguageServerCodeTransform\LspCommand\GenerateMethodCommand;
use Phpactor\Extension\LanguageServerCodeTransform\LspCommand\ImportNameCommand;
use Phpactor\Extension\LanguageServerCodeTransform\LspCommand\TransformCommand;
Expand Down Expand Up @@ -105,6 +108,18 @@ private function registerCommands(ContainerBuilder $container): void
'name' => GenerateMethodCommand::NAME
],
]);

$container->register(ExtractMethodCommand::class, function (Container $container) {
return new ExtractMethodCommand(
$container->get(ClientApi::class),
$container->get(LanguageServerExtension::SERVICE_SESSION_WORKSPACE),
$container->get(ExtractMethod::class)
);
}, [
LanguageServerExtension::TAG_COMMAND => [
'name' => ExtractMethodCommand::NAME
],
]);
}

private function registerCodeActions(ContainerBuilder $container): void
Expand Down Expand Up @@ -187,5 +202,13 @@ private function registerCodeActions(ContainerBuilder $container): void
LanguageServerExtension::TAG_DIAGNOSTICS_PROVIDER => [],
LanguageServerExtension::TAG_CODE_ACTION_PROVIDER => []
]);

$container->register(ExtractMethodProvider::class, function (Container $container) {
return new ExtractMethodProvider(
$container->get(ExtractMethod::class)
);
}, [
LanguageServerExtension::TAG_CODE_ACTION_PROVIDER => []
]);
}
}
67 changes: 67 additions & 0 deletions lib/LanguageServerCodeTransform/LspCommand/ExtractMethodCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

namespace Phpactor\Extension\LanguageServerCodeTransform\LspCommand;

use Amp\Promise;
use Amp\Success;
use Phpactor\CodeTransform\Domain\Exception\TransformException;
use Phpactor\CodeTransform\Domain\Refactor\ExtractMethod;
use Phpactor\CodeTransform\Domain\SourceCode;
use Phpactor\Extension\LanguageServerBridge\Converter\TextEditConverter;
use Phpactor\LanguageServerProtocol\ApplyWorkspaceEditResponse;
use Phpactor\LanguageServer\Core\Command\Command;
use Phpactor\LanguageServerProtocol\WorkspaceEdit;
use Phpactor\LanguageServer\Core\Server\ClientApi;
use Phpactor\LanguageServer\Core\Workspace\Workspace;

class ExtractMethodCommand implements Command
{
public const NAME = 'extract_method';
public const DEFAULT_METHOD_NAME = 'newMethod';

/**
* @var ExtractMethod
*/
private $extractMethod;
/**
* @var ClientApi
*/
private $clientApi;
/**
* @var Workspace
*/
private $workspace;

public function __construct(
ClientApi $clientApi,
Workspace $workspace,
ExtractMethod $extractMethod
) {
$this->extractMethod = $extractMethod;
$this->clientApi = $clientApi;
$this->workspace = $workspace;
}
/**
* @return Promise<ApplyWorkspaceEditResponse>
*/
public function __invoke(string $uri, int $startOffset, int $endOffset): Promise
{
$textDocument = $this->workspace->get($uri);

try {
$textEdits = $this->extractMethod->extractMethod(
SourceCode::fromStringAndPath($textDocument->text, $textDocument->uri),
$startOffset,
$endOffset,
self::DEFAULT_METHOD_NAME
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems we canot currently ask the user for input. Assuming this method will throw an exception if there is an existing method, you could add an increment to the default method name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually my guess was, since myMethod is so silly of a name, if you have one of these lying around, you have some more work to do until you extract another one. Go rename the other one, then extract this one :-) Kind of a code quality error. Otherwise doing a counter would be trivial, I think, if "method exists" is a catchable exception.

The perfect solution would be to either ask for input (I saw you found the same discussion), or enter rename mode immediately after completing. Probably it's best to wait for the former.

So, code quality wise, I'd leave it as it is, but I am happy to do a counter if think it'll be better.

);
} catch (TransformException $error) {
$this->clientApi->window()->showMessage()->warning($error->getMessage());
return new Success(null);
}

return $this->clientApi->workspace()->applyEdit(new WorkspaceEdit([
$uri => TextEditConverter::toLspTextEdits($textEdits->textEdits(), $textDocument->text)
]), 'Extract method');
}
}
93 changes: 93 additions & 0 deletions tests/LanguageServerCodeTransform/Unit/CodeAction/ExtractMethodProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<?php

namespace Phpactor\Extension\LanguageServerCodeTransform\Tests\Unit\CodeAction;

use PHPUnit\Framework\TestCase;
use Phpactor\CodeTransform\Domain\Refactor\ExtractMethod;
use Phpactor\CodeTransform\Domain\SourceCode;
use Phpactor\Extension\LanguageServerCodeTransform\CodeAction\ExtractMethodProvider;
use Phpactor\Extension\LanguageServerCodeTransform\LspCommand\ExtractMethodCommand;
use Phpactor\LanguageServerProtocol\CodeAction;
use Phpactor\LanguageServerProtocol\Command;
use Phpactor\LanguageServerProtocol\TextDocumentItem;
use Phpactor\LanguageServer\Test\ProtocolFactory;
use Prophecy\PhpUnit\ProphecyTrait;
use Prophecy\Prophecy\ObjectProphecy;
use function Amp\Promise\wait;
use Generator;

class ExtractMethodProviderTest extends TestCase
{
use ProphecyTrait;

const EXAMPLE_SOURCE = 'foobar';
const EXAMPLE_FILE = 'file:///somefile.php';
/**
* @var ObjectProphecy
*/
private $extractMethod;

public function setUp(): void
{
$this->extractMethod = $this->prophesize(ExtractMethod::class);
}
/**
* @dataProvider provideActionsData
*/
public function testProvideActions(bool $shouldSucceed, array $expectedValue): void
{
$textDocumentItem = new TextDocumentItem(self::EXAMPLE_FILE, 'php', 1, self::EXAMPLE_SOURCE);
$range = ProtocolFactory::range(0, 0, 0, 5);

// @phpstan-ignore-next-line
$this->extractMethod
->canExtractMethod(
SourceCode::fromStringAndPath($textDocumentItem->text, $textDocumentItem->uri),
$range->start->character,
$range->end->character
)
->willReturn($shouldSucceed)
->shouldBeCalled();

$this->assertEquals(
$expectedValue,
wait($this->createProvider()->provideActionsFor(
$textDocumentItem,
$range
))
);
}

public function provideActionsData(): Generator
{
yield 'Fail' => [
false,
[]
];
yield 'Success' => [
true,
[
CodeAction::fromArray([
'title' => 'Extract method',
'kind' => ExtractMethodProvider::KIND,
'diagnostics' => [],
'command' => new Command(
'Extract method',
ExtractMethodCommand::NAME,
[
self::EXAMPLE_FILE,
0,
5
]
)
])
]
];
}

private function createProvider(): ExtractMethodProvider
{
// @phpstan-ignore-next-line
return new ExtractMethodProvider($this->extractMethod->reveal());
}
}
109 changes: 109 additions & 0 deletions tests/LanguageServerCodeTransform/Unit/LspCommand/ExtractMethodCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
<?php

namespace Phpactor\Extension\LanguageServerCodeTransform\Tests\Unit\LspCommand;

use Exception;
use PHPUnit\Framework\TestCase;
use Phpactor\CodeTransform\Domain\Exception\TransformException;
use Phpactor\CodeTransform\Domain\Refactor\ExtractMethod;
use Phpactor\CodeTransform\Domain\SourceCode;
use Phpactor\Extension\LanguageServerBridge\Converter\TextEditConverter;
use Phpactor\Extension\LanguageServerCodeTransform\LspCommand\ExtractMethodCommand;
use Phpactor\LanguageServerProtocol\ApplyWorkspaceEditResponse;
use Phpactor\LanguageServerProtocol\MessageType;
use Phpactor\LanguageServerProtocol\WorkspaceEdit;
use Phpactor\LanguageServer\LanguageServerTesterBuilder;
use Phpactor\TextDocument\TextDocumentEdits;
use Phpactor\TextDocument\TextDocumentUri;
use Phpactor\TextDocument\TextEdits;
use Phpactor\TextDocument\TextEdit;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Prophecy\Prophecy\ObjectProphecy;

class ExtractMethodCommandTest extends TestCase
{
use ProphecyTrait;

const EXAMPLE_SOURCE = '<?php ';
const EXAMPLE_URI = 'file:///file.php';
const EXAMPLE_OFFSET = 5;

public function testSuccessfulCall(): void
{
$textEdits = new TextDocumentEdits(
TextDocumentUri::fromString(self::EXAMPLE_URI),
new TextEdits(TextEdit::create(self::EXAMPLE_OFFSET, 1, 'test'))
);

$extractMethod = $this->prophesize(ExtractMethod::class);
// @phpstan-ignore-next-line
$extractMethod->extractMethod(Argument::type(SourceCode::class), 0, self::EXAMPLE_OFFSET, ExtractMethodCommand::DEFAULT_METHOD_NAME)
->shouldBeCalled()
->willReturn($textEdits);

[$tester, $builder] = $this->createTester($extractMethod);
$tester->workspace()->executeCommand('extract_method', [self::EXAMPLE_URI, 0, self::EXAMPLE_OFFSET]);
$builder->responseWatcher()->resolveLastResponse(new ApplyWorkspaceEditResponse(true));

$applyEdit = $builder->transmitter()->filterByMethod('workspace/applyEdit')->shiftRequest();

self::assertNotNull($applyEdit);
self::assertEquals([
'edit' => new WorkspaceEdit([
(string)$textEdits->uri() => TextEditConverter::toLspTextEdits(
$textEdits->textEdits(),
self::EXAMPLE_SOURCE
)
]),
'label' => 'Extract method'
], $applyEdit->params);
}
/**
* @dataProvider provideExceptions
*/
public function testFailedCall(Exception $exception): void
{
$extractMethod = $this->prophesize(ExtractMethod::class);
// @phpstan-ignore-next-line
$extractMethod->extractMethod(Argument::type(SourceCode::class), 0, self::EXAMPLE_OFFSET, ExtractMethodCommand::DEFAULT_METHOD_NAME)
->shouldBeCalled()
->willThrow($exception);

[$tester, $builder] = $this->createTester($extractMethod);
$tester->workspace()->executeCommand('extract_method', [self::EXAMPLE_URI, 0, self::EXAMPLE_OFFSET]);
$showMessage = $builder->transmitter()->filterByMethod('window/showMessage')->shiftNotification();

self::assertNotNull($showMessage);
self::assertEquals([
'type' => MessageType::WARNING,
'message' => $exception->getMessage()
], $showMessage->params);
}

public function provideExceptions(): array
{
return [
TransformException::class => [ new TransformException('Error message!') ],
];
}
/**
* @return array
*/
private function createTester(ObjectProphecy $extractMethod): array
{
$builder = LanguageServerTesterBuilder::createBare()
->enableTextDocuments()
->enableCommands();
$builder->addCommand('extract_method', new ExtractMethodCommand(
$builder->clientApi(),
$builder->workspace(),
$extractMethod->reveal()
));

$tester = $builder->build();
$tester->textDocument()->open(self::EXAMPLE_URI, self::EXAMPLE_SOURCE);

return [$tester, $builder];
}
}