[DX] Aliases "check" & "fix"

[smnandre]

i'd like to have two aliases: check & fix , so i started to see how that could be done.

This seems to work perfectly by adding thoses aliases in the entry point, and two lines in the command

// bin/twig-cs-fixer

// ...

$command = new TwigCsFixerCommand();
$command->setAliases(['fix', 'check']);

$application = new Application();

// TwigCsFixerCommand.php


    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        if ('check' === $input->getArgument('command')) {
            $input->setOption('fix', false);
        } elseif ('fix' === $input->getArgument('command')) {
            $input->setOption('fix', true);
        }

Would you be open to that ? If yes do you think there is a better way to implement it ?

[VincentLanglet]

Would you be open to that ?

Yes, but I would say it requires some thought about what should be the right aliases.

If yes do you think there is a better way to implement it ?

I think setAliases can be called directly in TwigCsFixerCommand::configure.

My only concern is "What if someone use check --fix or fix --fix. There is multiple solution

• check --fix behave like lint --fix and fix --fix like fix
• --fix option fails with fix and check.

Another way to implement such alias would be to extends the command like it's done in the PHP-Cs-Fixer check command: https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/blob/master/src/Console/Command/CheckCommand.php

Or maybe we add a constructor

public function __construct(
    private string $name = self::DEFAULT_NAME,
    private string $description = self::DEFAULT_DESCRIPTION, 
    private ?bool $fix
) {
} 

with

protected function configure(): void
    {
        $this
            ->setName($this->name)
            ->setDescription($this->description)
            ->setDefinition(is_bool($this->fix) ? $inputOptionsWithoutFix : $inputOptions);

and

true === $this->fix || $input->hasOption('fix') && $input->getOption('fix') ? new Fixer($tokenizer) : null,

so it could be declared this way:

$command = new TwigCsFixerCommand();
$aliases = [
    new TwigCsFixerCommand('check', 'TODO', false),
    new TwigCsFixerCommand('fix', 'TODO', true),
];

$application = new Application();
$application->add($command);
foreach ($aliases as $alias) {
    $application->add($command);
}
$application->setDefaultCommand($command->getName());
$application->run();

I really dunno what is the best strategy...

[smnandre]

My only concern is "What if someone use check --fix or fix --fix. There is multiple solution

To me, fix --fix has no point, so i have no opinion on throwing or ignoring

And in the end, in my mind, check --fix should be tolerated.

--

Php-Cs-Fixer has a "--dry-run" option for the "fix" command, and a "check" alias for it

# Accepted, and works as expected
phpcs fix  --dry-run

# Accepted, and works as expected (so as "fix --dry-run)
phpcs check

# Error
phpcs check --dry-run
    The "--dry-run" option does not exist.

--

So that'd give here:

• define the check alias in the existing command (as it's 100% an alias)
• create a fix command that extends the current one and sets the "--fix" option)

WDYT ?

[VincentLanglet]

So that'd give here:

• define the check alias in the existing command (as it's 100% an alias)
• create a fix command that extends the current one and sets the "--fix" option)

WDYT ?

I agree, especially because aliases and another command are displayed differently

But I just remembered one reason which could not play in favor of a fix alias/command: all the rules are not automatically fixable. For instance, I don't fix automatically variable name since it could break the application https://github.com/VincentLanglet/Twig-CS-Fixer/blob/main/src/Rules/Variable/VariableNameRule.php#L80

Wouldn't be weird to have a fix command which doesn't fix every time and still output errors you have to fix manually ?

[smnandre]

Wouldn't be weird to have a fix command which doesn't fix every time and still output errors you have to fix manually ?

That does not change really more than the current situation (in my mind) .. If it's documented as today "Automatically fix all the fixable violations"

The question is more "what status code when all fixable violations have been fixed" i guess ?