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

[WIP] Add DatePeriod::getEndDate dynamic return type #762

Conversation

Alban-io
Copy link
Contributor

@Alban-io Alban-io commented Nov 9, 2021

@Alban-io Alban-io force-pushed the add-date-period-get-end-data-dynamic-return-type branch from 1133ea9 to eb401c8 Compare November 9, 2021 11:55
@Alban-io Alban-io changed the title Add DatePeriod::getEndDate dynamic return type [WIP] Add DatePeriod::getEndDate dynamic return type Nov 9, 2021
@Alban-io
Copy link
Contributor Author

Alban-io commented Nov 9, 2021

Currently my test works only because MethodCall is directly the constructor (new DatePeriod($start, $interval, $recurrences))->getEndDate().

Is there a way to retrieve the constructor arguments from this method ?

@staabm
Copy link
Contributor

staabm commented Nov 9, 2021

I think, the object on which the method is called on, is available via https://github.com/nikic/PHP-Parser/blob/99a24b6a556cb2f85c48f62a5311cbc1f8b6eb92/lib/PhpParser/Node/Expr/MethodCall.php#L13

and you might resolve it via the scope to the actual object (like you did with the method arguments)

@Alban-io
Copy link
Contributor Author

Alban-io commented Nov 9, 2021

I've already tried it but I couldn't find the constructor arguments types from MethodCall->var only object type 😕

@staabm
Copy link
Contributor

staabm commented Nov 9, 2021

isn't the 3rd arg returned by getRecurrences()?

@Alban-io
Copy link
Contributor Author

Alban-io commented Nov 9, 2021

Not really, DatePeriod are 3 constructors :

public __construct(
    DateTimeInterface $start,
    DateInterval $interval,
    int $recurrences,
    int $options = 0
)

public __construct(
    DateTimeInterface $start,
    DateInterval $interval,
    DateTimeInterface $end,
    int $options = 0
)

public __construct(string $isostr, int $options = 0)

The 3rd arg can have three types : null, int, DateTimeInterface.

null or int

public getEndDate(): null
public getRecurrences(): int

DateTimeInterface

public getEndDate(): DateTimeInterface
public getRecurrences(): null

@staabm
Copy link
Contributor

staabm commented Nov 9, 2021

I see. we need to wait for input from Ondrey ;-)

@Alban-io
Copy link
Contributor Author

I think DynamicMethodReturnTypeExtension may not be the right choice.

Another way :

Create an Type-Specifying Extensions but I feel like they are not available with constructor (New_), only available on Method, StaticMethod and Function.

@ondrejmirtes What is your point of view about that ?

@ondrejmirtes
Copy link
Member

What logic you want to implement in a dynamic return type extension for the constructor?

@ondrejmirtes
Copy link
Member

Superseded by #951 for now. Feel free to open a new PR with improvements.

@Alban-io
Copy link
Contributor Author

What logic you want to implement in a dynamic return type extension for the constructor?

More like an MethodTypeSpecifyingExtension to make specify types (TypeSpecifier).

Example with DatePeriod :

By default (without type specification) :

public getEndDate(): ?DateTimeInterface
public getRecurrences(): ?int

based on the type of the third constructor argument, we can specify the return type of the methods (getEndDate, getRecurrences) for object instance

// A
public __construct(
    DateTimeInterface $start,
    DateInterval $interval,
    DateTimeInterface $end,
    int $options = 0
)
// B
public __construct(
    DateTimeInterface $start,
    DateInterval $interval,
    int $recurrences,
    int $options = 0
)
// C
public __construct(string $isostr, int $options = 0)

in the case of the constructor A, we can remove null on getEndDate & and specify getRecurrences will always return null

public getEndDate(): DateTimeInterface
public getRecurrences(): null

inversely for constructor B & C

public getEndDate(): null
public getRecurrences(): int

@ondrejmirtes
Copy link
Member

This would require making DatePeriod generic with two template types - for endDate and recurrences count.

@Alban-io
Copy link
Contributor Author

Is it possible to make conditional return type with PHPStan ?

<?php

/**
 * @template T of int|\DateTimeInterface|null
 */
class TestDatePeriod {

	/** @var T */
	private $value;

	/**
	 * @param T $value
	 */
	public function __construct($value)
	{
		$this->value = $value;
	}

	/**
	 * @psalm-return (T is \DateTimeImmutable ? \DateTimeImmutable : null)
	 * @phpstan-return (T is \DateTimeImmutable ? \DateTimeImmutable : null)
	 */
	public function getEndDate(): ?DateTimeImmutable
	{
		return $this->value instanceof \DateTimeInterface ? new DateTimeImmutable() : null;
	}

	/**
	 * @psalm-return (T is \DateTimeInterface ? null : int)
	 * @phpstan-return (T is \DateTimeInterface ? null : int)
	 */
	public function getRecurrences(): ?int
	{
		return is_int($this->value) ? 0 : null;
	}
}

$intExpected = fn(int $value): int => $value + 1;
$dateExpected = fn(\DateTimeInterface $dateTime): \DateTimeInterface => $dateTime;

$datePeriod = new TestDatePeriod(new DateTimeImmutable());
$dateExpected($datePeriod->getEndDate());
$intExpected($datePeriod->getRecurrences());

$datePeriod = new TestDatePeriod(55);
$dateExpected($datePeriod->getEndDate());
$intExpected($datePeriod->getRecurrences());

$datePeriod = new TestDatePeriod(null);
$dateExpected($datePeriod->getEndDate());
$intExpected($datePeriod->getRecurrences());

@canvural
Copy link
Contributor

@Alban-io Not yet.

@ondrejmirtes
Copy link
Member

But you can write a dynamic return type extension for the same purpose.

@Alban-io
Copy link
Contributor Author

Thanks for your answers

I tried to make an implementation using DynamicStaticMethodReturnTypeExtension

I am able to determine the third argument type, but I can't find how to specify class methods return type

@ondrejmirtes
Copy link
Member

You need to write a DatePeriod generic stub that describes return types of thos two metods, and write a DynamicStaticMethodReturnTypeExtension for the constructor that returns different GenericObjectType based on the arguments.

The stub can look like this:

/**
 * @template TEnd of ?DateTimeInterface
 * @template TRecurrences of ?int
 */
class DatePeriod
{

	/**
	 * @return TEnd
	 */
	public function getEndDate()
	{
		
	}

	/**
	 * @return TRecurrences
	 */
	public function getRecurrences()
	{
		
	}
}

@Alban-io
Copy link
Contributor Author

It seems to work, great 😃 Alban-io@ad22b8b

Do you want a new PR or reopen this one ?

@ondrejmirtes
Copy link
Member

Please open a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants