Skip to content
Prev Previous commit
Next Next commit
♻️ cover more default value cases
  • Loading branch information
GautierDele committed Oct 28, 2024
commit 992c829bade5762f73b37c742c317e691d57f278
2 changes: 1 addition & 1 deletion src/DocBlock/Tags/Factory/MethodFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function (MethodTagValueParameterNode $param) use ($context) {
),
$param->isReference,
$param->isVariadic,
(string) $param->defaultValue
$param->defaultValue
);
},
$tagValue->parameters
Expand Down
81 changes: 81 additions & 0 deletions src/DocBlock/Tags/Factory/MethodParameterFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php

declare(strict_types=1);

/**
* This file is part of phpDocumentor.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*
* @link http://phpdoc.org
*/

namespace phpDocumentor\Reflection\DocBlock\Tags\Factory;

use phpDocumentor\Reflection\DocBlock\Tags\Formatter;
use function str_repeat;
use function strlen;

class MethodParameterFactory
Copy link
Member

Choose a reason for hiding this comment

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

Please mark this class as final and @internal as we should not make this part of the backward compatibility promise. Extending this class should be blocked as it is an internal only class. Maybe this should even be part of the Method tag? Unless there is a reason to extract this because you want to share this class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All good

For me, extracting this logic from the base code is a good idea for the maintanability
It could have been a trait but I did get inspired by the package way of working, could be wrong

{
/**
* Formats the given default value to a string-able mixin
*/
public function format($defaultValue): string
{
if (method_exists($this, $method = 'format'.ucfirst(gettype($defaultValue)))) {
return ' = ' . $this->{$method}($defaultValue);
}
return '';
}

protected function formatDouble(float $defaultValue): string
Copy link
Member

Choose a reason for hiding this comment

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

I do prefer private for all new methods as we do not want others to interfere with this code. This make it easier to refactor methods as private methods are internal only, and we can remove them at will.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All good

{
return var_export($defaultValue, true);
}

protected function formatNull($defaultValue): string
{
return 'null';
}

protected function formatInteger(int $defaultValue): string
{
return var_export($defaultValue, true);
}

protected function formatString(string $defaultValue): string
{
return var_export($defaultValue, true);
}

protected function formatBoolean(bool $defaultValue): string
{
return var_export($defaultValue, true);
}

protected function formatArray(array $defaultValue): string
{
$formatedValue = '[';

foreach ($defaultValue as $key => $value) {
if (method_exists($this, $method = 'format'.ucfirst(gettype($value)))) {
$formatedValue .= $this->{$method}($value);

if ($key !== array_key_last($defaultValue)) {
$formatedValue .= ',';
}
}
}

$formatedValue .= ']';

return $formatedValue;
}

protected function formatObject(object $defaultValue): string
{
return 'new '. get_class($defaultValue). '()';
}
}
13 changes: 1 addition & 12 deletions src/DocBlock/Tags/Method.php
Original file line number Diff line number Diff line change
Expand Up @@ -261,18 +261,7 @@ public function __toString(): string
{
$arguments = [];
foreach ($this->parameters as $parameter) {
$parameterDefaultValueStr = null;
if ($parameter->getDefaultValue() !== null) {
$parameterDefaultValueStr = $parameter->getDefaultValue();
settype($parameterDefaultValueStr, (string)$parameter->getType());
$parameterDefaultValueStr = sprintf(' = %s', var_export($parameterDefaultValueStr, true));
}

$arguments[] = $parameter->getType() . ' ' .
($parameter->isReference() ? '&' : '') .
($parameter->isVariadic() ? '...' : '') .
'$' . $parameter->getName() .
($parameterDefaultValueStr ?? '');
$arguments[] = (string) $parameter;
}

$argumentStr = '(' . implode(', ', $arguments) . ')';
Expand Down
18 changes: 15 additions & 3 deletions src/DocBlock/Tags/MethodParameter.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

namespace phpDocumentor\Reflection\DocBlock\Tags;

use phpDocumentor\Reflection\DocBlock\Tags\Factory\MethodParameterFactory;
use phpDocumentor\Reflection\Type;

final class MethodParameter
Expand All @@ -24,14 +25,16 @@ final class MethodParameter

private string $name;

private ?string $defaultValue = null;
private mixed $defaultValue;

private const NO_DEFAULT_VALUE = '__NO_VALUE__';

public function __construct(
string $name,
Type $type,
bool $isReference = false,
bool $isVariadic = false,
?string $defaultValue = null
$defaultValue = self::NO_DEFAULT_VALUE
) {
$this->type = $type;
$this->isReference = $isReference;
Expand Down Expand Up @@ -60,8 +63,17 @@ public function isVariadic(): bool
return $this->isVariadic;
}

public function getDefaultValue(): ?string
public function getDefaultValue(): mixed
Copy link
Member

Choose a reason for hiding this comment

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

Can we please revert this as I did notice before, because I see this as a backward compatibility break. I think we should not do that right now. This will also solve the failing tests

{
return $this->defaultValue;
}

public function __toString(): string
{
return $this->getType() . ' ' .
($this->isReference() ? '&' : '') .
($this->isVariadic() ? '...' : '') .
'$' . $this->getName() .
($this->getDefaultValue() !== self::NO_DEFAULT_VALUE ? (new MethodParameterFactory)->format($this->getDefaultValue()) : '');
}
}
104 changes: 104 additions & 0 deletions tests/unit/DocBlock/Tags/MethodParameterTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?php

declare(strict_types=1);

/**
* This file is part of phpDocumentor.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*
* @link http://phpdoc.org
*/

namespace phpDocumentor\Reflection\DocBlock\Tags;

use Mockery as m;
use phpDocumentor\Reflection\DocBlock\Description;
use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
use phpDocumentor\Reflection\Fqsen;
use phpDocumentor\Reflection\Type;
use phpDocumentor\Reflection\TypeResolver;
use phpDocumentor\Reflection\Types\Array_;
use phpDocumentor\Reflection\Types\Boolean;
use phpDocumentor\Reflection\Types\Compound;
use phpDocumentor\Reflection\Types\Context;
use phpDocumentor\Reflection\Types\Float_;
use phpDocumentor\Reflection\Types\Integer;
use phpDocumentor\Reflection\Types\Mixed_;
use phpDocumentor\Reflection\Types\Null_;
use phpDocumentor\Reflection\Types\Nullable;
use phpDocumentor\Reflection\Types\Object_;
use phpDocumentor\Reflection\Types\String_;
use phpDocumentor\Reflection\Types\This;
use phpDocumentor\Reflection\Types\Void_;
use PHPUnit\Framework\TestCase;

/**
* @coversDefaultClass \phpDocumentor\Reflection\DocBlock\Tags\Method
* @covers ::<private>
*/
class MethodParameterTest extends TestCase
{
/**
* Call Mockery::close after each test.
*/
public function tearDown(): void
{
m::close();
}

public function collectionDefaultValuesProvider(): array
{
return [
[new String_(), '1', '\'1\''],
[new Integer(), 1, '1'],
[new Boolean(), true, 'true'],
[new Float_(), 1.23, '1.23'],
[new Array_(), [1, '2', true], '[1,\'2\',true]'],
[new Array_(), [[1, 2], '2', true], '[[1,2],\'2\',true]'],
[new Nullable(new Float_()), null, 'null'],
[new Nullable(new Float_()), 1.23, '1.23'],
[new Object_(new Fqsen('\\stdClass')), new \stdClass(), 'new stdClass()'],
];
}

/**
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::__construct
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::getDefaultValue()
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::__toString
* @uses \phpDocumentor\Reflection\DocBlock\Tags\Formatter\PassthroughFormatter
*
* @dataProvider collectionDefaultValuesProvider
* @covers \phpDocumentor\Reflection\DocBlock\Tags\BaseTag::render
* @covers \phpDocumentor\Reflection\DocBlock\Tags\BaseTag::getName
*/
public function testIfTagCanBeRenderedUsingMethodParameterWithDefaultValue(Type $type, $defaultValue, string $defaultValueStr): void
{
$fixture = new MethodParameter('argument', $type, false, false, $defaultValue);

$this->assertSame(
sprintf('%s $argument = %s', $type, $defaultValueStr),
(string) $fixture
);
}

/**
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::__construct
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::getDefaultValue()
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::__toString
* @uses \phpDocumentor\Reflection\DocBlock\Tags\Formatter\PassthroughFormatter
*
* @covers \phpDocumentor\Reflection\DocBlock\Tags\BaseTag::render
* @covers \phpDocumentor\Reflection\DocBlock\Tags\BaseTag::getName
*/
public function testIfTagCanBeRenderedUsingMethodParameterWithNoDefaultValue(): void
{
$fixture = new MethodParameter('argument', new Float_());

$this->assertSame(
'float $argument',
(string) $fixture
);
}
}
38 changes: 1 addition & 37 deletions tests/unit/DocBlock/Tags/MethodTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use phpDocumentor\Reflection\DocBlock\Description;
use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
use phpDocumentor\Reflection\Fqsen;
use phpDocumentor\Reflection\Type;
use phpDocumentor\Reflection\TypeResolver;
use phpDocumentor\Reflection\Types\Array_;
use phpDocumentor\Reflection\Types\Boolean;
Expand Down Expand Up @@ -700,41 +701,4 @@ public function testCreateWithReference(): void
$this->assertSame($description, $fixture->getDescription());
$this->assertTrue($fixture->returnsReference());
}

/**
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::__construct
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::getDefaultValue()
* @uses \phpDocumentor\Reflection\DocBlock\Tags\MethodParameter::__toString
* @uses \phpDocumentor\Reflection\DocBlock\Tags\Formatter\PassthroughFormatter
*
* @covers \phpDocumentor\Reflection\DocBlock\Tags\BaseTag::render
* @covers \phpDocumentor\Reflection\DocBlock\Tags\BaseTag::getName
*/
public function testIfTagCanBeRenderedUsingMethodParameterWithDefaultValue(): void
{
$arguments = [
['name' => 'argument1', 'type' => new String_()],
['name' => 'argument2', 'type' => new Object_()],
];

$fixture = new Method(
'myMethod',
$arguments,
new Void_(),
false,
null,
false,
[
new MethodParameter('argument1', new String_(), false, false, '1'),
new MethodParameter('argument2', new Integer(), false, false, '1'),
new MethodParameter('argument3', new Boolean(), false, false, 'true'),
new MethodParameter('argument4', new Float_(), false, false, '1.23'),
]
);

$this->assertSame(
'@method void myMethod(string $argument1 = \'1\', int $argument2 = 1, bool $argument3 = true, float $argument4 = 1.23)',
$fixture->render()
);
}
}