Consider warning user when PHP runtime doesn't fit lowest PHP version from composer.json

[keradus]

if project XYZ supports php: ^7 || ^8 and one would run PHP CS Fixer on PHP 8.2, they could apply code changes that won't be valid for PHP 7.0.

idea: maybe we should warn user that they run it on high version and they should consider running the tool on lowest supported PHP of their repository?

[Wirone]

It sounds like a good idea. Specific fixers shouldn't be aware of Composer's context, but main process could determine PHP requirement and print some warning. I would even consider failing the process if it happens, because people don't read the warnings 😉.

[keradus]

let's start with the warning and see how to grow it further.

[localheinz]

A warning sounds good, but failing hard on users would make it more difficult for them to use the latest versions of php-cs-fixer/php-cs-fixer to apply fixes on a legacy code base.

I have had great success using the latest versions of php-cs-fixer/php-cs-fixer modernizing code bases running on PHP 5.3 to PHP 8.1 (and beyond).

It would be great if I could continue to do so, but I'm sure I would be able to find a way to work around such a limitation if it were put in place.

[keradus]

one can always first update composer.json, and then run the Fixer.
ultimately, if one uses Fixer to also migrate from PHPx to PHPy, we should run Fixer on PHPy and not promise that it would still work on PHPx.

[julienfalque]

What about introducing an environment variable, e.g. CODEBASE_LOWEST_PHP_VERSION that we would check instead of PHP_VERSION_ID constant? We could try to read composer.json in case it's not defined, showing a notice in CLI output in this case.

This variable would also allow us to prevent introducing changes that are not compatible with some old PHP version, even if the tool itself does not support running on that version anymore. However I guess that would also mean adding a lot of test cases.

[Wirone]

Not an environment variable, but rather "target PHP version" that can be set on config level. And then use PHP_VERSION_ID as default when it's not explicitly set. Fixers then should get that PHP version and align their inner logic to that desired level. This topic was discussed several times (here, here) and I am all for that. BUT it's pretty hard to achieve because of linting that is done before and after processing file, since the output code must be a valid syntax for the current PHP runtime.

[julienfalque]

Ah yes right, linting will be an issue.

[javaDeveloperKid]

Regarding the PHP version resolution

The most secure would be to require setting the PHP version in the config file, because falling back to the runtime version is not secure as it requires the user to remember about the runtime the tool is running in when fixing the code. Theoretically this required config option could be considered a BC Break and require major version release because after upgrading a tool it will fail. However I don't think that failing at the beginning with information about missing config value should be considered a BC Break. Especially for tool that is used only in CI, not in production.

Second option and I think most popular around such tools would be to fall back to the PHP runtime version. Most of the projects install php-cs-fixer as a dev dependency and run in the same PHP runtime as the application. However some project run php-cs-fixer as e.g. docker image for various of reasons with the most valuable being a chance to use newer version of the tool (new features, bugfixes) while application runtime is locked on lower one. If this is a case then one has to remember to set php version in the config.

Regarding applying fixer (or a part of a fixer) with changes no compatibile with the PHP version

IMO fixer should ignore features that do not exist in the targeted PHP version. There's no need to throw. We could display warning at the end .e.g.

The following fixes where not applied due to inconsistent PHP version:
  - MyFixer expects PHP >=8.0
  - MyOtherFixer for configuration option "parameters" expects PHP >=8.1

We could also consider adding cli option to control whether to fail or not on such warnings.
Other reason not to throw is that a company (or a team maintaining several application) may have one coding standards file that they copy past to every application but not every application is at the same PHP version. If so then it requires manual verification and adjustments for the ruleset and configuration (some rules and config might be hidden inside other sets what requires overriding the rule).
Also upgrading an app to higher PHP version would require revisiting the php-cs-fixer rulset and add new fixers or modify existing ones' configuration.

[Wirone]

It was already discussed, it's not possible to set target PHP version with current architecture (linting fixed file after applying changes). You just can't apply changes that introduce syntax not available in the runtime you're running the tool. It would require changing the philosophy and architecture, and major version of course.

[javaDeveloperKid]

Ok, I understand. Thank you for your response.
However maybe current linting could be configured to run againts given PHP version or delegated to other tool that can be configured? Just throwing ideas 😃

[Wirone]

Doesn't seem feasible, as external tooling is out of control of the Fixer.

However maybe current linting could be configured to run againts given PHP version

That would mean such PHP version is available, so why don't just run Fixer on it 😅?