Skip to content
This repository has been archived by the owner on Jul 2, 2019. It is now read-only.

Add RFC for Type Declarations #3

Closed
wants to merge 11 commits into from
128 changes: 128 additions & 0 deletions text/0000-type-declarations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
- Start Date: 2018-02-21
- RFC PR:
- WordPress Coding Standards Issue:

# Summary

[Type Declarations](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration) make code more readable. They also protect code, because Type Declarations are a way of enforcing specific data types in parameters passed to a function.

# Basic Example

You can force callers to pass an `array`, or a specific type of object, such as a `WP_REST_Request`. If a caller doesn't pass the data type expected, PHP 5 triggers a recoverable fatal error and PHP 7 throws a [TypeError](http://php.net/manual/en/class.typeerror.php) exception. This behavior catches problems right away.

```php
protected function foo( Bar $bar, array $items ) {
// $bar must be an instance of the Bar class.
// $items must be passed as an array.
}
```

# Motivation

PHP is a [weakly typed language](https://en.wikipedia.org/wiki/Strong_and_weak_typing). It doesn't require you to declare data types. However, variables still have data types associated with them (e.g., `string`, `integer`). In a weakly typed language you can do radical things like adding a `string` to an `integer` with no error. While this can be wonderful in some cases, it can also lead to unanticipated behavior and bugs.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... In a weakly typed language, you can do radical things like adding a string to an integer with no error.

Is this really radical? :D Maybe this should be reworded. Type juggling is a known thing in weakly typed languages

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I agree that this word is a little too strong.


For example, PHP functions and class methods accept parameters, such as `function foo( $bar, $items )`. If Type Declarations aren't being used, the only way a function knows what it's being passed is to run `is_*()` checks, `instanceof`, or use type casting. Otherwise, `$bar` could be anything, and there's a chance that invalid data types would go unnoticed (no error) and produce an unexpected or incorrect return value. Imagine type casting an array to an integer. That's forcing a wrong into a right, instead of correcting the underlying issue. A caller should pass the right data type to begin with.

Copy link

@schlessera schlessera Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add an additional paragraph here along the lines of:

Moreover, as the wrong value does not throw an immediate error, it might get passed down to
another function, and yet another, and so on. Most often, you'll end up with an error that is
thrown in a completely unrelated part of the code, or in the worst case, no error being
thrown at all, while the frontend happily renders bogus data.

Basically emphasizing why throwing an immediate error is so important.

So Type Declarations are advantageous, because ultimately, they produce improved error messages that catch problems right away. In the same way we _discourage_ use of the `@` error control operator and _encourage_ strict comparison `===`, Type Declarations shine a light a bugs. They're a way of being more explicit about the expected data type. An added benefit is more readable code.

# Detailed Design

## Valid Type Declarations in PHP 5.1+

- Class or interface name; e.g., `WP_REST_Request`
- `self`, which references own class or interface name
- `array`, to require an array

**Formatting:** There should be one space before and after a Type Declaration.

```php
protected function foo( Bar $bar, array $items ) {
// $bar must be an instance of the Bar class.
// $items must be passed as an array.
}
```

## Modern Versions of PHP

Other data types, such as `callable`, `string`, `int`, `float`, and `bool` [became available in PHP 7.0](http://php.net/manual/en/migration70.new-features.php). In addition, PHP 7.0 added support for [Return Type Declarations](http://php.net/manual/en/functions.returning-values.php#functions.returning-values.type-declaration) and [Strict Typing](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration.strict). Support for the `iterable` Type Declaration became available in PHP 7.1.

**Formatting:** A Return Type Declaration should immediately follow a function's round closing bracket `)`, with no space before `:`, and with one space before and after the data type.

```php
protected function foo( string $str, int $num, bool $flag, callable $callback ): bool {
// $str must be passed as a string.
// $num must be passed as an integer.
// $flag must be passed as true or false.
// $callback must be a callable function, method, closure.

return true; // must return true or false.
}
```

## WordPress Core Compatibility

At this time, the additional type declarations: `callable`, `string`, `int`, `float`, `bool`, and `iterable` must be avoided in WordPress Core. The same is true for [Return Type Declarations](http://php.net/manual/en/functions.returning-values.php#functions.returning-values.type-declaration) and [Strict Typing](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration.strict). Please see [WordPress requirements](https://wordpress.org/about/requirements/) (PHP 5.2.4+) and [this table](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration) for further details.

## When to Use Type Declarations

When you're writing a function or a class method that expects to receive an array or a specific object type, and there is no reason to accept anything other than that specific data type.

```php
protected function foo( Bar $bar, array $items ) {
// $bar must be an instance of the Bar class.
// $items must be passed as an array.
}
```

## When Not to Use Type Declarations

If the function you're writing is part of a public API and enforcing a specific data type would make the function less flexible in the eyes of a caller. For example, the [`get_post()`](https://developer.wordpress.org/reference/functions/get_post/) function in WordPress core accepts `int|WP_Post|null` as the first parameter. This maximizes flexibility for callers, making the function more convenient in a variety of circumstances.

Likewise, if you're writing a public function for an API and it needs a `WP_Post` instance, it's better not to enforce `WP_Post` with a type declaration. Instead, use the `get_post()` function to resolve the `$post` reference, making it possible for a caller to pass `int|WP_Post|null` to your function as well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph seems only partially related to type declarations. Although I agree that WP_Post should not be used, I think the most important reason is because it is a final class without an interface. Using that type declaration seals off any future extensibility. Using some other form of type checking is what throughout all of the WordPress code base anyway.


```php
function foo( $post ) {
$post = get_post( $post );
...
}
```

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the current PHP requirements, I think that type declarations should only be used right now with types that are:

  • interfaces or classes
  • considered to be part of the public interface
  • extensible

All other type declarations should be discouraged, as they cause more problems than they solve:

  • Using array immediately renders almost any future improvements using smart objects impossible. It makes it impossible for objects to stand in as collections of elements. Often times, when the doc-block states that a function needs an array, it actually means it needs a Traversable, because it wants to loop over it, but cannot do that as PHP 5.2 is missing that concept, or it needs an ArrayAccess because it wants to use indexed access. Using array should be avoided at all costs, unless there is a specific reason why it needs to be the actual internal type of array.
  • Using classes that are not extensible means that future changes need to be done by changing an element of the public interface, instead of _ extending_ it. This is again an unneeded hindrance to flexibility. This is why segregated interfaces (i.e. Renderable or Taggable instead of WP_Post) should always be preferred, however WordPress is not really big on using interfaces.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the above, I'd also recommend changing the examples in this document to not so prominently show the array type declaration.

# Drawbacks

## Runtime Errors

Using Type Declarations can lead to recoverable fatal errors in PHP 5, and [TypeError](http://php.net/manual/en/class.typeerror.php) exceptions in PHP 7. This is both a blessing and a curse. Runtime errors are mostly advantageous for reasons already stated elsewhere in this RFC; i.e., they help catch bugs right away.

However, unlike [`_doing_it_wrong()`](https://developer.wordpress.org/reference/functions/_doing_it_wrong/), errors associated with invalid data types are more difficult to suppress. Imagine a plugin author writing code that calls upon a core function. If the core function uses Type Declarations to enforce specific data types, and the plugin passes an invalid type, an HTTP error response or WSOD could occur.

# Adoption Strategy

Retrofitting existing functions with Type Declarations is generally discouraged. Doing so would suddenly enforce a rule that did not exist prior, causing runtime errors. For example, imagine themes and plugins calling core functions that previously accepted multiple data types, but now require a specific type. Not good! This scenario should be avoided.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retrofitting public/protected functions/methods should be avoided 💯

However, what about retrofitting private methods ? These are not publicly accessible and should therefore be relatively safe to retrofit.

The only problem there would be plugins which extend a Core class and change the visibility of a private method to protected or public.
In that case, they would receive a Declaration of Class::method(...) should be compatible with Parent::method(...) warning. This will not cause WSODs though, so I think encouraging this would not be a bad thing.
See https://3v4l.org/VHoO6

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be avoided 💯

I've updated to this stronger wording you suggested.


However, what about retrofitting private methods ? These are not publicly accessible and should therefore be relatively safe to retrofit.

I agree. Not quite sure how we'd like to word this yet, but giving it some thought. Maybe others have some input they'd like to share on this? It seems reasonably safe to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaswrks I would remove the :100: from the RFC text though.....


However, new functions (particularly protected and private methods of a class) are encouraged to use Type Declarations supported by the minimum version of PHP they are targeting. If targeting a modern version of PHP (7.0+), the use of [Return Type Declarations](http://php.net/manual/en/functions.returning-values.php#functions.returning-values.type-declaration) is encouraged also.

# Teaching Strategy

Type Declarations were once known as Type Hints in PHP 5. That name is no longer appropriate, because later versions of PHP added support for [Return Type Declarations](http://php.net/manual/en/functions.returning-values.php#functions.returning-values.type-declaration) and [Strict Typing](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration.strict). For this reason, please use the up-to-date and official terminology:

- [Type Declarations](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration)
- [Return Type Declarations](http://php.net/manual/en/functions.returning-values.php#functions.returning-values.type-declaration)
- [Strict Typing](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration.strict) (aka: Strict Mode)

## Limitations in WordPress Core

Given current minimum requirements in WordPress Core (PHP 5.2.4+), Return Type Declarations and Strict Typing should be avoided altogether, and only mentioned for the purpose of explaining why they cannot be used at this time.

The only Type Declarations supported in WordPress Core at this time, are:

- Class or interface name; e.g., `WP_REST_Request`
- `self`, which references own class or interface name
- `array`, to require an array

# Unresolved Questions

- If sniffs are added for Type Declarations:
- Are there any circumstances in which Type Declarations **MUST** be used?
- Are there any circumstances in which Type Declarations **MUST NOT** be used?

- Should steps be taken to suppress runtime errors caused by invalid data types when running in a production environment? If so, what options are available for consideration?