Codestyle verification

[wiese]

Added some checks to automatically verify our existing code style, fixed a few resulting violations. I used 3rd [1] party sniffs as squiz is not fully supporting php 7.1, yet, and some native sniffs replicating the wm-cs behaviour (which do not support modern CodeSniffer versions).

There is one effective change of the application (ValidateConfigCommand.php) where I opted against changing to a wrong mixed return type.

I would also suggest we add the following - some work (many violations), but they are not mere cosmetic but would actually make our code significantly better and more performant [2].

• SlevomatCodingStandard.Classes.ClassConstantVisibility
• SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue
• SlevomatCodingStandard.TypeHints.TypeHintDeclaration (partially)

To avoid misunderstandings: I do not propose to change the code style, but to enforce the rules that are already there but not clearly documented/technically checked for, but frowned upon when violated.

[1] https://github.com/slevomat/coding-standard/
[2] http://stackoverflow.com/questions/32940170/are-scalar-and-strict-types-in-php7-a-performance-enhancing-feature

[gbirke]

I like the improvements and the suggestion for null types.

not sure if making the visibility of constants explicit would add unnecessary clutter. OTOH it's consistent with methods and properties, so why not.

Do you have supporting evidence for the performance improvements?

[wiese]

As for the performance one would have to realistically say, that it opens the door for future gains - compile time optimization that only becomes possible when the hinting (incl nullable) information is provided. What it would definitely do now is improve our code for editors and static analysis tools.

[JeroenDeDauw]

not sure if making the visibility of constants explicit would add unnecessary clutter. OTOH it's consistent with methods and properties, so why not.

Whaaaaaat. It is indeed the same as for fields and methods. So why would you not do it? Else how do you know if it's meant to be accessed from outside or not? And for the cases where it is not, it becomes a lot easier to spot dead constants (which we did in several places when switching to 7.1.)

[JeroenDeDauw]

• SlevomatCodingStandard.Classes.ClassConstantVisibility
• SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue
• SlevomatCodingStandard.TypeHints.TypeHintDeclaration (partially)

As far as I can tell these have not been added yet. Doing that sounds like a good idea to me. Want to create a PR or shall we create a ticket?

[wiese]

@JeroenDeDauw I will create a ticket for the use of the 3 sniffs - was a whole bunch of changes needed. -> https://phabricator.wikimedia.org/T166118