-
Notifications
You must be signed in to change notification settings - Fork 138
chore: refactor to declare more types #771
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||||||
| <?php | ||||||||||
|
|
||||||||||
| declare(strict_types=1); | ||||||||||
|
|
||||||||||
| namespace Sabre\VObject; | ||||||||||
|
|
||||||||||
| use Sabre\Xml; | ||||||||||
|
|
@@ -32,19 +34,15 @@ class Parameter extends Node implements \Stringable | |||||||||
|
|
||||||||||
| /** | ||||||||||
| * Parameter value. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| * | ||||||||||
| * @var string|array|null | ||||||||||
| */ | ||||||||||
| protected $value; | ||||||||||
| protected array|string|null $value = null; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Sets up the object. | ||||||||||
| * | ||||||||||
| * It's recommended to use the create:: factory method instead. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| * | ||||||||||
| * @param string|array|null $value | ||||||||||
| */ | ||||||||||
| public function __construct(Document $root, ?string $name, $value = null) | ||||||||||
| public function __construct(Document $root, ?string $name, array|string|null $value = null) | ||||||||||
| { | ||||||||||
| $this->root = $root; | ||||||||||
| if (is_null($name)) { | ||||||||||
|
|
@@ -61,7 +59,9 @@ public function __construct(Document $root, ?string $name, $value = null) | |||||||||
| $this->noName = false; | ||||||||||
| $this->name = strtoupper($value); | ||||||||||
| } else { | ||||||||||
| $this->setValue($value); | ||||||||||
| if (null !== $value) { | ||||||||||
| $this->setValue($value); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
@@ -88,10 +88,8 @@ public static function guessParameterNameByValue(string $value): string | |||||||||
| * Updates the current value. | ||||||||||
| * | ||||||||||
| * This may be either a single, or multiple strings in an array. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I would keep the type just make it more correct |
||||||||||
| * | ||||||||||
| * @param string|array $value | ||||||||||
| */ | ||||||||||
| public function setValue($value): void | ||||||||||
| public function setValue(array|string $value): void | ||||||||||
| { | ||||||||||
| $this->value = $value; | ||||||||||
| } | ||||||||||
|
|
@@ -140,10 +138,8 @@ public function getParts(): array | |||||||||
| * | ||||||||||
| * If the argument is specified as an array, all items will be added to the | ||||||||||
| * parameter value list. | ||||||||||
| * | ||||||||||
| * @param string|array $part | ||||||||||
| */ | ||||||||||
| public function addValue($part): void | ||||||||||
| public function addValue(array|string $part): void | ||||||||||
| { | ||||||||||
| if (is_null($this->value)) { | ||||||||||
| $this->value = $part; | ||||||||||
|
|
@@ -206,7 +202,7 @@ function ($out, $item) { | |||||||||
| // But we've found that iCal (7.0, shipped with OSX 10.9) | ||||||||||
| // severely trips on + characters not being quoted, so we | ||||||||||
| // added + as well. | ||||||||||
| if (!preg_match('#(?: [\n":;\^,\+] )#x', $item)) { | ||||||||||
| if (!preg_match('#(?: [\n":;\^,\+] )#x', (string) $item)) { | ||||||||||
| return $out.$item; | ||||||||||
| } | ||||||||||
| // Enclosing in double-quotes, and using RFC6868 for encoding any | ||||||||||
|
|
@@ -228,20 +224,15 @@ function ($out, $item) { | |||||||||
| /** | ||||||||||
| * This method returns an array, with the representation as it should be | ||||||||||
| * encoded in JSON. This is used to create jCard or jCal documents. | ||||||||||
| * | ||||||||||
| * @return array|string|null | ||||||||||
| */ | ||||||||||
| #[\ReturnTypeWillChange] | ||||||||||
| public function jsonSerialize() | ||||||||||
| public function jsonSerialize(): array|string|null | ||||||||||
| { | ||||||||||
| return $this->value; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * This method serializes the data into XML. This is used to create xCard or | ||||||||||
| * xCal documents. | ||||||||||
| * | ||||||||||
| * @param Xml\Writer $writer XML writer | ||||||||||
| */ | ||||||||||
| public function xmlSerialize(Xml\Writer $writer): void | ||||||||||
| { | ||||||||||
|
|
@@ -261,7 +252,6 @@ public function __toString(): string | |||||||||
| /** | ||||||||||
| * Returns the iterator for this object. | ||||||||||
| */ | ||||||||||
| #[\ReturnTypeWillChange] | ||||||||||
| public function getIterator(): ElementList | ||||||||||
| { | ||||||||||
| if (!is_null($this->iterator)) { | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Sabre\VObject; | ||
|
|
||
| use PHPUnit\Framework\TestCase; | ||
|
|
@@ -30,17 +32,17 @@ public function testModify(): void | |
| $cal = new Component\VCalendar(); | ||
|
|
||
| $param = new Parameter($cal, 'name', null); | ||
| $param->addValue(1); | ||
| $param->addValue('1'); | ||
|
Comment on lines
-33
to
+35
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO |
||
| self::assertEquals([1], $param->getParts()); | ||
|
|
||
| $param->setParts([1, 2]); | ||
| self::assertEquals([1, 2], $param->getParts()); | ||
|
|
||
| $param->addValue(3); | ||
| $param->addValue('3'); | ||
| self::assertEquals([1, 2, 3], $param->getParts()); | ||
|
|
||
| $param->setValue(4); | ||
| $param->addValue(5); | ||
| $param->setValue('4'); | ||
| $param->addValue('5'); | ||
| self::assertEquals([4, 5], $param->getParts()); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Sabre\VObject\Parser; | ||
|
|
||
| use PHPUnit\Framework\TestCase; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These don't look so good. The PHPdoc used to say that
$offsetis anintBut the parent ArrayAccess::offsetExists(mixed $offset): bool
allows anything as parameter 1. So we can't writepublic function offsetExists(int $offset): bool`So we seem to lose something here, the PHPdoc type hint goes, but we can't preserve it in the actual parameter type declaration.