Backwards compatibility break in PHP/Messages constructors
- Dominant language
- C#
- Stars
- 41
- Forks
- 24
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 6
Description
## Relevant PHP concepts
PHP supports 'optional arguments' (arguments given an explicit default). This means a signature can change from:
```php
foo(string $bar)
```
to
```php
foo(string $bar, int $baz=0)
```
and the calling code `foo('hello')` still works as expected
Modern PHP additionally supports 'named arguments' so the same call can be written as `foo(name: 'hello')` but because this is a relatively recent addition, positional arguments are often used by developers.
## The problem in Messages 19.0.0
For Messages, all arguments are optional by design and in the documentation we encourage using named arguments
The new KeywordType field has changed the constructor of Step from:
```php
public function __construct(
Location $location = new Location(),
string $keyword = '',
string $text = '',
?DocString $docString = null,
?DataTable $dataTable = null,
string $id = '',
) {
```
to
```php
public function __construct(
Location $location = new Location(),
string $keyword = '',
?KeywordType $keywordType = null,
string $text = '',
?DocString $docString = null,
?DataTable $dataTable = null,
string $id = '',
) {
```
This means that while named arguments still work:
```php
$step = new Step(
keyword: 'Given',
text: 'I have an apple',
)
```
The equivalent with positional arguments is now a syntax error:
```php
$step = new Step(
new Location(),
'Given',
'I have an apple', # error because this is not a KeywordType
)
```
## Solutions
We could
1. Explicitly state that positional arguments are not supported. This is easy for us but relies on the developer reading the documentation and will lead to some friction
2. Only ever add new fields at the end of the schema; this could be enforced by adding some tests that check the objects are all constructable
Contributor guide
Research direction
Start with the PHP Step constructor shown in the issue and inspect the other Messages constructors for the same field-ordering pattern. Determine how positional construction should remain compatible, then add tests covering construction of these objects with existing positional arguments and verify that named arguments continue to work.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100