Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: allow non-union assertion types #8077

Merged
merged 18 commits into from Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a9daa3b
qa: add failing test for one-of assertion
boesing Jun 8, 2022
40971ff
bugfix: allow non-single union types in assertions
boesing Jun 8, 2022
7c85e0c
bugfix: only override templated values in case of literals
boesing Jun 11, 2022
2d9133b
bugfix: overriding types based on assertions have to pass more checks
boesing Jun 11, 2022
4ee1b3f
bugfix: only extend existing logic instead of hijacking it
boesing Jun 11, 2022
c68b6e9
bugfix: only allow narrowing down types in case the old type is not s…
boesing Jun 11, 2022
6c1ed90
qa: ensure we use multiple `isset` to verify the existence of `$arg_v…
boesing Jun 11, 2022
e13268f
qa: use `$assertion_var_id` over `$arg_var_id` for better type reflec…
boesing Jun 11, 2022
4068f5c
feature: add `Union#allFloatLiterals` as it was missing
boesing Jun 12, 2022
6a73fbd
bugfix: tighten the detection of types which narrow down other types
boesing Jun 12, 2022
083da24
qa: add more specific unit test(s) for the `oneOf` detection
boesing Jun 12, 2022
92b4bf1
qa: use dedicated variable for asserting a specific value
boesing Jun 12, 2022
02d4f0c
feature: add more accurate `equals` checks for `TLiteralFloat`, `TLit…
boesing Jun 12, 2022
ed1bb8a
bugfix: in case the old type is already more accurate than the new ty…
boesing Jun 12, 2022
7e033d8
bugfix: do not extend the type - only narrow down
boesing Jun 12, 2022
3fd7a8b
qa: refactor code to avoid too many nesting levels
boesing Jun 12, 2022
0d96766
bugfix: `single` does not mean that only one single atomic type is st…
boesing Jun 12, 2022
df927d0
qa: drop unnecessary `ensure_source_equality` check
boesing Jun 27, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
96 changes: 96 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
Expand Up @@ -770,6 +770,7 @@ public static function applyAssertionsToContext(
) {
continue;
}

$assertion_rule = clone $assertion_rule;
$assertion_rule->setAtomicType($atomic_type);
$orred_rules[] = $assertion_rule;
Expand All @@ -795,6 +796,23 @@ public static function applyAssertionsToContext(
);
}
}
} elseif (isset($context->vars_in_scope[$assertion_var_id])) {
$other_type = $context->vars_in_scope[$assertion_var_id];
$union = self::createUnionIntersectionFromOldType($union, $other_type);

if ($union !== null) {
foreach ($union->getAtomicTypes() as $atomic_type) {
if ($assertion_type instanceof TTemplateParam
&& $assertion_type->as->getId() === $atomic_type->getId()
) {
continue;
}

$assertion_rule = clone $assertion_rule;
$assertion_rule->setAtomicType($atomic_type);
$orred_rules[] = $assertion_rule;
}
}
}
} else {
$orred_rules[] = $assertion_rule;
Expand Down Expand Up @@ -1116,4 +1134,82 @@ public static function checkTemplateResult(
}
}
}

/**
* This method should detect if the new type narrows down the old type.
*/
private static function isNewTypeNarrowingDownOldType(Union $old_type, Union $new_type): bool
{
if ($new_type->isSingle()) {
return true;
}

// non-mixed is always better than mixed
if ($old_type->isMixed() && !$new_type->hasMixed()) {
return true;
}

// non-nullable is always better than nullable
if ($old_type->isNullable() && !$new_type->isNullable()) {
return true;
}

// Do not hassle around with non-single old types if they are not nullable
if (!$old_type->isSingle()) {
return false;
}

// Do not hassle around with single literals as they supposed to be more accurate than any new type assertion
if ($old_type->isSingleFloatLiteral()
|| $old_type->isSingleIntLiteral()
|| $old_type->isSingleStringLiteral()
) {
return false;
}

// Literals should always replace non-literals
if (($old_type->isString() && $new_type->allStringLiterals())
|| ($old_type->isInt() && $new_type->allIntLiterals())
|| ($old_type->isFloat() && $new_type->allFloatLiterals())
) {
return true;
}

return false;
}

/**
* This method should kick all literals within `new_type` which are not part of the already known `old_type`.
* So lets say we already know that the old type is one of "a", "b" or "c".
* If another assertion takes place to determine if the value is either "a", "c" or "d", we can kick "d" as that
* won't be possible.
*/
private static function createUnionIntersectionFromOldType(Union $new_type, Union $old_type): ?Union
{
if (!self::isNewTypeNarrowingDownOldType($old_type, $new_type)) {
return null;
}

if (!$new_type->allLiterals() || !$old_type->allLiterals()) {
return $new_type;
}

$equal_atomic_types = [];

foreach ($new_type->getAtomicTypes() as $new_atomic_type) {
foreach ($old_type->getAtomicTypes() as $old_atomic_type) {
if (!$new_atomic_type->equals($old_atomic_type, false)) {
continue;
}

$equal_atomic_types[] = $new_atomic_type;
}
}

if ($equal_atomic_types === []) {
return null;
}

return new Union($equal_atomic_types);
}
}
13 changes: 13 additions & 0 deletions src/Psalm/Type/Atomic/TLiteralFloat.php
Expand Up @@ -2,6 +2,10 @@

namespace Psalm\Type\Atomic;

use Psalm\Type\Atomic;

use function get_class;

/**
* Denotes a floating point value where the exact numeric value is known.
*/
Expand Down Expand Up @@ -41,4 +45,13 @@ public function toNamespacedString(
): string {
return 'float';
}

public function equals(Atomic $other_type, bool $ensure_source_equality): bool
{
if (get_class($other_type) !== static::class) {
return false;
}

return $this->value === $other_type->value;
}
}
19 changes: 19 additions & 0 deletions src/Psalm/Type/Atomic/TLiteralInt.php
Expand Up @@ -2,6 +2,10 @@

namespace Psalm\Type\Atomic;

use Psalm\Type\Atomic;

use function get_class;

/**
* Denotes an integer value where the exact numeric value is known.
*/
Expand Down Expand Up @@ -46,4 +50,19 @@ public function toNamespacedString(
): string {
return $use_phpdoc_format ? 'int' : (string) $this->value;
}

public function equals(Atomic $other_type, bool $ensure_source_equality): bool
{
if (get_class($other_type) !== static::class) {
return false;
}

if (($this->from_docblock && $ensure_source_equality)
|| ($other_type->from_docblock && $ensure_source_equality)
) {
return false;
}

return $this->value === $other_type->value;
}
}
18 changes: 18 additions & 0 deletions src/Psalm/Type/Atomic/TLiteralString.php
Expand Up @@ -2,7 +2,10 @@

namespace Psalm\Type\Atomic;

use Psalm\Type\Atomic;

use function addcslashes;
use function get_class;
use function mb_strlen;
use function mb_substr;

Expand Down Expand Up @@ -55,4 +58,19 @@ public function toNamespacedString(
): string {
return $use_phpdoc_format ? 'string' : "'" . $this->value . "'";
}

public function equals(Atomic $other_type, bool $ensure_source_equality): bool
{
if (get_class($other_type) !== static::class) {
return false;
}

if (($this->from_docblock && $ensure_source_equality)
|| ($other_type->from_docblock && $ensure_source_equality)
) {
return false;
}

return $this->value === $other_type->value;
}
}
11 changes: 11 additions & 0 deletions src/Psalm/Type/Union.php
Expand Up @@ -1267,6 +1267,17 @@ public function allIntLiterals(): bool
return true;
}

public function allFloatLiterals(): bool
{
foreach ($this->types as $atomic_key_type) {
if (!$atomic_key_type instanceof TLiteralFloat) {
return false;
}
}

return true;
}

public function allLiterals(): bool
{
foreach ($this->types as $atomic_key_type) {
Expand Down
60 changes: 59 additions & 1 deletion tests/AssertAnnotationTest.php
Expand Up @@ -90,7 +90,6 @@ function requiresString(string $_str): void {}
$this->analyzeFile('somefile.php', new Context());
}


/**
* @return iterable<string,array{code:string,assertions?:array<string,string>,ignored_issues?:list<string>}>
*/
Expand Down Expand Up @@ -2055,6 +2054,65 @@ function makeLowerNonEmpty(string $input): string
return $input;
}',
],
'assertOneOfValuesWithinArray' => [
'code' => '<?php

/**
* @template T
* @param mixed $input
* @param array<array-key,T> $values
* @psalm-assert T $input
*/
function assertOneOf($input, array $values): void {}

/** @param "a" $value */
function consumeSpecificStringValue(string $value): void {}

/** @param literal-string $value */
function consumeLiteralStringValue(string $value): void {}

function consumeAnyIntegerValue(int $value): void {}

function consumeAnyFloatValue(float $value): void {}

/** @var string $string */
$string;

/** @var string $anotherString */
$anotherString;

/** @var null|string $nullableString */
$nullableString;

/** @var mixed $maybeInt */
$maybeInt;
/** @var mixed $maybeFloat */
$maybeFloat;

assertOneOf($string, ["a"]);
consumeSpecificStringValue($string);

assertOneOf($anotherString, ["a", "b", "c"]);
consumeLiteralStringValue($anotherString);

assertOneOf($nullableString, ["a", "b", "c"]);
assertOneOf($nullableString, ["a", "c"]);

assertOneOf($maybeInt, [1, 2, 3]);
consumeAnyIntegerValue($maybeInt);

assertOneOf($maybeFloat, [1.5, 2.5, 3.5]);
consumeAnyFloatValue($maybeFloat);

/** @var "a"|"b"|"c" $abc */
$abc;

/** @param "a"|"b" $aOrB */
function consumeAOrB(string $aOrB): void {}
assertOneOf($abc, ["a", "b"]);
consumeAOrB($abc);
'
],
];
}

Expand Down