Wikibase/Coding conventions

This page describes the coding conventions used within the wider Wikibase codebase. This includes code formatting policies, high level architecture recommendations and everything in between.

A good portion of the Wikibase code is part of MediaWiki extensions, and almost all of the code has been written to support these MediaWiki extensions. As a result the guidelines used are based on the MediaWiki coding conventions, in particular the ones that apply to PHP and JavaScript. This document outlines the additions and modifications compared to these guidelines. In other words, you should generally follow the MediaWiki coding conventions unless they are contradicted by this document.

How to use this document

It is also important to consider that the more complicated the subject of the guideline, the more you are recommended to use critical thinking and not blindly apply said guideline. To be concrete, simple things such as code formatting should be viewed as policies and strictly adhered to in almost all cases. Subjects such as component design have a set of applicable recommendations, and rules that should generally be followed. Those rules will of course not always apply, and almost always still require one to make informed trade-offs. Good design cannot be achieved by mindlessly applying rules.

The coding conventions described in this document are also general. They apply mostly to most of the packages in the Wikibase codebase and closely associated projects. Some packages do not follow a specific guideline, which can be by design, or due to legacy reasons. One should carefully consider diverging from policies adhered to in these packages even if they conflict with the ones outlined here. If the package specific documentation does not provide more information on these divergences, it is recommended you contact the package maintainers for more information.

Tests

In general we apply the policy that all new behavior should be tested. There are exceptions for trivial code, such as getters and setters, in particular when the code is not expected to be widely dependent upon. Tests will make it easier for yourself to ensure your code is correct (testing through the user interface takes a lot more effort and is less effective), and will get your code through review quicker.

Use test doubles where appropriate. Try to avoid fakes and mocks if simple stubs or spies will suffice.

Most code should have real unit tests (that just test the unit of code) rather then component, integration or system tests. When writing a test that falls in one of the later categories, keep it separate, so people can run all unit tests without needing to wait for some database or network.

Follow Arrange Act Assert. Test only one logical thing with a single test method. Indicate what is being tested in the method name. testConstructor does not explain itself well, while testGivenInvalId_constructorThrowsException does.

General style guidelines

Comments

Prefer clean code over bad code with comments

"When you feel the need to write a comment, first try to refactor the code so that any comment becomes superfluous." -- Martin Fowler, Refactoring: Improving the Design of Existing Code, 1999

Generally try to avoid comments. Only add those that are really needed, and cannot be made superfluous by making the code itself more clear.

Avoid redundant comments

The comments here do not add any value. They therefore should be omitted.

/**
 * Sets the user name.
 *
 * @param string $userName The user name to set
 */
public function setUserName( $userName )

This is much better:

/**
 * @param string $userName
 */
public function setUserName( $userName )

Comments themselves should be clean

When running into a situation where a comment is needed, take care to write a short and precise comment that has correct grammar and punctuation.

Keep in mind the cost of comments

Comments are prone to being inaccurate, and tend to become more inaccurate over time, as code gets updated, while the comments do not. Unless great care is taken, comments will end up spreading disinformation, often subtly. At point they hurt more then helping. In this regard every comment is a liability.

General design guidelines

Class design

The SOLID principles should be kept in mind and generally adhered to.

Package design

The package principles should be kept in mind and generally adhered to.

PHP

We are using PHP CodeSniffer to automatically enforce certain style decisions. Our rule set is mostly based on the MediaWiki CodeSniffer rule set, with a few exceptions and additions. The differences are documented at https://github.com/wmde/WikibaseCodeSniffer/blob/master/Wikibase/ruleset.xml.

Basic Coding Standard

You must follow PSR-1. This includes following an autoloading standard, such as PSR-0 or PSR-4.

Namespace names must use SturdyCaps.

File level formatting

<?php

namespace Your\Package;

use InvalidArgumentException;
use Wikibase\DataModel\Entity\EntityId;

/**
 * Description of this class.
 *
 * @license GPL-2.0+
 * @author Nyan Cat < nyancat@example.com >
 */
class AwesomenessGenerator {}

Documentation header

The MediaWiki coding guidelines on this topic state one should use a verbose GPL header. Our policies state the opposite and strongly discourage inclusion of such verbose headers.

The general idea behind our policy is that only documentation relevant to developers should be included. Everything else is clutter that gets in the way. When someone opens a file, you do not want them to have to always scroll down first before they can actually see the relevant code. The code is more important then the address of the Free Software Foundation.

This means the following things should not be included:

• The verbose GPL header
• @file tags, either with or without file name
• Any @ingroup or @package tags – organization is already done by the directory structure and namespaces
• Any dates or other crust your default IDE template might contain

Things that need to be included:

• @license tag
• @author tag (there can be multiple)

Other things that are often included:

• @group tag in tests
• Any documentation the class or interface requires

Namespace, imports and newlines

Formatting should furthermore follow the above example. This means having the following sections, each separated with a blank line:

• The PHP opening tag
• The namespace your class resides in
• Any use statements to import resources from other namespaces
• The class documentation header directly followed by the class declaration

The namespace declaration and the use statements all use fully qualified names that do not start with a backslash.

@since tags

@since tags indicate in which version the annotated artefact has been introduced. This is important for people building on top of a component and want to provide compatibility with more then the latest development version.

@since tags should only be used in code bases that follow semantic versioning. This typically excludes extensions like Wikibase itself.

As with all manually maintained information in comments, these tags come with a cost. They are clutter to a lot of people that have no use for them, and they have the potential to be wrong. It is therefore discouraged to add these tags to all classes and methods. Adding @since tags for protected or private methods serves little purpose. The same goes for adding these tags to classes or interfaces that are private to a package. Public classes and interfaces, as well as the public methods defined in them, likely deserve a @since tag.

Referring to resources in other namespaces

Code inside classes should generally not contain any fully qualified names, and rather import the needed classes or interfaces.

Wrong:

class NyanCat {

    /**
     * @param \Geo\Location\LatLongPoint $destination
     * @throws \Awesome\ItsOverNineThousandException
     */
    public function flyAccrossTheSky( \Geo\Location\LatLongPoint $destination ) {
        // …
    }

}

Right:

use Geo\Location\LatLongPoint;
use Awesome\ItsOverNineThousandException;

class NyanCat {

    /**
     * @param LatLongPoint $destination
     * @throws ItsOverNineThousandException
     */
    public function flyAccrossTheSky( LatLongPoint $destination ) {
        // …
    }

}

Usage of relative references is also discouraged.

PHPUnit

• Data providers should be named in the format provideFooBar. The format fooBarProvider (as seen in the PHPUnit documentation) is also acceptable.
• Test methods and providers should not be made static unless there is specific reason to do so.

The PHPUnit mock API can be used. For test doubles that are needed in multiple tests, creating a dedicated class is often more clear.

JavaScript style policies

In general, the MediaWiki JavaScript coding conventions shall apply to the Wikibase extension as well. A few differences and additional specifications are outlined below.

Function and variable prefixes

Since JavaScript does not support encapsulation, we are using an underscore to mark functions and variables private discouraging the use out of their scope:

{
	/**
	 * @private
	 */
	_variableName: 'This variable is marked private.'
}

jQuery wrapped elements should be prefixed with a dollar sign to be able to distinguish those easily from other variables. In addition, this allows naming a jQuery node like the instance of a widget that is initialized on that node:

var $claimview = $( ':wikibase-claimview' ),
	claimview = $claimview.first().data( 'claimview' );

Whitespace

We follow the MediaWiki coding conventions:

a.foo = bar + baz;

if ( foo ) {
	foo.bar = doBar();
}

function foo() {
	return bar;
}

foo = function () {
	return 'bar';
};

foo = typeof bar;

Except for comments, spaces should, in general, not be used for the purpose of alignment within the code:

no	yes
longVariableName = 'foo'; shortName = 'bar';	longVariableName = 'foo'; shortName = 'bar';

Indentation

When chaining more than one function call, each call should be placed in a new line without applying any indentation to prevent ending up with deep indentation levels:

$node
.addClass( 'some-class' )
.append( $( '<div/>' ).addClass( 'some-other-class' ) );

This convention does not apply when assigning a variable. However, one should not declare other variables along with such a variable declaration. Just separate the variable declarations:

var $( '<div/>' )
	.addClass( 'some-class' )
	.append( $( '<div/>' ).addClass( 'some-other-class' ) );

var someOtherVariable = 0;

Line breaks

Like to the MediaWiki coding conventions, lines should not exceed the length of 100 characters (tab equals 4 spaces). When a line break within a statement cannot be avoided, the operator should be wrapped to the next line indented by an extra level to indicate the continuation of the statement. Closing parentheses should always be on the same level like their corresponding opening ones.

// No:
var someString = 'FooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFoo' +
	'FooFoo';

if ( mw.foo.hasBar() && mw.foo.getThis() === 'that' &&
	!mw.foo.getThatFrom( 'this' ) ) {
	return { first: 'Who', second: someString,
		third: 'I don\'t know'
	};
} else {
	return mw.foo( 'first', someString,
		'third' );
}

// Yes:
var someString = 'FooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFoo'
	+ 'Foo';

// (The if-statement should actually be in one single line, this is just to demonstrate breaking
// lines. Comments should not exceed a line length of 100 characters as well.)
if (
	mw.foo.hasBar()
	&& mw.foo.getThis() === 'that'
	&& !mw.foo.getThatFrom( 'this' )
) {
	return {
		first: 'Who',
		second: someString,
		third: 'I don\'t know'
	};
} else {
	return mw.foo(
		'first',
		someString,
		'third'
	);
}

jQuery DOM element creation

Although quick-closing is not necessary when creating elements, $( '<div/>' ) is preferred over $( '<div>' ) for consistency throughout the codebase.

Keeping context in callbacks: jQuery.proxy(); vs. var self = this;

For consistency throughout the codebase, usage of

var self = this;
foo( function someCallback() {
	self.bar();
} );

is preferred over

$.proxy( foo( function someCallback() {
	this.bar();
} ), this );

jQuery.Promise

When dealing with promises, the done, fail and progress methods should be used rather than the less expressive then. Callback parameters given when the promise gets resolved should be documented explicitly.

Accessing jQuery widget instances

For accessing jQuery widget instances, the .data() function should be used resulting in a more expressive code:

// No:
var site = $siteSelector.siteselector( 'getSelectedSite' );

// Yes:
var site = $siteselector.data( 'siteselector' ).getSelectedSite();

Documentation

Just like in MediaWiki, we use JSDuck for generating documentation. Consequently, only tags listed in the JSDuck syntax documentation should be applied. The only additional tag is "@license". Documentation should never be on file level, but on constructor/class level as well as on function and member level. Scoped global functions and variables should be marked with "@ignore" to not have them end up as class members in the documentation.

Types

Do not use scope arguments in the documentation, but be explicit in type and function parameter documentation since the generated documentation does not resolve scope arguments. Instead of …

/**
 * @property {wb.datamodel.Statement}
 */

… apply:

/**
 * @property {wikibase.datamodel.Statement}
 */

Optional arguments

In function documentations, optional function arguments should be documented with square brackets around the variable name making their optional nature more obvious.

Instead of …

/**
 * @param {string|undefined} variableName
 */

… use:

/**
 * @param {string} [variableName]
 */

Mixed arguments

Can be documented as

/**
 * @param {*} variableName
 */

jQuery Widget documentation

Since there should be not more than one widget per file, the file level documentation should be used to document a widget. In addition to the default documentation tags @license and @author, the documentation header should contain a description of the widget and documentation of the widget's options as well as the events triggered by the widget using the @option and @event tag respectively.

/**
 * […]
 * @option {wikibase.Claim[]|null} value
 *         The list of claims to be displayed this view. If null, the
 *         view will initialize an empty claimview with edit mode started.
 *         Default: null
 *
 * @event afterstopediting
 *        Triggered when one of the claimlistview's items has successfully
 *        left edit mode.
 *        - {jQuery.Event}
 *        - {boolean} Whether pending value has been dropped or saved.
 * […]
 */

(jQuery) promises

In order to avoid having to analyse a function body of a function returning a promise object, all values forwarded by promises should be documented, for example:

/**
 * @return {Object} jQuery.Promise
 * @return {Function} return.done
 * @return {string|null} return.done.formatted Formatted DataValue.
 * @return {dataValues.DataValue|null} return.done.dataValue DataValue object that has been
 *         formatted.
 * @return {Function} return.fail
 * @return {string} return.fail.message HTML error message.
 */

The format ensures a properly readable documentation getting generated.

Array iteration

We decided to use Array.prototype.forEach() and Array.prototype.every() instead of $.each().

Please keep in mind that .forEach( value, key ) and $.each( key, value ) have a different order of parameters and .forEach() doesn't support escaping the iteration by returning false. In this case one should use .every() instead of .forEach().

See also

• Wikidata external code review, May 2013