Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper PHPDoc for nullable class properties and method returns #604

Open
holtkamp opened this issue Feb 4, 2022 · 2 comments
Open

Proper PHPDoc for nullable class properties and method returns #604

holtkamp opened this issue Feb 4, 2022 · 2 comments
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@holtkamp
Copy link

holtkamp commented Feb 4, 2022

Related / follow-up of #172
Is your feature request related to a problem? Please describe.
Currently some classes have properties that

  • can be NULL
  • have PHPDoc which suggests it can not be NULL

For example:

* @var string
*/
public $date;

And

/**
* @return string
*/
public function getDate()
{
return $this->date;
}

When using static analysis using (for example) PHPStan, on code like this:

//All-day event has the date property set
if (is_string( $calendarEvent->getStart()->date)) {
   //Process all-day event
}
} elseif (is_string( $calendarEvent->getStart()->dateTime)) {
   //event is not all-day-even
}

This will result in an error like:

Elseif branch is unreachable because previous condition is always true.  

Describe the solution you'd like
It would be better to use PHPDoc to indicate that the property can be null, for example

  /**
   * @var string|null
   */
  public $date;

  /**
   * @return string|null
   */
  public function getDate()
  {
    return $this->date;
  }

Describe alternatives you've considered
For PHP versions that support typed properties and return types, the PHPDoc can be omitted:

  public ?string $date = null;
  public function getDate() : ?string
  {
    return $this->date;
  }

But this library supports PHP >=5.6, so this is probably not an option yet: https://github.com/googleapis/google-api-php-client-services/blob/main/composer.json#L9

@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Feb 5, 2022
@bshaffer bshaffer added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Mar 16, 2022
@bshaffer
Copy link
Contributor

This is related to #620

I am not sure how this will work across all APIs - what in the discovery doc decides if a field is nullable? There is a "required" keyword, but it's for the Resource classes making the requests, and not for the objects which are sent in the request.

So I believe the answer is that simply adding |null would be sufficient.

@yoshi-automation yoshi-automation removed triage me I really want to be triaged. 🚨 This issue needs some love. labels Mar 16, 2022
@razvanphp
Copy link

razvanphp commented Feb 5, 2024

This library now has minimum PHP version support for PHP 7.4 (#2854), so nullable return types can be added. Can we contribute those kind of changes @bshaffer, or is the repo completely auto generated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

4 participants