spryker / code-sniffer

Spryker Code Sniffer
https://spryker.com
MIT License
36 stars 11 forks source link

Messes up indentation when certain rules are excluded #259

Closed alfredbez closed 3 years ago

alfredbez commented 3 years ago

We noticed that the ruleset messes up indentation in some places when certain rules are excluded.

Example code:

<?php

declare(strict_types = 1);

class Foo
{

    protected function baz(): void
    {
    }

    protected function bar(): array
    {
        return [
            new stdClass(),
        ];
    }
}

The output of vendor/bin/phpcs --report=diff --standard=ruleset.xml looks like this:

--- Foo.php
+++ PHP_CodeSniffer
@@ -4,7 +4,6 @@

 class Foo
 {
-
     protected function baz(): void
     {
     }
@@ -12,7 +11,7 @@
     protected function bar(): array
     {
         return [
-            new stdClass(),
+        new stdClass(),
         ];
     }
 }

What we're confused about is: Why does the ruleset changing the indentation for new stdClass(),

This stops to happen if we exclude the rule Spryker.WhiteSpace.EmptyEnclosingLine.OpenBraceNewLine.

Here's a repo that reproduces that inside Github Actions (with excluded rule, without excluded rule).

dereuromark commented 3 years ago

The sniff most lkely has a side effect, that gets solved by including other sniffs that fix it.

A PR with a suggested fix to avoid side effects is welcome - this will be the speed process to resolve. Otherwise we will process this through a backlog and will investigate possible fixes on our side.

dereuromark commented 3 years ago

I looked into the sniff, and it only adds a newline So there must still be a different side effect playing into this for some reason then. Did anyone make an investigation on what causes it?

dereuromark commented 3 years ago

Can you check if https://github.com/spryker/code-sniffer/compare/bugfix/empty-enclosing resolves this? There was some issue of a token too much being modified it seems.

We can also add more tests on top, feel free to PR here any additional checks.

alfredbez commented 3 years ago

That looks better now, thanks:

Run vendor/bin/phpcs --report=diff --standard=ruleset.xml
--- Foo.php
+++ PHP_CodeSniffer
@@ -4,7 +4,6 @@

 class Foo
 {
-
     protected function baz(): void
     {
     }

Error: Process completed with exit code 2.

You can also have a look at this directly on github: https://github.com/alfredbez/spryker-code-sniffer-issue/runs/3301888579?check_suite_focus=true

dereuromark commented 3 years ago

Release is done :)