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

2.18.4 regression: anonymous class properties being inlined with anonymous class declarations #5575

Closed
Ocramius opened this issue Mar 25, 2021 · 6 comments
Labels

Comments

@Ocramius
Copy link

Ocramius commented Mar 25, 2021

While upgrading from 2.18.3 to 2.18.4:

         $url = $this->createProvider(
             new DummyPayoneGateway(null, function () {
-                throw new class() extends PaymentException {
-                    protected $message = 'my error';
+                throw new class() extends PaymentException { protected $message = 'my error';
                 };
             })
         )->buildPaymentRedirectUrl($order);

Overall, that protected $message should stay on a new line, I suppose?

.php_cs.dist:

<?php

$finder = PhpCsFixer\Finder::create()
    ->in([__DIR__ . '/src', __DIR__ . '/tests']);

return PhpCsFixer\Config::create()
    ->setRules([
        '@Symfony' => true,
        'strict_param' => true,
        'array_syntax' => ['syntax' => 'short'],
        'phpdoc_to_comment' => false,
        'no_unneeded_final_method' => false,
        'no_superfluous_phpdoc_tags' => ['allow_mixed' => true],
    ])
    ->setFinder(
        PhpCsFixer\Finder::create()
            ->in([__DIR__ . '/src', __DIR__ . '/tests'])
    );
@huye
Copy link

huye commented Mar 29, 2021

2.18.3:

class className {
}

2.18.4:

class className
{
}

I want to keep brace on the same line as 2.18.3. How do I set them?

@kubawerlos
Copy link
Contributor

@huye why did you post your message here? This issue is about anonymous classes where open brace is not a problem.

@kubawerlos
Copy link
Contributor

@Ocramius in 2.18.3 SingleLineThrowFixer had priority 1 and BracesFixer had -25, so BracesFixer was adding new lines after SingleLineThrowFixer removed them. In 2.18.4 priority of BracesFixer got changed to 35 so it runs first now.

Take a look at #5593 - is that making sense or should it be change the other way - that SingleLineThrowFixer is not touching anonymous class definition?

@Ocramius
Copy link
Author

Totally missed to provide feedback on this, @kubawerlos @keradus!

I'll see if I can test this on a new release, but I'm really unable to provide active help on understanding the underlying cause of the original issue due to lack of knowledge of the codebase :S

@Ocramius
Copy link
Author

Seems like this is still reproducible in 2.18.6:

--- Original
+++ New
@@ -229,8 +229,7 @@
 
         $url = $this->createProvider(
             new DummyPayoneGateway(null, function () {
-                throw new class() extends PaymentException {
-                    protected $message = 'my error';
+                throw new class() extends PaymentException { protected $message = 'my error';
                 };
             })
         )->buildPaymentRedirectUrl($order);
      ----------- end diff -----------

Should I open a new issue about this one? 🤔

@keradus
Copy link
Member

keradus commented Apr 27, 2021

The fix prepared by @kubawerlos - #5593 - was merged into 2.18.7, according to the milestone.

GitHub clsoes the ticket automatically when PR is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants