From b9845dd98c21a5a5fbefef0493e75ec2846be92c Mon Sep 17 00:00:00 2001 From: Yani Date: Tue, 10 Jan 2023 07:13:17 +0100 Subject: [PATCH 1/4] fix overloading behaviour --- features/access.feature | 4 ++++ src/Session.php | 4 ++-- tests/behavior/AccessContext.php | 38 ++++++++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/features/access.feature b/features/access.feature index d5b6add..6c2f3d5 100644 --- a/features/access.feature +++ b/features/access.feature @@ -33,3 +33,7 @@ Feature: Session access Scenario: Iterate over non-populated session When data does not exist Then data is not iterated + Scenario: Overload the Session object + When data does not exist + Then array overloading succeeds + And object overloading succeeds diff --git a/src/Session.php b/src/Session.php index 6e14c02..0b5a287 100644 --- a/src/Session.php +++ b/src/Session.php @@ -46,7 +46,7 @@ public function __construct(string $name, ?string $id = null, array $contents = * * @throws RuntimeException if not initialized */ - public function __get(string $name) + public function &__get(string $name) { if (!$this->isInitialized()) { throw new RuntimeException('Session not initialized'); @@ -133,7 +133,7 @@ public function offsetUnset($name): void unset($this->contents[$name]); } - public function offsetGet($name): mixed + public function &offsetGet($name): mixed { if (!$this->isInitialized()) { throw new RuntimeException('Session not initialized'); diff --git a/tests/behavior/AccessContext.php b/tests/behavior/AccessContext.php index 3bdc711..1ea7899 100644 --- a/tests/behavior/AccessContext.php +++ b/tests/behavior/AccessContext.php @@ -65,7 +65,7 @@ public function propertyReadTriggersNoticeError(): void { try { $errorThrown = false; - $bar = $this->session->bar; + $bar = clone $this->session->bar; // @phpstan-ignore-next-line } catch (Throwable $e) { $errorThrown = true; @@ -133,7 +133,7 @@ public function arrayAccessReadTriggersNoticeError(): void { try { $errorThrown = false; - $bar = $this->session['bar']; + $bar = clone $this->session['bar']; // @phpstan-ignore-next-line } catch (Throwable $e) { $errorThrown = true; @@ -199,4 +199,38 @@ public function iteratorFails(): void Assert::assertSame(0, $counter); } + + /** + * @Then array overloading succeeds + */ + public function arrayOverloadSucceeds(): void + { + $error = false; + + try { + $this->session['overload_me'] = []; + $this->session['overload_me'][] = 'foo'; + } catch (\Exception $ex) { + $error = true; + } + + Assert::assertFalse($error); + } + + /** + * @Then object overloading succeeds + */ + public function objectOverloadSucceeds(): void + { + $error = false; + + try { + $this->session->overload_me = []; + $this->session->overload_me[] = 'foo'; + } catch (\Exception $ex) { + $error = true; + } + + Assert::assertFalse($error); + } } From 1795eee6b4938bf5e57e8f902895048bab5c994c Mon Sep 17 00:00:00 2001 From: Jonathon Hill Date: Mon, 13 Feb 2023 19:08:08 -0500 Subject: [PATCH 2/4] Remove clone statements --- tests/behavior/AccessContext.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/behavior/AccessContext.php b/tests/behavior/AccessContext.php index 1ea7899..48d1b42 100644 --- a/tests/behavior/AccessContext.php +++ b/tests/behavior/AccessContext.php @@ -65,7 +65,7 @@ public function propertyReadTriggersNoticeError(): void { try { $errorThrown = false; - $bar = clone $this->session->bar; + $bar = $this->session->bar; // @phpstan-ignore-next-line } catch (Throwable $e) { $errorThrown = true; @@ -133,7 +133,7 @@ public function arrayAccessReadTriggersNoticeError(): void { try { $errorThrown = false; - $bar = clone $this->session['bar']; + $bar = $this->session['bar']; // @phpstan-ignore-next-line } catch (Throwable $e) { $errorThrown = true; From fc225e46a1f3f759131f6a92cc513252fb67f283 Mon Sep 17 00:00:00 2001 From: Yani Date: Wed, 15 Feb 2023 05:34:56 +0100 Subject: [PATCH 3/4] fix overloading usage and test cases --- features/access.feature | 10 ++++-- src/Session.php | 16 +++++++-- tests/behavior/AccessContext.php | 62 +++++++++++++++++++++++--------- 3 files changed, 67 insertions(+), 21 deletions(-) diff --git a/features/access.feature b/features/access.feature index 6c2f3d5..7229208 100644 --- a/features/access.feature +++ b/features/access.feature @@ -33,7 +33,11 @@ Feature: Session access Scenario: Iterate over non-populated session When data does not exist Then data is not iterated - Scenario: Overload the Session object + Scenario: Overload existing array + When empty array for overload exists + Then overloading using property access succeeds + And overloading using property access succeeds + Scenario: Overload non existing array When data does not exist - Then array overloading succeeds - And object overloading succeeds + Then overloading using array access fails + And overloading using property access fails diff --git a/src/Session.php b/src/Session.php index 0b5a287..d04051d 100644 --- a/src/Session.php +++ b/src/Session.php @@ -8,6 +8,7 @@ use Countable; use ArrayAccess; use RuntimeException; +use Stringable; class Session implements ArrayAccess, Iterator, Countable { @@ -52,7 +53,11 @@ public function &__get(string $name) throw new RuntimeException('Session not initialized'); } - // @phpstan-ignore-next-line + if(!isset($this->contents[$name])){ + \trigger_error("Array key not found: '$name'", \E_USER_NOTICE); + return null; + } + return $this->contents[$name]; } @@ -139,7 +144,14 @@ public function &offsetGet($name): mixed throw new RuntimeException('Session not initialized'); } - // @phpstan-ignore-next-line + + if(!isset($this->contents[$name])){ + if($name === null || \is_scalar($name) || $name instanceof Stringable){ + \trigger_error("Array key not found: '$name'", \E_USER_NOTICE); + } + return null; + } + return $this->contents[$name]; } diff --git a/tests/behavior/AccessContext.php b/tests/behavior/AccessContext.php index 48d1b42..0c95344 100644 --- a/tests/behavior/AccessContext.php +++ b/tests/behavior/AccessContext.php @@ -34,6 +34,16 @@ public function dataExists(): void Assert::assertCount(1, $this->session); } + /** + * @When empty array for overload exists + */ + public function emptyArrayForOverloadExists(): void + { + $this->session = new Session('foo', 'bar', ['foo' => []]); + Assert::assertTrue($this->session->isWriteable()); + Assert::assertCount(1, $this->session); + } + /** * @Then property check returns false */ @@ -205,16 +215,10 @@ public function iteratorFails(): void */ public function arrayOverloadSucceeds(): void { - $error = false; - - try { - $this->session['overload_me'] = []; - $this->session['overload_me'][] = 'foo'; - } catch (\Exception $ex) { - $error = true; - } - - Assert::assertFalse($error); + // @phpstan-ignore-next-line + $this->session['foo'][] = 'baz'; + // @phpstan-ignore-next-line + Assert::assertSame('baz', $this->session['foo'][0]); } /** @@ -222,15 +226,41 @@ public function arrayOverloadSucceeds(): void */ public function objectOverloadSucceeds(): void { - $error = false; + // @phpstan-ignore-next-line + $this->session->foo[] = 'baz'; + // @phpstan-ignore-next-line + Assert::assertSame('baz', $this->session->foo[0]); + } + /** + * @Then overloading using array access fails + */ + public function arrayOverloadFails(): void + { try { - $this->session->overload_me = []; - $this->session->overload_me[] = 'foo'; - } catch (\Exception $ex) { - $error = true; + $errorThrown = false; + // @phpstan-ignore-next-line + $this->session['foo'][] = 'baz'; + } catch (Throwable $e) { + $errorThrown = true; + } finally { + Assert::assertTrue($errorThrown); } + } - Assert::assertFalse($error); + /** + * @Then overloading using property access fails + */ + public function objectOverloadFails(): void + { + try { + $errorThrown = false; + // @phpstan-ignore-next-line + $this->session->foo[] = 'baz'; + } catch (Throwable $e) { + $errorThrown = true; + } finally { + Assert::assertTrue($errorThrown); + } } } From 6146cf4b96d43f9115740d1d900c8f6337045d57 Mon Sep 17 00:00:00 2001 From: Yani Date: Wed, 15 Feb 2023 05:57:44 +0100 Subject: [PATCH 4/4] fix name of overload testcases --- tests/behavior/AccessContext.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/behavior/AccessContext.php b/tests/behavior/AccessContext.php index 0c95344..199d107 100644 --- a/tests/behavior/AccessContext.php +++ b/tests/behavior/AccessContext.php @@ -211,7 +211,7 @@ public function iteratorFails(): void } /** - * @Then array overloading succeeds + * @Then overloading using array access succeeds */ public function arrayOverloadSucceeds(): void { @@ -222,7 +222,7 @@ public function arrayOverloadSucceeds(): void } /** - * @Then object overloading succeeds + * @Then overloading using property access succeeds */ public function objectOverloadSucceeds(): void {