ClassAttributesSeparationFixer - add option for no new lines between properties

[adri]

Implementation for #4755

Changes the class_attributes_separation option to take options for each type (const, method or property. The options can be either one or none, indicating one or no spaces between the types.

Example

A configuration like this:

'class_attributes_separation' => ['elements' => ['property' => 'none']],

Would change this example code:

<?php
class A
{
    private $a = null;

    public $b = 1;

    function A() {}
}

to this:

<?php
class A
{
    private $a = null;
    public $b = 1;

    function A() {}
}

[julienfalque]

I get this error when running the new test I added. What does it mean?

This is because the unit tests for fixers first check that the input code is changed to the expected code then it checks that the expected code is not changed again when used as input. It means the fixer changes code that is not expected to change.

[adri]

@juliendufresne @kubawerlos Thanks for checking this PR, with the help of @ruudk this PR is now ready for review.

[adri]

@kubawerlos Thanks! Fixed

[kubawerlos]

👍

[ruudk]

Thanks for providing this PR 🙌
@SpacePossum What do you think? You also replied in #4755 to provide a solution.

[ruudk]

@keradus @GrahamCampbell What do you think of this? Would love to be able to remove all those empty newlines from our project 🙏

[julienfalque]

@adri Before we merge this PR, could you please rebase and squash commits to keep one commit per authorship?

[SpacePossum]

Thanks @adri and @ruudk , great to have this in :)

Hope you guys @ TicketSwap are holding up and get through the lockdown in NL, love the service you provide!

[SpacePossum]

Sorry missed your comment @julienfalque and already compiled the list of PR te merge. Sadly the merge script didn't squash the commit, but we can do without it.

[ruudk]

Thanks for merging this. I see this is merged to master. Will this be released as 2.17.x?

Hope you guys @ TicketSwap are holding up and get through the lockdown in NL, love the service you provide!

@SpacePossum Thanks, hope it too 😊

[SpacePossum]

yeah this will be in 2.17.0 :)

[ruudk]

@SpacePossum What's the ETA for 2.17.0?

[SpacePossum]

My hope is within two months, however it depends on what we find during regression testing it and the time the maintainer has, maybe not ideal but we shouldn't rush it. People can help out a lot by using the master version for now and report any bugs they find on their repo's, as I think 2.17 is mostly feature-complete by this point.

[ruudk]

When properties have comments, the newlines are also removed. I don't think it should behave like that.

'class_attributes_separation' => ['elements' => ['property' => 'none']],

class Entity
{
    /**
     * @ORM\Column(name="id", type="uuid")
     * @ORM\Id
     */
    private UuidInterface $id;

    /**
     * @ORM\Column(name="description", type="text")
     */
    private string $description = '';
}

gets converted to

class Entity
{
    /**
     * @ORM\Column(name="id", type="uuid")
     * @ORM\Id
     */
    private UuidInterface $id;
    /**
     * @ORM\Column(name="description", type="text")
     */
    private string $description = '';
}

What do you think? How should this behave?

[julienfalque]

I would prefer keeping the empty line when there is a DocBlock comment. Same with constants and methods.

[ruudk]

Indeed, that's also what I expected, so maybe it's a bug in this PR? I don't know enough from the internals to know where this responsibility resides.

[kubawerlos]

I would expect it to be removed as the configuration chosen is none - maybe we need new config option like none_where_there_is_no_phpdoc_before_and_one_where_is_phpdoc_before.

[SpacePossum]

I agree with @kubawerlos here, for none I expect no exceptions. The new option is fine by me, although maybe a shorter name for it can be found.

[ruudk]

@SpacePossum @kubawerlos @julienfalque Implemented in #5060. What do you think?