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

Formatting results in Implements being commented out #231

Closed
FelixTHG opened this issue Nov 24, 2023 · 5 comments
Closed

Formatting results in Implements being commented out #231

FelixTHG opened this issue Nov 24, 2023 · 5 comments
Assignees

Comments

@FelixTHG
Copy link

Pint Version

1.13.6

PHP Version

8.2.5

Description

We have a validation rule that implements
use Illuminate\Contracts\Validation\DataAwareRule;
use Illuminate\Contracts\Validation\Rule;

we had commented out that the rule should implement Illuminate\Contracts\Validation\ValidatorAwareRule at the end of the implements line; like so:

class E164PhoneNumber implements DataAwareRule, Rule //, ValidatorAwareRule

when we format this file using pint fresh as is, it puts the "Rule" into the comment, breaking our validation.

Steps To Reproduce

Install pint on a laravel 10 project
used
composer require laravel/pint

create a rule file like so:

<?php

namespace App\Rules;

use App\Helpers\StringHelper;
use Illuminate\Contracts\Validation\DataAwareRule;
// use Illuminate\Contracts\Validation\ValidatorAwareRule;
use Illuminate\Contracts\Validation\Rule;

class E164PhoneNumber implements DataAwareRule, Rule //, ValidatorAwareRule
{
    public string $strippedPhone;

    public function passes($attribute, $value)
    {
        if (! isset($this->strippedPhone)) {
            return preg_match('/^\d{8,15}$/', $value);
        }

        return preg_match('/^\d{8,15}$/', $this->strippedPhone);
    }

    public function message()
    {
        return trans('validation.e164_phone_number');
    }

    public function setData($data): void
    {
        if (isset($data['phone'])) {
            $this->strippedPhone = StringHelper::Remove47FromPhone($data['phone']);
        }
    }
}

use pint to format the file by running ./vendor/bin/pint

@Jubeki
Copy link
Contributor

Jubeki commented Nov 24, 2023

In a fresh Laravel 10 Application with the example code put into "app/Rules/E164PhoneNumber.php" and then running the command vendor/bin/pint nothing happens.

Can you provide an example repository? Or do you have custom rules in your pint.json

Note: I use PHP 8.2.11 instead of PHP 8.2.5 (on MacOS)

@FelixTHG
Copy link
Author

FelixTHG commented Nov 24, 2023

Sorry, I guess I was a little quick.
you will have to change your class defining line to be like this:
class E164PhoneNumber implements Rule, DataAwareRule //, ValidatorAwareRule

Having it like so, gives me the bug, after doing a fresh laravel install (was in a more complex project earlier)

@Jubeki
Copy link
Contributor

Jubeki commented Nov 24, 2023

using the changed code from you, I can confirm the issue.

using this pint.json

{
    "rules": {
        "ordered_interfaces": false
    }
}

you can for now disable this behaviour, but this is an issue with PHP-CS-Fixer where you should report the problem.

@driesvints
Copy link
Member

Thanks @Jubeki

@FelixTHG feel free to report this to PHP CS Fixer 👍

@keradus
Copy link

keradus commented Nov 24, 2023

Fix released by Fixer team 🎉

for cross reference: PHP-CS-Fixer/PHP-CS-Fixer#7463 (comment)

btw folks, in some previous conversations we got lot of involvement of Laravel community pointing not big enough discussion, alignments etc - root cause is limited pool of contributors. Please, take it as good opportunity here - in future cases when you go deep dive to figure out which rule is having the problem, you may not only delegate the issue to core repo, but also contribute to repo with the fix 🙏🏻

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

No branches or pull requests

5 participants