From 388e804ed8394544576a7621d35944cd98d9ba32 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Thu, 5 Nov 2020 18:20:04 -0500 Subject: [PATCH] Allow opt-in to strict return type checking --- config.xsd | 1 + src/Psalm/Config.php | 6 ++ .../FunctionLike/ReturnTypeAnalyzer.php | 99 ++++++++++--------- tests/Template/ConditionalReturnTypeTest.php | 1 - 4 files changed, 59 insertions(+), 48 deletions(-) diff --git a/config.xsd b/config.xsd index 4f3ee262242..8f224b6dd22 100644 --- a/config.xsd +++ b/config.xsd @@ -76,6 +76,7 @@ + diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 74505ebba6c..0fb72a526af 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -433,6 +433,11 @@ class Config */ public $resolve_from_config_file = true; + /** + * @var bool + */ + public $restrict_return_types = false; + /** * @var string[] */ @@ -835,6 +840,7 @@ private static function fromXmlAndPaths(string $base_dir, string $file_contents, 'allowNamedArgumentCalls' => 'allow_named_arg_calls', 'findUnusedPsalmSuppress' => 'find_unused_psalm_suppress', 'reportInfo' => 'report_info', + 'restrictReturnTypes' => 'restrict_return_types', ]; foreach ($booleanAttributes as $xmlName => $internalName) { diff --git a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php index c9fa757ded2..f0358bbfdbb 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php @@ -525,63 +525,68 @@ public static function verifyReturnType( return false; } } - } elseif ($codebase->alter_code - && isset($project_analyzer->getIssuesToFix()['LessSpecificReturnType']) - && !in_array('LessSpecificReturnType', $suppressed_issues) - && !($function_like_storage instanceof MethodStorage && $function_like_storage->inheritdoc) - ) { - if (!UnionTypeComparator::isContainedBy( + } elseif (!$inferred_return_type->hasMixed() + && !UnionTypeComparator::isContainedBy( $codebase, $declared_return_type, $inferred_return_type, false, false - )) { - self::addOrUpdateReturnType( - $function, - $project_analyzer, - $inferred_return_type, - $source, - $compatible_method_ids - || (($project_analyzer->only_replace_php_types_with_non_docblock_types - || $unsafe_return_type) - && $inferred_return_type->from_docblock), - $function_like_storage - ); - - return null; - } - } elseif (!$inferred_return_type->hasMixed() - && ((!$inferred_return_type->isNullable() && $declared_return_type->isNullable()) - || (!$inferred_return_type->isFalsable() && $declared_return_type->isFalsable())) + ) ) { - if ($function instanceof Function_ - || $function instanceof Closure - || $function instanceof ArrowFunction - || $function->isPrivate() - ) { - $check_for_less_specific_type = true; - } elseif ($source instanceof StatementsAnalyzer) { - if ($function_like_storage instanceof MethodStorage) { - $check_for_less_specific_type = !$function_like_storage->overridden_somewhere; + if ($codebase->alter_code) { + if (isset($project_analyzer->getIssuesToFix()['LessSpecificReturnType']) + && !in_array('LessSpecificReturnType', $suppressed_issues) + && !($function_like_storage instanceof MethodStorage && $function_like_storage->inheritdoc) + ) { + self::addOrUpdateReturnType( + $function, + $project_analyzer, + $inferred_return_type, + $source, + $compatible_method_ids + || (($project_analyzer->only_replace_php_types_with_non_docblock_types + || $unsafe_return_type) + && $inferred_return_type->from_docblock), + $function_like_storage + ); + } + } else { + if ($function instanceof Function_ + || $function instanceof Closure + || $function instanceof ArrowFunction + || $function->isPrivate() + ) { + $check_for_less_specific_type = true; + } elseif ($source instanceof StatementsAnalyzer) { + if ($function_like_storage instanceof MethodStorage) { + $check_for_less_specific_type = !$function_like_storage->overridden_somewhere; + } else { + $check_for_less_specific_type = false; + } } else { $check_for_less_specific_type = false; } - } else { - $check_for_less_specific_type = false; - } - if ($check_for_less_specific_type) { - if (IssueBuffer::accepts( - new LessSpecificReturnType( - 'The inferred return type \'' . $inferred_return_type . '\' for ' . $cased_method_id . - ' is more specific than the declared return type \'' . $declared_return_type . '\'', - $return_type_location - ), - $suppressed_issues, - !($function_like_storage instanceof MethodStorage && $function_like_storage->inheritdoc) - )) { - return false; + if ($check_for_less_specific_type + && (\Psalm\Config::getInstance()->restrict_return_types + || (!$inferred_return_type->isNullable() && $declared_return_type->isNullable()) + || (!$inferred_return_type->isFalsable() && $declared_return_type->isFalsable())) + ) { + if (IssueBuffer::accepts( + new LessSpecificReturnType( + 'The inferred return type \'' + . $inferred_return_type->getId() + . '\' for ' . $cased_method_id + . ' is more specific than the declared return type \'' + . $declared_return_type->getId() . '\'', + $return_type_location + ), + $suppressed_issues, + !($function_like_storage instanceof MethodStorage && $function_like_storage->inheritdoc) + )) { + return false; + } } } } diff --git a/tests/Template/ConditionalReturnTypeTest.php b/tests/Template/ConditionalReturnTypeTest.php index aa5f781ad1b..3d096863c00 100644 --- a/tests/Template/ConditionalReturnTypeTest.php +++ b/tests/Template/ConditionalReturnTypeTest.php @@ -597,7 +597,6 @@ public static function ofType(string $type) { * @template T of mixed|false|null * @param T $i * @return (T is false ? no-return : T is null ? no-return : T) - * @psalm-suppress LessSpecificReturnType */ function orThrow($i) { if ($i === false || $i === null) {