From bbb4bb1a6921e436d0d0408f0e49762ac830c8fd Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 17:43:23 +0200 Subject: [PATCH 1/5] Demonstrate bug special attribute handling in ->getAttribute() --- tests/AttributesTest.hack | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/AttributesTest.hack b/tests/AttributesTest.hack index 04df5cb..d6d4571 100644 --- a/tests/AttributesTest.hack +++ b/tests/AttributesTest.hack @@ -9,6 +9,7 @@ use namespace Facebook\XHP\Core as x; use type Facebook\XHP\HTML\div; +use type Facebook\XHP\AttributeRequiredException; use function Facebook\FBExpect\expect; use namespace HH\Lib\Vec; @@ -44,7 +45,10 @@ final xhp class test:required_attributes extends x\element { } final xhp class test:default_attributes extends x\element { - attribute string mystring = 'mydefault'; + attribute string mystring = 'mydefault', + string data-with-initializer = 'initial', + string data-with-late-init @lateinit, + string data-plain; <<__Override>> protected async function renderAsync(): Awaitable { @@ -126,6 +130,10 @@ final class AttributesTest extends Facebook\HackTest\HackTest { public async function testProvidingDefaultAttributes(): Awaitable { $x = ; expect($x->:mystring)->toEqual('herp'); + expect($x->:data-plain)->toBeNull(); + expect($x->:data-that-does-not-exist)->toBeNull(); + expect($x->:data-with-initializer)->toEqual('initial'); + expect(() ==> $x->:data-with-late-init)->toThrow(AttributeRequiredException::class); expect(await $x->toStringAsync())->toEqual('
herp
'); } From bf24e5ecb35a72923db4aaac1ccbe6996465bd9c Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 17:44:30 +0200 Subject: [PATCH 2/5] Fix bug, default/@lateinit/@required of data- ignored --- src/core/Node.hack | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/core/Node.hack b/src/core/Node.hack index e21ebf2..329a0a5 100644 --- a/src/core/Node.hack +++ b/src/core/Node.hack @@ -289,20 +289,19 @@ abstract xhp class node implements \XHPChild { return $this->attributes[$attr]; } - if (!ReflectionXHPAttribute::isSpecial($attr)) { - // Get the declaration - $decl = static::__xhpReflectionAttribute($attr); - - if ($decl === null) { + // Get the declaration + $decl = static::__xhpReflectionAttribute($attr); + if ($decl === null) { + if (!ReflectionXHPAttribute::isSpecial($attr)) { throw new \Facebook\XHP\AttributeNotSupportedException($this, $attr); - } else if ($decl->isRequired()) { - throw new \Facebook\XHP\AttributeRequiredException($this, $attr); - } else { - return $decl->getDefaultValue(); } + } else if ($decl->isRequired()) { + throw new \Facebook\XHP\AttributeRequiredException($this, $attr); } else { - return null; + return $decl->getDefaultValue(); } + + return null; } final public static function __xhpReflectionAttribute( From 43de519c2bfd9bb00ec54ce1a2aeb4fd1f925d3f Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 17:48:10 +0200 Subject: [PATCH 3/5] Invert isSpecial in spreadElementImpl No behavior change, just improving clarity --- src/core/Node.hack | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/core/Node.hack b/src/core/Node.hack index 329a0a5..f7ea874 100644 --- a/src/core/Node.hack +++ b/src/core/Node.hack @@ -381,10 +381,8 @@ abstract xhp class node implements \XHPChild { foreach ($attrs as $attr_name => $value) { if ( $value === null || - !( - ReflectionXHPAttribute::isSpecial($attr_name) || - (static::__xhpReflectionAttribute($attr_name) !== null) - ) + static::__xhpReflectionAttribute($attr_name) === null && + !ReflectionXHPAttribute::isSpecial($attr_name) ) { continue; } From 8080b4220db9412e5f27732d327c17ea3fa20972 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 17:51:19 +0200 Subject: [PATCH 4/5] Formatting the affected files --- src/core/Node.hack | 85 +++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/src/core/Node.hack b/src/core/Node.hack index f7ea874..02893c5 100644 --- a/src/core/Node.hack +++ b/src/core/Node.hack @@ -53,11 +53,13 @@ abstract xhp class node implements \XHPChild { dynamic ...$debug_info ) { invariant( - $this->__xhpChildrenDeclaration() === self::__NO_LEGACY_CHILDREN_DECLARATION, + $this->__xhpChildrenDeclaration() === + self::__NO_LEGACY_CHILDREN_DECLARATION, 'The `children` keyword is no longer supported', ); invariant( - $this->__xhpCategoryDeclaration() === self::__NO_LEGACY_CATEGORY_DECLARATION, + $this->__xhpCategoryDeclaration() === + self::__NO_LEGACY_CATEGORY_DECLARATION, 'The `category` keyword is no longer supported', ); @@ -167,7 +169,8 @@ abstract xhp class node implements \XHPChild { /** * Fetches all direct children of this element of the given type. */ - public function getChildrenOfType<<<__Enforceable>> reify T as \XHPChild>(): vec { + public function getChildrenOfType<<<__Enforceable>> reify T as \XHPChild>( + ): vec { $children = vec[]; foreach ($this->children as $child) { if ($child is T) { @@ -202,7 +205,8 @@ abstract xhp class node implements \XHPChild { * * If no matching child is present, returns `null`. */ - public function getFirstChildOfType<<<__Enforceable>> reify T as \XHPChild>(): ?T { + public function getFirstChildOfType<<<__Enforceable>> reify T as \XHPChild>( + ): ?T { foreach ($this->children as $child) { if ($child is T) { return $child; @@ -216,7 +220,8 @@ abstract xhp class node implements \XHPChild { * * If no matching child is present, an exception is thrown. */ - public function getFirstChildOfTypex<<<__Enforceable>> reify T as \XHPChild>(): T { + public function getFirstChildOfTypex<<<__Enforceable>> reify T as \XHPChild>( + ): T { $child = $this->getFirstChildOfType(); invariant( $child is nonnull, @@ -253,7 +258,8 @@ abstract xhp class node implements \XHPChild { * * If the element has no matching children, `null` is returned. */ - public function getLastChildOfType<<<__Enforceable>> reify T as \XHPChild>(): ?T { + public function getLastChildOfType<<<__Enforceable>> reify T as \XHPChild>( + ): ?T { for ($i = C\count($this->children) - 1; $i >= 0; --$i) { $child = $this->children[$i]; if ($child is T) { @@ -268,9 +274,14 @@ abstract xhp class node implements \XHPChild { * * If the element has no matching children, an exception is thrown. */ - public function getLastChildOfTypex<<<__Enforceable>> reify T as \XHPChild>(): T { + public function getLastChildOfTypex<<<__Enforceable>> reify T as \XHPChild>( + ): T { $child = $this->getLastChildOfType(); - invariant($child is nonnull, '%s called with no matching child', __FUNCTION__); + invariant( + $child is nonnull, + '%s called with no matching child', + __FUNCTION__, + ); return $child; } @@ -300,7 +311,7 @@ abstract xhp class node implements \XHPChild { } else { return $decl->getDefaultValue(); } - + return null; } @@ -347,9 +358,8 @@ abstract xhp class node implements \XHPChild { // instance. <<__MemoizeLSB>> private static function emptyInstance(): this { - return ( - new \ReflectionClass(static::class) - )->newInstanceWithoutConstructor(); + return + (new \ReflectionClass(static::class))->newInstanceWithoutConstructor(); } final public function getAttributes(): dict { @@ -382,7 +392,7 @@ abstract xhp class node implements \XHPChild { if ( $value === null || static::__xhpReflectionAttribute($attr_name) === null && - !ReflectionXHPAttribute::isSpecial($attr_name) + !ReflectionXHPAttribute::isSpecial($attr_name) ) { continue; } @@ -465,7 +475,10 @@ abstract xhp class node implements \XHPChild { * @param $val value * @throws UseAfterRenderException */ - final public function forceAttribute_DEPRECATED(string $attr, mixed $value): this { + final public function forceAttribute_DEPRECATED( + string $attr, + mixed $value, + ): this { if ($this->__isRendered) { throw new UseAfterRenderException( Str\format("Can't %s after render", __FUNCTION__), @@ -586,7 +599,8 @@ abstract xhp class node implements \XHPChild { } const int __NO_LEGACY_CHILDREN_DECLARATION = -31337; - const dict __NO_LEGACY_CATEGORY_DECLARATION = dict["\0INVALID\0" => 0]; + const dict __NO_LEGACY_CATEGORY_DECLARATION = + dict["\0INVALID\0" => 0]; /** * Defined in elements by the `children` keyword. This returns a pattern of @@ -616,10 +630,8 @@ abstract xhp class node implements \XHPChild { return; } } - list($ret, $ii) = $this->validateChildrenExpression( - $decl->getExpression(), - 0, - ); + list($ret, $ii) = + $this->validateChildrenExpression($decl->getExpression(), 0); if (!$ret || $ii < C\count($this->children)) { if (($this->children[$ii] ?? null) is \Facebook\XHP\AlwaysValidChild) { return; @@ -663,15 +675,11 @@ abstract xhp class node implements \XHPChild { // Specific order -- :fb_thing, :fb_other_thing $oindex = $index; list($sub_expr_1, $sub_expr_2) = $expr->getSubExpressions(); - list($ret, $index) = $this->validateChildrenExpression( - $sub_expr_1, - $index, - ); + list($ret, $index) = + $this->validateChildrenExpression($sub_expr_1, $index); if ($ret) { - list($ret, $index) = $this->validateChildrenExpression( - $sub_expr_2, - $index, - ); + list($ret, $index) = + $this->validateChildrenExpression($sub_expr_2, $index); } if ($ret) { return tuple(true, $index); @@ -682,15 +690,11 @@ abstract xhp class node implements \XHPChild { // Either or -- :fb_thing | :fb_other_thing $oindex = $index; list($sub_expr_1, $sub_expr_2) = $expr->getSubExpressions(); - list($ret, $index) = $this->validateChildrenExpression( - $sub_expr_1, - $index, - ); + list($ret, $index) = + $this->validateChildrenExpression($sub_expr_1, $index); if (!$ret) { - list($ret, $index) = $this->validateChildrenExpression( - $sub_expr_2, - $index, - ); + list($ret, $index) = + $this->validateChildrenExpression($sub_expr_2, $index); } if ($ret) { return tuple(true, $index); @@ -747,10 +751,8 @@ abstract xhp class node implements \XHPChild { return tuple(true, $index + 1); case XHPChildrenConstraintType::SUB_EXPR: - return $this->validateChildrenExpression( - $expr->getSubExpression(), - $index, - ); + return + $this->validateChildrenExpression($expr->getSubExpression(), $index); } } @@ -812,9 +814,8 @@ abstract xhp class node implements \XHPChild { return await $child->toHTMLStringAsync(); } if ($child is Traversable<_>) { - throw new \Facebook\XHP\RenderArrayException( - 'Can not render traversables!', - ); + throw + new \Facebook\XHP\RenderArrayException('Can not render traversables!'); } /* HH_FIXME[4281] stringish migration */ From ce38877b6319a05f5cc5dc4b5b264cd0da945fd2 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 18:03:52 +0200 Subject: [PATCH 5/5] Run tests on lts versions and ubuntu 20.04 in CI --- .github/workflows/build-and-test.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index d713294..1d927c9 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -11,12 +11,12 @@ jobs: # Run tests on all OS's and HHVM versions, even if one fails fail-fast: false matrix: - os: [ ubuntu ] + os: [ ubuntu-20.04 ] hhvm: - '4.128' - - latest - - nightly - runs-on: ${{matrix.os}}-latest + - '4.153' + - '4.168' + runs-on: ${{matrix.os}} steps: - uses: actions/checkout@v2 - name: Create branch for version alias