Skip to content

Commit

Permalink
DOC Update coding conventions
Browse files Browse the repository at this point in the history
  • Loading branch information
@emteknetnz
emteknetnz committed Nov 13, 2023
1 parent 7cf345b commit 40b1e83
Showing 1 changed file with 127 additions and 163 deletions.
290 changes: 127 additions & 163 deletions en/05_Contributing/05_Coding_Conventions/01_PHP_Coding_Conventions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,190 +6,154 @@ iconBrand: php

# PHP Coding Conventions

This document provides guidelines for code formatting and documentation
to developers contributing to SilverStripe. It applies to all PHP files
in the `framework/` and `cms/` modules, as well as any supported additional modules.
This document provides guidelines for code formatting and documentation to developers contributing to Silverstripe. It applies to all PHP files in supported modules, as well as any additional modules under the `silverstripe` GitHub account.

Coding standards are an important aspect for every software project,
and facilitate collaboration by making code more consistent and readable.
Coding standards are an important aspect for every software project, and facilitate collaboration by making code more consistent and readable.

If you are unsure about a specific standard, imitate existing SilverStripe code.
These conventions will work well in most situations, though sometimes you may need to not follow them for practical reasons.

## PSR-2
These coding conventions are for new code. Existing Silverstripe CMS code is inconsistent whether or not it follows these conventions.

Our goal is [PSR-2 Coding Standards](https://www.php-fig.org/psr/psr-2/) compliance.
Since this affects existing APIs, some details like method casing will be iterated on in the next releases.
For example, many static methods will need to be changed from lower underscore to lower camel casing.

## Spelling
## General

All symbols and documentation should use UK-English spelling (e.g. "behaviour" instead of "behavior"),
except when necessitated by third party conventions (e.g using PHP's `Serializable` interface).

## Configuration Variables

Silverstripe CMS's [Config API](/developer_guides/configuration/configuration/) can read its defaults from variables declared as `private static` on classes.
As opposed to other variables, these should be declared as lower case with underscores.


```php
class MyClass
{
private static $my_config_variable = 'foo';
}
```

## Prefer identical (===) comparisons over equality (==)

Where possible, use type-strict identical comparisons instead of loosely typed equality comparisons.
Read more in the PHP documentation for [comparison operators](https://php.net/manual/en/language.operators.comparison.php) and [object comparison](https://php.net/manual/en/language.oop5.object-comparison.php).


```php
// good - only need to cast to (int) if $a might not already be an int
if ((int)$a === 100) {
doThis();
}

// bad
if ($a == 100) {
doThis();
}
```

## Separation of Logic and Presentation

Try to avoid using PHP's ability to mix HTML into the code.


```php
// PHP code
public function getTitle()
{
return "<h2>Bad Example</h2>";
}
```

```ss
// Template code
$Title
```

Better: Keep HTML in template files:


```php
// PHP code
public function getTitle()
{
return "Better Example";
}
```

```ss
// Template code
<h2>$Title</h2>
```

## Comments

Use [phpdoc](https://phpdoc.org/) syntax before each definition (see [tutorial](https://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_phpDocumentor.quickstart.pkg.html)
and [tag overview](https://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.pkg.html)).

* Methods should include at least `@param` and `@return`.
* Include a blank line after the description.
* Use `{@link MyOtherClass}` and `{@link MyOtherClass->otherMethod}` for inline references.
* Denote preformatted code examples in `<code></code>` blocks.
* Always start block-level comments containing phpdoc with two asterisks (`/** ... */`).

Example:


```php
/**
* My short description for this class.
* My longer description with
* multiple lines and richer formatting.
*
* Usage:
* <code>
* $c = new MyClass();
* $c->myMethod();
* </code>
*
* @package custom
*/
class MyClass extends Class
{
/**
* My Method.
* This method returns something cool. {@link MyParentMethod} has other cool stuff in it.
*
* @param string $colour The colour of cool things that you want
* @return DataList A list of everything cool
*/
public function myMethod($colour)
{
// ...
}

}
```
- Aim for [PSR-12 Coding Standards](https://www.php-fig.org/psr/psr-12/) compliance.
- Prioritise the human readability of code over "clever" code that reduces the number of lines at the expense of readability.
- All symbols and documentation should use UK-English spelling (e.g. "behaviour" instead of "behavior"), except when necessitated by third party conventions (e.g using PHP's `Serializable` interface).
- Avoid writing HTML in PHP code. Instead, put HTML is template files.

## Class Member Ordering
## Naming conventions

### Casing

- Follow [PSR-1](https://www.php-fig.org/psr/psr-1/#1-overview) for naming class names, method names, and class constants
- Use [snake_case](https://en.wikipedia.org/wiki/Snake_case) for configuration variables which are used in both `private static` class properties as well as yml configuration.
- Use [camelCase](https://en.wikipedia.org/wiki/Camel_case) for all other class properties

### Accessors

- Prefix with 'get' for getters and 'set' for setters, for example `function getValue():string { return $this->value; }` and `function setValue(string $value): void { $this->value = $value; }`.

### File suffixes and prefixes

Suffix (sometimes prefix) classnames with the type of class that they subclass. The suffix/prefix is sometimes a shortened version of the parent class because it reads better while retaining easy comprehension. Here are some common examples with the parent classes in brackets:

- `Admin` (ModelAdmin, LeftAndMain if included in the CMS Menu)
- `Block` (BaseElement)
- `DB` (DBField - prefix instead of suffix)
- `Controller` (Controller)
- `Exception` (Exception)
- `Extension` (Extension)
- `Factory` (Factory)
- `Filter` (SearchFilter)
- `Form` (Form)
- `Field` (FormField)
- `Handler` (RequestHandler)
- `Page` (SiteTree)
- `Job` (QueuedJob)
- `Service` (Class only instantiated as a singleton)
- `Task` (BuildTask)

## Directory naming - src dir

- Put similar types of classes in the same directory. For example, put Controllers in a Controllers directory
- Put DataObjects in a directory called Models
- If you end up with a lot of classes of a specific type with some clear grouping, you can use subdirectories to group them - e.g. `src/Models/Sports` if you have a ton of sports related models.
- Use initial caps pluralised for the directory naming e.g. use `Controllers` rather than `Controller` or `controllers`

## Properties and accessors

- Do not use public properties, instead use a combination of non-public properties and public getter and setter methods.
- For setter methods use a combination of a `:static` return type and `return $this` to allow for fluent programming.
- Do not use [dynamic properties](https://www.php.net/manual/en/language.oop5.properties.php#language.oop5.properties.dynamic-properties)

Put code into the classes in the following order (where applicable).
## Method, property, and constant non-public visibility

* Static variables
* Member variables
* Static methods
* Data-model definition static variables. (`$db`, `$has_one`, `$many_many`, etc)
* Commonly used methods like `getCMSFields()`
* Accessor methods (`getMyField()` and `setMyField()`)
* Controller action methods
* Template data-access methods (methods that will be called by a `$MethodName` or `<% loop $MethodName %>` construct in a template somewhere)
* Object methods

## Method/property visibility guidelines
- Prefer `private` over `protected` for all new properties. Subclasses can access the properties through the public getter and setter methods.
- Prefer `private` over `protected` for new methods, unless there is a clear use case for customising the specific functionality of that method on its own and the method implementation is unlikely to change in the near future.
- Prefer `private` over `protected` for constants, unless they are intended to be used in subclasses.

When adding new methods and properties, should you make them public, protected, or private?
## Interfaces

* Public: Use this for public APIs. You should add tests and documentation for these.
* Protected: Use for extensibility APIs. Note that compatibility for such APIs should be maintained in minor releases, and so you'll probably want to cover them in tests (i.e. test that these methods can be called from within a subclass and/or overridden). If that seems like overkill, consider whether it's safer to leave these as private for now.
* Private: Use for class internals
- Create an interface for type hinting if there are (or are likely to be in the future) multiple classes implementing a specific feature. Use the interface for type hints (e.g. for parameters, properties, and return types), and as the service key for injector.
- If there is only a single implementation and no foreseeable expectation of more implementations being added, then do not create an interface for the single class implementation because this would only be adding unnecessary complexity.

Above all, recognise that when you are making something public or protected, you are creating an API for other developers to use and rely on, and do so with care. Don't simply dump class internals in as protected members in the name of extensibility – doing so will be likely to break developers' projects in minor releases.
## Typing

## SQL Format
- Aim to strongly type all new properties, method parameters, and method return types. This includes files where there is no existing strong typing.
- Only provide docblock types for strongly typed parameters or return values if more context is provided.

If you have to use raw SQL, make sure your code works across databases. Make sure you escape your queries like below,
with the column or table name escaped with double quotes as below.
## Mixed types

- Where practical avoid using mixed types, including type unions, because this leads to complicated and brittle code.
- Avoid using nullable scalars and instead use the default value of the scalar instead of null. One exception to this is for when `null` means 'uninitialised'.
- Avoid using `func_get_args()`.

## Object instantiation

- For classes with the `Injectable` trait, use Injector to instantiate objects using the `::create()` static method rather than the `new` keyword. This is done as it allows projects to swap out classes with different implementations.
- For non-third-party classes without the `Injectable` trait, consider adding the `Injectable` trait, or use the `new` keyword.
- For third-party classes, [use `Injector` directly](/developer_guides/extending/injector/#basic-usage) to instantiate objects rather than the `new` keyword.
- For unit tests, always use the `new` keyword when instantiating objects unless you are explicitly testing the injector logic.

## Extensions and traits

- Use the Extension class for extending classes, including DataObjects.
- Do not use the DataExtension class, it will be deprecated in a future release.

Use a trait instead of an `Extension` when the composable functionality:

- Relies on per-instance properties, which cannot be added to extensions
- Does not add or modify configuration properties or values
- Is not expected to be applied to third-party code.

Use an `Extension` instead of a trait when the composable functionality:

- Does not rely on per-instance properties
- Adds or modifies configuration properties or values
- Could be expected to be applied to third-party code.

### Extension hooks

- For hooks that update a value before it's returned, name the hook `update{MethodName}` where `MethodName` has any prefix `get` removed. For example `getMyValue()` would have an extension hook named `updateMyValue()`.
- For hooks that apply before or after another method is called within the method, name the hook `onBefore{MethodName}` or `onAfter{MethodName}` respectively. For instance if a method calls `processSomething()` call the hook `onBeforeProcessSomething()`.

## Class Member Ordering

```php
MyClass::get()->where(['"Score" > ?' => 50]);
Order code in classes in the following order:

```
- Class constants
- Data-model definition static properties (`$db`, `$has_one`, `$many_many`, etc)
- Static properties
- Member properties
- Static methods
- Controller action methods
- Commonly used methods like `getCMSFields()`
- Accessor methods (`getMyField()` and `setMyField()`)
- Template data-access methods (methods that will be called by a `$MethodName` or `<% loop $MethodName %>` construct in a template somewhere)
- Object methods

It is preferable to use parameterised queries whenever necessary to provide conditions
to a SQL query, where values placeholders are each replaced with a single unquoted question mark.
If it's absolutely necessary to use literal values in a query make sure that values
are single quoted.
## Inline comments

- If there is a non-obvious reason for a code operation, include inline comments that explain *why* we are doing this operation. Remember that while something may be obvious to you now, it will not be obvious to someone else in 6 months.
- Self explanatory code with well-named methods and variables is preferred to inline comments that explain *what* the code does, though it's still OK to include these sorts of comments if you think it makes the code easier to follow. This is often the case for larger methods, though it may be better to simply split those methods into several private methods.

```php
MyClass::get()->where("\"Title\" = 'my title'");
```
### Unit testing

Use [ANSI SQL](https://en.wikipedia.org/wiki/SQL#Standardization) format where possible.
- Unit test method names should match the method that they're testing with the first letter of the method uppercased and the word `test` prefixed (e.g. for the method `myMethodName()` the unit test method should be called `testMyMethodName()`).
- Aim to use the `@dataProvider` annotation to provide test case data as it makes code much cleaner and it makes it very easy to add additional test cases.
- Data provider method names should be same as the test case method name they're providing for with the leading work `test` substituted for `provide` (e.g. for `testSomething()` the DataProvider is `provideSomething()`).
- Data provider array keys should describe the scenario they're testing.
- Unless you need to explicitly create dynamic fixtures, fixtures should be added via yaml fixture files.
- It's usually OK to use reflection to change the visibility of a private or protected method so that it can be tested independently. It is generally preferable to keep a smaller public API surface and use reflection to test functionality, rather than making the API public just so it can be tested.

## Secure Development
## Raw SQL

See [security](/developer_guides/security) for conventions related to handing security permissions.
- Avoid writing raw SQL where possible, instead use ORM methods.
- If you do write raw SQL, use double quotes around table/column names and use parameterised queries where possible e.g. `->where(['"Score" > ?' => 50])`.
- Use [ANSI SQL](https://en.wikipedia.org/wiki/SQL#Standardization) format where possible.

## Related
## Other convensions

* [PHPCS Coding Standard](https://github.com/silverstripe/silverstripe-framework/blob/4/phpcs.xml.dist)
* [JavaScript Coding Conventions](/contributing/javascript_coding_conventions)
* [Reference: CMS Architecture](/developer_guides/customising_the_admin_interface/cms_architecture)

- Prefer the identical `===` operator over the equality `==` operator for comparisons.
- If you directly reference a third-party dependency in code, then ensure the dependency is required in the module's `composer.json` file.
- Avoid hardcoding values when there is a method available to dynamically get a value, for example use `DataObjectSchema::tableName()` to get the table name for a DataObject rather than hard coding it.

0 comments on commit 40b1e83

Please sign in to comment.