Skip to content

Fix bad Pure attributes #1730

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix bad Pure attributes #1730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
73a8d84
`"classish"_exists()` functions are not pure
cs278 Apr 10, 2025
ecb8b80
Fix `Pure` attribute on `get_object_vars()` and `get_resource_type()`
cs278 Apr 10, 2025
bcf4cf1
Neither `ob_get_clean()` or `ob_get_flush()` are pure
cs278 Apr 10, 2025
fce8dc2
Remove `Pure` attribute from `array_intersect_uassoc()`
cs278 Apr 10, 2025
9d24dd5
Add note about not marking functions which takes callables as Pure
cs278 Apr 10, 2025
56d4178
Fix CI failure due to using wrong version of PHP
cs278 Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,4 @@ jobs:
run: docker compose -f docker-compose.yml run test_runner vendor/bin/phpunit --no-progress --testsuite Structure

- name: run cs fixer
run: docker run --rm -v .:/opt/project -w /opt/project php:8.3-cli sh -c 'curl -sS https://getcomposer.org/installer | php && php composer.phar run cs'
run: docker run --rm -e PHP_CS_FIXER_IGNORE_ENV=1 -v .:/opt/project -w /opt/project php:8.4-cli sh -c 'curl -sS https://getcomposer.org/installer | php && php composer.phar run cs'
8 changes: 2 additions & 6 deletions Core/Core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,6 @@ function property_exists($object_or_class, string $property): bool {}
* @link https://secure.php.net/manual/en/function.trait-exists.php
* @since 5.4
*/
#[Pure(true)]
function trait_exists(string $trait, bool $autoload = true): bool {}

/**
Expand All @@ -460,7 +459,6 @@ function trait_exists(string $trait, bool $autoload = true): bool {}
* @return bool true if <i>class_name</i> is a defined class,
* false otherwise.
*/
#[Pure(true)]
function class_exists(string $class, bool $autoload = true): bool {}

/**
Expand All @@ -476,7 +474,6 @@ function class_exists(string $class, bool $autoload = true): bool {}
* <i>interface_name</i> has been defined, false otherwise.
* @since 5.0.2
*/
#[Pure(true)]
function interface_exists(string $interface, bool $autoload = true): bool {}

/**
Expand Down Expand Up @@ -508,7 +505,6 @@ function function_exists(string $function): bool {}
* false otherwise.
* @since 8.1
*/
#[Pure(true)]
function enum_exists(string $enum, bool $autoload = true): bool {}

/**
Expand Down Expand Up @@ -609,7 +605,7 @@ function get_class_vars(string $class): array {}
* for the specified <i>object</i> in scope. If a property have
* not been assigned a value, it will be returned with a null value.
*/
#[Pure(true)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this change is also incorrect since function actually depends on global state. The function depends on the structure of the class from which the object was instantiated. If the class definition changes (e.g., due to autoloading), the result can change. Also if string property names or values are affected by locale settings (e.g., setlocale()), it might affect the output. Please correct me if I'm wrong

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cs278 Could you please comment on this? Am I right or do I miss something?

#[Pure]
function get_object_vars(object $object): array {}

/**
Expand Down Expand Up @@ -817,7 +813,7 @@ function create_function(string $args, string $code): false|string {}
* by this function, the return value will be the string
* Unknown.
*/
#[Pure(true)]
#[Pure]
function get_resource_type($resource): string {}

/**
Expand Down
1 change: 1 addition & 0 deletions meta/attributes/Pure.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
/**
* The attribute marks the function that has no impact on the program state or passed parameters used after the function execution.
* This means that a function call that resolves to such a function can be safely removed if the execution result is not used in code afterwards.
* Functions that take a callable which is called should not have this attribute applied.
*
* @since 8.0
*/
Expand Down
2 changes: 0 additions & 2 deletions standard/standard_8.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ function ob_end_clean(): bool {}
* @link https://php.net/manual/en/function.ob-get-flush.php
* @return string|false the output buffer or false if no buffering is active.
*/
#[Pure(true)]
function ob_get_flush(): string|false {}

/**
Expand All @@ -253,7 +252,6 @@ function ob_get_flush(): string|false {}
* @return string|false the contents of the output buffer and end output buffering.
* If output buffering isn't active then false is returned.
*/
#[Pure(true)]
function ob_get_clean(): string|false {}

/**
Expand Down
1 change: 0 additions & 1 deletion standard/standard_9.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,6 @@ function array_intersect_uassoc(
* array1 that are present in all the arguments.
* @meta
*/
#[Pure]
function array_uintersect_uassoc(
array $array,
#[PhpStormStubsElementAvailable(from: '5.3', to: '7.4')] array $array2,
Expand Down
Loading