Manual talk:Coding conventions

About this board

For discussion about PHP coding conventions prior to August 2020, see Manual talk:Coding conventions/PHP/Archive.

Consistency of nullable types in PHP documentation and type hints?

Jdforrester (WMF) (talkcontribs)

In our codebases we have a mix of old-style (Class|null) and new-style (?Class; "new" in PHP 7.1) annotations in our Doxygen blocks and so on; how would people feel about us standardising on one of these rather than having both?

In T250734 there has been some discussion about how to handle the use of nullable types in the more complicated scenario of multiple types (i.e. Class|SecondClass|null etc.), but I don't think we've had an up-front conversation at all about use of the nullable operator yet, and certainly our code is a messy mish-mash of multiple types. (Indeed, I've even seen ?Class $foo = null used.)

Given the lack of a way to handle these for multiple types, I think we should standardise on old-style annotations everywhere to create consistency for now, and write a rule (ideally with an auto-fixer) for the codesniffer tool.


Duesentrieb (talkcontribs)

Only the new style types work for type hints. Using ?Foo in the type hint and Foo|null in the doc block seems annoying and inconsistent. I'd opt for using the new style everywhere.

By the way: ?Foo $foo = null is different from Foo $foo = null and from ?Foo $foo: The first means "it's optional, but can be null explicitly", the second means "it'S optional, but shouldn't be null if given explicitly", and the third one means "it's required to be provided, but can be null".

Tgr (WMF) (talkcontribs)

+1 for using old-style for consistency. PSR-5 (a draft, and a quite old one at that, but the closest thing to a standard the PHP world has today) has no ?; combining it with unions is illegal in PHP 8, which we'll eventually adopt, at which point it would be very confusing to advertise a mistake in our type docs; and having to switch between ? and |null as types get added / removed would be annoying.

By the way: ?Foo $foo = null is different from Foo $foo = null and from ?Foo $foo: The first means "it's optional, but can be null explicitly", the second means "it'S optional, but shouldn't be null if given explicitly", and the third one means "it's required to be provided, but can be null".

That sounds logical but isn't how PHP actually works. The first two are identical, as a null default implicitly makes the parameter type nullable. The semantics also depend on the context: f(Foo $foo = null, Bar $bar) and f(?Foo $foo, Bar $bar) are identical (though only for B/C with pre-? legacy code).

Reply to "Consistency of nullable types in PHP documentation and type hints?"

[PHP] Spaces in array indexes

DKinzler (WMF) (talkcontribs)

The convention currently states: Do not put spaces in brackets when accessing array elements. However, this is awkward if the index is not a literal, especially if it is an expression that contains spaces. For example:

  $a[$i + 1]
  $a['x' . $name]
  $a[$this->get( $something, foo)]

We seem to already deviate from the convention in such cases, I found more than 100 places in core that use spaces in array indexes. I propose to change the convention as follows: Do not put spaces in brackets when accessing array elements if the index is a literal or variable. Do put spaces after the opening bracket and before the closing bracket if the index is an expression that by convention should contain spaces anywhere. For example:

  $a[ $i + 1 ]
  $a[ 'x' . $name ]
  $a[ $this->get( $something, foo) ]

I think that is much more readable and intuitive. What do you think?

I'd personally also be fine to always add spaces in array indexes, if that is preferable for consistency.

Jdforrester (WMF) (talkcontribs)

(I've noted in the title that you're talking about Manual:Coding conventions/PHP rather than Manual:Coding conventions because the talk pages were all merged together for visibility.)

I think that we should do this, and for a little bit further and just use spaces always in square brackets; this would align our PHP convention with our JS one, which would make the rule very simple to remember.

The old excuse that this would be a lot of work is not a real problem, as we now have both auto-fixers in the linters and an auto-upgrader bot that will fix every repo for us.

Tgr (WMF) (talkcontribs)

I think the concern with changes like that was less that it's a lot of work and more that it touches a lot of lines (and so is a pain in the ass when doing git blame).

Apparently GitHub supports a default ignore-revs file these days; not sure about other tools.

Jdforrester (WMF) (talkcontribs)
Reply to "[PHP] Spaces in array indexes"

Enforce quote styles in PHP

Tacsipacsi (talkcontribs)
Arlolra (talkcontribs)
Krinkle (talkcontribs)

I don't think this should be enforced in code review, PHPCS, or otherwise in CI. We have a general guideline to follow, but I don't see it helping us to require code to change back and forth between two formats on a regular basis just due to the addition or removal of a character. Local consistently and productivity are I think more important factors.

When all else is equal, e.g. writing new code, or most strings in a method or file are equally simple, then our convention says to default to single quotes. Beyond that I don't think it's worth worrying about. In my opinion, string quoting isn't the sort of thing where cognitive overhead is induced by having variants or where it adds friction during review/auditing/debugging. You are goint to encounter both variants no matter what.

In my view, the guideline mainly exists as a tie-breaker when there's uncertaintity so that nobody has to discuss which one to use if they're not sure. But when modifying existing code, or once code has been put up for review, it's imho not worth changing or discussing further.

Reply to "Enforce quote styles in PHP"

i do not understand text about vertical alignment

Qdinar (talkcontribs)

the width allowed for the left column constantly has to be increased as more items are added

which items are added? new columns added? what is "width allowed for the left column"? i think it is tab size. then, if you make longer strings in any column, not only left column, then tab size has to be increased, or, more tabs has to be used between columns, and, if there are elements of much different size, different count of tabs need to be used between columns.

Tgr (WMF) (talkcontribs)

The point is, you shouldn't have to change spacing in ten lines just because you replaced a name on one line. It makes for more confusing diffs. So we prefer

    'foo' => 0,
    'foobar' => 1,


    'foo'    => 0,
    'foobar' => 1,

(except for Puppet, apparently).

Reply to "i do not understand text about vertical alignment"
Nikerabbit (talkcontribs)

The example safe command no longer works as of version 2.0 which removed --disable and --enable command line flags:

In addition the other config file examples do not show hot run it for a single file.

Jdforrester (WMF) (talkcontribs)
Jdforrester (WMF) (talkcontribs)

Oh, fun. I'll update the docs to say to use SVGO v1.x for now.

Reply to "svgo example broken"
אלישיב ליפא (talkcontribs)

In the section "vertical alignment" there some mistake - I think some template was misused - "Template:English\note" is in the end of the first line, instead of a note saying that some diff engines can ignore white space changes.

Krinkle (talkcontribs)

I've proposed at T253461 that we phase out our AtEase library in favour of PHP's regular error suppression methods.

Based on the feedback gathered on the task so far, I think it's preferred that we continue to generally discourage use of error suppression (regardless of whether through @, AtEase, or error_reporting).

As such, I proposed that we keep the conventions mostly as-is, including that our PHPCS rule warns against use of the @-operator. What would change is that we'd use that same PHPCS rule as our way of opt-ing to it as-needed.


use Wikimedia\AtEase\AtEase;

$content = file_get_contents( $path );


// phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged
$content = @file_get_contents( $path );
Krinkle (talkcontribs)
RobinHood70 (talkcontribs)

The latest formatting recommendation from the PSR-12 is to always use 4 spaces to indent. While I'm a fan of tabs myself, we should consider whether we want to change the recommendation on the PHP subpage to match the PSR-12.

Krinkle (talkcontribs)

No. There is no meaningful interoperable benefit from which way to use spacing. Since the early 21st century, humanity has decided to indefinitely leave this question unanswered by forever more supporting both in the abstract. This is solved through EditorConfig and PHPCS. I don't expect contributors or text editor software to benefit from this switch, and more generally we don't use the rest of PSR coding style either. I'm boldly closing this as this would very likely otherwise become a distraction and waste of time, so I'm raising the bar a little by requiring the next person to re-open this if they feel compelled to convince others to spend time on this.

Dedicated pages per programming language

Krinkle (talkcontribs)

I've generalized this page to be a code conventions portal. Branching off specific languages to other dedicated pages. The "do as PHP unless stated otherwise" is getting old because JavaScript, for instance, is simply very different than PHP. And although the end result in syntax may be similar at times, the reasoning behind is very different. Therefor it's better to describe it in the right context with examples that make sense.

See also the Restructure on Project:Current issues.

קיפודנחש (talkcontribs)

i think it is sorely missing. User:Anomie ? or does it exist somewhere else?


Anomie (talkcontribs)

There isn't one, as far as I know. The convention generally follows mw:CC, and some bits extrapolated from mw:CC/PHP and mw:CC/JS (e.g. lowerCamelCase function names).

I wouldn't be opposed to a Manual:Coding conventions/Lua, if there's stuff that needs to go in it.

Return to "Coding conventions" page.