duster icon indicating copy to clipboard operation
duster copied to clipboard

Whitespace around re-ordered class properties and constants is incorrect

Open bakerkretzmar opened this issue 3 years ago • 4 comments

Anecdotally I'm noticing that this:

class Foo
{
    protected $foo = 'bar';
    public $bar = 'baz';
    public const FOO = 'bar';
    protected $bar = 'baz';
}

Is formatted to this:

class Foo
{
    public const FOO = 'bar';
    public $bar = 'baz';

    protected $foo = 'bar';
    protected $bar = 'baz';
}

The whitespace around/between properties and constants that get reordered isn't always consistent. Happy to work on this myself when I have time next week.

bakerkretzmar avatar Jan 04 '23 19:01 bakerkretzmar

The spacing is from TLint's OneLineBetweenClassVisibilityChanges

Class members of differing visibility must be separated by a blank line

driftingly avatar Jan 04 '23 20:01 driftingly

Sorry I should have been more specific—what's weird to me is that there isn't whitespace added after the public CONST. I would have expected either of these instead:

class Foo
{
    public const FOO = 'bar';
    public $bar = 'baz';
    protected $foo = 'bar';
    protected $bar = 'baz';
}
class Foo
{
    public const FOO = 'bar';

    public $bar = 'baz';

    protected $foo = 'bar';
    protected $bar = 'baz';
}

bakerkretzmar avatar Jan 04 '23 20:01 bakerkretzmar

Basically I want the formatter to fix all the whitespace for me, including in between consts and properties 😂

bakerkretzmar avatar Jan 04 '23 20:01 bakerkretzmar

Right now, OneLineBetweenClassVisibilityChanges doesn't distinguish between const/properties, it only looks at visibility changes. I think we would need an additional linter/formatter for class attribute groups, something like OneLineBetweenClassAttributeGroups.

Pint's Laravel preset uses class_attributes_separation to add a blank line between most:

'class_attributes_separation' => [
    'elements' => [
        'const' => 'one',
        'method' => 'one',
        'property' => 'one',
        'trait_import' => 'none',
    ],
],

We overwrite this to only require a blank line between method. I'm wondering if we should follow the laravel convention on this as well.

class Foo
{
    public const FOO = 'bar';

    public $bar = 'baz';

    protected $foo = 'bar';

    protected $bar = 'baz';
}

driftingly avatar Jan 04 '23 23:01 driftingly