php-cs-fixer-config icon indicating copy to clipboard operation
php-cs-fixer-config copied to clipboard

Customizing rules without overriding PHP-version-specific disabling

Open uuf6429 opened this issue 1 year ago • 0 comments

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 #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.

uuf6429 avatar Aug 25 '24 16:08 uuf6429