-
Notifications
You must be signed in to change notification settings - Fork 803
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
Php8 multiple signatures #950
Conversation
gd/gd.php
Outdated
* the image type is unsupported, the data is not in a recognised format, | ||
* or the image is corrupt and cannot be loaded. | ||
*/ | ||
#[Pure] | ||
function imagecreatefromstring ($image) {} | ||
|
||
#[PhpStormStubsElementAvailable('5.3','7.4')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the matter of code style I'd group multiple attributes together
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it but decided to insert the annotation before the PHPDoc for better readability but this is discussable I guess and depends on the navigation patterns:) If GoTo is used, it's better to have attributes together if scrolling - better to have a new attribute above.
💪 |
* on failure. | ||
* @see PDOStatement::setFetchMode For a full description of the second and following parameters. | ||
*/ | ||
public function query ($statement, $mode = PDO::ATTR_DEFAULT_FETCH_MODE, ...$fetch_mode_args) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be it makes sense to add property and return types (including union types) directly to signatures for entities marked with #[PhpStormStubsElementAvailable('8.0')]
? Thus we will strict types and improve type inference.
Should we somehow explicitly notify authors of 3rd party tools about such changes to allow them update tools to support duplicated items in stubs? |
Duplicates existed in stubs before this change, so nothing generally should change |
No description provided.