From 5da816f160475f462ea1f2cd73ecb3c987bb9bab Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Mon, 25 Jan 2021 23:41:42 -0500 Subject: [PATCH] Ensure getId() output can always be parsed as a type Ref #5105 --- .../Statements/Expression/AssertionFinder.php | 10 +++++----- .../Reflector/FunctionLikeDocblockScanner.php | 1 + src/Psalm/Type/Atomic/TIntMask.php | 8 +++++++- src/Psalm/Type/Atomic/TLiteralInt.php | 5 +++++ src/Psalm/Type/Atomic/TLiteralString.php | 11 ++++++++--- src/Psalm/Type/Atomic/TScalarClassConstant.php | 12 ++++++------ tests/ArrayFunctionCallTest.php | 2 +- tests/AssertAnnotationTest.php | 3 +-- tests/ClosureTest.php | 2 +- tests/JsonOutputTest.php | 4 ++-- tests/LanguageServer/SymbolLookupTest.php | 4 ++-- tests/Php56Test.php | 4 ++-- tests/ReturnTypeTest.php | 2 +- tests/Template/ClassTemplateTest.php | 4 ++-- tests/TypeCombinationTest.php | 18 +++++++++--------- tests/TypeParseTest.php | 12 ++++++------ tests/TypeReconciliation/ReconcilerTest.php | 2 +- 17 files changed, 60 insertions(+), 44 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php index 5993e0bd3..4e41f1c21 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php @@ -1219,7 +1219,7 @@ class AssertionFinder null, null, null - )->getId(); + )->getAssertionString(); } } } @@ -1284,7 +1284,7 @@ class AssertionFinder null, null, null - )->getId(); + )->getAssertionString(); } } } @@ -3696,7 +3696,7 @@ class AssertionFinder $literal_assertions = []; foreach ($array_literal_types as $array_literal_type) { - $literal_assertions[] = '=' . $array_literal_type->getId(); + $literal_assertions[] = '=' . $array_literal_type->getAssertionString(); } if ($value_type->isFalsable()) { @@ -3766,11 +3766,11 @@ class AssertionFinder if ($key_type->allStringLiterals() && !$key_type->possibly_undefined) { foreach ($key_type->getLiteralStrings() as $array_literal_type) { - $literal_assertions[] = '=' . $array_literal_type->getId(); + $literal_assertions[] = '=' . $array_literal_type->getAssertionString(); } } elseif ($key_type->allIntLiterals() && !$key_type->possibly_undefined) { foreach ($key_type->getLiteralInts() as $array_literal_type) { - $literal_assertions[] = '=' . $array_literal_type->getId(); + $literal_assertions[] = '=' . $array_literal_type->getAssertionString(); } } } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php index 8559a9353..c46248717 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php @@ -557,6 +557,7 @@ class FunctionLikeDocblockScanner foreach ($namespaced_type->getAtomicTypes() as $namespaced_type_part) { if ($namespaced_type_part instanceof Type\Atomic\TAssertionFalsy + || $namespaced_type_part instanceof Type\Atomic\TScalarClassConstant || ($namespaced_type_part instanceof Type\Atomic\TList && $namespaced_type_part->type_param->isMixed()) || ($namespaced_type_part instanceof Type\Atomic\TArray diff --git a/src/Psalm/Type/Atomic/TIntMask.php b/src/Psalm/Type/Atomic/TIntMask.php index 0b8f54cef..eec25e8d6 100644 --- a/src/Psalm/Type/Atomic/TIntMask.php +++ b/src/Psalm/Type/Atomic/TIntMask.php @@ -31,7 +31,13 @@ class TIntMask extends TInt public function getId(bool $nested = false): string { - return $this->getKey(); + $s = ''; + + foreach ($this->values as $value) { + $s .= $value->getId() . ', '; + } + + return 'int-mask<' . substr($s, 0, -2) . '>'; } /** diff --git a/src/Psalm/Type/Atomic/TLiteralInt.php b/src/Psalm/Type/Atomic/TLiteralInt.php index 0286b7479..1666ca85c 100644 --- a/src/Psalm/Type/Atomic/TLiteralInt.php +++ b/src/Psalm/Type/Atomic/TLiteralInt.php @@ -20,6 +20,11 @@ class TLiteralInt extends TInt } public function getId(bool $nested = false): string + { + return (string) $this->value; + } + + public function getAssertionString(bool $exact = false): string { return 'int(' . $this->value . ')'; } diff --git a/src/Psalm/Type/Atomic/TLiteralString.php b/src/Psalm/Type/Atomic/TLiteralString.php index 003fbabdc..5c5859236 100644 --- a/src/Psalm/Type/Atomic/TLiteralString.php +++ b/src/Psalm/Type/Atomic/TLiteralString.php @@ -20,7 +20,7 @@ class TLiteralString extends TString public function getKey(bool $include_extra = true) : string { - return $this->getId(); + return 'string(' . $this->value . ')'; } public function __toString(): string @@ -32,10 +32,15 @@ class TLiteralString extends TString { $no_newline_value = preg_replace("/\n/m", '\n', $this->value); if (strlen($this->value) > 80) { - return 'string(' . substr($no_newline_value, 0, 80) . '...' . ')'; + return '"' . substr($no_newline_value, 0, 80) . '...' . '"'; } - return 'string(' . $no_newline_value . ')'; + return '"' . $no_newline_value . '"'; + } + + public function getAssertionString(bool $exact = false): string + { + return 'string(' . $this->value . ')'; } /** diff --git a/src/Psalm/Type/Atomic/TScalarClassConstant.php b/src/Psalm/Type/Atomic/TScalarClassConstant.php index e2c256e56..5e7f8c7b1 100644 --- a/src/Psalm/Type/Atomic/TScalarClassConstant.php +++ b/src/Psalm/Type/Atomic/TScalarClassConstant.php @@ -30,7 +30,12 @@ class TScalarClassConstant extends Scalar public function getId(bool $nested = false): string { - return $this->getKey(); + return $this->fq_classlike_name . '::' . $this->const_name; + } + + public function getAssertionString(bool $exact = false): string + { + return 'scalar-class-constant(' . $this->fq_classlike_name . '::' . $this->const_name . ')'; } /** @@ -69,9 +74,4 @@ class TScalarClassConstant extends Scalar . '::' . $this->const_name; } - - public function getAssertionString(bool $exact = false): string - { - return 'mixed'; - } } diff --git a/tests/ArrayFunctionCallTest.php b/tests/ArrayFunctionCallTest.php index f566746c6..4825c5691 100644 --- a/tests/ArrayFunctionCallTest.php +++ b/tests/ArrayFunctionCallTest.php @@ -688,7 +688,7 @@ class ArrayFunctionCallTest extends TestCase ARRAY_FILTER_USE_KEY );', 'assertions' => [ - '$foo' => 'array', + '$foo' => 'array', ], ], 'ignoreFalsableCurrent' => [ diff --git a/tests/AssertAnnotationTest.php b/tests/AssertAnnotationTest.php index e35516475..16324040c 100644 --- a/tests/AssertAnnotationTest.php +++ b/tests/AssertAnnotationTest.php @@ -1244,7 +1244,7 @@ class AssertAnnotationTest extends TestCase 'convertConstStringType' => [ ' [ - '$a' => 'pure-Closure():pure-Closure():string(hello)', + '$a' => 'pure-Closure():pure-Closure():"hello"', '$b' => 'string', ], ], diff --git a/tests/JsonOutputTest.php b/tests/JsonOutputTest.php index b02ce435f..43d3fa430 100644 --- a/tests/JsonOutputTest.php +++ b/tests/JsonOutputTest.php @@ -98,7 +98,7 @@ class JsonOutputTest extends TestCase function fooFoo() { return "hello"; }', - 'message' => 'Method fooFoo does not have a return type, expecting string(hello)', + 'message' => 'Method fooFoo does not have a return type, expecting "hello"', 'line' => 2, 'error' => 'fooFoo', ], @@ -110,7 +110,7 @@ class JsonOutputTest extends TestCase function fooFoo() { return "hello"; }', - 'message' => "The inferred type 'string(hello)' does not match the declared return type 'int' for fooFoo", + 'message' => "The inferred type '\"hello\"' does not match the declared return type 'int' for fooFoo", 'line' => 6, 'error' => '"hello"', ], diff --git a/tests/LanguageServer/SymbolLookupTest.php b/tests/LanguageServer/SymbolLookupTest.php index a522bac97..a04d8e503 100644 --- a/tests/LanguageServer/SymbolLookupTest.php +++ b/tests/LanguageServer/SymbolLookupTest.php @@ -216,13 +216,13 @@ class SymbolLookupTest extends \Psalm\Tests\TestCase $this->assertNotNull($symbol_at_position); - $this->assertSame('213-214:int(1)', $symbol_at_position[0]); + $this->assertSame('213-214:1', $symbol_at_position[0]); $symbol_at_position = $codebase->getReferenceAtPosition('somefile.php', new Position(17, 30)); $this->assertNotNull($symbol_at_position); - $this->assertSame('425-426:int(2)', $symbol_at_position[0]); + $this->assertSame('425-426:2', $symbol_at_position[0]); } public function testGetSymbolPositionMissingArg(): void diff --git a/tests/Php56Test.php b/tests/Php56Test.php index be0c4f493..7ea0aa920 100644 --- a/tests/Php56Test.php +++ b/tests/Php56Test.php @@ -59,8 +59,8 @@ class Php56Test extends TestCase $bitxor = C::BITXOR;', 'assertions' => [ '$c1' => 'int', - '$c2===' => 'int(2)', - '$c3===' => 'int(3)', + '$c2===' => '2', + '$c3===' => '3', '$c1_3rd' => 'float|int', '$c_sentence' => 'string', '$cf' => 'int', diff --git a/tests/ReturnTypeTest.php b/tests/ReturnTypeTest.php index f7f390662..0d838c0cd 100644 --- a/tests/ReturnTypeTest.php +++ b/tests/ReturnTypeTest.php @@ -1288,7 +1288,7 @@ class ReturnTypeTest extends TestCase return 1; }; }', - 'error_message' => 'InvalidReturnStatement - src' . DIRECTORY_SEPARATOR . 'somefile.php:9:28 - The inferred type \'pure-Closure(iterable):int(1)\' does not match the declared return type \'callable(iterable):iterable\' for map', + 'error_message' => 'InvalidReturnStatement - src' . DIRECTORY_SEPARATOR . 'somefile.php:9:28 - The inferred type \'pure-Closure(iterable):1\' does not match the declared return type \'callable(iterable):iterable\' for map', ], 'cannotInferReturnClosureWithDifferentTypes' => [ ' [ - '$c===' => 'C', + '$c===' => 'C<"hello">', ], ], 'SKIPPED-templateDefaultConstant' => [ @@ -3601,7 +3601,7 @@ class ClassTemplateTest extends TestCase $mario = new CharacterRow(["id" => 5, "name" => "Mario", "height" => 3.5]); $mario->ame = "Luigi";', - 'error_message' => 'InvalidArgument - src' . DIRECTORY_SEPARATOR . 'somefile.php:47:29 - Argument 1 of CharacterRow::__set expects string(height)|string(id)|string(name), string(ame) provided', + 'error_message' => 'InvalidArgument - src' . DIRECTORY_SEPARATOR . 'somefile.php:47:29 - Argument 1 of CharacterRow::__set expects "height"|"id"|"name", "ame" provided', ], 'specialiseTypeBeforeReturning' => [ ' [ - 'array', + 'array<"a"|int, int|string>', [ 'array{a: int}', 'array', ], ], 'combineNestedObjectTypeWithTKeyedArrayIntKeyedArray' => [ - 'array{a: array}', + 'array{a: array<"a"|int, int|string>}', [ 'array{a: array{a: int}}', 'array{a: array}', ], ], 'combineIntKeyedObjectTypeWithNestedIntKeyedArray' => [ - 'array>', + 'array>', [ 'array', 'array>', ], ], 'combineNestedObjectTypeWithNestedIntKeyedArray' => [ - 'array>', + 'array<"a"|int, array<"a"|int, int|string>>', [ 'array{a: array{a: int}}', 'array>', @@ -431,7 +431,7 @@ class TypeCombinationTest extends TestCase ], ], 'objectLikePlusArrayEqualsArray' => [ - 'array', + 'array<"a"|"b"|"c", 1|2|3>', [ 'array<"a"|"b"|"c", 1|2|3>', 'array{a: 1|2, b: 2|3, c: 1|3}', @@ -571,14 +571,14 @@ class TypeCombinationTest extends TestCase ], ], 'combineZeroAndPositiveInt' => [ - 'int(0)|positive-int', + '0|positive-int', [ '0', 'positive-int', ], ], 'combinePositiveIntAndZero' => [ - 'int(0)|positive-int', + '0|positive-int', [ 'positive-int', '0', @@ -615,7 +615,7 @@ class TypeCombinationTest extends TestCase ], ], 'combineZeroOneAndPositiveInt' => [ - 'int(0)|positive-int', + '0|positive-int', [ '0', '1', @@ -623,7 +623,7 @@ class TypeCombinationTest extends TestCase ], ], 'combinePositiveIntOneAndZero' => [ - 'int(0)|positive-int', + '0|positive-int', [ 'positive-int', '1', diff --git a/tests/TypeParseTest.php b/tests/TypeParseTest.php index 78a1dc05b..600c5e156 100644 --- a/tests/TypeParseTest.php +++ b/tests/TypeParseTest.php @@ -717,7 +717,7 @@ class TypeParseTest extends TestCase public function testCombineLiteralStringWithClassString(): void { $this->assertSame( - 'class-string|string(array)', + '"array"|class-string', Type::parseString('"array"|class-string')->getId() ); } @@ -902,26 +902,26 @@ class TypeParseTest extends TestCase { $docblock_type = Type::parseString('int-mask<0, 1, 2, 4>'); - $this->assertSame('int(0)|int(1)|int(2)|int(3)|int(4)|int(5)|int(6)|int(7)', $docblock_type->getId()); + $this->assertSame('0|1|2|3|4|5|6|7', $docblock_type->getId()); $docblock_type = Type::parseString('int-mask<1, 2, 4>'); - $this->assertSame('int(1)|int(2)|int(3)|int(4)|int(5)|int(6)|int(7)', $docblock_type->getId()); + $this->assertSame('1|2|3|4|5|6|7', $docblock_type->getId()); $docblock_type = Type::parseString('int-mask<1, 4>'); - $this->assertSame('int(1)|int(4)|int(5)', $docblock_type->getId()); + $this->assertSame('1|4|5', $docblock_type->getId()); $docblock_type = Type::parseString('int-mask'); - $this->assertSame('int(1)|int(256)|int(257)|int(512)|int(513)|int(768)|int(769)', $docblock_type->getId()); + $this->assertSame('1|256|257|512|513|768|769', $docblock_type->getId()); } public function testIntMaskWithClassConstant(): void { $docblock_type = Type::parseString('int-mask<0, A::FOO, A::BAR>'); - $this->assertSame('int-mask', $docblock_type->getId()); + $this->assertSame('int-mask<0, A::FOO, A::BAR>', $docblock_type->getId()); } public function testIntMaskWithInvalidClassConstant(): void diff --git a/tests/TypeReconciliation/ReconcilerTest.php b/tests/TypeReconciliation/ReconcilerTest.php index db3a06f2f..a626ead52 100644 --- a/tests/TypeReconciliation/ReconcilerTest.php +++ b/tests/TypeReconciliation/ReconcilerTest.php @@ -101,7 +101,7 @@ class ReconcilerTest extends \Psalm\Tests\TestCase 'falsyWithMyObjectPipeBool' => ['false', 'falsy', 'MyObject|bool'], 'falsyWithMixed' => ['empty-mixed', 'falsy', 'mixed'], 'falsyWithBool' => ['false', 'falsy', 'bool'], - 'falsyWithStringOrNull' => ['null|string()|string(0)', 'falsy', 'string|null'], + 'falsyWithStringOrNull' => ['""|"0"|null', 'falsy', 'string|null'], 'falsyWithScalarOrNull' => ['empty-scalar', 'falsy', 'scalar'], 'notMyObjectWithMyObjectPipeBool' => ['bool', '!MyObject', 'MyObject|bool'],