sabre-io / cs Goto Github PK

static is the 'more important keyword' I believe, so it should be turned around.

Should we also use the ternary_spaces fixer?

Would e.g. enforce spaces for $a ? $b : $c.

See https://travis-ci.org/fruux/sabre-katana/jobs/63172493#L289.

sabre/cs is based on php-cs-fixer 1.1 which is by now deprecated and not compatible with latest php versions (warnings are poping up on php7.3)

Porting sabre/cs to php-cs-fixer 2.0 is quite some effort since interfaces changed. From the first look it seems all custom fixer rules need to be reimplemented.

I hereby question the necessity of these custom rules and propose to deprecate sabre/CS and use pure php-cs-fixer.

@staabm @evert

Too many to name. See commit ;)

If there is no test, then we should remove the tests/ directory :-).

I've done this so incredibly inconsistently, that it makes sense to finally make a decision here.

I don't know what looks nicer anymore, I've been going back and forward. Because of this, it's better to pick one: lets go for the PSR-2 approach.

PSR-2 dictates that every import must have 1 use statement.

Because php-cs-fixer automatically skips vendor/, we can safely do:

$ bin/sabre-cs-fixer fix --dry-run --diff .

It will include the tests/ directory this way. And it's much better. All projects must update to this way of doing.

We don't like these whitespaces:

• if ()
• for ()
• foreach ()
• while ()
• catch ()
• function ()
• ...

Creating a ticket so people see the change and might want to argue.

Currently the CS fixer will change this:

$foo = 1;
$longvariable = 2;

to

$foo          = 1;
$longvariable = 2;

While I think this is totally more legible in some blocks of code, especially blocks where there's many similar things being defined, I don't think it should be applied everywhere. So I'd want to remove this rule.

@DominikTo found out that public properties cause internal errors in sabre-cs-fixer.

class Foo
{
}

Is currently not automatically fixed into:

class Foo {
}