PHP: Use type hints for method setters instead of GPBUtil

[bshaffer]

Enhancement

The generated messages use the GPBUtil class to check types. This is fine in some cases, such as with RepeatedField or Map types, where the setter needs to verify the keys and/or the values of an iterable type. However, for other setters which only accept a class, it would be better to let PHP type hinting do the magic:

For primitive types (Timestamp.php):

public function getSeconds(): int
{
      return $this->seconds;
}

public function setSeconds(int $seconds)
{
    $this->seconds = $seconds;

    return $this;
}

For message classes (Api.php)

public function getSourceContext(): \Google\Protobuf\SourceContext
{
    return $this->source_context;
}

public function setSourceContext(\Google\Protobuf\SourceContext $sourceContext)
{
    $this->source_context = $sourceContext;

    return $this;
}

For map fields and repeated fields, we can still typehint better than we are currently doing by using the iterable typehint (Struct.php):

public function getFields(): iterable
{
    return $this->fields;
}

public function setFields(iterable $fields)
{
    $arr = GPBUtil::checkMapField($fields, \Google\Protobuf\Internal\GPBType::STRING, \Google\Protobuf\Internal\GPBType::MESSAGE, \Google\Protobuf\Value::class);
    $this->fields = $arr;

    return $this;
}

[haberman]

Sorry for the slow reply. Does PHP provide a guarantee? What happens if you call setSeconds("123") in your example?

[bshaffer]

By default, PHP would cast the string "123" into the int 123. If the file in which the method is called implements declare(strict_types=1), then a TypeError is thrown, same as if setSeconds("abc") is called.

function foo(int $b) {
    var_dump($b);
}
foo(123);   // int(123)
foo('123'); // int(123)
foo('abc'); // TypeError
// When "declare(strict_types=1);" is set at the top of the invoking file
foo('123'); // TypeError

[github-actions[bot]]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

[github-actions[bot]]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

[bshaffer]

This is still a valid (and important) change that should be made.