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

Remove or add redundant readonly property modifier #7915

Open
derrabus opened this issue Mar 28, 2024 · 4 comments
Open

Remove or add redundant readonly property modifier #7915

derrabus opened this issue Mar 28, 2024 · 4 comments
Labels
kind/feature request topic/PHP8.2 Related to features available in PHP 8.2+

Comments

@derrabus
Copy link
Contributor

derrabus commented Mar 28, 2024

Feature request

In readonly classes, I can still flag individual properties as readonly, for example:

readonly class MyService
{
    private readonly Foo $foo;

    public function __construct(
        FooFactory $fooFactory,
        private readonly Bar $bar,
    ) {
        $this->foo = $fooFactory->create();
    }
}

In this context, the readonly modifiers on the properties are 100% redundant. However, in codebases where readonly and mutatable properties are mixed, these redundant modifiers can still provide explicitness to the reader.

Proposal

I'd like to propose a fixer that normalizes readonly property modifiers on read-only classes. That fixer should come with a boolean setting to let me choose between:

  • Always add readonly to all properties of a readonly class. This is essential the code snippet above.
  • Always remove readonly from all properties of a readonly class. This setting wold be my preferred default. Running the fixer with that setting on the code above would yield the following result:
readonly class MyService
{
    private Foo $foo;

    public function __construct(
        FooFactory $fooFactory,
        private Bar $bar,
    ) {
        $this->foo = $fooFactory->create();
    }
}

Related work

@derrabus derrabus changed the title Remove redundant readonly property modifier Remove or add redundant readonly property modifier Mar 28, 2024
@Wirone Wirone added the topic/PHP8.2 Related to features available in PHP 8.2+ label Mar 28, 2024
@julienfalque
Copy link
Member

in codebases where readonly and mutatable properties are mixed, these redundant modifiers can still provide explicitness to the reader.

While I see the value of removing the redundant readonly markers from properties, I don't understand what would be the value of the opposite. If you can add readonly to properties because the class itself is readonly, then you don't have a mix of mutable and readonly properties, you already have readonly properties only.

@derrabus
Copy link
Contributor Author

derrabus commented Apr 3, 2024

If you can add readonly to properties because the class itself is readonly, then you don't have a mix of mutable and readonly properties, you already have readonly properties only.

Sure, but I have to look at the class definition to see that. I have to jump to a different line of code while reading a file.

That being said, I can totally live with a fixer that is not configurable and that is only able to remove redundant readonly from properties.

@Wirone
Copy link
Member

Wirone commented Apr 3, 2024

If you can add readonly to properties because the class itself is readonly, then you don't have a mix of mutable and readonly properties, you already have readonly properties only.

Sure, but I have to look at the class definition to see that. I have to jump to a different line of code while reading a file.

You don't mark methods as final when class is final though, right? How's that different with readonly?

@derrabus
Copy link
Contributor Author

derrabus commented Apr 3, 2024

You don't mark methods as final when class is final though, right?

But I could.

How's that different with readonly?

It isn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature request topic/PHP8.2 Related to features available in PHP 8.2+
Projects
None yet
Development

No branches or pull requests

3 participants