`goto` - should we use it or not? If yes - when?


#1

Hello all!

When I’m working on ZendCodingStandard I was thinking about adding sniff which discourage goto language construct.
Recently someone suggested to create this sniff in PHP_CodeSniffer:
https://github.com/squizlabs/PHP_CodeSniffer/issues/1662
and there is already the PR with that sniff:
https://github.com/squizlabs/PHP_CodeSniffer/pull/1664

I’ve checked where in zendframework org we are using goto instruction and I have found it only in zend-code (really old part of the library - scanners - and as I remember correctly, it is going to be removed with v4), and also in zend-http:


(it could be easily eliminated there).

(I can see it also in ZF1 and some old libraries like ZendMarkup…)

I cannot see any valid example where goto is better than refactor and split code into smaller functions which do simpler actions. IMHO we should then discourage of using goto.

@mwop is against adding that rule, please see comments:
https://github.com/zendframework/zend-coding-standard/pull/1#issuecomment-330650893
https://github.com/zendframework/zend-coding-standard/pull/1#discussion_r139795720

@mwop can you please provide some examples, where goto is going to be better than code refactor and smaller functions?

What do you think, guys? Should we disallow using goto or not?


#2

@webimpress If we were to enable such a rule by default, is it possible to disable it per-project via phpcs.xml? If so, I retract any objections, so long as we can document how to do that (as well as alter settings for any other rules).


#3

@mwop - yes we can’t disable it per-project in phpcs.xml. Still with the zend-coding-standard library we will be able to have some customization (if needed) in phpcs.xml. It is possible to disable each sniff for all files or just for specified files.

For example in zend-code I would set for now:

    <rule ref="ZendCodingStandard.NamingConventions.ValidVariableName.NotCamelCaps">
        <exclude-pattern>src/Scanner/AnnotationScanner.php</exclude-pattern>
        <exclude-pattern>src/Scanner/ClassScanner.php</exclude-pattern>
        <exclude-pattern>src/Scanner/DocBlockScanner.php</exclude-pattern>
        <exclude-pattern>src/Scanner/MethodScanner.php</exclude-pattern>
        <exclude-pattern>src/Scanner/TokenArrayScanner.php</exclude-pattern>
    </rule>

#4

Okay, perfect. I can live with that solution: make it difficult, but not impossible, to use such features. By making a change in the phpcs.xml, they also signal that this is a potentially contentious feature, which will draw extra scrutiny during code review.


#5

Maybe I wasn’t clear enough - in above files in zend-code we have variables like: $MACRO_TOKEN_ADVANCE.

Example of customization phpcs.xml was just to show how we can disable some sniff for specific files in the project. We can also disable sniff for all files:

<rule ref="ZendCodingStandard.RulesNamespace.SniffName">
    <severity>0</severity>
</rule>

More can be found in PHP_CodeSniffer documentation: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-ruleset.xml#the-annotated-sample-file