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

Rule declare_strict_types does not work on files that start with a "hashbang" line #7634

Closed
krzysztof-sikorski opened this issue Dec 28, 2023 · 4 comments · Fixed by #7687
Closed

Comments

@krzysztof-sikorski
Copy link

Bug report

Description

The rule declare_strict_types does not work on files that start with a "hashbang" line:

#!/usr/bin/env php
<?php
// the rest of the file

I discovered the bug when I tried to reformat a bin/console file generated by Symfony CLI,
but further testing soon revealed that the rule simply ignores "mixed content" files
that contain any non-PHP lines before the <php opening tag.

While I understand that the behaviour might be intentional (to avoid corrupting files
where the declare statement would have no effect or wrong effect), I also think that
the hashbang line should be an exception and files that start with it but contain no other
non-PHP content should be treated as "pure PHP" files and modified by the rule.

Runtime version

PHP CS Fixer 3.43.0 Three Keys by Fabien Potencier and Dariusz Ruminski.
PHP runtime: 8.3.1

Used command

vendor/bin/php-cs-fixer -vvv fix

Configuration file

<?php
$rules = [
    'declare_strict_types' => true,
];

$finder = new PhpCsFixer\Finder();
$finder->in(__DIR__);

$config = new PhpCsFixer\Config();
$config->setRiskyAllowed(true);
$config->setRules($rules);
$config->setFinder($finder);

return $config;

Code snippet that reproduces the problem

#!/usr/bin/env php
<?php
echo 'PHP-CS-Fixer should add declare statement above!';

Expected result:

#!/usr/bin/env php
<?php declare(strict_types=1);
echo 'PHP-CS-Fixer should add declare statement above!';

Actual result: file is not changed at all.

Here is a simple repository that demonstrates the bug:
https://github.com/krzysztof-sikorski/php-cs-fixer-declare-strict-types-bug

Just pull the code, execute composer install, and then vendor/bin/php-cs-fixer -vvv fix.

@keradus
Copy link
Member

keradus commented Dec 30, 2023

potential testcase:

diff --git a/tests/Fixer/Strict/DeclareStrictTypesFixerTest.php b/tests/Fixer/Strict/DeclareStrictTypesFixerTest.php
index 709d50456..f42e138a6 100644
--- a/tests/Fixer/Strict/DeclareStrictTypesFixerTest.php
+++ b/tests/Fixer/Strict/DeclareStrictTypesFixerTest.php
@@ -130,6 +130,15 @@ class A {
             '<?php declare(strict_types=0);',
         ];
 
+        yield [
+            "#!/usr/bin/env php
+<?php declare(strict_types=1);
+echo 'foo';",
+            "#!/usr/bin/env php
+<?php
+echo 'foo';",
+        ];
  • class to handle the fix: DeclareStrictTypesFixer
  • potential helper to use: $tokens->isMonolithicPhp(), that is already checking for shebang

@krzysztof-sikorski , would you like to give it a try?

@keradus
Copy link
Member

keradus commented Dec 30, 2023

@krzysztof-sikorski
Copy link
Author

@keradus I'm not sure if I fully understand the project's architecture but yes, I am willing to try.

Because this issue got classified as a feature instead of bug, then maybe the new behaviour should be configurable and disabled by default, to keep backward compatibility?

@keradus
Copy link
Member

keradus commented Dec 30, 2023

Thanks!

I'm open to have it behind config

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

Successfully merging a pull request may close this issue.

2 participants