[PHP8.2] mark class readonly if all props are readonly

[keradus]

before

class Post
{
    public function __construct(
        public readonly string $title, 
        public readonly Author $author,
        public readonly string $body,
        public readonly DateTime $publishedAt,
    ) {}
}

after

readonly class Post
{
    public function __construct(
        public string $title, 
        public Author $author,
        public string $body,
        public DateTime $publishedAt,
    ) {}
}

[mvorisek]

I do not think this is a good idea. Especially as PHP-CS-Fixer rules cannot be individually ignored. Imagine class with one readonly property. The class can be perfectly mutable class that is allowed to be extended by another non-readonly class.

[kubawerlos]

Especially as PHP-CS-Fixer rules cannot be individually ignored

What do you mean? You can disable any rule you want.

[keradus]

I guess he mentioned "per-file".

[keradus]

I guess we can start with converting phpdoc tag to readonly keyword [ref #6640 ]

[simensen]

I do not think this is a good idea. Especially as PHP-CS-Fixer rules cannot be individually ignored. Imagine class with one readonly property. The class can be perfectly mutable class that is allowed to be extended by another non-readonly class.

If readonly class would be inherited by children, it would indeed cause potential issues. Could we make it configurable? For example, as I've learned more about php-cs-fixer, I'd imagine something like this could be part of the design? For example, default behavior would be to not offer to promote a class to readonly if it is abstract, but give the option to include those as well?

[

  // Default behavior ( ['even_abstract' => false] )
  'promote_readonly_class' => true,

  // Can be overridden to 
  'promote_readonly_class' => [
    'even_abstract' => true,
  ],

I've spent very little time thinking about naming, here, but hopefully this helps show what I'm suggesting. :)

[Wirone]

Another option is to convert only final classes with all properties marked as readonly. For non-final classes we can break the code in inherited classes, so it's too risky IMHO.

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

I will close it if no further activity occurs within the next 30 days.

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

[ByNetherdude]

+1 for the final option suggested by @Wirone.

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

[janisint]

still needed, please unstale.

To the maintaners: I'd be happy to give it a go with a PR, just let me know

[hockdudu]

It appears there's a Rector rule for this: https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#readonlyclassrector

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

[janisint]

Please keep this open

[Wirone]

@janisint there's no need to wait for green light from maintainers when they created and commented the issue, and suggested how it should be implemented 🙂. Issue is open = accepted for implementation.