PHP has several modifiers that can be applied to method definitions:
public,protected,private
static
abstract, final

We have not defined the order of those modifiers in the coding standards. In Drupal core we have some typical patterns:
"public" before "static"
"abstract" before "protected"
"public" and "final" mixed

PSR-2 recommends:

Visibility MUST be declared on all properties and methods; abstract and final MUST be declared before the visibility; static MUST be declared after the visibility.

That roughly matches what we do in core anyway, so we can just conform to PSR-2 in this case.
Example:

(abstract or final or nothing) (public or protected or private) (static or nothing) function example() {}

Proposed change to https://www.drupal.org/docs/develop/coding-standards/object-oriented-cod... :
Old text:

All methods and properties of classes must specify their visibility: public, protected, or private. The PHP 4-style "var" declaration must not be used.

New text:

All methods and properties of classes must specify their visibility: public, protected, or private. abstract and final, if present, must be declared before the visibility; static, if present, must be declared after the visibility. The PHP 4-style "var" declaration must not be used.

Comments

klausi created an issue. See original summary.

klausi’s picture

I think this issue is not really controversial and reflects our current practices anyway, so I went ahead and implemented a rule for Coder to enforce the proposal http://cgit.drupalcode.org/coder/commit/?id=317447a
This will be released with Coder 8.2.11 soon.

drunken monkey’s picture

Issue summary: View changes

+1 for this.
I'd just change the new text a bit to clearly reflect the non-visibility modifiers are optional:

All methods and properties of classes must specify their visibility: public, protected, or private. abstract and final, if present, must be declared before the visibility; static, if present, must be declared after the visibility. The PHP 4-style "var" declaration must not be used.

Also, not sure what our current stance on this is, but don't we want to capitalize the RFC 2119 verbs, at least when making changes to text?

klausi’s picture

I don't like all upper case MUST words, it feels like a document is screaming at me. RFC 2119 is nice to clarify that "must" really means "must", but I would not apply the upper case convention to our more casual coding standards document.

But I don't really care, feel free to change that if it is important to you.

Anonymous’s picture

+1 for this.

Committing to conforming to PSR-2 sounds like a logical progression.

DuaelFr’s picture

+1 for standards :)

Wim Leers’s picture

+1

klausi’s picture

Status: Active » Reviewed & tested by the community
Issue tags: +needs announcement for final discussion

Thanks, with that agreement we can proceed.

bojanz’s picture

Late +1 :)

dawehner’s picture

+1

tobiasb’s picture

+1

borisson_’s picture

Another +1 here.

tizzo’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: -needs announcement for final discussion +final discussion

This issue is being included in today's coding standards announcement, updating status and issue tags to follow the defined coding standards ratification process.

andypost’s picture

-1 to "must add public" that's too common to skip it as default in language

borisson_’s picture

-1 to "must add public" that's too common to skip it as default in language

This was already enforced in the coding standards, see also the "old text" in the IS. This is just about adding a consistent order to the other keywords.

NormySan’s picture

+1, and awesome that we can conform to the PSR-standard in this case!

rfulcher’s picture

+1

xjm’s picture

This seems like a good standard since it puts the most important information first. It's also minimally disruptive to adopt:

[ibnsina:drupal | Wed 14:52:10] $ grep -r "static protected" * | grep -v "vendor" | wc -l
      11
[ibnsina:drupal | Wed 14:52:20] $ grep -r "static public" * | grep -v "vendor" | wc -l
       3
[ibnsina:drupal | Wed 14:52:33] $ grep -r "static private" * | grep -v "vendor" | wc -l
       0
[ibnsina:drupal | Wed 14:52:43] $ grep -r "public abstract" * | grep -v "vendor" | wc -l
       0
[ibnsina:drupal | Wed 14:53:19] $ grep -r "protected abstract" * | grep -v "vendor" | wc -l
       0
[ibnsina:drupal | Wed 14:53:27] $ grep -r "public final" * | grep -v "vendor" | wc -l
       0
[ibnsina:drupal | Wed 14:55:00] $ grep -r "protected final" * | grep -v "vendor" | wc -l
       0
claudiu.cristea’s picture

Status: Needs review » Reviewed & tested by the community

Let's do it.