Saeven / zf3-circlical-user

Turnkey Authentication, Identity, and RBAC for Laminas and Zend Framework 3. Supports Doctrine and Middleware.
Mozilla Public License 2.0
36 stars 15 forks source link

Laminas coding style #82

Closed CreativeNative closed 2 years ago

CreativeNative commented 2 years ago

fixes #81

  1. adds phpcodesniffer with laminas codings style
  2. adds phpstan for testing
  3. updates composer to prefer stable versions
Saeven commented 2 years ago

I'd seen the Laminas Standard; and had chosen not to adopt it because it grinds my gears a bit. There are some things in the Laminas standard that I find a bit too opinionated. Such as aligning equals signs, which drives me up the wall. I used to do it this way with my team, but the time wasted aligning things is completely pedantic & catastrophic. There's always the next guy that rolls around with a new const that forces all the equals signs seven spaces to the right. Absolute eyeball scratcher to read that mess in Bitbucket (main CI/CD at work) all day long.

Also, adding spaces after "not" operators. I just can't get used to it.

Then, the 120 char limit. Let people code to the full extend of their screen real estate imo! Most guys on my team are packing 4k 24+ inch monitors. Capping it to 80-120 chars made sense to me in the Terminal days (when I used to actually code in a Terminal in the late 90s). It's just column-snobbery at this point ;)

Lastly, the camel_case issue on Doctrine entities - we have some automation that lines up app logs automatically based on name, so that's another I wouldn't be able to move on.

Now, on the positive side, there are some things that I do plan to align more with the Laminas standard, such as fixing legacy @params and applying strict_types=1 everywhere.

I'm happy to apply some "tightening" to PSR1/2 to bring it closer where good reason stands 👍🏼. However, would remain unwavering on the items mentioned above, so we'd need what I'd consider some basic QoL exceptions on the Laminas whitespace dictatorship ;)

sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

CreativeNative commented 2 years ago

For aligning code you have to use "composer cs-fix" and I would recommend to insert in your routine before committing a "composer cs-check". Like this you see all errors and can fix it automatically with cs-fix. It make the code more readable and above all it prevents a lot of errors. To get you an idea have a look on this file:

src/CirclicalUser/Mapper/UserMapper.php

use CirclicalUser\Provider\UserInterface;
use CirclicalUser\Provider\UserInterface as User; <- that's not used at all

or here src/CirclicalUser/Listener/AccessListener.php

Type Application\Controller\IndexController is not used in this file.
Type Application\Controller\LoginController is not used in this file.

We can implement the laminas coding standard and overwrite or exclude everything what we not like.

For example like so:

<rule ref="PSR2">
    <!-- or to disable the whole sniff -->
    <exclude name="Generic.Files.LineLength"/>
</rule>

<rule ref="Generic.Files.LineLength">
    <properties>
        <property name="lineLimit" value="220"/>
        <property name="absoluteLineLimit" value="220"/>
    </properties>
</rule>

I added this to the branch. Please do me a favor and try it. Checkout the branch a hit "composer cs-check". After you read some errors try "composer cs-fix". A TOTAL OF 770 ERRORS WERE FIXED IN 86 FILES!

In the beginning there is a lot to fix that can't be fixed automatically. Also some stuff is broken because of the "strict types". For example:

97 ! returns true when someone is there (235ms) exception [err:TypeError("hash_hmac() expects parameter 3 to be string, object given")] has been thrown.

You see it when you run the tests. But if everything is green, it is very satisfying. And your team will thank you, because only this check will prevent A LOT of errors. Trust me.

Saeven commented 2 years ago

Only declined in edict. Relaxed a bit and solved with: https://github.com/Saeven/zf3-circlical-user/pull/88