verify tests - #6387
Closed
staabm wants to merge 9 commits into
Closed
Conversation
…jectType` in `ParserNodeTypeToPHPStanType`
* `ParserNodeTypeToPHPStanType::resolve()` turned every non-builtin native type name into an `ObjectType`. PhpStorm stubs express pre-PHP 8 signatures with a native `resource` type (via `#[LanguageLevelTypeAware(..., default: 'resource')]`), so those signatures ended up with `ObjectType('resource')`, which describes itself as `resource` but is unrelated to `ResourceType` - hence "expects resource, resource given".
* The check runs before the `self`/`static`/`parent` resolution so a class whose own name is `resource` keeps working with those keywords. This mirrors what `TypeNodeResolver` already does for the `resource` PHPDoc type.
* The single choke point fixes the whole family at once, not just `finfo_buffer`: parameter types (`finfo_file`, `curl_getinfo`, `ftp_alloc`, `ftp_quit`, `pg_clientencoding`, `pg_errormessage`, `pg_fieldname`, `pg_fieldnum`, `pg_fieldsize`, `pg_fieldtype`, `pg_freeresult`, `pg_getlastoid`, `pg_numfields`, `pg_numrows`, ...) and return types (`pg_exec`, `pg_loopen`), plus the native types merged into signature-map entries.
* Return types were the nastier case: `is_resource()` on the result of `pg_exec()` narrowed to `*NEVER*` and reported "If condition is always false".
* Probed the other type names appearing in `LanguageLevelTypeAware` attributes across phpstorm-stubs - `resource` is the only pseudo-type there; every other name is a real class, so no analogous mapping is needed. No internal class methods, properties or constants declare a `resource` native type today, but they go through the same choke point.
* New tests: `CallToFunctionParametersRulePhp7Test` (rule-level false positives, PHP 7.4) and `ResourceTypePhp7Test` (`is_resource()` narrowing and inferred/native types, PHP 7.4).
…rker Drops the dedicated ResourceTypePhp7Test and its `nodeScopeResolverPhp7.neon` config in favour of the existing convention: the data file lives in tests/PHPStan/Analyser/nsrt and declares its PHP version requirement with `<?php // lint < 8.0`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uses `#[RequiresPhp('< 8.0.0')]` on the test method instead of a separate
test class pinned to `phpVersion: 70400` via its own NEON config.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PHP has no `resource` native type - it reads such a typehint as a class name (with a "not a supported builtin type" warning), so a userland class named `Resource` used as a typehint has to keep resolving to an object type. The pseudo-type only ever reaches `ParserNodeTypeToPHPStanType` from the PhpStorm stubs, which spell it lowercase, so compare the name case-sensitively instead of lowercasing it first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Asserts the results of https://phpstan.org/r/47fe50ab-e9de-4e20-8093-b24400e53d66 are unchanged, both on the runtime PHP version and with `phpVersion: 70400`, where the PhpStorm stubs describe signatures with the `resource` pseudo-type. The same PHP 7.4 run covers the issue's reproducer, which the unit tests can only exercise on a PHP 7 runtime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Declaring a top-level class in a rule test data file pollutes the class namespace shared by the rest of the test suite. The e2e test in `e2e/bug-15141` already covers the userland `Resource` class case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
|
You've opened the pull request against the latest branch 2.3.x. PHPStan 2.3 is not going to be released for months. If your code is relevant on 2.2.x and you want it to be released sooner, please rebase your pull request and change its target to 2.2.x. |
Contributor
Author
|
tests work as expected |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.