From 5bae869dc69d567337344d400af2b8ac9d062b49 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Sun, 7 Jan 2018 00:11:23 -0500 Subject: [PATCH] Break file manipulation out into Psalter --- composer.json | 2 +- psalter | 2 + src/Psalm/Checker/CommentChecker.php | 8 +- src/Psalm/Checker/FunctionLikeChecker.php | 162 ++++++++++++----- src/Psalm/Checker/ProjectChecker.php | 37 ++-- src/Psalm/Checker/StatementsChecker.php | 1 - .../FunctionDocblockManipulator.php | 132 ++++++++++++-- src/Psalm/Issue/FixableCodeIssue.php | 28 --- src/Psalm/IssueBuffer.php | 17 +- src/Psalm/Type/Atomic.php | 15 ++ src/Psalm/Type/Atomic/ObjectLike.php | 18 ++ src/Psalm/Type/Atomic/Scalar.php | 20 +++ src/Psalm/Type/Atomic/T.php | 8 + src/Psalm/Type/Atomic/TArray.php | 18 ++ src/Psalm/Type/Atomic/TBool.php | 13 ++ src/Psalm/Type/Atomic/TCallable.php | 18 ++ src/Psalm/Type/Atomic/TEmpty.php | 13 ++ src/Psalm/Type/Atomic/TFalse.php | 5 + src/Psalm/Type/Atomic/TGenericObject.php | 8 + src/Psalm/Type/Atomic/TMixed.php | 18 ++ src/Psalm/Type/Atomic/TNamedObject.php | 18 ++ src/Psalm/Type/Atomic/TNull.php | 18 ++ src/Psalm/Type/Atomic/TNumeric.php | 13 ++ src/Psalm/Type/Atomic/TNumericString.php | 8 + src/Psalm/Type/Atomic/TObject.php | 18 ++ src/Psalm/Type/Atomic/TResource.php | 18 ++ src/Psalm/Type/Atomic/TScalar.php | 18 ++ src/Psalm/Type/Atomic/TTrue.php | 5 + src/Psalm/Type/Atomic/TVoid.php | 18 ++ src/Psalm/Type/Union.php | 72 ++++++++ src/command_functions.php | 133 ++++++++++++++ src/psalm.php | 161 ++--------------- src/psalter.php | 165 ++++++++++++++++++ tests/FileManipulationTest.php | 159 +++++++++++++++-- 34 files changed, 1076 insertions(+), 291 deletions(-) create mode 100755 psalter create mode 100644 src/command_functions.php create mode 100644 src/psalter.php diff --git a/composer.json b/composer.json index 3022dbc50..06fd8e13d 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ "openlss/lib-array2xml": "^0.0.10||^0.5.1", "muglug/package-versions-56": "1.2.3" }, - "bin": ["psalm"], + "bin": ["psalm", "psalter"], "autoload": { "psr-4": { "Psalm\\": "src/Psalm" diff --git a/psalter b/psalter new file mode 100755 index 000000000..da332d144 --- /dev/null +++ b/psalter @@ -0,0 +1,2 @@ +#!/usr/bin/env php + $lines) { - if ($last_type !== null && ($last_type !== 'return' || $type !== 'psalm-return')) { + if ($last_type !== null && ($last_type !== 'return' || $last_type !== 'psalm-return')) { $doc_comment_text .= $left_padding . ' *' . PHP_EOL; } foreach ($lines as $line) { - $doc_comment_text .= $left_padding . ' * @' . str_pad($type, $special_type_width) . $line . PHP_EOL; + $doc_comment_text .= $left_padding . ' * @' . $type . ' ' . $line . PHP_EOL; } $last_type = $type; diff --git a/src/Psalm/Checker/FunctionLikeChecker.php b/src/Psalm/Checker/FunctionLikeChecker.php index 2793c89cc..7827e7140 100644 --- a/src/Psalm/Checker/FunctionLikeChecker.php +++ b/src/Psalm/Checker/FunctionLikeChecker.php @@ -309,8 +309,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo new MismatchingDocblockParamType( 'Parameter $' . $function_param->name . ' has wrong type \'' . $param_type . '\', should be \'' . $signature_type . '\'', - $function_param->type_location, - (string)$signature_type + $function_param->type_location ), $storage->suppressed_issues )) { @@ -413,12 +412,19 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo $fleshed_out_signature_type ) ) { + if ($project_checker->alter_code + && isset($project_checker->getIssuesToFix()['MismatchingDocblockReturnType']) + ) { + $this->addOrUpdateReturnType($project_checker, $storage->signature_return_type, true); + + return null; + } + if (IssueBuffer::accepts( new MismatchingDocblockReturnType( 'Docblock has incorrect return type \'' . $storage->return_type . '\', should be \'' . $storage->signature_return_type . '\'', - $storage->return_type_location, - (string) $storage->signature_return_type + $storage->return_type_location ), $storage->suppressed_issues )) { @@ -1164,35 +1170,6 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo ) ); - if (!$return_type && !$project_checker->add_docblocks && !$is_to_string) { - if ($this->function instanceof Closure) { - if (IssueBuffer::accepts( - new MissingClosureReturnType( - 'Closure does not have a return type, expecting ' . $inferred_return_type, - new CodeLocation($this, $this->function, null, true) - ), - $this->suppressed_issues - )) { - // fall through - } - - return null; - } - - if (IssueBuffer::accepts( - new MissingReturnType( - 'Method ' . $cased_method_id . ' does not have a return type' . - (!$inferred_return_type->isMixed() ? ', expecting ' . $inferred_return_type : ''), - new CodeLocation($this, $this->function, null, true) - ), - $this->suppressed_issues - )) { - // fall through - } - - return null; - } - if ($is_to_string) { if (!$inferred_return_type->isMixed() && (string)$inferred_return_type !== 'string') { if (IssueBuffer::accepts( @@ -1210,9 +1187,53 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } if (!$return_type) { - if (!$inferred_return_type->isMixed()) { - // $project_checker->add_docblocks is always true here - $this->addDocblockReturnType($project_checker, $inferred_return_type); + if ($this->function instanceof Closure) { + if ($project_checker->alter_code + && isset($project_checker->getIssuesToFix()['MissingClosureReturnType']) + ) { + if ($inferred_return_type->isMixed()) { + return null; + } + + $this->addOrUpdateReturnType($project_checker, $inferred_return_type); + + return null; + } + + if (IssueBuffer::accepts( + new MissingClosureReturnType( + 'Closure does not have a return type, expecting ' . $inferred_return_type, + new CodeLocation($this, $this->function, null, true) + ), + $this->suppressed_issues + )) { + // fall through + } + + return null; + } + + if ($project_checker->alter_code + && isset($project_checker->getIssuesToFix()['MissingReturnType']) + ) { + if ($inferred_return_type->isMixed()) { + return null; + } + + $this->addOrUpdateReturnType($project_checker, $inferred_return_type); + + return null; + } + + if (IssueBuffer::accepts( + new MissingReturnType( + 'Method ' . $cased_method_id . ' does not have a return type' . + (!$inferred_return_type->isMixed() ? ', expecting ' . $inferred_return_type : ''), + new CodeLocation($this, $this->function, null, true) + ), + $this->suppressed_issues + )) { + // fall through } return null; @@ -1237,6 +1258,12 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo return null; } + if ($project_checker->alter_code && isset(getIssuesToFix()['InvalidReturnType'])) { + $this->addOrUpdateReturnType($project_checker, Type::getVoid()); + + return null; + } + if (IssueBuffer::accepts( new InvalidReturnType( 'No return statements were found for method ' . $cased_method_id . @@ -1285,12 +1312,19 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo && !$declared_return_type->isNullable() && !$declared_return_type->isVoid() ) { + if ($project_checker->alter_code + && isset($project_checker->getIssuesToFix()['NullableInferredReturnType']) + ) { + $this->addOrUpdateReturnType($project_checker, $inferred_return_type); + + return null; + } + if (IssueBuffer::accepts( new NullableInferredReturnType( 'The declared return type \'' . $declared_return_type . '\' for ' . $cased_method_id . ' is not nullable, but \'' . $inferred_return_type . '\' contains null', - $return_type_location, - (string) $inferred_return_type + $return_type_location ), $this->suppressed_issues )) { @@ -1302,12 +1336,19 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo && !$declared_return_type->isFalsable() && !$declared_return_type->hasBool() ) { + if ($project_checker->alter_code + && isset($project_checker->getIssuesToFix()['FalsableInferredReturnType']) + ) { + $this->addOrUpdateReturnType($project_checker, $inferred_return_type); + + return null; + } + if (IssueBuffer::accepts( new FalsableInferredReturnType( 'The declared return type \'' . $declared_return_type . '\' for ' . $cased_method_id . ' does not allow false, but \'' . $inferred_return_type . '\' contains false', - $return_type_location, - (string) $inferred_return_type + $return_type_location ), $this->suppressed_issues )) { @@ -1338,12 +1379,19 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo return false; } } else { + if ($project_checker->alter_code + && isset($project_checker->getIssuesToFix()['InvalidReturnType']) + ) { + $this->addOrUpdateReturnType($project_checker, $inferred_return_type); + + return null; + } + if (IssueBuffer::accepts( new InvalidReturnType( 'The declared return type \'' . $declared_return_type . '\' for ' . $cased_method_id . ' is incorrect, got \'' . $inferred_return_type . '\'', - $return_type_location, - (string) $inferred_return_type + $return_type_location ), $this->suppressed_issues )) { @@ -1351,6 +1399,14 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } } } elseif (!$inferred_return_type->isNullable() && $declared_return_type->isNullable()) { + if ($project_checker->alter_code + && isset($project_checker->getIssuesToFix()['LessSpecificReturnType']) + ) { + $this->addOrUpdateReturnType($project_checker, $inferred_return_type); + + return null; + } + if (IssueBuffer::accepts( new LessSpecificReturnType( 'The inferred return type \'' . $inferred_return_type . '\' for ' . $cased_method_id . @@ -1368,20 +1424,29 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } /** - * @param Type\Union $inferred_return_type + * @param bool $docblock_only * * @return void */ - private function addDocblockReturnType(ProjectChecker $project_checker, Type\Union $inferred_return_type) - { + private function addOrUpdateReturnType( + ProjectChecker $project_checker, + Type\Union $inferred_return_type, + $docblock_only = false + ) { $manipulator = FunctionDocblockManipulator::getForFunction( $project_checker, $this->source->getFilePath(), $this->getMethodId(), $this->function ); - - $manipulator->setDocblockReturnType( + $manipulator->setReturnType( + !$docblock_only && $project_checker->php_major_version >= 7 + ? $inferred_return_type->toPhpString( + $this->source->getAliasedClassesFlipped(), + $this->source->getFQCLN(), + $project_checker->php_major_version, + $project_checker->php_minor_version + ) : null, $inferred_return_type->toNamespacedString( $this->source->getAliasedClassesFlipped(), $this->source->getFQCLN(), @@ -1391,7 +1456,8 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo $this->source->getAliasedClassesFlipped(), $this->source->getFQCLN(), true - ) + ), + $inferred_return_type->canBeFullyExpressedInPhp() ); } diff --git a/src/Psalm/Checker/ProjectChecker.php b/src/Psalm/Checker/ProjectChecker.php index 720b41dc6..0967110d6 100644 --- a/src/Psalm/Checker/ProjectChecker.php +++ b/src/Psalm/Checker/ProjectChecker.php @@ -87,7 +87,7 @@ class ProjectChecker /** * @var bool */ - public $add_docblocks = false; + public $alter_code = false; /** * @var bool @@ -259,6 +259,16 @@ class ProjectChecker */ private $issues_to_fix = []; + /** + * @var int + */ + public $php_major_version = PHP_MAJOR_VERSION; + + /** + * @var int + */ + public $php_minor_version = PHP_MINOR_VERSION; + const TYPE_CONSOLE = 'console'; const TYPE_JSON = 'json'; const TYPE_EMACS = 'emacs'; @@ -1004,7 +1014,7 @@ class ProjectChecker } } - if ($this->replace_code || $this->add_docblocks || $this->issues_to_fix) { + if ($this->replace_code || $this->alter_code || $this->issues_to_fix) { foreach ($this->files_to_report as $file_path) { $this->updateFile($file_path, true); } @@ -1019,7 +1029,7 @@ class ProjectChecker */ public function updateFile($file_path, $output_changes = false) { - if ($this->add_docblocks) { + if ($this->alter_code) { $new_return_type_manipulations = FunctionDocblockManipulator::getManipulationsForFile($file_path); } else { $new_return_type_manipulations = []; @@ -2082,19 +2092,16 @@ class ProjectChecker } /** + * @param int $php_major_version + * @param int $php_minor_version + * * @return void */ - public function addDocblocksAfterCompletion() + public function alterCodeAfterCompletion($php_major_version, $php_minor_version) { - $this->add_docblocks = true; - } - - /** - * @return void - */ - public function replaceCodeAfterCompletion() - { - $this->replace_code = true; + $this->alter_code = true; + $this->php_major_version = $php_major_version; + $this->php_minor_version = $php_minor_version; } /** @@ -2102,13 +2109,15 @@ class ProjectChecker * * @return void */ - public function fixIssuesAfterCompletion(array $issues) + public function setIssuesToFix(array $issues) { $this->issues_to_fix = $issues; } /** * @return array + * + * @psalm-suppress PossiblyUnusedMethod - need to fix #422 */ public function getIssuesToFix() { diff --git a/src/Psalm/Checker/StatementsChecker.php b/src/Psalm/Checker/StatementsChecker.php index a1dde0c7e..a46777d28 100644 --- a/src/Psalm/Checker/StatementsChecker.php +++ b/src/Psalm/Checker/StatementsChecker.php @@ -1023,7 +1023,6 @@ class StatementsChecker extends SourceChecker implements StatementsSource if ($storage->return_type && !$storage->return_type->isMixed() - && !$project_checker->add_docblocks ) { $inferred_type = ExpressionChecker::fleshOutType( $project_checker, diff --git a/src/Psalm/FileManipulation/FunctionDocblockManipulator.php b/src/Psalm/FileManipulation/FunctionDocblockManipulator.php index 8b0460b8e..2bc394bea 100644 --- a/src/Psalm/FileManipulation/FunctionDocblockManipulator.php +++ b/src/Psalm/FileManipulation/FunctionDocblockManipulator.php @@ -2,6 +2,7 @@ namespace Psalm\FileManipulation; use PhpParser\Node\Expr\Closure; +use PhpParser\Node\FunctionLike; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; use Psalm\Checker\CommentChecker; @@ -28,6 +29,21 @@ class FunctionDocblockManipulator /** @var int */ private $docblock_end; + /** @var int */ + private $return_typehint_area_start; + + /** @var ?int */ + private $return_typehint_start; + + /** @var ?int */ + private $return_typehint_end; + + /** @var ?string */ + private $new_php_return_type; + + /** @var bool */ + private $return_type_is_php_compatible = false; + /** @var ?string */ private $new_phpdoc_return_type; @@ -44,8 +60,12 @@ class FunctionDocblockManipulator * * @return self */ - public static function getForFunction(ProjectChecker $project_checker, $file_path, $function_id, $stmt) - { + public static function getForFunction( + ProjectChecker $project_checker, + $file_path, + $function_id, + FunctionLike $stmt + ) { if (isset(self::$manipulators[$file_path][$function_id])) { return self::$manipulators[$file_path][$function_id]; } @@ -62,32 +82,99 @@ class FunctionDocblockManipulator * @param string $file_path * @param Closure|Function_|ClassMethod $stmt */ - private function __construct($file_path, $stmt, ProjectChecker $project_checker) + private function __construct($file_path, FunctionLike $stmt, ProjectChecker $project_checker) { $this->stmt = $stmt; $docblock = $stmt->getDocComment(); $this->docblock_start = $docblock ? $docblock->getFilePos() : (int)$stmt->getAttribute('startFilePos'); - $this->docblock_end = (int)$stmt->getAttribute('startFilePos'); + $this->docblock_end = $function_start = (int)$stmt->getAttribute('startFilePos'); + $function_end = (int)$stmt->getAttribute('endFilePos'); + + $file_contents = $project_checker->getFileContents($file_path); + + $last_arg_position = $stmt->params + ? (int) $stmt->params[count($stmt->params) - 1]->getAttribute('endFilePos') + : null; + + if ($stmt instanceof Closure && $stmt->uses) { + $last_arg_position = (int) $stmt->uses[count($stmt->uses) - 1]->getAttribute('endFilePos'); + } + + $end_bracket_position = (int) strpos($file_contents, ')', $last_arg_position ?: $function_start); + + $this->return_typehint_area_start = $end_bracket_position + 1; + + $function_code = substr($file_contents, $function_start, $function_end); + + $function_code_after_bracket = substr($function_code, $end_bracket_position + 1 - $function_start); + + // do a little parsing here + /** @var array */ + $chars = str_split($function_code_after_bracket); + + foreach ($chars as $i => $char) { + switch ($char) { + case PHP_EOL: + continue; + + case ':': + continue; + + case '/': + // @todo handle comments in this area + throw new \UnexpectedValueException('Not expecting comments where return types should live'); + + case '{': + break 2; + + case '?': + $this->return_typehint_start = $i + $end_bracket_position + 1; + break; + } + + if (preg_match('/\w/', $char)) { + if ($this->return_typehint_start === null) { + $this->return_typehint_start = $i + $end_bracket_position + 1; + } + + if (!preg_match('/\w/', $chars[$i + 1])) { + $this->return_typehint_end = $i + $end_bracket_position + 2; + break; + } + } + } + + $preceding_newline_pos = strrpos($file_contents, PHP_EOL, $this->docblock_end - strlen($file_contents)); + + if ($preceding_newline_pos === false) { + $this->indentation = ''; + + return; + } + + $first_line = substr($file_contents, $preceding_newline_pos + 1, $this->docblock_end - $preceding_newline_pos); - $file_lines = explode(PHP_EOL, $project_checker->getFileContents($file_path)); - $first_line = $file_lines[$stmt->getLine() - 1]; $this->indentation = str_replace(ltrim($first_line), '', $first_line); } /** * Sets the new docblock return type * + * @param ?string $php_type * @param string $new_type * @param string $phpdoc_type + * @param bool $is_php_compatible * * @return void */ - public function setDocblockReturnType($new_type, $phpdoc_type) + public function setReturnType($php_type, $new_type, $phpdoc_type, $is_php_compatible) { $new_type = str_replace(['', ''], '', $new_type); + $this->new_php_return_type = $php_type; $this->new_phpdoc_return_type = $phpdoc_type; $this->new_psalm_return_type = $new_type; + $this->return_type_is_php_compatible = $is_php_compatible; } /** @@ -131,11 +218,32 @@ class FunctionDocblockManipulator $file_manipulations = []; foreach (self::$ordered_manipulators[$file_path] as $manipulator) { - $file_manipulations[$manipulator->docblock_start] = new FileManipulation( - $manipulator->docblock_start, - $manipulator->docblock_end, - $manipulator->getDocblock() - ); + if ($manipulator->new_php_return_type) { + if ($manipulator->return_typehint_start && $manipulator->return_typehint_end) { + $file_manipulations[$manipulator->return_typehint_start] = new FileManipulation( + $manipulator->return_typehint_start, + $manipulator->return_typehint_end, + $manipulator->new_php_return_type + ); + } else { + $file_manipulations[$manipulator->return_typehint_area_start] = new FileManipulation( + $manipulator->return_typehint_area_start, + $manipulator->return_typehint_area_start, + ' : ' . $manipulator->new_php_return_type + ); + } + } + + if (!$manipulator->new_php_return_type + || !$manipulator->return_type_is_php_compatible + || $manipulator->docblock_start !== $manipulator->docblock_end + ) { + $file_manipulations[$manipulator->docblock_start] = new FileManipulation( + $manipulator->docblock_start, + $manipulator->docblock_end, + $manipulator->getDocblock() + ); + } } return $file_manipulations; diff --git a/src/Psalm/Issue/FixableCodeIssue.php b/src/Psalm/Issue/FixableCodeIssue.php index 812397a3c..b67acff64 100644 --- a/src/Psalm/Issue/FixableCodeIssue.php +++ b/src/Psalm/Issue/FixableCodeIssue.php @@ -1,34 +1,6 @@ replacement_text = $replacement_text; - } - - /** - * @return ?string - */ - public function getReplacementText() - { - return $this->replacement_text; - } } diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php index ad4995e10..bd3238586 100644 --- a/src/Psalm/IssueBuffer.php +++ b/src/Psalm/IssueBuffer.php @@ -3,8 +3,6 @@ namespace Psalm; use LSS\Array2XML; use Psalm\Checker\ProjectChecker; -use Psalm\FileManipulation\FileManipulation; -use Psalm\FileManipulation\FileManipulationBuffer; use Psalm\Issue\CodeIssue; class IssueBuffer @@ -81,24 +79,13 @@ class IssueBuffer public static function add(CodeIssue $e) { $config = Config::getInstance(); - $project_checker = ProjectChecker::getInstance(); $fqcn_parts = explode('\\', get_class($e)); $issue_type = array_pop($fqcn_parts); - $issues_to_fix = \Psalm\Checker\ProjectChecker::getInstance()->getIssuesToFix(); - - if (isset($issues_to_fix[$issue_type]) - && $e instanceof \Psalm\Issue\FixableCodeIssue - && $replacement_text = $e->getReplacementText() - ) { - $code_location = $e->getLocation(); - $bounds = $code_location->getSelectionBounds(); - FileManipulationBuffer::add( - $e->getFilePath(), - [new FileManipulation($bounds[0], $bounds[1], $replacement_text)] - ); + $project_checker = ProjectChecker::getInstance(); + if ($project_checker->alter_code) { return false; } diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php index 324bae6f5..9f1cd2a41 100644 --- a/src/Psalm/Type/Atomic.php +++ b/src/Psalm/Type/Atomic.php @@ -251,6 +251,21 @@ abstract class Atomic return $this->getKey(); } + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + abstract public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version); + + /** + * @return bool + */ + abstract public function canBeFullyExpressedInPhp(); + /** * @return void */ diff --git a/src/Psalm/Type/Atomic/ObjectLike.php b/src/Psalm/Type/Atomic/ObjectLike.php index 7a6eb38da..abed26540 100644 --- a/src/Psalm/Type/Atomic/ObjectLike.php +++ b/src/Psalm/Type/Atomic/ObjectLike.php @@ -80,6 +80,24 @@ class ObjectLike extends \Psalm\Type\Atomic '}'; } + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return $this->getKey(); + } + + public function canBeFullyExpressedInPhp() + { + return false; + } + /** * @return Union */ diff --git a/src/Psalm/Type/Atomic/Scalar.php b/src/Psalm/Type/Atomic/Scalar.php index 9ddc67b64..a65a103d6 100644 --- a/src/Psalm/Type/Atomic/Scalar.php +++ b/src/Psalm/Type/Atomic/Scalar.php @@ -3,4 +3,24 @@ namespace Psalm\Type\Atomic; abstract class Scalar extends \Psalm\Type\Atomic { + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return $php_major_version >= 7 ? $this->getKey() : null; + } + + /** + * @return bool + */ + public function canBeFullyExpressedInPhp() + { + return true; + } } diff --git a/src/Psalm/Type/Atomic/T.php b/src/Psalm/Type/Atomic/T.php index db4acb380..b56fea7fa 100644 --- a/src/Psalm/Type/Atomic/T.php +++ b/src/Psalm/Type/Atomic/T.php @@ -17,4 +17,12 @@ abstract class T extends TString { $this->typeof = $typeof; } + + /** + * @return bool + */ + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TArray.php b/src/Psalm/Type/Atomic/TArray.php index 1e3cce967..c7298e1d1 100644 --- a/src/Psalm/Type/Atomic/TArray.php +++ b/src/Psalm/Type/Atomic/TArray.php @@ -27,4 +27,22 @@ class TArray extends \Psalm\Type\Atomic implements Generic { return 'array'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return $this->getKey(); + } + + public function canBeFullyExpressedInPhp() + { + return $this->type_params[0]->isMixed() && $this->type_params[1]->isMixed(); + } } diff --git a/src/Psalm/Type/Atomic/TBool.php b/src/Psalm/Type/Atomic/TBool.php index f54da514b..e8d2950c2 100644 --- a/src/Psalm/Type/Atomic/TBool.php +++ b/src/Psalm/Type/Atomic/TBool.php @@ -15,4 +15,17 @@ class TBool extends Scalar { return 'bool'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return $php_major_version >= 7 ? 'bool' : null; + } } diff --git a/src/Psalm/Type/Atomic/TCallable.php b/src/Psalm/Type/Atomic/TCallable.php index cba0a3fef..96d7528f8 100644 --- a/src/Psalm/Type/Atomic/TCallable.php +++ b/src/Psalm/Type/Atomic/TCallable.php @@ -15,4 +15,22 @@ class TCallable extends \Psalm\Type\Atomic { return 'callable'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return null; + } + + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TEmpty.php b/src/Psalm/Type/Atomic/TEmpty.php index 301556af5..6cd296bf7 100644 --- a/src/Psalm/Type/Atomic/TEmpty.php +++ b/src/Psalm/Type/Atomic/TEmpty.php @@ -15,4 +15,17 @@ class TEmpty extends Scalar { return 'empty'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return null; + } } diff --git a/src/Psalm/Type/Atomic/TFalse.php b/src/Psalm/Type/Atomic/TFalse.php index 39c9b110c..181e2f214 100644 --- a/src/Psalm/Type/Atomic/TFalse.php +++ b/src/Psalm/Type/Atomic/TFalse.php @@ -15,4 +15,9 @@ class TFalse extends TBool { return 'false'; } + + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TGenericObject.php b/src/Psalm/Type/Atomic/TGenericObject.php index 134396429..67dab2fa5 100644 --- a/src/Psalm/Type/Atomic/TGenericObject.php +++ b/src/Psalm/Type/Atomic/TGenericObject.php @@ -17,4 +17,12 @@ class TGenericObject extends TNamedObject implements Generic $this->value = $value; $this->type_params = $type_params; } + + /** + * @return bool + */ + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TMixed.php b/src/Psalm/Type/Atomic/TMixed.php index b760cfa85..3089980d8 100644 --- a/src/Psalm/Type/Atomic/TMixed.php +++ b/src/Psalm/Type/Atomic/TMixed.php @@ -15,4 +15,22 @@ class TMixed extends \Psalm\Type\Atomic { return 'mixed'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return null; + } + + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TNamedObject.php b/src/Psalm/Type/Atomic/TNamedObject.php index 0f73f1a2f..c2cb0a70f 100644 --- a/src/Psalm/Type/Atomic/TNamedObject.php +++ b/src/Psalm/Type/Atomic/TNamedObject.php @@ -67,6 +67,24 @@ class TNamedObject extends Atomic return '\\' . $this->value; } + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return $this->toNamespacedString($aliased_classes, $this_class, false); + } + + public function canBeFullyExpressedInPhp() + { + return true; + } + /** * @param TNamedObject $type * diff --git a/src/Psalm/Type/Atomic/TNull.php b/src/Psalm/Type/Atomic/TNull.php index 6d3178a0a..66e0b2443 100644 --- a/src/Psalm/Type/Atomic/TNull.php +++ b/src/Psalm/Type/Atomic/TNull.php @@ -15,4 +15,22 @@ class TNull extends \Psalm\Type\Atomic { return 'null'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return null; + } + + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TNumeric.php b/src/Psalm/Type/Atomic/TNumeric.php index 2d9cdbcbd..23c872ef3 100644 --- a/src/Psalm/Type/Atomic/TNumeric.php +++ b/src/Psalm/Type/Atomic/TNumeric.php @@ -15,4 +15,17 @@ class TNumeric extends Scalar { return 'numeric'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return null; + } } diff --git a/src/Psalm/Type/Atomic/TNumericString.php b/src/Psalm/Type/Atomic/TNumericString.php index 87811ed9a..c00032298 100644 --- a/src/Psalm/Type/Atomic/TNumericString.php +++ b/src/Psalm/Type/Atomic/TNumericString.php @@ -15,4 +15,12 @@ class TNumericString extends TString { return $this->getKey(); } + + /** + * @return bool + */ + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TObject.php b/src/Psalm/Type/Atomic/TObject.php index c04f0c995..18f4b44dc 100644 --- a/src/Psalm/Type/Atomic/TObject.php +++ b/src/Psalm/Type/Atomic/TObject.php @@ -15,4 +15,22 @@ class TObject extends \Psalm\Type\Atomic { return 'object'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return $php_major_version >= 7 && $php_minor_version >= 2 ? $this->getKey() : null; + } + + public function canBeFullyExpressedInPhp() + { + return true; + } } diff --git a/src/Psalm/Type/Atomic/TResource.php b/src/Psalm/Type/Atomic/TResource.php index 253b08c5f..8410b75e5 100644 --- a/src/Psalm/Type/Atomic/TResource.php +++ b/src/Psalm/Type/Atomic/TResource.php @@ -15,4 +15,22 @@ class TResource extends \Psalm\Type\Atomic { return 'resource'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return null; + } + + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TScalar.php b/src/Psalm/Type/Atomic/TScalar.php index 5f5e6ced7..7297079a8 100644 --- a/src/Psalm/Type/Atomic/TScalar.php +++ b/src/Psalm/Type/Atomic/TScalar.php @@ -15,4 +15,22 @@ class TScalar extends Scalar { return 'scalar'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return null; + } + + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TTrue.php b/src/Psalm/Type/Atomic/TTrue.php index 9a4486d58..248293c01 100644 --- a/src/Psalm/Type/Atomic/TTrue.php +++ b/src/Psalm/Type/Atomic/TTrue.php @@ -15,4 +15,9 @@ class TTrue extends TBool { return 'true'; } + + public function canBeFullyExpressedInPhp() + { + return false; + } } diff --git a/src/Psalm/Type/Atomic/TVoid.php b/src/Psalm/Type/Atomic/TVoid.php index d2ab89df3..258358e22 100644 --- a/src/Psalm/Type/Atomic/TVoid.php +++ b/src/Psalm/Type/Atomic/TVoid.php @@ -15,4 +15,22 @@ class TVoid extends \Psalm\Type\Atomic { return 'void'; } + + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + return $php_major_version >= 7 && $php_minor_version >= 1 ? $this->getKey() : null; + } + + public function canBeFullyExpressedInPhp() + { + return true; + } } diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index 953f29004..2ecf9596c 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -111,6 +111,78 @@ class Union ); } + /** + * @param array $aliased_classes + * @param string|null $this_class + * @param int $php_major_version + * @param int $php_minor_version + * + * @return ?string + */ + public function toPhpString(array $aliased_classes, $this_class, $php_major_version, $php_minor_version) + { + $nullable = false; + + if (count($this->types) > 2 + || ( + count($this->types) === 2 + && (!isset($this->types['null']) + || $php_major_version < 7 + || $php_minor_version < 1) + ) + ) { + return null; + } + + $types = $this->types; + + if (isset($types['null'])) { + unset($types['null']); + + $nullable = true; + } + + $atomic_type = array_values($types)[0]; + + $atomic_type_string = $atomic_type->toPhpString( + $aliased_classes, + $this_class, + $php_major_version, + $php_minor_version + ); + + if ($atomic_type_string) { + return ($nullable ? '?' : '') . $atomic_type_string; + } + + return null; + } + + /** + * @return bool + */ + public function canBeFullyExpressedInPhp() + { + if (count($this->types) > 2 + || ( + count($this->types) === 2 + && !isset($this->types['null']) + ) + ) { + return false; + } + + $types = $this->types; + + if (isset($types['null'])) { + unset($types['null']); + } + + $atomic_type = array_values($types)[0]; + + return $atomic_type->canBeFullyExpressedInPhp(); + } + /** * @return void */ diff --git a/src/command_functions.php b/src/command_functions.php new file mode 100644 index 000000000..95ca41ce9 --- /dev/null +++ b/src/command_functions.php @@ -0,0 +1,133 @@ + 2) { + continue; + } + + $filtered_input_paths[] = $input_path; + } + + stream_set_blocking(STDIN, false); + + if ($filtered_input_paths === ['-'] && $stdin = fgets(STDIN)) { + $filtered_input_paths = preg_split('/\s+/', trim($stdin)); + } + + foreach ($filtered_input_paths as $i => $path_to_check) { + if ($path_to_check[0] === '-') { + die('Invalid usage, expecting psalm [options] [file...]' . PHP_EOL); + } + + if (!file_exists($path_to_check)) { + die('Cannot locate ' . $path_to_check . PHP_EOL); + } + + $path_to_check = realpath($path_to_check); + + if (!$path_to_check) { + die('Error getting realpath for file' . PHP_EOL); + } + + $paths_to_check[] = $path_to_check; + } + + if (!$paths_to_check) { + $paths_to_check = null; + } + } + + return $paths_to_check; +} diff --git a/src/psalm.php b/src/psalm.php index 3960e1076..e28d13225 100644 --- a/src/psalm.php +++ b/src/psalm.php @@ -1,4 +1,5 @@ 2) { - continue; - } - - $filtered_input_paths[] = $input_path; - } - - stream_set_blocking(STDIN, false); - - if ($filtered_input_paths === ['-'] && $stdin = fgets(STDIN)) { - $filtered_input_paths = preg_split('/\s+/', trim($stdin)); - } - - foreach ($filtered_input_paths as $i => $path_to_check) { - if ($path_to_check[0] === '-') { - die('Invalid usage, expecting psalm [options] [file...]' . PHP_EOL); - } - - if (!file_exists($path_to_check)) { - die('Cannot locate ' . $path_to_check . PHP_EOL); - } - - $path_to_check = realpath($path_to_check); - - if (!$path_to_check) { - die('Error getting realpath for file' . PHP_EOL); - } - - $paths_to_check[] = $path_to_check; - } - - if (!$paths_to_check) { - $paths_to_check = null; - } -} +$paths_to_check = getPathsToCheck(isset($options['f']) ? $options['f'] : null); $plugins = []; @@ -353,11 +235,9 @@ if ($path_to_config === false) { die('Could not resolve path to config ' . (string)$options['c'] . PHP_EOL); } -$use_color = !array_key_exists('m', $options); - $show_info = isset($options['show-info']) - ? $options['show-info'] !== 'false' && $options['show-info'] !== '0' - : true; + ? $options['show-info'] !== 'false' && $options['show-info'] !== '0' + : true; $is_diff = isset($options['diff']); @@ -367,8 +247,6 @@ $find_references_to = isset($options['find-references-to']) && is_string($option ? $options['find-references-to'] : null; -$add_docblocks = isset($options['add-docblocks']); - $threads = isset($options['threads']) ? (int)$options['threads'] : 1; $cache_provider = isset($options['no-cache']) @@ -378,11 +256,11 @@ $cache_provider = isset($options['no-cache']) $project_checker = new ProjectChecker( new Psalm\Provider\FileProvider(), $cache_provider, - $use_color, + !array_key_exists('m', $options), $show_info, $output_format, $threads, - $debug, + array_key_exists('debug', $options), $find_dead_code || $find_references_to !== null, $find_references_to, isset($options['report']) && is_string($options['report']) ? $options['report'] : null @@ -417,25 +295,6 @@ foreach ($plugins as $plugin_path) { Config::getInstance()->addPluginPath($current_dir . DIRECTORY_SEPARATOR . $plugin_path); } -if (isset($options['replace-code'])) { - $project_checker->replaceCodeAfterCompletion(); -} - -if (isset($options['fix-issues'])) { - if (!is_string($options['fix-issues']) || !$options['fix-issues']) { - die('Expecting a comma-separated string of issues' . PHP_EOL); - } - - $issues = explode(',', $options['fix-issues']); - - $keyed_issues = []; - foreach ($issues as $issue) { - $keyed_issues[$issue] = true; - } - - $project_checker->fixIssuesAfterCompletion($keyed_issues); -} - /** @psalm-suppress MixedArgument */ \Psalm\IssueBuffer::setStartTime(microtime(true)); diff --git a/src/psalter.php b/src/psalter.php new file mode 100644 index 000000000..7cabcf912 --- /dev/null +++ b/src/psalter.php @@ -0,0 +1,165 @@ +setConfigXML($path_to_config, $current_dir); +} + +$config = $project_checker->getConfig(); + +if (!$config) { + $project_checker->getConfigForPath($current_dir, $current_dir); +} + +if (!is_string($options['issues']) || !$options['issues']) { + die('Expecting a comma-separated list of issues' . PHP_EOL); +} + +$issues = explode(',', $options['issues']); + +$keyed_issues = []; +foreach ($issues as $issue) { + $keyed_issues[$issue] = true; +} + +$php_major_version = PHP_MAJOR_VERSION; +$php_minor_version = PHP_MINOR_VERSION; + +if (isset($options['php-version'])) { + if (!is_string($options['php-version']) || !preg_match('/^(5\.[456]|7\.[012])^/', $options['php-version'])) { + die('Expecting a version number in the format x.y' . PHP_EOL); + } + + list($php_major_version, $php_minor_version) = explode('.', $options['php-version']); +} + +$project_checker->alterCodeAfterCompletion((int) $php_major_version, (int) $php_minor_version); +$project_checker->setIssuesToFix($keyed_issues); + +/** @psalm-suppress MixedArgument */ +\Psalm\IssueBuffer::setStartTime(microtime(true)); + +if ($paths_to_check === null) { + $project_checker->check($current_dir); +} elseif ($paths_to_check) { + foreach ($paths_to_check as $path_to_check) { + if (is_dir($path_to_check)) { + $project_checker->checkDir($path_to_check); + } else { + $project_checker->checkFile($path_to_check); + } + } +} diff --git a/tests/FileManipulationTest.php b/tests/FileManipulationTest.php index eeee716f4..905c11951 100644 --- a/tests/FileManipulationTest.php +++ b/tests/FileManipulationTest.php @@ -30,8 +30,6 @@ class FileManipulationTest extends TestCase } $this->project_checker->setConfig(self::$config); - - $this->project_checker->add_docblocks = true; } /** @@ -39,10 +37,12 @@ class FileManipulationTest extends TestCase * * @param string $input_code * @param string $output_code + * @param string $php_version + * @param string[] $issues_to_fix * * @return void */ - public function testValidCode($input_code, $output_code) + public function testValidCode($input_code, $output_code, $php_version, array $issues_to_fix) { $test_name = $this->getName(); if (strpos($test_name, 'PHP7-') !== false) { @@ -64,9 +64,19 @@ class FileManipulationTest extends TestCase $input_code ); + list($php_major_version, $php_minor_version) = explode('.', $php_version); + + $keyed_issues_to_fix = []; + + foreach ($issues_to_fix as $issue) { + $keyed_issues_to_fix[$issue] = true; + } + $file_checker = new FileChecker($file_path, $this->project_checker); - $this->project_checker->addDocblocksAfterCompletion(); - $this->project_checker->fixIssuesAfterCompletion(['InvalidReturnType' => true]); + + $this->project_checker->setIssuesToFix($keyed_issues_to_fix); + $this->project_checker->alterCodeAfterCompletion((int) $php_major_version, (int) $php_minor_version); + $file_checker->visitAndAnalyzeMethods($context); $this->project_checker->updateFile($file_path); $this->assertSame($output_code, $this->project_checker->getFileContents($file_path)); @@ -78,7 +88,7 @@ class FileManipulationTest extends TestCase public function providerFileCheckerValidCodeParse() { return [ - 'doesNothing' => [ + 'addMissingVoidReturnType56' => [ ' [ + 'addMissingVoidReturnType70' => [ + ' [ + ' [ ' [ + 'addMissingStringReturnType70' => [ + ' [ + ' [ + ' [ + ' [ + ' [ + ' [ ' [ + 'fixInvalidIntReturnType70' => [ ' [ '