Skip to content

Commit

Permalink
Allow opt-in to strict return type checking
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed Nov 5, 2020
1 parent d47d817 commit 388e804
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 48 deletions.
1 change: 1 addition & 0 deletions config.xsd
Expand Up @@ -76,6 +76,7 @@
<xs:attribute name="allowInternalNamedArgumentCalls" type="xs:boolean" default="true" />
<xs:attribute name="allowNamedArgumentCalls" type="xs:boolean" default="true" />
<xs:attribute name="reportInfo" type="xs:boolean" default="true" />
<xs:attribute name="restrictReturnTypes" type="xs:boolean" default="false" />
</xs:complexType>

<xs:complexType name="ProjectFilesType">
Expand Down
6 changes: 6 additions & 0 deletions src/Psalm/Config.php
Expand Up @@ -433,6 +433,11 @@ class Config
*/
public $resolve_from_config_file = true;

/**
* @var bool
*/
public $restrict_return_types = false;

/**
* @var string[]
*/
Expand Down Expand Up @@ -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) {
Expand Down
99 changes: 52 additions & 47 deletions src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php
Expand Up @@ -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;
}
}
}
}
Expand Down
1 change: 0 additions & 1 deletion tests/Template/ConditionalReturnTypeTest.php
Expand Up @@ -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) {
Expand Down

0 comments on commit 388e804

Please sign in to comment.