Merge pull request #579 from phel-lang/fix/578-native-global-array-push

Fix apush, aset and aunset for global php arrays
phel-lang · Feb 25, 2023 · dff3056 · dff3056
2 parents f9298bc + a7eb8d2
commit dff3056
Show file tree

Hide file tree

Showing 14 changed files with 144 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file.
 * Added `--testdox` argument to `phel test` command (#567)
 * Added support for fluid configuration in `phel-config.php` (#494)
 * Enable gacela cache filesystem by default (#576)
+* Fix `php/apush`, `php/aset` and `php/aunset` for global php arrays (#579)
 
 ## 0.9.0 (2023-02-05)
 

diff --git a/src/php/Compiler/Domain/Analyzer/Ast/GlobalVarNode.php b/src/php/Compiler/Domain/Analyzer/Ast/GlobalVarNode.php
@@ -41,4 +41,9 @@ public function isMacro(): bool
     {
         return $this->meta[Keyword::create('macro')] === true;
     }
+
+    public function useReference(): bool
+    {
+        return $this->getEnv()->useGlobalReference();
+    }
 }
diff --git a/src/php/Compiler/Domain/Analyzer/Environment/NodeEnvironment.php b/src/php/Compiler/Domain/Analyzer/Environment/NodeEnvironment.php
@@ -16,6 +16,9 @@ final class NodeEnvironment implements NodeEnvironmentInterface
     /** Def inside of def should not work. This flag help us to keep track of this. */
     private bool $defAllowed = true;
 
+    /** Use Registery::getDefinitionReference() instead of Registry::getDefinition() */
+    private bool $globalReference = false;
+
     /**
      * @param array<int, Symbol> $locals A list of local symbols
      * @param string $context The current context (Expression, Statement or Return)
@@ -126,6 +129,14 @@ public function withContext(string $context): NodeEnvironmentInterface
         return $result;
     }
 
+    public function withUseGlobalReference(bool $useGlobalReference): NodeEnvironmentInterface
+    {
+        $result = clone $this;
+        $result->globalReference = $useGlobalReference;
+
+        return $result;
+    }
+
     public function withAddedRecurFrame(RecurFrame $frame): NodeEnvironmentInterface
     {
         $result = clone $this;
@@ -176,4 +187,9 @@ public function isDefAllowed(): bool
     {
         return $this->defAllowed;
     }
+
+    public function useGlobalReference(): bool
+    {
+        return $this->globalReference;
+    }
 }
diff --git a/src/php/Compiler/Domain/Analyzer/Environment/NodeEnvironmentInterface.php b/src/php/Compiler/Domain/Analyzer/Environment/NodeEnvironmentInterface.php
@@ -50,6 +50,8 @@ public function withLocals(array $locals): self;
 
     public function withContext(string $context): self;
 
+    public function withUseGlobalReference(bool $useGlobalReference): self;
+
     public function withAddedRecurFrame(RecurFrame $frame): self;
 
     public function withDisallowRecurFrame(): self;
@@ -63,4 +65,6 @@ public function getCurrentRecurFrame(): ?RecurFrame;
     public function getBoundTo(): string;
 
     public function isDefAllowed(): bool;
+
+    public function useGlobalReference(): bool;
 }
diff --git a/src/php/Compiler/Domain/Analyzer/TypeAnalyzer/SpecialForm/PhpAPushSymbol.php b/src/php/Compiler/Domain/Analyzer/TypeAnalyzer/SpecialForm/PhpAPushSymbol.php
@@ -17,7 +17,7 @@ public function analyze(PersistentListInterface $list, NodeEnvironmentInterface
     {
         return new PhpArrayPushNode(
             $env,
-            $this->analyzer->analyze($list->get(1), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)),
+            $this->analyzer->analyze($list->get(1), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)->withUseGlobalReference(true)),
             $this->analyzer->analyze($list->get(2), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)),
             $list->getStartLocation(),
         );

diff --git a/src/php/Compiler/Domain/Analyzer/TypeAnalyzer/SpecialForm/PhpASetSymbol.php b/src/php/Compiler/Domain/Analyzer/TypeAnalyzer/SpecialForm/PhpASetSymbol.php
@@ -17,7 +17,7 @@ public function analyze(PersistentListInterface $list, NodeEnvironmentInterface
     {
         return new PhpArraySetNode(
             $env,
-            $this->analyzer->analyze($list->get(1), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)),
+            $this->analyzer->analyze($list->get(1), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)->withUseGlobalReference(true)),
             $this->analyzer->analyze($list->get(2), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)),
             $this->analyzer->analyze($list->get(3), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)),
             $list->getStartLocation(),

diff --git a/src/php/Compiler/Domain/Analyzer/TypeAnalyzer/SpecialForm/PhpAUnsetSymbol.php b/src/php/Compiler/Domain/Analyzer/TypeAnalyzer/SpecialForm/PhpAUnsetSymbol.php
@@ -22,7 +22,7 @@ public function analyze(PersistentListInterface $list, NodeEnvironmentInterface
 
         return new PhpArrayUnsetNode(
             $env,
-            $this->analyzer->analyze($list->get(1), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)),
+            $this->analyzer->analyze($list->get(1), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)->withUseGlobalReference(true)),
             $this->analyzer->analyze($list->get(2), $env->withContext(NodeEnvironmentInterface::CONTEXT_EXPRESSION)),
             $list->getStartLocation(),
         );

diff --git a/src/php/Compiler/Domain/Emitter/OutputEmitter/NodeEmitter/GlobalVarEmitter.php b/src/php/Compiler/Domain/Emitter/OutputEmitter/NodeEmitter/GlobalVarEmitter.php
@@ -19,7 +19,11 @@ public function emit(AbstractNode $node): void
         assert($node instanceof GlobalVarNode);
 
         $this->outputEmitter->emitContextPrefix($node->getEnv(), $node->getStartSourceLocation());
-        $this->outputEmitter->emitStr('\\Phel\\Lang\\Registry::getInstance()->getDefinition("');
+        if ($node->useReference()) {
+            $this->outputEmitter->emitStr('\\Phel\\Lang\\Registry::getInstance()->getDefinitionReference("');
+        } else {
+            $this->outputEmitter->emitStr('\\Phel\\Lang\\Registry::getInstance()->getDefinition("');
+        }
         $this->outputEmitter->emitStr(addslashes($this->outputEmitter->mungeEncodeNs($node->getNamespace())));
         $this->outputEmitter->emitStr('", "');
         $this->outputEmitter->emitStr(addslashes($node->getName()->getName()));

diff --git a/src/php/Lang/Registry.php b/src/php/Lang/Registry.php
@@ -6,6 +6,8 @@
 
 use Phel\Lang\Collections\Map\PersistentMapInterface;
 
+use RuntimeException;
+
 use function array_key_exists;
 
 final class Registry
@@ -54,6 +56,16 @@ public function getDefinition(string $ns, string $name): mixed
         return $this->definitions[$ns][$name] ?? null;
     }
 
+    public function &getDefinitionReference(string $ns, string $name): mixed
+    {
+        if (isset($this->definitions[$ns][$name])) {
+            $value = &$this->definitions[$ns][$name];
+            return $value;
+        }
+
+        throw new RuntimeException('Only variables can be returned by reference');
+    }
+
     public function getDefinitionMetaData(string $ns, string $name): ?PersistentMapInterface
     {
         if (array_key_exists($ns, $this->definitions) && array_key_exists($name, $this->definitions[$ns])) {

diff --git a/src/php/Run/Infrastructure/Command/ReplCommand.php b/src/php/Run/Infrastructure/Command/ReplCommand.php
@@ -158,6 +158,7 @@ private function addLineFromPromptToBuffer(): void
 
         ++$this->lineNumber;
 
+        /** @psalm-suppress RedundantCondition */
         if ($input === null && $isInitialInput) {
             // Ctrl+D will exit the repl
             $this->inputBuffer[] = self::EXIT_REPL;

diff --git a/tests/phel/test/core/sequence-operation.phel b/tests/phel/test/core/sequence-operation.phel
@@ -7,6 +7,24 @@
   (is (= 3 (peek (php/array 1 2 3))) "peek on php array")
   (is (nil? (peek (php/array))) "peek on empty php array"))
 
+(def- testing-set-global-array (php/array 1 2 3))
+
+(deftest test-native-global-array-set
+  (php/aset testing-set-global-array 0 4)
+  (is (= (php/array 4 2 3) testing-set-global-array) "native global set on PHP array"))
+
+(def- testing-unset-global-array (php/array 1 2 3))
+
+(deftest test-native-global-array-unset
+  (php/aunset testing-unset-global-array 2)
+  (is (= (php/array 1 2) testing-unset-global-array) "native global unset on PHP array"))
+
+(def- testing-push-global-array (php/array 1 2 3))
+
+(deftest test-native-global-array-push
+  (php/apush testing-push-global-array 4)
+  (is (= (php/array 1 2 3 4) testing-push-global-array) "native global push on PHP array"))
+
 (deftest test-push
   (let [x (php/array)]
     (php/apush x 1)

diff --git a/tests/php/Integration/Fixtures/Call/php-push-global-reference.test b/tests/php/Integration/Fixtures/Call/php-push-global-reference.test
@@ -0,0 +1,22 @@
+--PHEL--
+(def arr (php/array 1 2 3))
+(php/apush arr 4)
+--PHP--
+\Phel\Lang\Registry::getInstance()->addDefinition(
+  "user",
+  "arr",
+  array(1, 2, 3),
+  \Phel\Lang\TypeFactory::getInstance()->persistentMapFromKVs(
+    \Phel\Lang\Keyword::create("start-location"), \Phel\Lang\TypeFactory::getInstance()->persistentMapFromKVs(
+      \Phel\Lang\Keyword::create("file"), "Call/php-push-global-reference.test",
+      \Phel\Lang\Keyword::create("line"), 1,
+      \Phel\Lang\Keyword::create("column"), 0
+    ),
+    \Phel\Lang\Keyword::create("end-location"), \Phel\Lang\TypeFactory::getInstance()->persistentMapFromKVs(
+      \Phel\Lang\Keyword::create("file"), "Call/php-push-global-reference.test",
+      \Phel\Lang\Keyword::create("line"), 1,
+      \Phel\Lang\Keyword::create("column"), 27
+    )
+  )
+);
+(\Phel\Lang\Registry::getInstance()->getDefinitionReference("user", "arr"))[] = 4;
diff --git a/tests/php/Integration/Fixtures/Call/php-push.test b/tests/php/Integration/Fixtures/Call/php-push.test
@@ -3,4 +3,4 @@
   (php/apush arr 4))
 --PHP--
 $arr_1 = array(1, 2, 3);
-($arr_1)[] = 4;
+($arr_1)[] = 4;
diff --git a/tests/php/Unit/Lang/RegistryTest.php b/tests/php/Unit/Lang/RegistryTest.php
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhelTest\Unit\Lang;
+
+use Phel\Lang\Registry;
+use PHPUnit\Framework\TestCase;
+
+final class RegistryTest extends TestCase
+{
+    private Registry $registry;
+
+    public static function tearDownAfterClass(): void
+    {
+        Registry::getInstance()->clear();
+    }
+
+    protected function setUp(): void
+    {
+        $this->registry = Registry::getInstance();
+        $this->registry->clear();
+    }
+
+    public function test_null_when_non_existing_definition_by_value(): void
+    {
+        $actual = $this->registry->getDefinition('ns', 'non-existing');
+
+        self::assertNull($actual);
+    }
+
+    public function test_value_definition(): void
+    {
+        $this->registry->addDefinition('ns', 'array', [1, 2, 3]);
+        $this->registry->getDefinition('ns', 'array')[] = 4;
+        $actual = $this->registry->getDefinition('ns', 'array');
+
+        self::assertSame([1, 2, 3], $actual);
+    }
+
+    public function test_error_when_non_existing_definition_by_reference(): void
+    {
+        $this->expectExceptionMessage('Only variables can be returned by reference');
+
+        $this->registry->getDefinitionReference('ns', 'non-existing');
+    }
+
+    public function test_reference_definition(): void
+    {
+        $this->registry->addDefinition('ns', 'array', [1, 2, 3]);
+        $this->registry->getDefinitionReference('ns', 'array')[] = 4;
+        $actual = $this->registry->getDefinition('ns', 'array');
+
+        self::assertSame([1, 2, 3, 4], $actual);
+    }
+}