Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anonymous classes with interfaces missing space before opening bracket cause parse error with PSR-12 ruleset #3790

Closed
jwittorf opened this issue Apr 1, 2023 · 10 comments · Fixed by PHPCSStandards/PHP_CodeSniffer#56

Comments

@jwittorf
Copy link

jwittorf commented Apr 1, 2023

Describe the bug
In some cases anonymous classes with extends and implements cause a ParseError after "fixing" the file.

It happens if there is no space between the last interface-name and the {, see following code samples.

The following happens, see code samples at the end:

  1. The new keyword gets additional () braces
  2. The interface-names don't get indented and lined up correctly
  3. The opening { for the class gets removed
  4. The methods get wrong

Code sample

<?php

namespace Jwittorf\Psr12Linting\Psr12;

use Jwittorf\Psr12Linting\UseableClass;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfaces;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesB;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesC;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesD;

$instance = new class() extends UseableClass implements MultipleInterfaces,
    MultipleInterfacesB,
    MultipleInterfacesC, MultipleInterfacesD{
    public function __construct()
    {
        echo "Created anonymous class in file '" . __FILE__ . "'.\n";
    }

    public function someFunctionToBeImplemented(): void
    {
        echo "This text was created with the function " . __FUNCTION__ . " in file '" . __FILE__ . "'.\n";
    }
};

$instance2 = new class extends UseableClass implements MultipleInterfaces,
                                                       MultipleInterfacesB,
                                                       MultipleInterfacesC, MultipleInterfacesD{
    public function __construct()
    {
        echo "Created anonymous class in file '" . __FILE__ . "'.\n";
    }

    public function someFunctionToBeImplemented(): void
    {
        echo "This text was created with the function " . __FUNCTION__ . " in file '" . __FILE__ . "'.\n";
    }
};

Custom ruleset

<?xml version="1.0"?>
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="PHP_CodeSniffer">
    <file>src</file>

    <!-- Don't hide tokenizer exceptions -->
    <rule ref="Internal.Tokenizer.Exception">
        <type>error</type>
    </rule>
    
    <rule ref="PSR12"/>
</ruleset>

To reproduce
Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php ...
  3. See messed up PHP code below
<?php

namespace Jwittorf\Psr12Linting\Psr12;

use Jwittorf\Psr12Linting\UseableClass;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfaces;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesB;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesC;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesD;

$instance = new() class () extends UseableClass implements
MultipleInterfaces,
    MultipleInterfacesB,
    MultipleInterfacesC, MultipleInterfacesD

public function __construct()
{
    echo "Created anonymous class in file '" . __FILE__ . "'.\n";
}

public function someFunctionToBeImplemented(): void
{
    echo "This text was created with the function " . __FUNCTION__ . " in file '" . __FILE__ . "'.\n";
}
};

$instance2 = new() class extends UseableClass implements
MultipleInterfaces,
    MultipleInterfacesB,
    MultipleInterfacesC, MultipleInterfacesD

public function __construct()
{
    echo "Created anonymous class in file '" . __FILE__ . "'.\n";
}

public function someFunctionToBeImplemented(): void
{
    echo "This text was created with the function " . __FUNCTION__ . " in file '" . __FILE__ . "'.\n";
}
};

Expected behavior
I would expect the correct formatting like the code below with following "steps":

  1. Put every interface-name on an own line, indented 4 spaces
  2. Move the opening { into an own line, no indention
  3. DON'T try to add () after the new keyword
<?php

namespace Jwittorf\Psr12Linting\Psr12;

use Jwittorf\Psr12Linting\UseableClass;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfaces;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesB;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesC;
use Jwittorf\Psr12Linting\Psr4\MultipleInterfacesD;

$instance = new class() extends UseableClass implements
    MultipleInterfaces,
    MultipleInterfacesB,
    MultipleInterfacesC,
    MultipleInterfacesD
{
    public function __construct()
    {
        echo "Created anonymous class in file '" . __FILE__ . "'.\n";
    }

    public function someFunctionToBeImplemented(): void
    {
        echo "This text was created with the function " . __FUNCTION__ . " in file '" . __FILE__ . "'.\n";
    }
};

$instance2 = new class extends UseableClass implements
    MultipleInterfaces,
    MultipleInterfacesB,
    MultipleInterfacesC,
    MultipleInterfacesD
{
    public function __construct()
    {
        echo "Created anonymous class in file '" . __FILE__ . "'.\n";
    }

    public function someFunctionToBeImplemented(): void
    {
        echo "This text was created with the function " . __FUNCTION__ . " in file '" . __FILE__ . "'.\n";
    }
};

Versions:

  • OS: MacOS 12.6
  • PHP: 7.4.33
  • PHPCS: 3.7.1
  • Standard: PSR12
@jrfnl
Copy link
Contributor

jrfnl commented Apr 2, 2023

@jwittorf Thank you for reporting this. I've been able to reproduce the issues and am working on a fix.

@jrfnl
Copy link
Contributor

jrfnl commented Apr 2, 2023

Okay, done. PR #3791 should fix this. Testing appreciated.

@jwittorf
Copy link
Author

jwittorf commented Apr 2, 2023

Thanks for your quick response and fix @jrfnl! I've tested it in my setup, looks good :)

But the checks for PHP 8.2 still fail?

@jrfnl
Copy link
Contributor

jrfnl commented Apr 2, 2023

Thanks for your quick response and fix @jrfnl! I've tested it in my setup, looks good :)

But the checks for PHP 8.2 still fail?

Thanks for testing, Re: the PHP 8.2 test run, that is unrelated to this issue. See #3731

@fredden

This comment was marked as off-topic.

@jrfnl
Copy link
Contributor

jrfnl commented Apr 3, 2023

@fredden As I pointed out before, they are related. Also: different discussion, not in this issue.

@fredden
Copy link
Contributor

fredden commented Apr 3, 2023

@jrfnl it looks like #3791 only addresses one of the four bullet points raised in this issue. Did you determine that the other three are false-alerts / side effects?

  1. The new keyword gets additional () braces
  2. The interface-names don't get indented and lined up correctly
  3. The opening { for the class gets removed
  4. The methods get wrong

From what I can tell:

  1. This seems unrelated to the fix. Perhaps there's a quirk somewhere that adds these parenthesises when the code is mal-formed / invalid?
  2. The test case that you've added in PSR12/AnonClassDeclaration: prevent fixer creating parse error #3791 doesn't cover this. Is it covered elsewhere and a side effect of the invalid code?
  3. This is directly fixed by PSR12/AnonClassDeclaration: prevent fixer creating parse error #3791.
  4. This is fixed by PSR12/AnonClassDeclaration: prevent fixer creating parse error #3791, if I'm reading "wrong" correctly as "not indented enough", because it seems like a side effect from removing the opening brace by mistake.

@jrfnl
Copy link
Contributor

jrfnl commented Apr 3, 2023

@jrfnl it looks like #3791 only addresses one of the four bullet points raised in this issue. Did you determine that the other three are false-alerts / side effects?

Yes, I did, see my comment in the PR. They are all side-effects. There is nothing else to do.

@fredden
Copy link
Contributor

fredden commented Apr 3, 2023

Sorry @jrfnl, I've now seen the note on the pull request which specifically addresses my query.

@jrfnl
Copy link
Contributor

jrfnl commented Dec 8, 2023

FYI: the fix for this issue is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment