Symplify: [EasyCodingStandard] Rule to verify type declaration location

[azdanov]

Is there a way to verify that declare(strict_types=1); is declared right after the opening tag?

For example, if I have this snippet of code, then all the checks pass, but the app will crash.

<?php

use App\Providers\AppServiceProvider;

declare(strict_types=1);

Of course, there is strict_types declaration must be the very first statement in the script exception, but it would be cool to enforce this with a linter.

This is the rule that enables strict checking:

https://github.com/Symplify/EasyCodingStandard/blob/a9fd83574988c8a442dc05b9baa4b06edcf8577c/config/php70.yml#L15

[TomasVotruba]

Hi, there is one useful command that will help you in situations like these:

Try the one I highlithed :)

[azdanov]

Thanks for the quick reply.

This command does work, but only when declare(strict_types=1); is missing. When declare(strict_types=1); is declared as in the snippet in the first post, then I get [OK] No errors found. Great job - your code is shiny in style! but the code is invalid.

I wonder if there is a way to check declare(strict_types=1); for validity.

An example from http://www.php.net/manual/en/control-structures.declare.php:

As directives are handled as the file is being compiled, only literals may be given as directive values. Variables and constants cannot be used.

<?php
// This is valid:
declare(ticks=1);
const TICK_VALUE = 1;

<?php
// This is invalid:
const TICK_VALUE = 1;
declare(ticks=TICK_VALUE);

Here's my config for reference:

easy-coding-standard.yml

imports:
  - { resource: "vendor/symplify/easy-coding-standard/config/clean-code.yml" }
  - { resource: "vendor/symplify/easy-coding-standard/config/common.yml" }
  - { resource: "vendor/symplify/easy-coding-standard/config/psr12.yml" }
  - { resource: "vendor/symplify/easy-coding-standard/config/php71.yml" }
  - { resource: "vendor/symplify/easy-coding-standard/config/symfony.yml" }
  - { resource: "vendor/symplify/easy-coding-standard/config/symfony-risky.yml" }
  - { resource: "vendor/symplify/easy-coding-standard/config/symplify.yml" }
services:
    PhpCsFixer\Fixer\Operator\ConcatSpaceFixer:
        spacing: 'one'
parameters:
    exclude_checkers:
        - 'PhpCsFixer\Fixer\Operator\NotOperatorWithSuccessorSpaceFixer'
        - 'SlevomatCodingStandard\Sniffs\Exceptions\ReferenceThrowableOnlySniff'
        - 'SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff'

[TomasVotruba]

I see. How did you managed to add declare(strict_types=1) to position that makes code invalid?

Have you tried all the other checker that were in that result? There is small change, that one could handle it. If not, there is issue about position of declare(strict_types=1) in PHP CS Fixer: https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/2062

Please continue there.

P.S: Cool trick with "summary", didn't now that :+1:

[azdanov]

PHPStorm auto-fix feature. It's not too serious, more of an edge case. And php will throw an exception, so it's easy to spot.

I guess there are currently no rules for this case. Should probably close this issue?

PS: Yeah, it's awesome for longer configuration files and console logs.

[TomasVotruba]

In that case, switch from PHPStorm to coding standards :) they're better tested by community and CI ready (it also handles the cases in your screenshot).

Yes, please close and continue in here: https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/2062
Mention it's needed because PHPStorm create wrong issue, it might help somebody to grab it and create it.

[TomasVotruba]

Btw, can I ask you for a feedback on ECS? How is using it for you?
What do you find most useful and what is there to improve?

[azdanov]

I've found the culprit, it was https://github.com/kalessil/phpinspectionsea, with Use ::class instead rule. I've created an issue there.

As for ECS, I love it. 😄

Before I've used a combination of phpcs, phpcbf, php-cs-fixer and phpstorm. Now I use ECS, and sometimes PHPStorm since it offers more formatting options.

It would be cool if there was a plugin for PHPStorm, or VSCode to show linting errors in real-time, without having to run the check command periodically. Similarly the fix command, I'm used to eslint which offers fixing on demand, or even auto fixing on file save.

I like how easy it is to configure, although I've had to experiment a little to find which rules to use. For example, in Laravel, an auto-fix rule from php70 set was breaking the Exception.php method. So I had manually to disable some rules.

I wonder if a specific set for Laravel could be useful since the framework quite popular. Or some way to ensure that certain rules won't break the functionality when used together. For example, if I include Laravel set with other sets, then Laravel will have priority.

I've already created a nice workflow via external tools in PHPStorm, where on a keypress a file or a directory is passed to ECS to get fixed. Could be a cool to mention in the readme file how to integrate ECS with other tools.

There are slowdowns, but I guess it depends on number of checks enabled and on how many files a processed.

I've actually considered using ECS some time ago, but Neon format wasn't that appealing to me, so I started using other tools. Now with Yaml it's cool. Although I've been missing on a lot of things.

Whew. 😅

[TomasVotruba]

Wow, thanks for such a long answer :+1: , I don't have time now, but I'll love to read it whole later.

As for IDE plugins, I tried them but they work kinda 50:50. See this discussion for more:
https://www.tomasvotruba.cz/blog/2018/06/04/how-to-migrate-from-php-code-sniffer-to-easy-coding-standard/#disqus_thread

[azdanov]

I see. This makes sense.

And I've achieved more or less the same workflow as before. Now that I think about this more, it is much nicer to see no errors during development, and only sometimes run the fix command.

I wonder if there are ways to use ECS with Git Hooks, to lint on either a commit or a push. Could be useful to mention in the readme.

Also some instructions on how to integrate with some popular CI, such as Travis or CircleCI. It's not hard, but seeing an example always makes it easier.

[TomasVotruba]

Thanks for much fur such extensive reply, I'm very happy to read and I'm amazed how much in detail you're thinking about codings standards. It's one of my passion, you know ;)

I've had to experiment a little to find which rules to use.

I do such experiment with every new release of PHP CS Fixer or PHP_CodeSniffer.
This is my best selection so far: https://github.com/Symplify/Symplify/blob/a221f71bddc26ab7c8cc74e45aec733894748401/ecs.yml#L1-L25 I love the CognitiveComplexitySniff the most, really gold helper.

So I had manually to disable some rules.

I do that too, that's a case when 3rd party code isn't typehinted. E.g. Sniffs have @param int typehint, that is not in the code. Luckily, it's easy to exclude such files:

parameters:
    skip:
        SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff:
            - '*packages/CodingStandard/src/Sniffs/*/*Sniff.php'

As for Laravel, there were some issues at PHP CS Fixer, yet I think PSR 2 is just fine and the rest is optional.

I've already created a nice workflow via external tools in PHPStorm, where on a keypress a file or a directory is passed to ECS to get fixed. Could be a cool to mention in the readme file how to integrate ECS with other tools.

Cool! Yea sure, send PR and I'd be happy to merge it.

...but Neon format wasn't that appealing to me.

I feel you, I got more feedback like these so after some experiment I switched ECS to more common and known format.

Although I've been missing on a lot of things.

What do you mean exactly?

[TomasVotruba]

I wonder if there are ways to use ECS with Git Hooks, to lint on either a commit or a push. Could be useful to mention in the readme.

I don't use it that way. Just in CI.

Also some instructions on how to integrate with some popular CI, such as Travis or CircleCI. It's not hard, but seeing an example always makes it easier.

Have you managed to integrate it? If so, please send PR, it's always better to have these setup first hand by somebody who just did that.

If not, I'll prepare it :)

[azdanov]

I will certainly borrow some rules from your config, quite interesting.

As for Laravel, I see now that using a personal style is encouraged.

Although I've been missing on a lot of things.

What I meant to say is that I should've started using ECS earlier. It's really good. I've spent too much time assembling the tools than learning PHP and Laravel.

I will see about those PRs. Probably will create integration with VSCode and PHPStorm first. And CI later. It could be a good experience for me to create a plugin for VSCode, that replaces the inbuilt formatter with ECS.

What I'd like to see is a similar eco-system to javascript. Prettier for style formatting, and ESLint for linting. With these two it's so easy to have a uniform code with specialized checks either for react, vue, lodash or even plain node.

What I find useful in ESLint are comments that disable rules. This way I can disable only a certain rule in a file, or even on a line.

Prettier is really good at enforcing a style. The closest thing in PHP is php-cs-fixer and it's still not as precise.

Also, there are google-java-format and black which serve as an inspiration to me.

My passion, I guess, is being lazy. Especially when automated tools do my job. 😄

[TomasVotruba]

I've noticed I missed this comment, so I'm replying now.

My passion, I guess, is being lazy. Especially when automated tools do my job. 😄

Well said :D

Personally I try to move from linting and styling to architecture sniffs/fixers. Like Final or Abstract class, solid sniffs etc.

Something all these clean code posts are all about, just not on programmer's shoulders but enforced by coding standard.

I see there are to much information to learn lately. People don't have time and thus don't learn anything. But that's not their mistake, I think its the tooling mistake. Instead of focusing on checking spaces, there should be tools that check best practices for us. So we don't have to teach the world to know them, just provide a package to do the job for everyone.

[azdanov]

Wow. Now that's a vision I'd like to support.

It seems that the JavaScript community is slowly but surely going in that direction (ESLint, Prettier). It would be really nice to see something similar in PHP.

Right now there's a bunch of competing tools that try to solve the same problems over and over again.

I really like what kalessil/phpinspectionsea is trying to achieve. Although it depends too much PHPStorm which can't be used in CI/CD..

[TomasVotruba]

Yes, that was my main idea behind ECS. I tried to help both maintainers of PHP_CodeSniffer and PHP CS Fixer to get closer many years, but they have very strong opinions about their own position.

I mean, I get it, I myself have sometimes problems to let go, just because there is somebody else doing the same. I want to compete, be better or just take care of myself. That's natural for people. I just want to change this nature to cooperation by default.

And that's hard :) but I see it works. Thanks to ECS I see many people don't care about which tools is it, if it does the job right and they're able to let go of one or the other side.

Finding tool that is super easy to use, fits CI and people agree on that is also another task.