DOC Update coding conventions

en/05_Contributing/14_PHP_Coding_Conventions.md

@@ -15,9 +15,9 @@ and facilitate collaboration by making code more consistent and readable.
 
 If you are unsure about a specific standard, imitate existing SilverStripe code.
 
-## PSR-2
+## PSR-12
 
-Our goal is [PSR-2 Coding Standards](https://www.php-fig.org/psr/psr-2/) compliance.
+Our goal is [PSR-12 Coding Standards](https://www.php-fig.org/psr/psr-12/) 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. 
 
@@ -136,6 +136,133 @@ class MyClass extends Class
 }
 ```
 
+## Code style
+
+- Prioritise the human readability of code. Avoid "clever" code that aims to reduce the number of lines at the expense of readbability. This is particularly important when writing Javascript.
+- Avoid writing any new "magic" code as it is very hard to follow.
+- Within reason, favour larger files that have everything contained in them rather than multiple small files that require developers to jump around multiple files to follow a single code path.
+
+## Inline comments
+- If there is a non-obvious reason for a code operation, include verbose inline comments that explain *why* we are doing this operation. Remember that while something may be obvious to you now, it won't be obvious to someone else in 6 months.
+- Inline comments that explain *what* the code is doing are generally fine as it can make code easier to follow and is particularly useful for larger methods, though it may be better to simply split larger methods in to several private methods.
+
+## Naming conventions
+
+### Casing
+
+- Use `camelCase` for variables, method names and parameters
+- Use `UpperCamelCase` for class names, interfaces, traits and enums
+- Use `snake_case` for private static configuration properties.
+- Use `SCREAMING_SNAKE_CASE` for constants including class contants, and environment variables
+- Note that there will be some inconsistencies with legacy code where static methods have been a combination of `MyClass::camelCase()` and `MyClass::snake_case()`
+
+### Accessors
+
+- For accessors omit the leading 'get' with getters and prefix with 'set' for setters. e.g. `function myValue():sting { return $this->value; }` and `function setMyValue(string $value): void { $this->value = $value; }`.
+- Note that there will be some inconsistencies with legacy code as get accessors have been a combination of "with get", "without get", and "without get with the first letter uppercased"
+
+### File suffixes and prefixes
+
+- This is for new classes and interfaces that are created. Legacy Silvesrtripe code is inconsistent whether or not it follows these conventions.
+- Suffix the classnames with the type of class that is subclassed. The suffixed is sometimes a shortened version of the parent class because it reads better while retaining easy comprehension. Here are some common examples with the parents classes in brackets
+  - `Exception` (Exception)
+  - `Controller` (Controller)
+  - `Extension` (Extension, DataExtension)
+  - `Admin` (ModelAdmin)
+  - `Handler` (RequestHandler)
+  - `Filter` (SearchFilter)
+  - `Form` (Form)
+  - `Field` (FormField)
+  - `Job` (QueuedJob)
+  - `Task` (BuildTask)
+  - `Service` (generic class type that has no parent class)
+Some parent classes have unorthodox naming, in which case suffix with something more standard
+  - `Page` (SiteTree)
+  - `Controller` (LeftAndMain, CMSMain)
+- Do not suffix classes if they are a subclass any of the following:
+  - `DataObject`
+- Suffix interfaces with `Interface`
+- Suffix traits with `Trait`
+- Prefix abstract classes with `Abstract`
+
+### Other naming conventions
+
+- Extensions should match the name of the class they are extending. For example, an extension class that's added to LeftAndMain should be called LeftAndMainExtension
+- When choosing names for variables and methods use the guiding principle "make it as obvious as possible"
+- Ensure that variable names and method naming is consistent. For example, if you are creating a new class and have a variable named `$itemValue`` which is passed to another method in the same class, the paramater name for that method should also be `$itemValue``.
+
+## 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
+- Do not group different class types together in a directory relating to a single feature
+- Use initial caps pluralised for the directory naming e.g. use `Controllers` rather than `Controller` or `controllers`
+
+## Properties and accessors
+
+- Do not use public properites, instead use a combination of private properites and public getters and setters
+- For setters 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)
+
+## Method and property visibility
+
+- Use private over protected as this reduces the maintained API surface. Only use protected if there's an immediate reason to upgrade from private.
+- Note that there is a lot of legacy Silverstripe code where protected was used over private.
+
+## Interfaces
+
+- If there are or there's a realistic expection that there will be multiple class implementations of something then create an interface and use that for parameter and return types, and as the service key for injector.
+- If there is only a single implemention and no forseeable expectation of more implementaitons being added, then do not create an interface for the single class implementation because this would only be adding unecessarily complexity.
+
+## Typing
+
+- Aim to strongly type all new properties, method parameters, and method return types.
+- The exception to this is adding a new method or property to a class which is entirely untyped and adding typed API would be very inconsistent with the rest of the file. In this instance use PHP docblocks to define types.
+- If there are duplicate strong types and docblock types then remove the old docblock types.
+- Do not strongly type existing API outside of a new major version as it's an API breakage.
+
+## Mixed types
+
+- Where possible avoid using mixed types, including type unions, because this leads to complicated and brittle code.
+- Try to 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 the use of the splat `...` parameter as this makes API enforcement difficult. If you need to support multiple arguments then use an array parameter type.
+
+## Object instantiation
+
+- Use Injector to instantiate objects rather than the `new` keyword. This is done as it allows projects to swap out classes with different implementations.
+- Note there is only a minimal performance penalty for this, though there is a downside that IDE type hinting stops working. During development you may wish to use the new keyword for this reason and change it to use injector before submitting your PR.
+- For classes that use the Injectable trait use the `::create()` static method.
+- For objects that do not have the Injectable trait, use `Injector::inst()->get({classname})`
+- This also includes instantiation of third party classes `<<< @todo why? discuss`
+- The exception to this is for unit-tests where you should use the `new` because there you need to ensure you're testing a specific class
+
+## Extensions and traits
+
+- Use the DataExtension class for extending DataObjects.
+- Use the Extension class for extending non-DataObjects.
+- Typically you shouldn't use native PHP traits. A common gotcha is mistakenly using a trait on a DataObject to add a private static config property. This doesn't work because private static config properties are a Silverstripe specific thing and the Silverstripe DataExtension will correctly merge them. However traits are unable to merge private static config properites.
+
+### Extension hooks
+
+- Naming convention for extension hooks is `update{methodName}` where methodName has any leading `get` omitted has the remaining first letter uppercased.
+
+### Unit testing
+
+- The unit test method name should match the method that it's 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 DataProvider's to provide test case data as it makes code much cleaner and it makes it very easy to add additional test cases.
+- The DataProvider method name should be same as the test case method name it's providing for with the leading work `test` substituted for `provide` e.g. for `testSomething()` the DataProvider is `provideSomething()`
+- Typically you should use yml fixture files for creating fixtures, though this is a fairly loose recommendation. Often it will make sense to "create them manually" in PHP.
+- Using reflection to change the visibility of a private or protected method so that it can be independently tested is usually OK. At a practical level sometimes only testing public methods is overly clunky.
+
+### Admin-only controllers
+
+- Controllers that are only intended to be used from within the CMS should extend [`LeftAndMain`](api:SilverStripe\Admin\LeftAndMain). This will handle routing for you and ensures that only authenticated users are able to access their endpoints.
+- For small modules that are expected to only have a single admin-only controller, the `$url_segment` private static configuration property should match the second half of the composer name of the module e.g. for a `LinkFieldController` in `silverstripe/linkfield` the controllers `$url_segment` should be "linkfield".
+
+## Calling third-party dependencies
+
+- If you directly call a third-party dependency, then ensure the dependency is required in the modules `composer.json`.
+
 ## Class Member Ordering
 
 Put code into the classes in the following order (where applicable).
@@ -192,4 +319,3 @@ See [security](/developer_guides/security) for conventions related to handing se
  * [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)
-