ClassAttributesSeparationFixer - Introduce only_if_meta spacing option

[paulbalandan]

Closes #4014 .

This PR introduces a new spacing option only_if_meta to class_attributes_separation fixer. The behavior of this option is technically the same with none except that when there is a T_DOC_COMMENT or T_ATTRIBUTE (or both) above the class element the option will now behave as one.

Examples:

['elements' => ['const' => 'only_if_meta']]

// Before
class Sample
{
    /** @var int */
    const FOO = 1;
    /** @var int */
    const BAR = 2;
    #[Foo\Baz]
    const BAZ = 3;
    const OTHER = 4;
}

// After
class Sample
{
    /** @var int */
    const FOO = 1;

    /** @var int */
    const BAR = 2;

    #[Foo\Baz]
    const BAZ = 3;
    const OTHER = 4;
}

['elements' => ['property' => 'only_if_meta']]

// Before
class Sample
{
    public $a;
    public $b;
    /** @var string */
    public $c;
}

// After
class Sample
{
    public $a;
    public $b;

    /** @var string */
    public $c;
}

[julienfalque]

Nice 👍

[coveralls]

Coverage increased (+0.01%) to 92.194% when pulling 79bfe09 on paulbalandan:class-attr-sep-hybrid-option into e5ebc7f on FriendsOfPHP:master.

[ruudk]

🙌 Amazing! Thanks for tackling this @paulbalandan

I tried this in the past but couldn't get it to work.

Instead of calling the option one_if_phpdoc shouldn't we also think about PHP 8 Attributes? How do we want to support that in the future? Maybe the option should be called one_if_phpdoc_or_attribute and support Attributes as well?

Because properties can be annotated with only an attribute, with an attribute and phpdoc, only a phpdoc, or nothing at all.

[julienfalque]

@ruudk Good point! I agree PHP 8 attributes should be supported. Maybe a name like only_if_meta would be more appropriate? Anyway, @paulbalandan, could you please make the changes for that?

[paulbalandan]

@julienfalque sure, I'll look into that. Just to confirm, meta means phpdocs and attributes only, right?

[julienfalque]

Thanks! Yep, meta means "PHPDocs or attritbute".

[ruudk]

Does it matter if it's a phpdoc or regular comment? I think as a user you expect the same behavior:

    const FOO = 1;
    
    // Some comment to explain more
    const BAR = 2;
    const BAZ = 3;

?

[julienfalque]

IMO we can start with PHPDocs and attributes only and see if the option needs adjustements afterwards.

[ruudk]

I was looking at the example in the OP:

// Before
class Sample
{
    /** @var int */
    const FOO = 1;
    /** @var int */
    const BAR = 2;
    #[Foo\Baz]
    const BAZ = 3;
    const OTHER = 4;
}

// After
class Sample
{
    /** @var int */
    const FOO = 1;

    /** @var int */
    const BAR = 2;

    #[Foo\Baz]
    const BAZ = 3;
    const OTHER = 4;
}

Shouldn't there also be a newline between BAZ and OTHER because BAZ has meta? That's how I would manually format this.

Like this:

// Before
class Sample
{
    /** @var int */
    const FOO = 1;
    /** @var int */
    const BAR = 2;
    #[Foo\Baz]
    const BAZ = 3;
    const OTHER = 4;
    const OTHER2 = 5;
}

// After
class Sample
{
    /** @var int */
    const FOO = 1;

    /** @var int */
    const BAR = 2;

    #[Foo\Baz]
    const BAZ = 3;

    const OTHER = 4;
    const OTHER2 = 5;
}

[paulbalandan]

The rule adds a new line if the element itself has a meta above it. It will not be affected if the element above it has the meta instead.

[SpacePossum]

closes #5800 as well

[SpacePossum]

I like this PR a lot and know it is a feature request for a lot for some time 👍

Left a minor and kicked the CI, so lets see on that.
Another thing I was wondering is #5704 (comment) , I would like the style with each PHPDoc + element being separated by itself, so basically agreeing with @ruudk . What do others think and what would be the effort to adjust?

[keradus]

Thank you @paulbalandan.

[ruudk]

Thank you @paulbalandan for this amazing contribution ❤️ 🙌