Enhancement: Implement Files\DeclareStrictTypesRule

[dmecke]

This PR

• implements a Files\DeclareStrictTypesRule, which reports an error when a non-empty file does not contain a declare(strict_types=1); declaration

[localheinz]

@dmecke

Thank you for your contribution!

It currently checks classes only. If I understand the way nikic/php-parser works right, there is no way to check a file as it is no token. So this check needs to be implemented again for everything that could be defined, right?

Unless I am mistaken, this could be implemented for other tokens as well, take a look, for example, at

• Localheinz\PHPStan\Rules\Functions\NoNullableReturnTypeDeclarationRule
• Localheinz\PHPStan\Rules\Methods\NoNullableReturnTypeDeclarationRule

php-cs-fixer adds the missing directive in the Failure test directory. It could be discussed if a coding standard tool should change a directive as it actually changes behaviour. Another approach could be to exclude this specific file from fixing.

You could circumvent this by applying the following patch:

diff --git a/.php_cs.fixture b/.php_cs.fixture
index 492256b..e4f85cf 100644
--- a/.php_cs.fixture
+++ b/.php_cs.fixture
@@ -5,6 +5,7 @@ declare(strict_types=1);
 use Localheinz\PhpCsFixer\Config;

 $config = Config\Factory::fromRuleSet(new Config\RuleSet\Php71(''), [
+    'declare_strict_types' => false,
     'lowercase_constants' => false,
     'magic_method_casing' => false,
     'static_lambda' => false,

Now, before you jump in into making these changes, I'm not entirely sure whether these rules would be very useful. That is, while I agree that it makes sense to point out the lack of a strict types statement, this issue could be automatically fixed by enabling the declare_strict_types fixer of friendsofphp/php-cs-fixer (or probably some alternative when using squizlabs/php_codesniffer).

What do you think?

[dmecke]

I find it hard at times to cut the line between static analyzers like phpstan and coding standard checkers. And I think the declare_strict is a good example as it's not perfectly clear which tool should take care.
One could argue that it's the projects coding standard to declare everything as strict. On the other hand I'd say that - opposed to brackets in one line or another - the strict declaration actually changes the behaviour of the code and therefor is outside of the scope of a coding standard tool.

In a legacy project I work on 95% of the code does not have the declaration (as it was coded before php 7). While we define it in every new file, we cannot change it in the old code though as it would break things. So in this project this is definitively "code".

That being said, I'd totally understand if you'd like to not have this rule in here, so don't worry to deny it. :-)

[ondrejmirtes]

I think that this is a much better job for: https://github.com/slevomat/coding-standard#slevomatcodingstandardtypehintsdeclarestricttypes-

This rule as it's currently implemented has some downsides:

* Will not report missing strict_types for files without classes
* Will not report it for interfaces and traits

• Will not report it for functions
• If a file has multiple classes in it, it will be reported multiple times.

Right now, there would have to be some hardcoded virtual node for this in PHPStan (something like FileBeginningNode that would carry this information and that you could hook this rule onto) so that it could work the same way as the linked PHPCS sniff.

[localheinz]

@dmecke

I hope you don't mind that I'm closing this - for reasoning see above comments.

Regardless, thanks a lot for your contribution!

[localheinz]

@dmecke

You might like phpstan/phpstan@ed81c3a!

[localheinz]

@dmecke

Also see phpstan/phpstan-strict-rules#59.

[localheinz]

@dmecke

Let's wait for the next release of phpstan/phpstan, and then adjust this to use the newly created FileNode!

[localheinz]

@dmecke

What do you think about my amendments?

I turned this into a Files\DeclareStrictTypesRule!

[codecov]

Codecov Report

Merging #79 into master will decrease coverage by 0.29%.
The diff coverage is 78.94%.

@@             Coverage Diff             @@
##             master      #79     +/-   ##
===========================================
- Coverage     83.89%   83.59%   -0.3%     
- Complexity       82       91      +9     
===========================================
  Files            13       14      +1     
  Lines           298      317     +19     
===========================================
+ Hits            250      265     +15     
- Misses           48       52      +4

Impacted Files	Coverage Δ	Complexity Δ
src/Files/DeclareStrictTypesRule.php	78.94% <78.94%> (ø)	9 <9> (?)
src/Classes/FinalRule.php	88.88% <0%> (ø)	17% <0%> (ø)	⬇️
src/Classes/NoExtendsRule.php	84.61% <0%> (ø)	7% <0%> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edd848a...b1845c7. Read the comment docs.

[localheinz]

👍

Thank you for your patience, @dmecke!

[dmecke]

Nice, thanks for your effort!

[localheinz]

Thank you, @dmecke!

Note that I still owe you in #77!