danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-21 21:31:13 +01:00
Code Issues Projects Releases Wiki Activity

Add safeguards to prevent bad refactor input

Browse Source
This commit is contained in:
Matthew Brown 2019-06-02 23:33:57 -04:00
parent 2439a9f6a0
commit fc0f625f62
7 changed files with 114 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/Psalm/Exception/RefactorException.php Normal file
View File

@ -0,0 +1,6 @@
<?php
namespace Psalm\Exception;
class RefactorException extends \Exception
{
}

62
src/Psalm/Internal/Analyzer/ProjectAnalyzer.php
View File

@ -152,6 +152,11 @@ class ProjectAnalyzer
*/
private $project_files;
/**
* @var array<string, string>
*/
private $to_refactor = [];
const TYPE_COMPACT = 'compact';
const TYPE_CONSOLE = 'console';
const TYPE_PYLINT = 'pylint';
@ -524,6 +529,55 @@ class ProjectAnalyzer
);
}
public function interpretRefactors() : void
{
if (!$this->codebase->alter_code) {
throw new \UnexpectedValueException('Should not be checking references');
}
foreach ($this->to_refactor as $source => $destination) {
$source_parts = explode('::', $source);
$destination_parts = explode('::', $destination);
if (count($source_parts) === 1 || count($destination_parts) === 1) {
throw new \Psalm\Exception\RefactorException('Cannot yet refactor classes');
}
if ($this->codebase->methods->methodExists($source)) {
if ($this->codebase->methods->methodExists($destination)) {
throw new \Psalm\Exception\RefactorException(
'Destination ' . $destination . ' already exists'
);
} elseif (!$this->codebase->classlikes->classExists($destination_parts[0])) {
throw new \Psalm\Exception\RefactorException(
'Destination class ' . $destination_parts[0] . ' doesn’t exist'
);
}
if (strtolower($source_parts[0]) !== strtolower($destination_parts[0])) {
$source_storage = $this->codebase->methods->getStorage($source);
if (!$source_storage->is_static) {
throw new \Psalm\Exception\RefactorException(
'Cannot move non-static method ' . $source
);
}
$this->codebase->methods_to_move[strtolower($source)] = $destination;
} else {
$this->codebase->methods_to_rename[strtolower($source)] = $destination_parts[1];
}
$this->codebase->call_transforms[strtolower($source) . '\((.*\))'] = $destination . '($1)';
continue;
}
throw new \Psalm\Exception\RefactorException(
'At present Psalm can only move static methods (attempted to move ' . $source . ')'
);
}
}
public function prepareMigration() : void
{
if (!$this->codebase->alter_code) {
@ -884,15 +938,13 @@ class ProjectAnalyzer
}
/**
* @param int $php_major_version
* @param int $php_minor_version
* @param bool $dry_run
* @param bool $safe_types
* @param array<string, string> $to_refactor
*
* @return void
*/
public function refactorCodeAfterCompletion()
public function refactorCodeAfterCompletion(array $to_refactor)
{
$this->to_refactor = $to_refactor;
$this->codebase->alter_code = true;
$this->show_issues = false;
}

4
src/Psalm/Internal/Codebase/Analyzer.php
View File

@ -222,6 +222,10 @@ class Analyzer
$filetype_analyzers = $this->config->getFiletypeAnalyzers();
$codebase = $project_analyzer->getCodebase();
if ($alter_code) {
$project_analyzer->interpretRefactors();
}
$analysis_worker =
/**
* @param int $_

21
src/psalm-refactor.php
View File

@ -156,8 +156,7 @@ $args = getArguments();
$operation = null;
$last_arg = null;
$to_move = [];
$to_rename = [];
$to_refactor = [];
foreach ($args as $arg) {
if ($arg === '--move') {
@ -213,14 +212,13 @@ foreach ($args as $arg) {
foreach ($last_arg_parts as $last_arg_part) {
list(, $identifier_name) = explode('::', $last_arg_part);
$to_move[strtolower($last_arg_part)] = $arg . '::' . $identifier_name;
$to_rename[strtolower($last_arg_part) . '\((.*\))'] = $arg . '::' . $identifier_name . '($1)';
$to_refactor[$last_arg_part] = $arg . '::' . $identifier_name;
}
} elseif ($operation === 'move_and_rename_to') {
$to_move[strtolower($last_arg)] = $arg;
$to_rename[strtolower($last_arg) . '\((.*\))'] = $arg . '($1)';
$to_refactor[$last_arg] = $arg;
} else {
$to_rename[strtolower($last_arg) . '\((.*\))'] = $arg . '($1)';
list(, $identifier_name) = explode('::', $last_arg);
$to_refactor[$last_arg] = $arg . '::' . $identifier_name;
}
$last_arg = null;
@ -237,7 +235,7 @@ foreach ($args as $arg) {
die('Unexpected argument "' . $arg . '"' . PHP_EOL);
}
if (!$to_move && !$to_rename) {
if (!$to_refactor) {
die('No --move or --rename arguments supplied' . PHP_EOL);
}
@ -279,12 +277,7 @@ $project_analyzer = new ProjectAnalyzer(
$config->visitComposerAutoloadFiles($project_analyzer);
$codebase = $project_analyzer->getCodebase();
$codebase->methods_to_move = $to_move;
$codebase->call_transforms = $to_rename;
$project_analyzer->refactorCodeAfterCompletion();
$project_analyzer->refactorCodeAfterCompletion($to_refactor);
$start_time = microtime(true);

75
tests/FileManipulation/MethodMoveTest.php
View File

@ -32,8 +32,7 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
public function testValidCode(
string $input_code,
string $output_code,
array $methods_to_move,
array $call_transforms
array $methods_to_move
) {
$test_name = $this->getTestName();
if (strpos($test_name, 'SKIPPED-') !== false) {
@ -61,10 +60,7 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
$codebase = $this->project_analyzer->getCodebase();
$codebase->methods_to_move = $methods_to_move;
$codebase->call_transforms = $call_transforms;
$this->project_analyzer->refactorCodeAfterCompletion();
$this->project_analyzer->refactorCodeAfterCompletion($methods_to_move);
$this->analyzeFile($file_path, $context);
@ -78,12 +74,12 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
}
/**
* @return array<string,array{string,string,array<string, string>,array<string, string>}>
* @return array<string,array{string,string,array<string, string>}>
*/
public function providerValidCodeParse()
{
return [
'moveStaticMethodReferenceOnly' => [
'moveSimpleStaticMethod' => [
'<?php
namespace Ns;
@ -115,12 +111,7 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
class A {
const C = 5;
/**
* @return ArrayObject<int, int>
*/
public static function Foo() {
return new ArrayObject([self::C]);
}
}
class B {
@ -129,13 +120,19 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
foreach (B::Fe() as $f) {}
}
/**
* @return ArrayObject<int, int>
*/
public static function Fe() {
return new ArrayObject([A::C]);
}
}',
[],
[
'ns\a::foo\((.*\))' => 'Ns\B::Fe($1)',
'Ns\A::Foo' => 'Ns\B::Fe',
]
],
'moveStaticMethodReferenceOnlyIntoNamespaceWithExistingUse' => [
'moveStaticMethodIntoNamespaceWithExistingUse' => [
'<?php
namespace {
class A {
@ -154,12 +151,14 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
}
namespace Ns\A {
class B {}
class B {
}
}',
'<?php
namespace {
class A {
public static function Foo() : void {}
}
}
@ -174,11 +173,14 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
}
namespace Ns\A {
class B {}
class B {
public static function Fedcba() : void {}
}
}',
[],
[
'a::foo\((.*\))' => 'Ns\A\B::Fedcba($1)',
'A::Foo' => 'Ns\A\B::Fedcba',
]
],
'moveEmptyStaticMethodOnly' => [
@ -209,10 +211,8 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
public static function Fedcba() : void {}
}',
[
'ns\a::foo' => 'Ns\B::Fedcba',
'Ns\A::Foo' => 'Ns\B::Fedcba',
],
[
]
],
'moveStaticMethodOnly' => [
'<?php
@ -289,9 +289,8 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
}
}',
[
'ns\a::foo' => 'Ns\B::Fedbca',
],
[]
'Ns\A::Foo' => 'Ns\B::Fedbca',
]
],
'moveStaticMethodAndReferencesFromAbove' => [
'<?php
@ -335,10 +334,7 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
}
}',
[
'ns\a::foo' => 'Ns\B::Fe',
],
[
'ns\a::foo\((.*\))' => 'Ns\B::Fe($1)',
'Ns\A::Foo' => 'Ns\B::Fe',
]
],
'moveStaticMethodAndReferencesFromBelow' => [
@ -382,11 +378,8 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
}',
[
'ns\a::foo' => 'Ns\B::Fe',
'Ns\A::Foo' => 'Ns\B::Fe',
],
[
'ns\a::foo\((.*\))' => 'Ns\B::Fe($1)',
]
],
'moveStaticMethodAndReferencesAcrossNamespaces' => [
'<?php
@ -434,10 +427,7 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
}
}',
[
'ns1\a::foo' => 'Ns2\Ns3\B::Fe',
],
[
'ns1\a::foo\((.*\))' => 'Ns2\Ns3\B::Fe($1)',
'Ns1\A::Foo' => 'Ns2\Ns3\B::Fe',
]
],
'moveStaticMethodAndReferencesAcrossNamespacesWithExistingUse' => [
@ -490,10 +480,7 @@ class MoveMethodTest extends \Psalm\Tests\TestCase
}
}',
[
'ns1\a::foo' => 'Ns2\Ns3\B::Fedcba',
],
[
'ns1\a::foo\((.*\))' => 'Ns2\Ns3\B::Fedcba($1)',
'Ns1\A::Foo' => 'Ns2\Ns3\B::Fedcba',
]
],
];

18
tests/FileManipulation/MethodRenameTest.php
View File

@ -24,16 +24,14 @@ class MethodRenameTest extends \Psalm\Tests\TestCase
*
* @param string $input_code
* @param string $output_code
* @param array<string, string> $methods_to_rename
* @param array<string, string> $call_transforms
* @param array<string, string> $to_refactor
*
* @return void
*/
public function testValidCode(
string $input_code,
string $output_code,
array $methods_to_rename,
array $call_transforms
array $to_refactor
) {
$test_name = $this->getTestName();
if (strpos($test_name, 'SKIPPED-') !== false) {
@ -61,10 +59,7 @@ class MethodRenameTest extends \Psalm\Tests\TestCase
$codebase = $this->project_analyzer->getCodebase();
$codebase->call_transforms = $call_transforms;
$codebase->methods_to_rename = $methods_to_rename;
$this->project_analyzer->refactorCodeAfterCompletion();
$this->project_analyzer->refactorCodeAfterCompletion($to_refactor);
$this->analyzeFile($file_path, $context);
@ -78,7 +73,7 @@ class MethodRenameTest extends \Psalm\Tests\TestCase
}
/**
* @return array<string,array{string,string,array<string, string>,array<string, string>}>
* @return array<string,array{string,string,array<string, string>}>
*/
public function providerValidCodeParse()
{
@ -141,11 +136,8 @@ class MethodRenameTest extends \Psalm\Tests\TestCase
}
}',
[
'ns\a::foo' => 'Fedcba',
'Ns\A::foo' => 'Ns\A::Fedcba',
],
[
'ns\a::foo\((.*\))' => 'Ns\A::Fedcba($1)',
]
],
];
}

4
tests/TestCase.php
View File

@ -101,6 +101,10 @@ class TestCase extends BaseTestCase
$codebase->config->visitStubFiles($codebase);
if ($codebase->alter_code) {
$this->project_analyzer->interpretRefactors();
}
$file_analyzer = new FileAnalyzer(
$this->project_analyzer,
$file_path,