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

psalm --alter generates invalid PHP code #9642

Closed
sebastianbergmann opened this issue Apr 12, 2023 · 5 comments
Closed

psalm --alter generates invalid PHP code #9642

sebastianbergmann opened this issue Apr 12, 2023 · 5 comments

Comments

@sebastianbergmann
Copy link
Contributor

$ git remote -v
origin	[email protected]:sebastianbergmann/phpunit.git (fetch)
origin	[email protected]:sebastianbergmann/phpunit.git (push)
$ git show --oneline -s
66eb85ba89 (HEAD -> main) Reorganize
$ ./tools/psalm --version                                                   
Psalm 5.9.0@8b9ad1eb9e8b7d3101f949291da2b9f7767cd163
$ ./tools/psalm --config=.psalm/config.xml --alter --issues=MissingParamType --no-progress
------------------------------
                              
       No errors found!       
                              
------------------------------

Checks took 12.44 seconds and used 302.228MB of memory
Psalm was able to infer types for 97.0460% of the codebase
diff --git a/src/Framework/Assert.php b/src/Framework/Assert.php
index 5735abf10..5b2cc3b0c 100644
--- a/src/Framework/Assert.php
+++ b/src/Framework/Assert.php
@@ -268,8 +268,11 @@ final public static function assertNotCount(int $expectedCount, Countable|iterab
      * Asserts that two variables are equal.
      *
      * @throws ExpectationFailedException
+     *
+     * @param \DOMDocument|string $expected
+     * @param \DOMDocument|string $actual
      */
-    final public static function assertEquals(mixed $expected, mixed $actual, string $message = ''): void
+    final public static function assertEquals(string|\DOMDocumentd $expected, string|\DOMDocumentd $actual, string $message = ''): void
     {
         $constraint = new IsEqual($expected);
 
@@ -319,8 +322,11 @@ final public static function assertEqualsWithDelta(mixed $expected, mixed $actua
      * Asserts that two variables are not equal.
      *
      * @throws ExpectationFailedException
+     *
+     * @param \DOMDocument|bool $expected
+     * @param \DOMDocument|bool $actual
      */
-    final public static function assertNotEquals(mixed $expected, mixed $actual, string $message = ''): void
+    final public static function assertNotEquals(bool|\DOMDocumentd $expected, bool|\DOMDocumentd $actual, string $message = ''): void
     {
         $constraint = new LogicalNot(
             new IsEqual($expected)
@@ -1822,8 +1828,10 @@ final public static function assertXmlStringNotEqualsXmlString(string $expectedX
      * Evaluates a PHPUnit\Framework\Constraint matcher object.
      *
      * @throws ExpectationFailedException
+     *
+     * @param false|int|iterable|null|object|string $value
      */
-    final public static function assertThat(mixed $value, Constraint $constraint, string $message = ''): void
+    final public static function assertThat(iterable|object|string|int|false|nulld $value, Constraint $constraint, string $message = ''): void
     {
         self::$count += count($constraint);
 
@@ -1990,7 +1998,7 @@ final public static function logicalAnd(mixed ...$constraints): LogicalAnd
         return LogicalAnd::fromConstraints(...$constraints);
     }
 
-    final public static function logicalOr(mixed ...$constraints): LogicalOr
+    final public static function logicalOr(IsEquald ...$constraints): LogicalOr
     {
         return LogicalOr::fromConstraints(...$constraints);
     }
diff --git a/src/Framework/Constraint/Cardinality/Count.php b/src/Framework/Constraint/Cardinality/Count.php
index 512013ce6..4735b30b8 100644
--- a/src/Framework/Constraint/Cardinality/Count.php
+++ b/src/Framework/Constraint/Cardinality/Count.php
@@ -54,8 +54,10 @@ protected function matches(mixed $other): bool
 
     /**
      * @throws Exception
+     *
+     * @param \Countable|iterable $other
      */
-    protected function getCountOf(mixed $other): ?int
+    protected function getCountOf(iterable|\Countabled $other): ?int
     {
         if (is_countable($other)) {
             return count($other);
diff --git a/src/Framework/Constraint/Constraint.php b/src/Framework/Constraint/Constraint.php
index 274927d31..caecf0263 100644
--- a/src/Framework/Constraint/Constraint.php
+++ b/src/Framework/Constraint/Constraint.php
@@ -35,7 +35,7 @@ abstract class Constraint implements Countable, SelfDescribing
      *
      * @throws ExpectationFailedException
      */
-    public function evaluate(mixed $other, string $description = '', bool $returnResult = false): ?bool
+    public function evaluate(stringd $other, string $description = '', bool $returnResult = false): ?bool
     {
         $success = false;
 
@@ -87,7 +87,7 @@ protected function matches(mixed $other): bool
      *
      * @throws ExpectationFailedException
      */
-    protected function fail(mixed $other, string $description, ComparisonFailure $comparisonFailure = null): never
+    protected function fail(stringd $other, string $description, ComparisonFailure $comparisonFailure = null): never
     {
         $failureDescription = sprintf(
             'Failed asserting that %s.',
@@ -146,8 +146,10 @@ protected function failureDescription(mixed $other): string
      *
      * The method shall return empty string, when it does not handle
      * customization by itself.
+     *
+     * @psalm-param int<min, max> $role
      */
-    protected function toStringInContext(Operator $operator, mixed $role): string
+    protected function toStringInContext(Operator $operator, intd $role): string
     {
         return '';
     }
@@ -163,8 +165,10 @@ protected function toStringInContext(Operator $operator, mixed $role): string
      *
      * The method shall return empty string, when it does not handle
      * customization by itself.
+     *
+     * @psalm-param 0 $role
      */
-    protected function failureDescriptionInContext(Operator $operator, mixed $role, mixed $other): string
+    protected function failureDescriptionInContext(Operator $operator, intd $role, mixed $other): string
     {
         $string = $this->toStringInContext($operator, $role);
 
diff --git a/src/Util/GlobalState.php b/src/Util/GlobalState.php
index e61c2ff8f..e26aa945d 100644
--- a/src/Util/GlobalState.php
+++ b/src/Util/GlobalState.php
@@ -251,6 +251,11 @@ public static function getGlobalsAsString(): string
         return $result;
     }
 
+    /**
+     * @param (int|string)|array|null|resource|scalar $variable
+     *
+     * @psalm-param array|array-key|null|resource|scalar $variable
+     */
     private static function exportVariable(mixed $variable): string
     {
         if (is_scalar($variable) || $variable === null ||

TL;DR: psalm --alter transforms mixed $role into intd $role, for instance.

@psalm-github-bot
Copy link

Hey @sebastianbergmann, can you reproduce the issue on https://psalm.dev ?

@BenjaminHoegh
Copy link

funny, I have a similar issue where private $createAnchorIDCallback = null; becomes private ?callable $createAnchorIDCallback = null; but that aren't allowed in PHP 7 using --php-version=7.4

@weirdan
Copy link
Collaborator

weirdan commented Feb 3, 2024

Fixed in #9803, the fix released in 5.12.0.

@weirdan weirdan closed this as completed Feb 3, 2024
@BenjaminHoegh
Copy link

Fixed in #9803, the fix released in 5.12.0.

Looks like its back then

./vendor/bin/psalm -v                                              
Psalm 5.21.1@8c473e2437be8b6a8fd8f630f0f11a16b114c494

./vendor/bin/psalm --alter --php-version=7.4 --issues=all --dry-run
Target PHP version: 7.4 (set by CLI argument).
Scanning files...
Analyzing files...

Altering files...
@@ -38,7 +38,7 @@
-    private $createAnchorIDCallback = null;
+    private ?callable $createAnchorIDCallback = null;

@weirdan
Copy link
Collaborator

weirdan commented Feb 3, 2024

The original issue (appending d to the type when replacing mixed) was fixed. Yours looks very different, so please report it as a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants