Implement readonly properties - #7089

Closed
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties
Closed

Implement readonly properties#7089
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties

Conversation

@nikic

@nikicnikic commented Jun 2, 2021

Copy link
Copy Markdown
Member

@nikicnikic added the RFC label Jun 2, 2021
Comment threadZend/zend_language_parser.y
Comment threadZend/zend_object_handlers.c
@matthieu88160

Copy link
Copy Markdown

Hi all,
Shouldn't this feature be extended to the class constant rather than properties only?

@iluuu1994

iluuu1994 commented Jun 16, 2021

Copy link
Copy Markdown
Member

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

@matthieu88160

Copy link
Copy Markdown

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

My bad, I was thinking modification was allowed.

@iluuu1994

Copy link
Copy Markdown
Member

@nikic Here are two more tests for useful use-cases of overriding a readonly property:

--TEST--
Visibility can change in readonly property
--FILE--
<?phpclass A {
protected readonly int $prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
publicreadonlyint$prop;
}
$a = newA();
try {
var_dump($a->prop);
} catch (\Error$error) {
echo$error->getMessage() . "\n";
}
$b = newB();
var_dump($b->prop);
?>
--EXPECT--
Cannot access protected property A::$prop
int(42)

--TEST--
Can override readonly property with attributes
--FILE--
<?php
#[Attribute]
class FooAttribute {}
class A {
publicreadonlyint$prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
#[FooAttribute]
publicreadonlyint$prop;
}
var_dump((newReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance());
?>
--EXPECT--
object(FooAttribute)#1 (0) {
}

Comment threadext/reflection/tests/readonly_properties.phpt
@reyadkhan

reyadkhan commented Jul 2, 2021

Copy link
Copy Markdown

Method parameters can be readonly:
foo(readonly string $bar){}

@kolardavid

Copy link
Copy Markdown

Hey guys, I would like to add my few objections you might want to consider:

  1. Personally, I would allow unset() on readonly property, which would unlock it for writing again. This can be done only in private scope and can be useful. It would then work as controlled lockable property within scope, which has knowledge about the object. Also it would be consistent with typed properties, which can be unset, which returns them back in state, where reading throws exception (in oppose to writing).
  2. PHP is imho missing simple function to check if typed or readonly property is initialized, which might be harder to safely detect if reading/writing will throw an exception or is permitted. People usually use isset(), which gives false negative if typed/readonly is already set to explicit null. It would be probably helpful to add is_initialized() function. You can't even use property_exists() for this, since it returns (correctly) true for uninitialized properties.
  3. I would personally vote for not using readonly keyword, rather use writeonce/immutable/locked. readonly imho does not describe the usage that well as the other. Also, I would reserve readonly for other purpose:
  4. readonly would be imho much more useful for different scenario, where property is writable within private scope and readonly outside of private scope. Actually I believe I would use this scenario even more than the currently proposed readonly feature. It would safely expose private properties to outside without risk of modification. It would be basically syntactic sugar for private property and public getter without setter (the above proposed behavior of being able to unset and set it again would be more close to this scenario, even though it would always require unset before set to be safe, which is annoying). I can imagine 2 ways of behavior:
    a. private readonly int $prop; would mean that property is writable in private scope and readonly in protected and public scope (in which case public readonly would be non-practical).
    b. public readonly int $prop; would mean the same, but changes meaning of visibility specifier (in which case private readonly would be non-practical)
    Basically the only difference would be if private/protected/public would specify where is readonly visible and it would be always writable only in private scope (which would work similarly to current readonly behavior, but you will be unable to make it writable from protected scope) or it would be always publicly visible, but you would specify if it is writable in private/protected/public scope. That would be imho more practical, but would be confusing and required explanation, because it would make visibility modifiers works differently than usual.

Of course there is option to merge both behaviors by adding modifier to readonly - so having public readonly working as readonly from outside, writable from inside and public readonly writeonce as current readonly proposal, making both things more consistent. The downside is that readonly writeonce is longer to write, but makes most sense to me.

Thank you for your attention and considering this opinions :)

@kocsismate

Copy link
Copy Markdown
Member

Hey guys, I would like to add my few objections you might want to consider:

Please note that the feature is currently in voting, so any change should be warranted very much.

Points 3 and 4 have already been discussed to death, I believe the RFC provides background information why the readonly keyword (rather than writeonce) was chosen, and it also talks about the relation of "readonly properties" to the "asymmetric visibility" feature you also described.

Implementing an is_initialized() function could make sense, but it's IMO also orthogonal to this feature, since it doesn't only affect readonly properties. Also, it's already possible to detect if a property has been initialized via reflection.

Point 1 would render the feature broken, since any property value could be changed in private scope at any time, simply by unsetting it first.

@nikic

nikic commented Jul 5, 2021

Copy link
Copy Markdown
MemberAuthor

is_initialized() was recently discussed and declined (https://externals.io/message/114607).

@kolardavid

Copy link
Copy Markdown

OK guys, thanks for your reply and sorry to bother you :)

@nikicnikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikicforce-pushed the readonly-properties branch 7 times, most recently from 1bc5f49 to c890185CompareJuly 15, 2021 13:07
@rela589n

This comment has been minimized.

@kolardavid

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@KalleZ

Copy link
Copy Markdown
Member

Please keep these comments to the mailing list, the PR is not a forum for such

@rela589n

rela589n commented Jul 16, 2021 via email

Copy link
Copy Markdown

@nikic
nikicforce-pushed the readonly-properties branch 3 times, most recently from fe4c9e0 to 2137465CompareJuly 16, 2021 14:00
@nikic
nikicforce-pushed the readonly-properties branch from 8af4aae to 968a399CompareJuly 20, 2021 09:12
@nikicnikic closed this in 6780aaaJul 20, 2021
@jellynoone

Copy link
Copy Markdown
Contributor

Hi, @nikic just tried this out, is it intentional that it's now possible to skip the public visibility modifer in promoted properties and regular readonly properties?

class ClassName
{
readonlyint$prop1;
function__construct(
readonly array $prop2,
) {
$this->prop1 = \count($prop2);
}
}
\var_dump(newClassName([1,2]));

@jrfnl

jrfnl commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Just FYI as I didn't see mention of the impact of readonly becoming a semi-reserved keyword in the potential for BC-breaks section of the RFC, nor a code-scan of top PHP projects to gauge the impact:
Global functions, classes et al named readonly will have to be renamed as this will now be a parse error.
And by extension, all calls to these functions/instantiations of these classes will have to be adjusted too as those will also be a parse error.

If nothing else, this affects WordPress - and by extension potentially all plugins and themes in its ecosphere. So far, the impact is estimated to be relatively small as the function (to add the readonly attribute to HTML input tags) is not that widely used, but still...

@rela589n

This comment has been minimized.

@iluuu1994

Copy link
Copy Markdown
Member

Please use the mailing list for these discussions. The repercussions of a keyword are clear to voters. The suggestion of no $ was mentioned on the mailing list and not well received. While it avoids the BC break it's very subtle and doesn't communicate intent well. It's also way to late for this discussion in the first place. This RFC was in discussion for months.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@nikic@matthieu88160@iluuu1994@reyadkhan@kolardavid@kocsismate@rela589n@KalleZ@TysonAndre@jellynoone@jrfnl@mvorisek@dstogov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Implement readonly properties - #7089

Closed
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties
Closed

Implement readonly properties#7089
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties

Conversation

@nikic

@nikicnikic commented Jun 2, 2021

Copy link
Copy Markdown
Member

@nikicnikic added the RFC label Jun 2, 2021
Comment threadZend/zend_language_parser.y
Comment threadZend/zend_object_handlers.c
@matthieu88160

Copy link
Copy Markdown

Hi all,
Shouldn't this feature be extended to the class constant rather than properties only?

@iluuu1994

iluuu1994 commented Jun 16, 2021

Copy link
Copy Markdown
Member

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

@matthieu88160

Copy link
Copy Markdown

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

My bad, I was thinking modification was allowed.

@iluuu1994

Copy link
Copy Markdown
Member

@nikic Here are two more tests for useful use-cases of overriding a readonly property:

--TEST--
Visibility can change in readonly property
--FILE--
<?phpclass A {
protected readonly int $prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
publicreadonlyint$prop;
}
$a = newA();
try {
var_dump($a->prop);
} catch (\Error$error) {
echo$error->getMessage() . "\n";
}
$b = newB();
var_dump($b->prop);
?>
--EXPECT--
Cannot access protected property A::$prop
int(42)

--TEST--
Can override readonly property with attributes
--FILE--
<?php
#[Attribute]
class FooAttribute {}
class A {
publicreadonlyint$prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
#[FooAttribute]
publicreadonlyint$prop;
}
var_dump((newReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance());
?>
--EXPECT--
object(FooAttribute)#1 (0) {
}

Comment threadext/reflection/tests/readonly_properties.phpt
@reyadkhan

reyadkhan commented Jul 2, 2021

Copy link
Copy Markdown

Method parameters can be readonly:
foo(readonly string $bar){}

@kolardavid

Copy link
Copy Markdown

Hey guys, I would like to add my few objections you might want to consider:

  1. Personally, I would allow unset() on readonly property, which would unlock it for writing again. This can be done only in private scope and can be useful. It would then work as controlled lockable property within scope, which has knowledge about the object. Also it would be consistent with typed properties, which can be unset, which returns them back in state, where reading throws exception (in oppose to writing).
  2. PHP is imho missing simple function to check if typed or readonly property is initialized, which might be harder to safely detect if reading/writing will throw an exception or is permitted. People usually use isset(), which gives false negative if typed/readonly is already set to explicit null. It would be probably helpful to add is_initialized() function. You can't even use property_exists() for this, since it returns (correctly) true for uninitialized properties.
  3. I would personally vote for not using readonly keyword, rather use writeonce/immutable/locked. readonly imho does not describe the usage that well as the other. Also, I would reserve readonly for other purpose:
  4. readonly would be imho much more useful for different scenario, where property is writable within private scope and readonly outside of private scope. Actually I believe I would use this scenario even more than the currently proposed readonly feature. It would safely expose private properties to outside without risk of modification. It would be basically syntactic sugar for private property and public getter without setter (the above proposed behavior of being able to unset and set it again would be more close to this scenario, even though it would always require unset before set to be safe, which is annoying). I can imagine 2 ways of behavior:
    a. private readonly int $prop; would mean that property is writable in private scope and readonly in protected and public scope (in which case public readonly would be non-practical).
    b. public readonly int $prop; would mean the same, but changes meaning of visibility specifier (in which case private readonly would be non-practical)
    Basically the only difference would be if private/protected/public would specify where is readonly visible and it would be always writable only in private scope (which would work similarly to current readonly behavior, but you will be unable to make it writable from protected scope) or it would be always publicly visible, but you would specify if it is writable in private/protected/public scope. That would be imho more practical, but would be confusing and required explanation, because it would make visibility modifiers works differently than usual.

Of course there is option to merge both behaviors by adding modifier to readonly - so having public readonly working as readonly from outside, writable from inside and public readonly writeonce as current readonly proposal, making both things more consistent. The downside is that readonly writeonce is longer to write, but makes most sense to me.

Thank you for your attention and considering this opinions :)

@kocsismate

Copy link
Copy Markdown
Member

Hey guys, I would like to add my few objections you might want to consider:

Please note that the feature is currently in voting, so any change should be warranted very much.

Points 3 and 4 have already been discussed to death, I believe the RFC provides background information why the readonly keyword (rather than writeonce) was chosen, and it also talks about the relation of "readonly properties" to the "asymmetric visibility" feature you also described.

Implementing an is_initialized() function could make sense, but it's IMO also orthogonal to this feature, since it doesn't only affect readonly properties. Also, it's already possible to detect if a property has been initialized via reflection.

Point 1 would render the feature broken, since any property value could be changed in private scope at any time, simply by unsetting it first.

@nikic

nikic commented Jul 5, 2021

Copy link
Copy Markdown
MemberAuthor

is_initialized() was recently discussed and declined (https://externals.io/message/114607).

@kolardavid

Copy link
Copy Markdown

OK guys, thanks for your reply and sorry to bother you :)

@nikicnikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikicforce-pushed the readonly-properties branch 7 times, most recently from 1bc5f49 to c890185CompareJuly 15, 2021 13:07
@rela589n

This comment has been minimized.

@kolardavid

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@KalleZ

Copy link
Copy Markdown
Member

Please keep these comments to the mailing list, the PR is not a forum for such

@rela589n

rela589n commented Jul 16, 2021 via email

Copy link
Copy Markdown

@nikic
nikicforce-pushed the readonly-properties branch 3 times, most recently from fe4c9e0 to 2137465CompareJuly 16, 2021 14:00
@nikic
nikicforce-pushed the readonly-properties branch from 8af4aae to 968a399CompareJuly 20, 2021 09:12
@nikicnikic closed this in 6780aaaJul 20, 2021
@jellynoone

Copy link
Copy Markdown
Contributor

Hi, @nikic just tried this out, is it intentional that it's now possible to skip the public visibility modifer in promoted properties and regular readonly properties?

class ClassName
{
readonlyint$prop1;
function__construct(
readonly array $prop2,
) {
$this->prop1 = \count($prop2);
}
}
\var_dump(newClassName([1,2]));

@jrfnl

jrfnl commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Just FYI as I didn't see mention of the impact of readonly becoming a semi-reserved keyword in the potential for BC-breaks section of the RFC, nor a code-scan of top PHP projects to gauge the impact:
Global functions, classes et al named readonly will have to be renamed as this will now be a parse error.
And by extension, all calls to these functions/instantiations of these classes will have to be adjusted too as those will also be a parse error.

If nothing else, this affects WordPress - and by extension potentially all plugins and themes in its ecosphere. So far, the impact is estimated to be relatively small as the function (to add the readonly attribute to HTML input tags) is not that widely used, but still...

@rela589n

This comment has been minimized.

@iluuu1994

Copy link
Copy Markdown
Member

Please use the mailing list for these discussions. The repercussions of a keyword are clear to voters. The suggestion of no $ was mentioned on the mailing list and not well received. While it avoids the BC break it's very subtle and doesn't communicate intent well. It's also way to late for this discussion in the first place. This RFC was in discussion for months.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@nikic@matthieu88160@iluuu1994@reyadkhan@kolardavid@kocsismate@rela589n@KalleZ@TysonAndre@jellynoone@jrfnl@mvorisek@dstogov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Implement readonly properties - #7089

Closed
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties
Closed

Implement readonly properties#7089
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties

Conversation

@nikic

@nikicnikic commented Jun 2, 2021

Copy link
Copy Markdown
Member

@nikicnikic added the RFC label Jun 2, 2021
Comment threadZend/zend_language_parser.y
Comment threadZend/zend_object_handlers.c
@matthieu88160

Copy link
Copy Markdown

Hi all,
Shouldn't this feature be extended to the class constant rather than properties only?

@iluuu1994

iluuu1994 commented Jun 16, 2021

Copy link
Copy Markdown
Member

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

@matthieu88160

Copy link
Copy Markdown

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

My bad, I was thinking modification was allowed.

@iluuu1994

Copy link
Copy Markdown
Member

@nikic Here are two more tests for useful use-cases of overriding a readonly property:

--TEST--
Visibility can change in readonly property
--FILE--
<?phpclass A {
protected readonly int $prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
publicreadonlyint$prop;
}
$a = newA();
try {
var_dump($a->prop);
} catch (\Error$error) {
echo$error->getMessage() . "\n";
}
$b = newB();
var_dump($b->prop);
?>
--EXPECT--
Cannot access protected property A::$prop
int(42)

--TEST--
Can override readonly property with attributes
--FILE--
<?php
#[Attribute]
class FooAttribute {}
class A {
publicreadonlyint$prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
#[FooAttribute]
publicreadonlyint$prop;
}
var_dump((newReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance());
?>
--EXPECT--
object(FooAttribute)#1 (0) {
}

Comment threadext/reflection/tests/readonly_properties.phpt
@reyadkhan

reyadkhan commented Jul 2, 2021

Copy link
Copy Markdown

Method parameters can be readonly:
foo(readonly string $bar){}

@kolardavid

Copy link
Copy Markdown

Hey guys, I would like to add my few objections you might want to consider:

  1. Personally, I would allow unset() on readonly property, which would unlock it for writing again. This can be done only in private scope and can be useful. It would then work as controlled lockable property within scope, which has knowledge about the object. Also it would be consistent with typed properties, which can be unset, which returns them back in state, where reading throws exception (in oppose to writing).
  2. PHP is imho missing simple function to check if typed or readonly property is initialized, which might be harder to safely detect if reading/writing will throw an exception or is permitted. People usually use isset(), which gives false negative if typed/readonly is already set to explicit null. It would be probably helpful to add is_initialized() function. You can't even use property_exists() for this, since it returns (correctly) true for uninitialized properties.
  3. I would personally vote for not using readonly keyword, rather use writeonce/immutable/locked. readonly imho does not describe the usage that well as the other. Also, I would reserve readonly for other purpose:
  4. readonly would be imho much more useful for different scenario, where property is writable within private scope and readonly outside of private scope. Actually I believe I would use this scenario even more than the currently proposed readonly feature. It would safely expose private properties to outside without risk of modification. It would be basically syntactic sugar for private property and public getter without setter (the above proposed behavior of being able to unset and set it again would be more close to this scenario, even though it would always require unset before set to be safe, which is annoying). I can imagine 2 ways of behavior:
    a. private readonly int $prop; would mean that property is writable in private scope and readonly in protected and public scope (in which case public readonly would be non-practical).
    b. public readonly int $prop; would mean the same, but changes meaning of visibility specifier (in which case private readonly would be non-practical)
    Basically the only difference would be if private/protected/public would specify where is readonly visible and it would be always writable only in private scope (which would work similarly to current readonly behavior, but you will be unable to make it writable from protected scope) or it would be always publicly visible, but you would specify if it is writable in private/protected/public scope. That would be imho more practical, but would be confusing and required explanation, because it would make visibility modifiers works differently than usual.

Of course there is option to merge both behaviors by adding modifier to readonly - so having public readonly working as readonly from outside, writable from inside and public readonly writeonce as current readonly proposal, making both things more consistent. The downside is that readonly writeonce is longer to write, but makes most sense to me.

Thank you for your attention and considering this opinions :)

@kocsismate

Copy link
Copy Markdown
Member

Hey guys, I would like to add my few objections you might want to consider:

Please note that the feature is currently in voting, so any change should be warranted very much.

Points 3 and 4 have already been discussed to death, I believe the RFC provides background information why the readonly keyword (rather than writeonce) was chosen, and it also talks about the relation of "readonly properties" to the "asymmetric visibility" feature you also described.

Implementing an is_initialized() function could make sense, but it's IMO also orthogonal to this feature, since it doesn't only affect readonly properties. Also, it's already possible to detect if a property has been initialized via reflection.

Point 1 would render the feature broken, since any property value could be changed in private scope at any time, simply by unsetting it first.

@nikic

nikic commented Jul 5, 2021

Copy link
Copy Markdown
MemberAuthor

is_initialized() was recently discussed and declined (https://externals.io/message/114607).

@kolardavid

Copy link
Copy Markdown

OK guys, thanks for your reply and sorry to bother you :)

@nikicnikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikicforce-pushed the readonly-properties branch 7 times, most recently from 1bc5f49 to c890185CompareJuly 15, 2021 13:07
@rela589n

This comment has been minimized.

@kolardavid

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@KalleZ

Copy link
Copy Markdown
Member

Please keep these comments to the mailing list, the PR is not a forum for such

@rela589n

rela589n commented Jul 16, 2021 via email

Copy link
Copy Markdown

@nikic
nikicforce-pushed the readonly-properties branch 3 times, most recently from fe4c9e0 to 2137465CompareJuly 16, 2021 14:00
@nikic
nikicforce-pushed the readonly-properties branch from 8af4aae to 968a399CompareJuly 20, 2021 09:12
@nikicnikic closed this in 6780aaaJul 20, 2021
@jellynoone

Copy link
Copy Markdown
Contributor

Hi, @nikic just tried this out, is it intentional that it's now possible to skip the public visibility modifer in promoted properties and regular readonly properties?

class ClassName
{
readonlyint$prop1;
function__construct(
readonly array $prop2,
) {
$this->prop1 = \count($prop2);
}
}
\var_dump(newClassName([1,2]));

@jrfnl

jrfnl commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Just FYI as I didn't see mention of the impact of readonly becoming a semi-reserved keyword in the potential for BC-breaks section of the RFC, nor a code-scan of top PHP projects to gauge the impact:
Global functions, classes et al named readonly will have to be renamed as this will now be a parse error.
And by extension, all calls to these functions/instantiations of these classes will have to be adjusted too as those will also be a parse error.

If nothing else, this affects WordPress - and by extension potentially all plugins and themes in its ecosphere. So far, the impact is estimated to be relatively small as the function (to add the readonly attribute to HTML input tags) is not that widely used, but still...

@rela589n

This comment has been minimized.

@iluuu1994

Copy link
Copy Markdown
Member

Please use the mailing list for these discussions. The repercussions of a keyword are clear to voters. The suggestion of no $ was mentioned on the mailing list and not well received. While it avoids the BC break it's very subtle and doesn't communicate intent well. It's also way to late for this discussion in the first place. This RFC was in discussion for months.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@nikic@matthieu88160@iluuu1994@reyadkhan@kolardavid@kocsismate@rela589n@KalleZ@TysonAndre@jellynoone@jrfnl@mvorisek@dstogov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Implement readonly properties - #7089

Closed
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties
Closed

Implement readonly properties#7089
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties

Conversation

@nikic

@nikicnikic commented Jun 2, 2021

Copy link
Copy Markdown
Member

@nikicnikic added the RFC label Jun 2, 2021
Comment threadZend/zend_language_parser.y
Comment threadZend/zend_object_handlers.c
@matthieu88160

Copy link
Copy Markdown

Hi all,
Shouldn't this feature be extended to the class constant rather than properties only?

@iluuu1994

iluuu1994 commented Jun 16, 2021

Copy link
Copy Markdown
Member

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

@matthieu88160

Copy link
Copy Markdown

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

My bad, I was thinking modification was allowed.

@iluuu1994

Copy link
Copy Markdown
Member

@nikic Here are two more tests for useful use-cases of overriding a readonly property:

--TEST--
Visibility can change in readonly property
--FILE--
<?phpclass A {
protected readonly int $prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
publicreadonlyint$prop;
}
$a = newA();
try {
var_dump($a->prop);
} catch (\Error$error) {
echo$error->getMessage() . "\n";
}
$b = newB();
var_dump($b->prop);
?>
--EXPECT--
Cannot access protected property A::$prop
int(42)

--TEST--
Can override readonly property with attributes
--FILE--
<?php
#[Attribute]
class FooAttribute {}
class A {
publicreadonlyint$prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
#[FooAttribute]
publicreadonlyint$prop;
}
var_dump((newReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance());
?>
--EXPECT--
object(FooAttribute)#1 (0) {
}

Comment threadext/reflection/tests/readonly_properties.phpt
@reyadkhan

reyadkhan commented Jul 2, 2021

Copy link
Copy Markdown

Method parameters can be readonly:
foo(readonly string $bar){}

@kolardavid

Copy link
Copy Markdown

Hey guys, I would like to add my few objections you might want to consider:

  1. Personally, I would allow unset() on readonly property, which would unlock it for writing again. This can be done only in private scope and can be useful. It would then work as controlled lockable property within scope, which has knowledge about the object. Also it would be consistent with typed properties, which can be unset, which returns them back in state, where reading throws exception (in oppose to writing).
  2. PHP is imho missing simple function to check if typed or readonly property is initialized, which might be harder to safely detect if reading/writing will throw an exception or is permitted. People usually use isset(), which gives false negative if typed/readonly is already set to explicit null. It would be probably helpful to add is_initialized() function. You can't even use property_exists() for this, since it returns (correctly) true for uninitialized properties.
  3. I would personally vote for not using readonly keyword, rather use writeonce/immutable/locked. readonly imho does not describe the usage that well as the other. Also, I would reserve readonly for other purpose:
  4. readonly would be imho much more useful for different scenario, where property is writable within private scope and readonly outside of private scope. Actually I believe I would use this scenario even more than the currently proposed readonly feature. It would safely expose private properties to outside without risk of modification. It would be basically syntactic sugar for private property and public getter without setter (the above proposed behavior of being able to unset and set it again would be more close to this scenario, even though it would always require unset before set to be safe, which is annoying). I can imagine 2 ways of behavior:
    a. private readonly int $prop; would mean that property is writable in private scope and readonly in protected and public scope (in which case public readonly would be non-practical).
    b. public readonly int $prop; would mean the same, but changes meaning of visibility specifier (in which case private readonly would be non-practical)
    Basically the only difference would be if private/protected/public would specify where is readonly visible and it would be always writable only in private scope (which would work similarly to current readonly behavior, but you will be unable to make it writable from protected scope) or it would be always publicly visible, but you would specify if it is writable in private/protected/public scope. That would be imho more practical, but would be confusing and required explanation, because it would make visibility modifiers works differently than usual.

Of course there is option to merge both behaviors by adding modifier to readonly - so having public readonly working as readonly from outside, writable from inside and public readonly writeonce as current readonly proposal, making both things more consistent. The downside is that readonly writeonce is longer to write, but makes most sense to me.

Thank you for your attention and considering this opinions :)

@kocsismate

Copy link
Copy Markdown
Member

Hey guys, I would like to add my few objections you might want to consider:

Please note that the feature is currently in voting, so any change should be warranted very much.

Points 3 and 4 have already been discussed to death, I believe the RFC provides background information why the readonly keyword (rather than writeonce) was chosen, and it also talks about the relation of "readonly properties" to the "asymmetric visibility" feature you also described.

Implementing an is_initialized() function could make sense, but it's IMO also orthogonal to this feature, since it doesn't only affect readonly properties. Also, it's already possible to detect if a property has been initialized via reflection.

Point 1 would render the feature broken, since any property value could be changed in private scope at any time, simply by unsetting it first.

@nikic

nikic commented Jul 5, 2021

Copy link
Copy Markdown
MemberAuthor

is_initialized() was recently discussed and declined (https://externals.io/message/114607).

@kolardavid

Copy link
Copy Markdown

OK guys, thanks for your reply and sorry to bother you :)

@nikicnikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikicforce-pushed the readonly-properties branch 7 times, most recently from 1bc5f49 to c890185CompareJuly 15, 2021 13:07
@rela589n

This comment has been minimized.

@kolardavid

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@KalleZ

Copy link
Copy Markdown
Member

Please keep these comments to the mailing list, the PR is not a forum for such

@rela589n

rela589n commented Jul 16, 2021 via email

Copy link
Copy Markdown

@nikic
nikicforce-pushed the readonly-properties branch 3 times, most recently from fe4c9e0 to 2137465CompareJuly 16, 2021 14:00
@nikic
nikicforce-pushed the readonly-properties branch from 8af4aae to 968a399CompareJuly 20, 2021 09:12
@nikicnikic closed this in 6780aaaJul 20, 2021
@jellynoone

Copy link
Copy Markdown
Contributor

Hi, @nikic just tried this out, is it intentional that it's now possible to skip the public visibility modifer in promoted properties and regular readonly properties?

class ClassName
{
readonlyint$prop1;
function__construct(
readonly array $prop2,
) {
$this->prop1 = \count($prop2);
}
}
\var_dump(newClassName([1,2]));

@jrfnl

jrfnl commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Just FYI as I didn't see mention of the impact of readonly becoming a semi-reserved keyword in the potential for BC-breaks section of the RFC, nor a code-scan of top PHP projects to gauge the impact:
Global functions, classes et al named readonly will have to be renamed as this will now be a parse error.
And by extension, all calls to these functions/instantiations of these classes will have to be adjusted too as those will also be a parse error.

If nothing else, this affects WordPress - and by extension potentially all plugins and themes in its ecosphere. So far, the impact is estimated to be relatively small as the function (to add the readonly attribute to HTML input tags) is not that widely used, but still...

@rela589n

This comment has been minimized.

@iluuu1994

Copy link
Copy Markdown
Member

Please use the mailing list for these discussions. The repercussions of a keyword are clear to voters. The suggestion of no $ was mentioned on the mailing list and not well received. While it avoids the BC break it's very subtle and doesn't communicate intent well. It's also way to late for this discussion in the first place. This RFC was in discussion for months.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@nikic@matthieu88160@iluuu1994@reyadkhan@kolardavid@kocsismate@rela589n@KalleZ@TysonAndre@jellynoone@jrfnl@mvorisek@dstogov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Implement readonly properties - #7089

Closed
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties
Closed

Implement readonly properties#7089
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties

Conversation

@nikic

@nikicnikic commented Jun 2, 2021

Copy link
Copy Markdown
Member

@nikicnikic added the RFC label Jun 2, 2021
Comment threadZend/zend_language_parser.y
Comment threadZend/zend_object_handlers.c
@matthieu88160

Copy link
Copy Markdown

Hi all,
Shouldn't this feature be extended to the class constant rather than properties only?

@iluuu1994

iluuu1994 commented Jun 16, 2021

Copy link
Copy Markdown
Member

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

@matthieu88160

Copy link
Copy Markdown

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

My bad, I was thinking modification was allowed.

@iluuu1994

Copy link
Copy Markdown
Member

@nikic Here are two more tests for useful use-cases of overriding a readonly property:

--TEST--
Visibility can change in readonly property
--FILE--
<?phpclass A {
protected readonly int $prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
publicreadonlyint$prop;
}
$a = newA();
try {
var_dump($a->prop);
} catch (\Error$error) {
echo$error->getMessage() . "\n";
}
$b = newB();
var_dump($b->prop);
?>
--EXPECT--
Cannot access protected property A::$prop
int(42)

--TEST--
Can override readonly property with attributes
--FILE--
<?php
#[Attribute]
class FooAttribute {}
class A {
publicreadonlyint$prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
#[FooAttribute]
publicreadonlyint$prop;
}
var_dump((newReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance());
?>
--EXPECT--
object(FooAttribute)#1 (0) {
}

Comment threadext/reflection/tests/readonly_properties.phpt
@reyadkhan

reyadkhan commented Jul 2, 2021

Copy link
Copy Markdown

Method parameters can be readonly:
foo(readonly string $bar){}

@kolardavid

Copy link
Copy Markdown

Hey guys, I would like to add my few objections you might want to consider:

  1. Personally, I would allow unset() on readonly property, which would unlock it for writing again. This can be done only in private scope and can be useful. It would then work as controlled lockable property within scope, which has knowledge about the object. Also it would be consistent with typed properties, which can be unset, which returns them back in state, where reading throws exception (in oppose to writing).
  2. PHP is imho missing simple function to check if typed or readonly property is initialized, which might be harder to safely detect if reading/writing will throw an exception or is permitted. People usually use isset(), which gives false negative if typed/readonly is already set to explicit null. It would be probably helpful to add is_initialized() function. You can't even use property_exists() for this, since it returns (correctly) true for uninitialized properties.
  3. I would personally vote for not using readonly keyword, rather use writeonce/immutable/locked. readonly imho does not describe the usage that well as the other. Also, I would reserve readonly for other purpose:
  4. readonly would be imho much more useful for different scenario, where property is writable within private scope and readonly outside of private scope. Actually I believe I would use this scenario even more than the currently proposed readonly feature. It would safely expose private properties to outside without risk of modification. It would be basically syntactic sugar for private property and public getter without setter (the above proposed behavior of being able to unset and set it again would be more close to this scenario, even though it would always require unset before set to be safe, which is annoying). I can imagine 2 ways of behavior:
    a. private readonly int $prop; would mean that property is writable in private scope and readonly in protected and public scope (in which case public readonly would be non-practical).
    b. public readonly int $prop; would mean the same, but changes meaning of visibility specifier (in which case private readonly would be non-practical)
    Basically the only difference would be if private/protected/public would specify where is readonly visible and it would be always writable only in private scope (which would work similarly to current readonly behavior, but you will be unable to make it writable from protected scope) or it would be always publicly visible, but you would specify if it is writable in private/protected/public scope. That would be imho more practical, but would be confusing and required explanation, because it would make visibility modifiers works differently than usual.

Of course there is option to merge both behaviors by adding modifier to readonly - so having public readonly working as readonly from outside, writable from inside and public readonly writeonce as current readonly proposal, making both things more consistent. The downside is that readonly writeonce is longer to write, but makes most sense to me.

Thank you for your attention and considering this opinions :)

@kocsismate

Copy link
Copy Markdown
Member

Hey guys, I would like to add my few objections you might want to consider:

Please note that the feature is currently in voting, so any change should be warranted very much.

Points 3 and 4 have already been discussed to death, I believe the RFC provides background information why the readonly keyword (rather than writeonce) was chosen, and it also talks about the relation of "readonly properties" to the "asymmetric visibility" feature you also described.

Implementing an is_initialized() function could make sense, but it's IMO also orthogonal to this feature, since it doesn't only affect readonly properties. Also, it's already possible to detect if a property has been initialized via reflection.

Point 1 would render the feature broken, since any property value could be changed in private scope at any time, simply by unsetting it first.

@nikic

nikic commented Jul 5, 2021

Copy link
Copy Markdown
MemberAuthor

is_initialized() was recently discussed and declined (https://externals.io/message/114607).

@kolardavid

Copy link
Copy Markdown

OK guys, thanks for your reply and sorry to bother you :)

@nikicnikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikicforce-pushed the readonly-properties branch 7 times, most recently from 1bc5f49 to c890185CompareJuly 15, 2021 13:07
@rela589n

This comment has been minimized.

@kolardavid

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@KalleZ

Copy link
Copy Markdown
Member

Please keep these comments to the mailing list, the PR is not a forum for such

@rela589n

rela589n commented Jul 16, 2021 via email

Copy link
Copy Markdown

@nikic
nikicforce-pushed the readonly-properties branch 3 times, most recently from fe4c9e0 to 2137465CompareJuly 16, 2021 14:00
@nikic
nikicforce-pushed the readonly-properties branch from 8af4aae to 968a399CompareJuly 20, 2021 09:12
@nikicnikic closed this in 6780aaaJul 20, 2021
@jellynoone

Copy link
Copy Markdown
Contributor

Hi, @nikic just tried this out, is it intentional that it's now possible to skip the public visibility modifer in promoted properties and regular readonly properties?

class ClassName
{
readonlyint$prop1;
function__construct(
readonly array $prop2,
) {
$this->prop1 = \count($prop2);
}
}
\var_dump(newClassName([1,2]));

@jrfnl

jrfnl commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Just FYI as I didn't see mention of the impact of readonly becoming a semi-reserved keyword in the potential for BC-breaks section of the RFC, nor a code-scan of top PHP projects to gauge the impact:
Global functions, classes et al named readonly will have to be renamed as this will now be a parse error.
And by extension, all calls to these functions/instantiations of these classes will have to be adjusted too as those will also be a parse error.

If nothing else, this affects WordPress - and by extension potentially all plugins and themes in its ecosphere. So far, the impact is estimated to be relatively small as the function (to add the readonly attribute to HTML input tags) is not that widely used, but still...

@rela589n

This comment has been minimized.

@iluuu1994

Copy link
Copy Markdown
Member

Please use the mailing list for these discussions. The repercussions of a keyword are clear to voters. The suggestion of no $ was mentioned on the mailing list and not well received. While it avoids the BC break it's very subtle and doesn't communicate intent well. It's also way to late for this discussion in the first place. This RFC was in discussion for months.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@nikic@matthieu88160@iluuu1994@reyadkhan@kolardavid@kocsismate@rela589n@KalleZ@TysonAndre@jellynoone@jrfnl@mvorisek@dstogov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Implement readonly properties - #7089

Closed
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties
Closed

Implement readonly properties#7089
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties

Conversation

@nikic

@nikicnikic commented Jun 2, 2021

Copy link
Copy Markdown
Member

@nikicnikic added the RFC label Jun 2, 2021
Comment threadZend/zend_language_parser.y
Comment threadZend/zend_object_handlers.c
@matthieu88160

Copy link
Copy Markdown

Hi all,
Shouldn't this feature be extended to the class constant rather than properties only?

@iluuu1994

iluuu1994 commented Jun 16, 2021

Copy link
Copy Markdown
Member

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

@matthieu88160

Copy link
Copy Markdown

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

My bad, I was thinking modification was allowed.

@iluuu1994

Copy link
Copy Markdown
Member

@nikic Here are two more tests for useful use-cases of overriding a readonly property:

--TEST--
Visibility can change in readonly property
--FILE--
<?phpclass A {
protected readonly int $prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
publicreadonlyint$prop;
}
$a = newA();
try {
var_dump($a->prop);
} catch (\Error$error) {
echo$error->getMessage() . "\n";
}
$b = newB();
var_dump($b->prop);
?>
--EXPECT--
Cannot access protected property A::$prop
int(42)

--TEST--
Can override readonly property with attributes
--FILE--
<?php
#[Attribute]
class FooAttribute {}
class A {
publicreadonlyint$prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
#[FooAttribute]
publicreadonlyint$prop;
}
var_dump((newReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance());
?>
--EXPECT--
object(FooAttribute)#1 (0) {
}

Comment threadext/reflection/tests/readonly_properties.phpt
@reyadkhan

reyadkhan commented Jul 2, 2021

Copy link
Copy Markdown

Method parameters can be readonly:
foo(readonly string $bar){}

@kolardavid

Copy link
Copy Markdown

Hey guys, I would like to add my few objections you might want to consider:

  1. Personally, I would allow unset() on readonly property, which would unlock it for writing again. This can be done only in private scope and can be useful. It would then work as controlled lockable property within scope, which has knowledge about the object. Also it would be consistent with typed properties, which can be unset, which returns them back in state, where reading throws exception (in oppose to writing).
  2. PHP is imho missing simple function to check if typed or readonly property is initialized, which might be harder to safely detect if reading/writing will throw an exception or is permitted. People usually use isset(), which gives false negative if typed/readonly is already set to explicit null. It would be probably helpful to add is_initialized() function. You can't even use property_exists() for this, since it returns (correctly) true for uninitialized properties.
  3. I would personally vote for not using readonly keyword, rather use writeonce/immutable/locked. readonly imho does not describe the usage that well as the other. Also, I would reserve readonly for other purpose:
  4. readonly would be imho much more useful for different scenario, where property is writable within private scope and readonly outside of private scope. Actually I believe I would use this scenario even more than the currently proposed readonly feature. It would safely expose private properties to outside without risk of modification. It would be basically syntactic sugar for private property and public getter without setter (the above proposed behavior of being able to unset and set it again would be more close to this scenario, even though it would always require unset before set to be safe, which is annoying). I can imagine 2 ways of behavior:
    a. private readonly int $prop; would mean that property is writable in private scope and readonly in protected and public scope (in which case public readonly would be non-practical).
    b. public readonly int $prop; would mean the same, but changes meaning of visibility specifier (in which case private readonly would be non-practical)
    Basically the only difference would be if private/protected/public would specify where is readonly visible and it would be always writable only in private scope (which would work similarly to current readonly behavior, but you will be unable to make it writable from protected scope) or it would be always publicly visible, but you would specify if it is writable in private/protected/public scope. That would be imho more practical, but would be confusing and required explanation, because it would make visibility modifiers works differently than usual.

Of course there is option to merge both behaviors by adding modifier to readonly - so having public readonly working as readonly from outside, writable from inside and public readonly writeonce as current readonly proposal, making both things more consistent. The downside is that readonly writeonce is longer to write, but makes most sense to me.

Thank you for your attention and considering this opinions :)

@kocsismate

Copy link
Copy Markdown
Member

Hey guys, I would like to add my few objections you might want to consider:

Please note that the feature is currently in voting, so any change should be warranted very much.

Points 3 and 4 have already been discussed to death, I believe the RFC provides background information why the readonly keyword (rather than writeonce) was chosen, and it also talks about the relation of "readonly properties" to the "asymmetric visibility" feature you also described.

Implementing an is_initialized() function could make sense, but it's IMO also orthogonal to this feature, since it doesn't only affect readonly properties. Also, it's already possible to detect if a property has been initialized via reflection.

Point 1 would render the feature broken, since any property value could be changed in private scope at any time, simply by unsetting it first.

@nikic

nikic commented Jul 5, 2021

Copy link
Copy Markdown
MemberAuthor

is_initialized() was recently discussed and declined (https://externals.io/message/114607).

@kolardavid

Copy link
Copy Markdown

OK guys, thanks for your reply and sorry to bother you :)

@nikicnikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikicforce-pushed the readonly-properties branch 7 times, most recently from 1bc5f49 to c890185CompareJuly 15, 2021 13:07
@rela589n

This comment has been minimized.

@kolardavid

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@KalleZ

Copy link
Copy Markdown
Member

Please keep these comments to the mailing list, the PR is not a forum for such

@rela589n

rela589n commented Jul 16, 2021 via email

Copy link
Copy Markdown

@nikic
nikicforce-pushed the readonly-properties branch 3 times, most recently from fe4c9e0 to 2137465CompareJuly 16, 2021 14:00
@nikic
nikicforce-pushed the readonly-properties branch from 8af4aae to 968a399CompareJuly 20, 2021 09:12
@nikicnikic closed this in 6780aaaJul 20, 2021
@jellynoone

Copy link
Copy Markdown
Contributor

Hi, @nikic just tried this out, is it intentional that it's now possible to skip the public visibility modifer in promoted properties and regular readonly properties?

class ClassName
{
readonlyint$prop1;
function__construct(
readonly array $prop2,
) {
$this->prop1 = \count($prop2);
}
}
\var_dump(newClassName([1,2]));

@jrfnl

jrfnl commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Just FYI as I didn't see mention of the impact of readonly becoming a semi-reserved keyword in the potential for BC-breaks section of the RFC, nor a code-scan of top PHP projects to gauge the impact:
Global functions, classes et al named readonly will have to be renamed as this will now be a parse error.
And by extension, all calls to these functions/instantiations of these classes will have to be adjusted too as those will also be a parse error.

If nothing else, this affects WordPress - and by extension potentially all plugins and themes in its ecosphere. So far, the impact is estimated to be relatively small as the function (to add the readonly attribute to HTML input tags) is not that widely used, but still...

@rela589n

This comment has been minimized.

@iluuu1994

Copy link
Copy Markdown
Member

Please use the mailing list for these discussions. The repercussions of a keyword are clear to voters. The suggestion of no $ was mentioned on the mailing list and not well received. While it avoids the BC break it's very subtle and doesn't communicate intent well. It's also way to late for this discussion in the first place. This RFC was in discussion for months.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@nikic@matthieu88160@iluuu1994@reyadkhan@kolardavid@kocsismate@rela589n@KalleZ@TysonAndre@jellynoone@jrfnl@mvorisek@dstogov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Implement readonly properties - #7089

Closed
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties
Closed

Implement readonly properties#7089
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties

Conversation

@nikic

@nikicnikic commented Jun 2, 2021

Copy link
Copy Markdown
Member

@nikicnikic added the RFC label Jun 2, 2021
Comment threadZend/zend_language_parser.y
Comment threadZend/zend_object_handlers.c
@matthieu88160

Copy link
Copy Markdown

Hi all,
Shouldn't this feature be extended to the class constant rather than properties only?

@iluuu1994

iluuu1994 commented Jun 16, 2021

Copy link
Copy Markdown
Member

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

@matthieu88160

Copy link
Copy Markdown

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

My bad, I was thinking modification was allowed.

@iluuu1994

Copy link
Copy Markdown
Member

@nikic Here are two more tests for useful use-cases of overriding a readonly property:

--TEST--
Visibility can change in readonly property
--FILE--
<?phpclass A {
protected readonly int $prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
publicreadonlyint$prop;
}
$a = newA();
try {
var_dump($a->prop);
} catch (\Error$error) {
echo$error->getMessage() . "\n";
}
$b = newB();
var_dump($b->prop);
?>
--EXPECT--
Cannot access protected property A::$prop
int(42)

--TEST--
Can override readonly property with attributes
--FILE--
<?php
#[Attribute]
class FooAttribute {}
class A {
publicreadonlyint$prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
#[FooAttribute]
publicreadonlyint$prop;
}
var_dump((newReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance());
?>
--EXPECT--
object(FooAttribute)#1 (0) {
}

Comment threadext/reflection/tests/readonly_properties.phpt
@reyadkhan

reyadkhan commented Jul 2, 2021

Copy link
Copy Markdown

Method parameters can be readonly:
foo(readonly string $bar){}

@kolardavid

Copy link
Copy Markdown

Hey guys, I would like to add my few objections you might want to consider:

  1. Personally, I would allow unset() on readonly property, which would unlock it for writing again. This can be done only in private scope and can be useful. It would then work as controlled lockable property within scope, which has knowledge about the object. Also it would be consistent with typed properties, which can be unset, which returns them back in state, where reading throws exception (in oppose to writing).
  2. PHP is imho missing simple function to check if typed or readonly property is initialized, which might be harder to safely detect if reading/writing will throw an exception or is permitted. People usually use isset(), which gives false negative if typed/readonly is already set to explicit null. It would be probably helpful to add is_initialized() function. You can't even use property_exists() for this, since it returns (correctly) true for uninitialized properties.
  3. I would personally vote for not using readonly keyword, rather use writeonce/immutable/locked. readonly imho does not describe the usage that well as the other. Also, I would reserve readonly for other purpose:
  4. readonly would be imho much more useful for different scenario, where property is writable within private scope and readonly outside of private scope. Actually I believe I would use this scenario even more than the currently proposed readonly feature. It would safely expose private properties to outside without risk of modification. It would be basically syntactic sugar for private property and public getter without setter (the above proposed behavior of being able to unset and set it again would be more close to this scenario, even though it would always require unset before set to be safe, which is annoying). I can imagine 2 ways of behavior:
    a. private readonly int $prop; would mean that property is writable in private scope and readonly in protected and public scope (in which case public readonly would be non-practical).
    b. public readonly int $prop; would mean the same, but changes meaning of visibility specifier (in which case private readonly would be non-practical)
    Basically the only difference would be if private/protected/public would specify where is readonly visible and it would be always writable only in private scope (which would work similarly to current readonly behavior, but you will be unable to make it writable from protected scope) or it would be always publicly visible, but you would specify if it is writable in private/protected/public scope. That would be imho more practical, but would be confusing and required explanation, because it would make visibility modifiers works differently than usual.

Of course there is option to merge both behaviors by adding modifier to readonly - so having public readonly working as readonly from outside, writable from inside and public readonly writeonce as current readonly proposal, making both things more consistent. The downside is that readonly writeonce is longer to write, but makes most sense to me.

Thank you for your attention and considering this opinions :)

@kocsismate

Copy link
Copy Markdown
Member

Hey guys, I would like to add my few objections you might want to consider:

Please note that the feature is currently in voting, so any change should be warranted very much.

Points 3 and 4 have already been discussed to death, I believe the RFC provides background information why the readonly keyword (rather than writeonce) was chosen, and it also talks about the relation of "readonly properties" to the "asymmetric visibility" feature you also described.

Implementing an is_initialized() function could make sense, but it's IMO also orthogonal to this feature, since it doesn't only affect readonly properties. Also, it's already possible to detect if a property has been initialized via reflection.

Point 1 would render the feature broken, since any property value could be changed in private scope at any time, simply by unsetting it first.

@nikic

nikic commented Jul 5, 2021

Copy link
Copy Markdown
MemberAuthor

is_initialized() was recently discussed and declined (https://externals.io/message/114607).

@kolardavid

Copy link
Copy Markdown

OK guys, thanks for your reply and sorry to bother you :)

@nikicnikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikicforce-pushed the readonly-properties branch 7 times, most recently from 1bc5f49 to c890185CompareJuly 15, 2021 13:07
@rela589n

This comment has been minimized.

@kolardavid

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@KalleZ

Copy link
Copy Markdown
Member

Please keep these comments to the mailing list, the PR is not a forum for such

@rela589n

rela589n commented Jul 16, 2021 via email

Copy link
Copy Markdown

@nikic
nikicforce-pushed the readonly-properties branch 3 times, most recently from fe4c9e0 to 2137465CompareJuly 16, 2021 14:00
@nikic
nikicforce-pushed the readonly-properties branch from 8af4aae to 968a399CompareJuly 20, 2021 09:12
@nikicnikic closed this in 6780aaaJul 20, 2021
@jellynoone

Copy link
Copy Markdown
Contributor

Hi, @nikic just tried this out, is it intentional that it's now possible to skip the public visibility modifer in promoted properties and regular readonly properties?

class ClassName
{
readonlyint$prop1;
function__construct(
readonly array $prop2,
) {
$this->prop1 = \count($prop2);
}
}
\var_dump(newClassName([1,2]));

@jrfnl

jrfnl commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Just FYI as I didn't see mention of the impact of readonly becoming a semi-reserved keyword in the potential for BC-breaks section of the RFC, nor a code-scan of top PHP projects to gauge the impact:
Global functions, classes et al named readonly will have to be renamed as this will now be a parse error.
And by extension, all calls to these functions/instantiations of these classes will have to be adjusted too as those will also be a parse error.

If nothing else, this affects WordPress - and by extension potentially all plugins and themes in its ecosphere. So far, the impact is estimated to be relatively small as the function (to add the readonly attribute to HTML input tags) is not that widely used, but still...

@rela589n

This comment has been minimized.

@iluuu1994

Copy link
Copy Markdown
Member

Please use the mailing list for these discussions. The repercussions of a keyword are clear to voters. The suggestion of no $ was mentioned on the mailing list and not well received. While it avoids the BC break it's very subtle and doesn't communicate intent well. It's also way to late for this discussion in the first place. This RFC was in discussion for months.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@nikic@matthieu88160@iluuu1994@reyadkhan@kolardavid@kocsismate@rela589n@KalleZ@TysonAndre@jellynoone@jrfnl@mvorisek@dstogov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Implement readonly properties - #7089

Closed
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties
Closed

Implement readonly properties#7089
nikic wants to merge 20 commits into
php:masterfrom
nikic:readonly-properties

Conversation

@nikic

@nikicnikic commented Jun 2, 2021

Copy link
Copy Markdown
Member

@nikicnikic added the RFC label Jun 2, 2021
Comment threadZend/zend_language_parser.y
Comment threadZend/zend_object_handlers.c
@matthieu88160

Copy link
Copy Markdown

Hi all,
Shouldn't this feature be extended to the class constant rather than properties only?

@iluuu1994

iluuu1994 commented Jun 16, 2021

Copy link
Copy Markdown
Member

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

@matthieu88160

Copy link
Copy Markdown

@matthieu88160 Constants are by definition readonly. Not sure how that would make sense. Also note that the mailing list is the primary tool for feedback when it comes to specification.

My bad, I was thinking modification was allowed.

@iluuu1994

Copy link
Copy Markdown
Member

@nikic Here are two more tests for useful use-cases of overriding a readonly property:

--TEST--
Visibility can change in readonly property
--FILE--
<?phpclass A {
protected readonly int $prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
publicreadonlyint$prop;
}
$a = newA();
try {
var_dump($a->prop);
} catch (\Error$error) {
echo$error->getMessage() . "\n";
}
$b = newB();
var_dump($b->prop);
?>
--EXPECT--
Cannot access protected property A::$prop
int(42)

--TEST--
Can override readonly property with attributes
--FILE--
<?php
#[Attribute]
class FooAttribute {}
class A {
publicreadonlyint$prop;
publicfunction__construct() {
$this->prop = 42;
}
}
class B extends A {
#[FooAttribute]
publicreadonlyint$prop;
}
var_dump((newReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance());
?>
--EXPECT--
object(FooAttribute)#1 (0) {
}

Comment threadext/reflection/tests/readonly_properties.phpt
@reyadkhan

reyadkhan commented Jul 2, 2021

Copy link
Copy Markdown

Method parameters can be readonly:
foo(readonly string $bar){}

@kolardavid

Copy link
Copy Markdown

Hey guys, I would like to add my few objections you might want to consider:

  1. Personally, I would allow unset() on readonly property, which would unlock it for writing again. This can be done only in private scope and can be useful. It would then work as controlled lockable property within scope, which has knowledge about the object. Also it would be consistent with typed properties, which can be unset, which returns them back in state, where reading throws exception (in oppose to writing).
  2. PHP is imho missing simple function to check if typed or readonly property is initialized, which might be harder to safely detect if reading/writing will throw an exception or is permitted. People usually use isset(), which gives false negative if typed/readonly is already set to explicit null. It would be probably helpful to add is_initialized() function. You can't even use property_exists() for this, since it returns (correctly) true for uninitialized properties.
  3. I would personally vote for not using readonly keyword, rather use writeonce/immutable/locked. readonly imho does not describe the usage that well as the other. Also, I would reserve readonly for other purpose:
  4. readonly would be imho much more useful for different scenario, where property is writable within private scope and readonly outside of private scope. Actually I believe I would use this scenario even more than the currently proposed readonly feature. It would safely expose private properties to outside without risk of modification. It would be basically syntactic sugar for private property and public getter without setter (the above proposed behavior of being able to unset and set it again would be more close to this scenario, even though it would always require unset before set to be safe, which is annoying). I can imagine 2 ways of behavior:
    a. private readonly int $prop; would mean that property is writable in private scope and readonly in protected and public scope (in which case public readonly would be non-practical).
    b. public readonly int $prop; would mean the same, but changes meaning of visibility specifier (in which case private readonly would be non-practical)
    Basically the only difference would be if private/protected/public would specify where is readonly visible and it would be always writable only in private scope (which would work similarly to current readonly behavior, but you will be unable to make it writable from protected scope) or it would be always publicly visible, but you would specify if it is writable in private/protected/public scope. That would be imho more practical, but would be confusing and required explanation, because it would make visibility modifiers works differently than usual.

Of course there is option to merge both behaviors by adding modifier to readonly - so having public readonly working as readonly from outside, writable from inside and public readonly writeonce as current readonly proposal, making both things more consistent. The downside is that readonly writeonce is longer to write, but makes most sense to me.

Thank you for your attention and considering this opinions :)

@kocsismate

Copy link
Copy Markdown
Member

Hey guys, I would like to add my few objections you might want to consider:

Please note that the feature is currently in voting, so any change should be warranted very much.

Points 3 and 4 have already been discussed to death, I believe the RFC provides background information why the readonly keyword (rather than writeonce) was chosen, and it also talks about the relation of "readonly properties" to the "asymmetric visibility" feature you also described.

Implementing an is_initialized() function could make sense, but it's IMO also orthogonal to this feature, since it doesn't only affect readonly properties. Also, it's already possible to detect if a property has been initialized via reflection.

Point 1 would render the feature broken, since any property value could be changed in private scope at any time, simply by unsetting it first.

@nikic

nikic commented Jul 5, 2021

Copy link
Copy Markdown
MemberAuthor

is_initialized() was recently discussed and declined (https://externals.io/message/114607).

@kolardavid

Copy link
Copy Markdown

OK guys, thanks for your reply and sorry to bother you :)

@nikicnikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikicforce-pushed the readonly-properties branch 7 times, most recently from 1bc5f49 to c890185CompareJuly 15, 2021 13:07
@rela589n

This comment has been minimized.

@kolardavid

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@rela589n

This comment has been minimized.

@KalleZ

Copy link
Copy Markdown
Member

Please keep these comments to the mailing list, the PR is not a forum for such

@rela589n

rela589n commented Jul 16, 2021 via email

Copy link
Copy Markdown

@nikic
nikicforce-pushed the readonly-properties branch 3 times, most recently from fe4c9e0 to 2137465CompareJuly 16, 2021 14:00
@nikic
nikicforce-pushed the readonly-properties branch from 8af4aae to 968a399CompareJuly 20, 2021 09:12
@nikicnikic closed this in 6780aaaJul 20, 2021
@jellynoone

Copy link
Copy Markdown
Contributor

Hi, @nikic just tried this out, is it intentional that it's now possible to skip the public visibility modifer in promoted properties and regular readonly properties?

class ClassName
{
readonlyint$prop1;
function__construct(
readonly array $prop2,
) {
$this->prop1 = \count($prop2);
}
}
\var_dump(newClassName([1,2]));

@jrfnl

jrfnl commented Aug 3, 2021

Copy link
Copy Markdown
Contributor

Just FYI as I didn't see mention of the impact of readonly becoming a semi-reserved keyword in the potential for BC-breaks section of the RFC, nor a code-scan of top PHP projects to gauge the impact:
Global functions, classes et al named readonly will have to be renamed as this will now be a parse error.
And by extension, all calls to these functions/instantiations of these classes will have to be adjusted too as those will also be a parse error.

If nothing else, this affects WordPress - and by extension potentially all plugins and themes in its ecosphere. So far, the impact is estimated to be relatively small as the function (to add the readonly attribute to HTML input tags) is not that widely used, but still...

@rela589n

This comment has been minimized.

@iluuu1994

Copy link
Copy Markdown
Member

Please use the mailing list for these discussions. The repercussions of a keyword are clear to voters. The suggestion of no $ was mentioned on the mailing list and not well received. While it avoids the BC break it's very subtle and doesn't communicate intent well. It's also way to late for this discussion in the first place. This RFC was in discussion for months.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@nikic@matthieu88160@iluuu1994@reyadkhan@kolardavid@kocsismate@rela589n@KalleZ@TysonAndre@jellynoone@jrfnl@mvorisek@dstogov