tighten / duster

Automatic configuration for Laravel apps to apply Tighten's standard linting & code standards.
MIT License
507 stars 16 forks source link

Whitespace around re-ordered class properties and constants is incorrect #49

Open bakerkretzmar opened 1 year ago

bakerkretzmar commented 1 year ago

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.

driftingly commented 1 year ago

The spacing is from TLint's OneLineBetweenClassVisibilityChanges

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

bakerkretzmar commented 1 year ago

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 commented 1 year ago

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

driftingly commented 1 year ago

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';
}