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

Customizing rules without overriding PHP-version-specific disabling #1099

Open
uuf6429 opened this issue Aug 25, 2024 · 0 comments
Open

Customizing rules without overriding PHP-version-specific disabling #1099

uuf6429 opened this issue Aug 25, 2024 · 0 comments
Assignees
Labels

Comments

@uuf6429
Copy link
Contributor

uuf6429 commented Aug 25, 2024

First of all, I think this package is a very good idea. I especially like the fact that I can avoid doing my own, which was already in the planning phase. :)

As you can see from #1098, eventually I would like to automate the version-specific aspect as much as possible.

The problem I see is that when customizing some rule, I would have to keep in mind the PHP version as well. This might have been a "given" with version-specific rulesets, but it's particular annoying with the automated-version-ruleset.
Consider the following:

// in a PHP 8.3 project
$companyRuleSet = Config\RuleSet\Php56::create()->withRules(Config\Rules::fromArray([
    'void_return' => true,
]));
// in a PHP 5.6 project
$companyRuleSet = Config\RuleSet\Php56::create()->withRules(Config\Rules::fromArray([
    //'void_return' => true,      <- we can't enable this for PHP 5.6
]));

// ^ In the two cases above, we'll need to have (and maintain) a different ruleset for each PHP version, rendering this package useless.

// in any PHP project thanks to automatic version check
$companyRuleSet = Config\RuleSet\PhpAuto::create()->withRules(Config\Rules::fromArray([
    'void_return' => true,      // <- this will still not work for e.g. PHP 5.6
]));

// ^ That skips one step, but there's still the PHP version problem

Solutions

  1. A bit of a hack: generating some special value (e.g. object) which is than resolved correctly in the factory:

    $companyRuleSet = Config\RuleSet\PhpAuto::create()->withRules(Config\Rules::fromArray([
        'void_return' => since('7.1'),
    ]));

    Implementing it as a child repo/package means skipping the usefulness of this repo. The 'since' thingy is otherwise rather readable.. could be something to consider in the current repo.

  2. Applying the PHP-version-specific changes after defining the customization:

    $companyRuleSet = CompanyRuleSet::create()->withoutRules(Config\RuleSet\PhpAuto::create());

    I don't think this would work though since some rules in this package seem to be deactivated based on preference rather than PHP-version (also following/considering Why forcing all the builtin rules to be configured? #2).

  3. The cleanest approach I can think of (and that would still be backword-compatible):

    // Backword-compatible change in PHP 7.0 ruleset. All other rulesets should be implemented similarly
    class Php70 {
        public static function create() {
            return self::disableIncompatibleRules(Php71::create());
        }
    
        public static function disableIncompatibleRules(RuleSet $set) {
            return $set->withRules([
                'void_return' => false,
                // ...
            ]);
        }
    }
    // Note: At the top of the hierarchy, `Php53::create()` would be calling `PhpBase::create()` where you define the defaults for all versions and an empty `disableIncompatibleRules`.
    
    // company php-cs-fixer config repo
    class CompanyRuleSet {
        // all the customizations we want
    }
    
    // company php-cs-fixer config file for any project
    $effectiveRuleSet = Config\RuleSet\PhpAuto::disableIncompatibleRules(CompanyRuleSet::create()); // TADA 🤩

Thoughts? Other ideas? If you like the 3rd approach, I'd gladly implement it in a PR.

@localheinz localheinz self-assigned this Nov 20, 2024
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

2 participants