magento-coding-standard icon indicating copy to clipboard operation
magento-coding-standard copied to clipboard

[New Rule] Add declare(strict_types=1) to your PHP files

Open lenaorobei opened this issue 5 years ago • 11 comments

Rule

All new PHP files MUST have strict type mode enabled by starting with declare(strict_types=1);. All updated PHP files SHOULD have strict type mode enabled. PHP interfaces SHOULD NOT have this declaration.

Reason

Source: Magento Technical Guidelines.

With PHP 7, it is possible to add type hinting to your code. However, this doesn't mean that types are actually enforced, unless strict typing is enabled by adding declare(strict_types=1) to the top of each PHP file.

PHP code becomes more robust when type hinting (argument types, return types) are added. With the declare(strict_types=1) added, there is less chance for bugs that related to type casting.

Implementation

Please refer to ExtDN an generic PHP CodeSniffer (not merged yet) implementations.

Important details

This sniff should be applicable to Magento >=2.2, because older versions of Magento supports PHP 5.5 and 5.6. Follow the discussion about version specific sniffs for more details.

lenaorobei avatar Feb 01 '19 14:02 lenaorobei

@lenaorobei I can implement this

larsroettig avatar Feb 01 '19 15:02 larsroettig

@larsroettig sounds good, but first we need to discuss the approach for version specific sniffs.

lenaorobei avatar Feb 01 '19 16:02 lenaorobei

When implementing, please remove a static test that cover the same thing.

buskamuza avatar Feb 13 '19 12:02 buskamuza

As per architects discussion we need to determine how strict this rule should be:

  • Applicable only to new files?
  • Or only to changed files?
  • How to check vendors extensions?

lenaorobei avatar Feb 13 '19 17:02 lenaorobei

There is a good sniff which checks that your code satisfies php7 styling code (type hints, return types, declaring strict type etc) https://github.com/slevomat/coding-standard, can we reuse it ?

sergeynezbritskiy avatar Apr 13 '19 15:04 sergeynezbritskiy

Any progress here? Is there help needed?

tmotyl avatar Jan 09 '20 00:01 tmotyl

@tmotyl This coding standard is used to check older Magento 2.x versions that are compatible with PHP < 7.0. This is the reason why this rule is not beeng added here. We support only one version of coding standard, so there is no specific branches for separate Magento releases.

lenaorobei avatar Jan 09 '20 17:01 lenaorobei

Since PHP 8 is now released, is it time to drop support for PHP < 7.0 and merge this in?

markshust avatar Jan 28 '21 20:01 markshust

It looks like this can be closed. PHP version 8 has this feature enabled and does not require the extra line of code in each file. When modules (both core & third-party) support PHP version 8 this check will not be required. There's no value having this line on PHP written for version 8, so adding a check to require this line isn't useful.

According to the system requirements page on devdocs, Magento version 2.4.4 and later will require PHP version 8.1+.

fredden avatar Sep 17 '21 08:09 fredden

It looks like this can be closed. PHP version 8 has this feature enabled and does not require the extra line of code in each file.

This is not true. You are probably thinking of this. This has nothing to do with strict types.

guvra avatar Aug 25 '22 07:08 guvra

This is not true.

Sorry, I was wrong. I've marked my comment as hidden to avoid confusion going forward. Thanks for calling this out.

fredden avatar Aug 25 '22 08:08 fredden