Drop Psalm in favor of PHPStan #6719

greg0ire · 2025-01-15T07:27:16Z

As per the decision we made during the hackathon.

psalm-immutable annotations have been dropped, as they were only
introduced to sastisfy Psalm

Also, the phpstan- prefix has been dropped from some phpdoc annotation
where I believe it will not confuse any up-to-date IDE.

this means fresher dependencies

$ composer update -v
Loading composer repositories with package information
Updating dependencies
Dependency resolution completed in 0.001 seconds
Analyzed 240 packages to resolve dependencies
Analyzed 526 rules to resolve dependencies
Dependency resolution completed in 0.000 seconds
Lock file operations: 0 installs, 2 updates, 19 removals
Updates: nikic/php-parser:v5.4.0, symfony/console:v7.2.1
Removals: webmozart/path-util, webmozart/assert, vimeo/psalm, symfony/polyfill-php80, psalm/plugin-phpunit, phpdocumentor/type-resolver, phpdocumentor/reflection-docblock, phpdocumentor/reflection-common, openlss/lib-array2xml, netresearch/jsonmapper, felixfbecker/language-server-protocol, felixfbecker/advanced-json-rpc, dnoegel/php-xdg-base-dir, composer/xdebug-handler, composer/semver, composer/pcre, composer/package-versions-deprecated, amphp/byte-stream, amphp/amp
  - Removing amphp/amp (v2.6.4)
  - Removing amphp/byte-stream (v1.8.2)
  - Removing composer/package-versions-deprecated (1.11.99.5)
  - Removing composer/pcre (3.3.2)
  - Removing composer/semver (3.4.3)
  - Removing composer/xdebug-handler (3.0.5)
  - Removing dnoegel/php-xdg-base-dir (v0.1.1)
  - Removing felixfbecker/advanced-json-rpc (v3.2.1)
  - Removing felixfbecker/language-server-protocol (v1.5.3)
  - Removing netresearch/jsonmapper (v4.5.0)
  - Removing openlss/lib-array2xml (1.0.0)
  - Removing phpdocumentor/reflection-common (2.2.0)
  - Removing phpdocumentor/reflection-docblock (5.6.1)
  - Removing phpdocumentor/type-resolver (1.10.0)
  - Removing psalm/plugin-phpunit (0.18.4)
  - Removing symfony/polyfill-php80 (v1.31.0)
  - Removing vimeo/psalm (4.30.0)
  - Removing webmozart/assert (1.11.0)
  - Removing webmozart/path-util (2.3.0)
  - Upgrading nikic/php-parser (v4.19.4 => v5.4.0)
  - Upgrading symfony/console (v6.4.17 => v7.2.1)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 0 installs, 2 updates, 19 removals
Updates: nikic/php-parser:v5.4.0, symfony/console:v7.2.1
Removals: webmozart/path-util, webmozart/assert, vimeo/psalm, symfony/polyfill-php80, psalm/plugin-phpunit, phpdocumentor/type-resolver, phpdocumentor/reflection-docblock, phpdocumentor/reflection-common, openlss/lib-array2xml, netresearch/jsonmapper, felixfbecker/language-server-protocol, felixfbecker/advanced-json-rpc, dnoegel/php-xdg-base-dir, composer/xdebug-handler, composer/semver, composer/pcre, composer/package-versions-deprecated, amphp/byte-stream, amphp/amp
  - Removing webmozart/path-util (2.3.0)
  - Removing webmozart/assert (1.11.0)
  - Removing vimeo/psalm (4.30.0)
  - Removing symfony/polyfill-php80 (v1.31.0)
  - Removing psalm/plugin-phpunit (0.18.4)
  - Removing phpdocumentor/type-resolver (1.10.0)
  - Removing phpdocumentor/reflection-docblock (5.6.1)
  - Removing phpdocumentor/reflection-common (2.2.0)
  - Removing openlss/lib-array2xml (1.0.0)
  - Removing netresearch/jsonmapper (v4.5.0)
  - Removing felixfbecker/language-server-protocol (v1.5.3)
  - Removing felixfbecker/advanced-json-rpc (v3.2.1)
  - Removing dnoegel/php-xdg-base-dir (v0.1.1)
  - Removing composer/xdebug-handler (3.0.5)
  - Removing composer/semver (3.4.3)
  - Removing composer/pcre (3.3.2)
  - Removing composer/package-versions-deprecated (1.11.99.5)
  - Removing amphp/byte-stream (v1.8.2)
  - Removing amphp/amp (v2.6.4)
  - Upgrading nikic/php-parser (v4.19.4 => v5.4.0): Extracting archive
  - Upgrading symfony/console (v6.4.17 => v7.2.1): Extracting archive
Generating autoload files
42 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
> post-update-cmd: PHPCSStandards\Composer\Plugin\Installers\PHPCodeSniffer\Plugin->onDependenciesChangedEvent
Running PHPCodeSniffer Composer Installer
No PHPCS standards to install or update
No security vulnerability advisories found.

greg0ire · 2025-01-15T07:39:49Z

When merging this up, we'll have to be careful to convert existing psalm- annotations present in upper branches.

derrabus · 2025-01-15T10:36:30Z

On 3.x, we don't run PHPStan on the tests yet. If we drop Psalm now, we'd lose coverage. 😕

morozov · 2025-01-15T16:35:14Z

.github/workflows/static-analysis.yml

-        run: vendor/bin/psalm --shepherd
-
-      - name: Run type inference tests with Vimeo Psalm
-        run: vendor/bin/psalm --config=psalm-strict.xml --shepherd


In this case, we should remove type coverage information from the README:

dbal/README.md

Line 41 in 179e73d

[TypeCov]: https://shepherd.dev/github/doctrine/dbal

morozov · 2025-01-15T16:37:21Z

src/ArrayParameters/Exception/MissingNamedParameter.php

@@ -7,7 +7,7 @@

 use function sprintf;

-/** @psalm-immutable */
+/** @phpstan-immutable */


I remember that we had to add this annotations to satisfy Psalm. Maybe let's double check what value they provide and, as an option, remove them instead of updating.

morozov · 2025-01-15T16:39:26Z

src/Driver/IBMDB2/Exception/CannotCopyStreamToStream.php

 */
 final class CannotCopyStreamToStream extends AbstractException
 {
-    /** @psalm-param array{message: string}|null $error */
+    /** @phpstan-param array{message: string}|null $error */


Probably, at this point it could be just @param. We use this syntax in @return annotations in the test data providers a lot.

morozov · 2025-01-15T16:42:05Z

src/Driver/PgSQL/Statement.php

@@ -40,7 +40,7 @@ final class Statement implements StatementInterface
    /** @var array<int, mixed> */
    private array $parameters = [];

-    /** @psalm-var array<int, int> */
+    /** @phpstan-var array<int, int> */


This also could be a @var.

morozov · 2025-01-15T16:52:54Z

I'd revisit all @var, @param and @return annotations (everything about types) and double-checked if we can drop the vendor prefix instead of replacing it. That should be the case for simple types (no sure how simple).

Also, what do we do with tracking the usage of deprecated APIs? So far, Psalm was the only tool that we used that provided this tracking, however, in hindsight, it reported the usage of our own APIs in 99.99% of cases, which we use on purpose to establish backward compatibility. I imagine that tracking of deprecated PHP APIs is done via stubs, which may be outdated (i.e. not fully accurate), so it feels like losing this tracking is not a big loss. Am I right?

As per the decision we made during the hackathon. psalm-immutable annotations have been dropped, as they were only introduced to sastisfy Psalm

In the cases at hand, having an unprefixed phpdoc annotation should not confused any up-to-date IDE.

greg0ire · 2025-01-15T17:59:04Z

I'd revisit all @var, @param and @return annotations (everything about types) and double-checked if we can drop the vendor prefix instead of replacing it. That should be the case for simple types (no sure how simple).

I'm not sure either, so I removed the syntax you mentioned. We're left with this:

tests/DriverManagerTest.php
162:     * @phpstan-param Params|string $url

tests/Platforms/SQLServerPlatformTest.php
1839:     * @phpstan-param LockMode::* $lockMode

src/Result.php
267:     * @phpstan-param FetchMode::* $mode
306:     * @phpstan-param FetchMode::* $mode

tests/Functional/PrimaryReadReplicaConnectionTest.php
51:     * @phpstan-return Params

src/DriverManager.php
101:     * @phpstan-var array<string, key-of<self::DRIVER_MAP>>
156:     * @phpstan-param Params $params
158:     * @phpstan-return ($params is array{wrapperClass: class-string<T>} ? T : Connection)
204:     * @phpstan-return list<key-of<self::DRIVER_MAP>>
241:     * @phpstan-param Params $params
245:     * @phpstan-return Params

tests/Functional/ExceptionTest.php
324:     * @phpstan-param Params $params

src/Driver.php
24:     * @phpstan-param Params $params All connection parameters.

src/Connection.php
139:     * @phpstan-var Params
183:     * @phpstan-param Params $params
247:     * @phpstan-return Params

src/ArrayParameterType.php
30:     * @phpstan-param self::* $type
32:     * @phpstan-return ParameterType::INTEGER|ParameterType::STRING|ParameterType::ASCII|ParameterType::BINARY

tests/Tools/DsnParserTest.php
17:     * @phpstan-param Params $expected
34:    /** @phpstan-return iterable<string, array{string, Params}> */

src/Connections/PrimaryReadReplicaConnection.php
101:     * @phpstan-param Params $params

src/Platforms/SqlitePlatform.php
1028:     * @phpstan-param int-mask-of<AbstractPlatform::CREATE_*>|null $createFlags

src/Platforms/AbstractPlatform.php
1797:     * @phpstan-param LockMode::* $lockMode
2056:     * @phpstan-param int-mask-of<self::CREATE_*> $createFlags
4567:     * @phpstan-return class-string<KeywordList>

tests/Schema/MySQLInheritCharsetTest.php
81:     * @phpstan-param Params $params

src/Types/DateType.php
37:     * @phpstan-param T $value

src/Types/DateTimeTzImmutableType.php
25:     * @phpstan-param T $value

src/Tools/DsnParser.php
35:     * @phpstan-return Params

src/Query/QueryBuilder.php
116:     * @phpstan-var self::SELECT|self::DELETE|self::UPDATE|self::INSERT
123:     * @phpstan-var self::STATE_*
222:     * @phpstan-return self::STATE_*

src/Query/Expression/CompositeExpression.php
152:     * @phpstan-return int<0, max>

src/Portability/Converter.php
154:     * @phpstan-return (T is string ? string : T)
231:                * @phpstan-return (T is false ? false : T)

tests/Functional/Driver/PgSQL/ResultTest.php
152:    /** @phpstan-return Generator<string, array{string, mixed, (Types::*)}> */

src/Driver/PDO/Result.php
94:     * @phpstan-param PDO::FETCH_* $mode
110:     * @phpstan-param PDO::FETCH_* $mode

src/Driver/PDO/ParameterTypeMap.php
27:     * @phpstan-return PDO::PARAM_*

src/Driver/SQLite3/Statement.php
124:     * @phpstan-return value-of<self::PARAM_TYPE_MAP>

src/Driver/API/OCI/ExceptionConverter.php
35:        /** @phpstan-var int|'HY000' $code */ // @phpstan-ignore varTag.type

Feel free to request more removals, anybody.

Also, what do we do with tracking the usage of deprecated APIs? So far, Psalm was the only tool that we used that provided this tracking, however, in hindsight, it reported the usage of our own APIs in 99.99% of cases, which we use on purpose to establish backward compatibility. I imagine that tracking of deprecated PHP APIs is done via stubs, which may be outdated (i.e. not fully accurate), so it feels like losing this tracking is not a big loss. Am I right?

I forgot to set it up, despite that being mentioned here: https://github.com/doctrine/.github/releases/tag/5.2.0
However, given what you're telling us, I think we can indeed omit it, at least for now.

morozov

Feel free to request more removals, anybody.

The remaining annotations with the vendor prefix look valid.

Thanks @greg0ire for the cleanup, much awaited.

greg0ire force-pushed the drop-psalm branch 2 times, most recently from b2debc7 to 0fef7ef Compare January 15, 2025 07:31

greg0ire requested review from derrabus, morozov and SenseException January 15, 2025 07:39

greg0ire force-pushed the drop-psalm branch from 0fef7ef to 0130f4b Compare January 15, 2025 07:40

derrabus added the Static Analysis label Jan 15, 2025

morozov reviewed Jan 15, 2025

View reviewed changes

greg0ire and others added 3 commits January 15, 2025 18:58

Drop Psalm in favor of PHPStan

80e7ec0

As per the decision we made during the hackathon. psalm-immutable annotations have been dropped, as they were only introduced to sastisfy Psalm

Run PHPStan on tests

6ebb10d

Remove phpstan- prefix

6cea6d4

In the cases at hand, having an unprefixed phpdoc annotation should not confused any up-to-date IDE.

greg0ire force-pushed the drop-psalm branch from a181af4 to 6cea6d4 Compare January 15, 2025 17:58

greg0ire requested a review from morozov January 15, 2025 17:59

morozov approved these changes Jan 15, 2025

View reviewed changes

greg0ire added this to the 3.9.4 milestone Jan 15, 2025

greg0ire merged commit 9f28dee into doctrine:3.9.x Jan 15, 2025
68 checks passed

greg0ire deleted the drop-psalm branch January 15, 2025 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop Psalm in favor of PHPStan #6719

Drop Psalm in favor of PHPStan #6719

greg0ire commented Jan 15, 2025 •

edited

Loading

greg0ire commented Jan 15, 2025

derrabus commented Jan 15, 2025 •

edited

Loading

morozov Jan 15, 2025

morozov Jan 15, 2025 •

edited

Loading

morozov Jan 15, 2025 •

edited

Loading

morozov Jan 15, 2025

morozov commented Jan 15, 2025 •

edited

Loading

greg0ire commented Jan 15, 2025

morozov left a comment

Drop Psalm in favor of PHPStan #6719

Drop Psalm in favor of PHPStan #6719

Conversation

greg0ire commented Jan 15, 2025 • edited Loading

greg0ire commented Jan 15, 2025

derrabus commented Jan 15, 2025 • edited Loading

morozov Jan 15, 2025

Choose a reason for hiding this comment

morozov Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

morozov Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

morozov Jan 15, 2025

Choose a reason for hiding this comment

morozov commented Jan 15, 2025 • edited Loading

greg0ire commented Jan 15, 2025

morozov left a comment

Choose a reason for hiding this comment

greg0ire commented Jan 15, 2025 •

edited

Loading

derrabus commented Jan 15, 2025 •

edited

Loading

morozov Jan 15, 2025 •

edited

Loading

morozov Jan 15, 2025 •

edited

Loading

morozov commented Jan 15, 2025 •

edited

Loading