Skip to content

Conversation

kkmuffme
Copy link
Contributor

Fix #1775

@kkmuffme
Copy link
Contributor Author

Quick question before I fix the test: the test fails on 7.4, however this test should not ever have passed on 7.4, since this test is a fatal error in PHP 7.4
Can I just check the PHP_VERSION_ID and skip the test in that version? Or are there any other ways implemented to run tests version specific?

Any plans to add a php -l linting pipeline to CI to prevent tests that are actually fatal errors ending up in the codebase, as well as to become aware of any issues that might be fatal in newer/older PHP versions?

@kkmuffme kkmuffme marked this pull request as ready for review August 24, 2025 23:37
@kukulich
Copy link
Contributor

PHPCS is parser so it does not run code. PHPCS on PHP 7.4 can parse and fix code with PHP 8.4 syntax.

@kkmuffme
Copy link
Contributor Author

No, you misunderstood me: PHPCS on PHP 7.4 with config php_version 80400 can fix code with PHP 8.4 syntax - but "php84 -l file.php" should pass without errors
Similarly: PHPCS on 8.4 with config php_version 70400 can fix code with PHP 7.4 syntax - "php74 -l file.php" should pass without errors.
This is what I meant should be checked in the CI (like other tools e.g. psalm do)

Therefore:

Can I just check the PHP_VERSION_ID and skip the test in that version? Or are there any other ways implemented to run tests version specific?

using PHP_VERSION_ID would be wrong, I'd need to check the php_version of phpcs config for that case and skip the test in that case. It seems there are no other tests that are version specific? Does this mean I can just remove the failing test, since it's only failing in PHP 7.4?

return;
}

if ($this->shortNullable !== self::YES && SniffSettingsHelper::isEnabledByPhpVersion(null, 80000) === false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not set the value, when user set it. Only null value should be rewritten.

@kukulich
Copy link
Contributor

I don't usually test default values - only specific values of the options (yes and no here). However you can test it by standard ways of PHPUnit.

See other tests data for the lint check for specific PHP versions.

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

Successfully merging this pull request may close these issues.

TypeHints.DNFTypeHintFormat shortNullable should be "yes" for PHP < 8
2 participants