Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.2 | PSR1/SideEffects: allow for readonly classes #3728

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Dec 10, 2022

Includes unit test.

Ref:

Fixes #3727

@michnovka
Copy link

Any update regarding this?

@l-you
Copy link

l-you commented Feb 3, 2023

Any update regarding this?

@ldebrouwer
Copy link

It'd be amazing if we could get this merged. I'm itching to start using readonly classes under PHP 8.2.

@eerison
Copy link

eerison commented Feb 16, 2023

what is missing here?

@HSken
Copy link

HSken commented Feb 20, 2023

what is missing here?

I suspect this is waiting to have all test's successful, this one is still failing
https://github.com/squizlabs/PHP_CodeSniffer/actions/runs/3664153242/jobs/6194396619

@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 20, 2023

what is missing here?

I suspect this is waiting to have all test's successful, this one is still failing https://github.com/squizlabs/PHP_CodeSniffer/actions/runs/3664153242/jobs/6194396619

No, it's not. PR #3686 has been open for longer than this PR to address that particular issue, which you could have known if you'd read the original issue: #3727 (comment).

What it's waiting for is maintainer time.

@aykutersoyrmn
Copy link

It'd be amazing to start using readonly classes in our projects. I hope the maintainer will find some time to merge and release this PR 🤞

@gsherwood gsherwood added this to the 3.8.0 milestone Feb 22, 2023
@michal-swietochowski-tg

Waiting for this too. Is there anything blocking this small change?

@janedbal
Copy link

@gsherwood Hi, did you consider adding few more people as maintainers to this project so that it does not block ton of companies to adapt new PHP features?

I can confirm that every year I actually need to wait months for this project to onboard new PHP syntax to be able to use it in our projects.


I understand that open-source is hard and takes ton of time, but I believe the community will help you with that on such core features.

Thank you for considering this.

@kyryl-bogach
Copy link

kyryl-bogach commented Mar 28, 2023

I'm down to help! 👍🏻

@helyakin
Copy link

helyakin commented Apr 25, 2023

I'm totally down to help as well.

@janedbal is right, I actually have a PR on phpinsights wich will also needs this improvement on php code sniffer to be merged.
And I'm sur a tons of other code sniffer dependent tools with this project do need this.

I'd be happy to contribute in any way, so this kind of mandatory tool for every project keeps on living.

@jrfnl
Copy link
Contributor Author

jrfnl commented May 19, 2023

After seeing all the offers for help here, I've spend a little time creating a more extensive contributor guide. I look forward to seeing your contributions!

As for this PR, after the announcement a couple of weeks ago, I spend some time creating a PR priority list, which we are currently working our way through. This PR is first on the list for our next maintainers call session in two weeks time (and no, I will not merge my own PRs).

gsherwood added a commit that referenced this pull request May 22, 2023
@gsherwood gsherwood merged commit 7c452b0 into squizlabs:master May 22, 2023
@jrfnl jrfnl deleted the php-8.2/psr1-sideeffects-allow-for-readonly-classes branch May 26, 2023 00:10
@janedbal janedbal mentioned this pull request Aug 1, 2023
@asanikovich
Copy link

When will be release of 3.8.0 ?

@talkinnl
Copy link

In addition to the extra maintainers / or release date discussion:

Maybe the current 3.8.0 scope is too big? It currently contains 46 closed, 25 open tickets.

If some/most open tickets could be postponed to a later version, it would enable getting all the nice done work published. :)

@delolmo
Copy link

delolmo commented Nov 22, 2023

For the time being, anyone encountering this problem can bypass this error by using this phpcs comment before the class declaration:

// phpcs:disable PSR1.Files.SideEffects
final readonly class WhateverClassName

This way CI/CD tests won't fail and you can revert the file to the original state once version 3.8 is released.

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 8, 2023

FYI: this fix is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).

@eerison
Copy link

eerison commented Dec 9, 2023

@jrfnl is it possible to mark this repo as abandoned and archive? Maybe add something in composer to inform

@fredden
Copy link
Contributor

fredden commented Dec 9, 2023

@eerison please see #3932 and #3933.

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 9, 2023

@eerison As @fredden pointed out, a PR to add a note at the top of the readme is open.

Archiving the repo will possibly be done in six/twelve months time or so, as once the repo is archived you cannot comment on issues/PRs anymore, which is very limiting. For example, I wouldn't have been able to post my previous message informing you of the fix being in the 3.8.0 release, so archiving the repo now would likely hinder instead of help the information about the repo move spreading.

@asanikovich
Copy link

@jrfnl
Maybe you should update Readme.md with the latest updates about The Future of PHP_CodeSniffer?

@fredden
Copy link
Contributor

fredden commented Dec 12, 2023

@asanikovich please see #3933.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not work with readonly classes introduced in PHP 8.2