Refactor how annotations with non-constant arguments are emitted

composer.json

@@ -6,12 +6,12 @@
   "description" : "XP Compiler",
   "keywords": ["module", "xp"],
   "require" : {
-    "xp-framework/core": "^11.0 | ^10.0",
+    "xp-framework/core": "^11.6 | ^10.16",
     "xp-framework/ast": "^10.1",
     "php" : ">=7.0.0"
   },
   "require-dev" : {
-    "xp-framework/reflection": "^2.11",
+    "xp-framework/reflection": "^2.13",
     "xp-framework/test": "^1.5"
   },
   "bin": ["bin/xp.xp-framework.compiler.compile", "bin/xp.xp-framework.compiler.ast"],

src/main/php/lang/ast/emit/AttributesAsComments.class.php

@@ -16,6 +16,27 @@ protected function emitAnnotation($result, $annotation) {
     $result->out->write('\\'.$annotation->name);
     if (empty($annotation->arguments)) return;
 
+    // Check whether arguments are constant, enclose in `eval` array
+    // otherwise. This is not strictly necessary but will ensure
+    // forward compatibility with PHP 8
+    foreach ($annotation->arguments as $argument) {
+      if ($this->isConstant($result, $argument)) continue;
+
+      $escaping= new Escaping($result->out, ["'" => "\\'", '\\' => '\\\\']);
+      $result->out->write('(eval: [');
+      foreach ($annotation->arguments as $name => $argument) {
+        is_string($name) && $result->out->write("'{$name}'=>");
+
+        $result->out->write("'");
+        $result->out= $escaping;
+        $this->emitOne($result, $argument);
+        $result->out= $escaping->original();
+        $result->out->write("',");
+      }
+      $result->out->write('])');
+      return;
+    }
+
     // We can use named arguments here as PHP 8 attributes are parsed
     // by the XP reflection API when using PHP 7. However, we may not
     // emit trailing commas here!

src/main/php/lang/ast/emit/PHP.class.php

@@ -495,24 +495,18 @@ protected function emitAnnotation($result, $annotation) {
       if ($this->isConstant($result, $argument)) continue;
 
       // Found first non-constant argument, enclose in `eval`
-      $result->out->write('(eval: \'');
-      $result->out= new Escaping($result->out, ["'" => "\\'", '\\' => '\\\\']);
-
-      // If exactly one unnamed argument exists, emit its value directly
-      if (1 === sizeof($annotation->arguments) && 0 === key($annotation->arguments)) {
-        $this->emitOne($result, current($annotation->arguments));
-      } else {
-        $result->out->write('[');
-        foreach ($annotation->arguments as $key => $argument) {
-          $result->out->write("'{$key}'=>");
-          $this->emitOne($result, $argument);
-          $result->out->write(',');
-        }
-        $result->out->write(']');
+      $escaping= new Escaping($result->out, ["'" => "\\'", '\\' => '\\\\']);
+      $result->out->write('(eval: [');
+      foreach ($annotation->arguments as $name => $argument) {
+        is_string($name) && $result->out->write("'{$name}'=>");
+
+        $result->out->write("'");
+        $result->out= $escaping;
+        $this->emitOne($result, $argument);
+        $result->out= $escaping->original();
+        $result->out->write("',");
       }
-
-      $result->out= $result->out->original();
-      $result->out->write('\')');
+      $result->out->write('])');
       return;
     }
 

src/test/php/lang/ast/unittest/emit/AnnotationSupport.class.php

@@ -1,6 +1,6 @@
 <?php namespace lang\ast\unittest\emit;
 
-use lang\IllegalArgumentException;
+use lang\{Reflection, IllegalArgumentException};
 use test\{Assert, Expect, Test, Values};
 
 /**
@@ -11,138 +11,192 @@
  */
 abstract class AnnotationSupport extends EmittingTest {
 
+  /**
+   * Declares annotations, optionally including a type
+   *
+   * @param  string $declaration
+   * @return lang.reflection.Type
+   */
+  private function declare($declaration) {
+    return Reflection::type($this->type(
+      $declaration.(strstr($declaration, '<T>') ? '' : ' class <T> { }')
+    ));
+  }
+
+  /**
+   * Returns annotations present in the given type
+   *
+   * @param  lang.reflection.Annotated $annotated
+   * @return [:var[]]
+   */
+  private function annotations($annotated) {
+    $r= [];
+    foreach ($annotated->annotations() as $name => $annotation) {
+      $r[$name]= $annotation->arguments();
+    }
+    return $r;
+  }
+
   #[Test]
   public function without_value() {
-    $t= $this->type('#[Test] class <T> { }');
-    Assert::equals(['test' => null], $t->getAnnotations());
+    Assert::equals(
+      ['Test' => []],
+      $this->annotations($this->declare('#[Test]'))
+    );
   }
 
   #[Test]
   public function within_namespace() {
-    $t= $this->type('namespace tests; #[Test] class <T> { }');
-    Assert::equals(['test' => null], $t->getAnnotations());
+    Assert::equals(
+      ['tests\\Test' => []],
+      $this->annotations($this->declare('namespace tests; #[Test]'))
+    );
   }
 
   #[Test]
   public function resolved_against_import() {
-    $t= $this->type('use unittest\Test; #[Test] class <T> { }');
-    Assert::equals(['test' => null], $t->getAnnotations());
+    Assert::equals(
+      ['unittest\\Test' => []],
+      $this->annotations($this->declare('use unittest\Test; #[Test]'))
+    );
   }
 
   #[Test]
   public function primitive_value() {
-    $t= $this->type('#[Author("Timm")] class <T> { }');
-    Assert::equals(['author' => 'Timm'], $t->getAnnotations());
+    Assert::equals(
+      ['Author' => ['Timm']],
+      $this->annotations($this->declare('#[Author("Timm")]'))
+    );
   }
 
   #[Test]
   public function array_value() {
-    $t= $this->type('#[Authors(["Timm", "Alex"])] class <T> { }');
-    Assert::equals(['authors' => ['Timm', 'Alex']], $t->getAnnotations());
+    Assert::equals(
+      ['Authors' => [['Timm', 'Alex']]],
+      $this->annotations($this->declare('#[Authors(["Timm", "Alex"])]'))
+    );
   }
 
   #[Test]
   public function map_value() {
-    $t= $this->type('#[Expect(["class" => \lang\IllegalArgumentException::class])] class <T> { }');
-    Assert::equals(['expect' => ['class' => IllegalArgumentException::class]], $t->getAnnotations());
+    Assert::equals(
+      ['Expect' => [['class' => IllegalArgumentException::class]]],
+      $this->annotations($this->declare('#[Expect(["class" => \lang\IllegalArgumentException::class])]'))
+    );
   }
 
   #[Test]
   public function named_argument() {
-    $t= $this->type('#[Expect(class: \lang\IllegalArgumentException::class)] class <T> { }');
-    Assert::equals(['expect' => ['class' => IllegalArgumentException::class]], $t->getAnnotations());
+    Assert::equals(
+      ['Expect' => ['class' => IllegalArgumentException::class]],
+      $this->annotations($this->declare('#[Expect(class: \lang\IllegalArgumentException::class)]'))
+    );
   }
 
   #[Test]
   public function closure_value() {
-    $t= $this->type('#[Verify(function($arg) { return $arg; })] class <T> { }');
-    $f= $t->getAnnotation('verify');
-    Assert::equals('test', $f('test'));
+    $verify= $this->annotations($this->declare('#[Verify(function($arg) { return $arg; })]'))['Verify'];
+    Assert::equals('test', $verify[0]('test'));
   }
 
   #[Test]
   public function arrow_function_value() {
-    $t= $this->type('#[Verify(fn($arg) => $arg)] class <T> { }');
-    $f= $t->getAnnotation('verify');
-    Assert::equals('test', $f('test'));
+    $verify= $this->annotations($this->declare('#[Verify(fn($arg) => $arg)]'))['Verify'];
+    Assert::equals('test', $verify[0]('test'));
   }
 
   #[Test]
   public function array_of_arrow_function_value() {
-    $t= $this->type('#[Verify([fn($arg) => $arg])] class <T> { }');
-    $f= $t->getAnnotation('verify');
-    Assert::equals('test', $f[0]('test'));
+    $verify= $this->annotations($this->declare('#[Verify([fn($arg) => $arg])]'))['Verify'];
+    Assert::equals('test', $verify[0][0]('test'));
   }
 
   #[Test]
   public function named_arrow_function_value() {
-    $t= $this->type('#[Verify(func: fn($arg) => $arg)] class <T> { }');
-    $f= $t->getAnnotation('verify');
-    Assert::equals('test', $f['func']('test'));
+    $verify= $this->annotations($this->declare('#[Verify(func: fn($arg) => $arg)]'))['Verify'];
+    Assert::equals('test', $verify['func']('test'));
   }
 
   #[Test]
   public function single_quoted_string_inside_non_constant_expression() {
-    $t= $this->type('#[Verify(fn($arg) => \'php\\\\\'.$arg)] class <T> { }');
-    $f= $t->getAnnotation('verify');
-    Assert::equals('php\\test', $f('test'));
+    $verify= $this->annotations($this->declare('#[Verify(fn($arg) => \'php\\\\\'.$arg)]'))['Verify'];
+    Assert::equals('php\\test', $verify[0]('test'));
   }
 
   #[Test]
   public function has_access_to_class() {
-    $t= $this->type('#[Expect(self::SUCCESS)] class <T> { const SUCCESS = true; }');
-    Assert::equals(['expect' => true], $t->getAnnotations());
+    Assert::equals(
+      ['Expect' => [true]],
+      $this->annotations($this->declare('#[Expect(self::SUCCESS)] class <T> { const SUCCESS = true; }'))
+    );
   }
 
   #[Test]
   public function method() {
-    $t= $this->type('class <T> { #[Test] public function fixture() { } }');
-    Assert::equals(['test' => null], $t->getMethod('fixture')->getAnnotations());
+    $t= $this->declare('class <T> { #[Test] public function fixture() { } }');
+    Assert::equals(
+      ['Test' => []],
+      $this->annotations($t->method('fixture'))
+    );
   }
 
   #[Test]
   public function field() {
-    $t= $this->type('class <T> { #[Test] public $fixture; }');
-    Assert::equals(['test' => null], $t->getField('fixture')->getAnnotations());
+    $t= $this->declare('class <T> { #[Test] public $fixture; }');
+    Assert::equals(
+      ['Test' => []],
+      $this->annotations($t->property('fixture'))
+    );
   }
 
   #[Test]
   public function param() {
-    $t= $this->type('class <T> { public function fixture(#[Test] $param) { } }');
-    Assert::equals(['test' => null], $t->getMethod('fixture')->getParameter(0)->getAnnotations());
+    $t= $this->declare('class <T> { public function fixture(#[Test] $param) { } }');
+    Assert::equals(
+      ['Test' => []],
+      $this->annotations($t->method('fixture')->parameter(0))
+    );
   }
 
   #[Test]
   public function params() {
-    $t= $this->type('class <T> { public function fixture(#[Inject(["name" => "a"])] $a, #[Inject] $b) { } }');
-    $m= $t->getMethod('fixture');
+    $t= $this->declare('class <T> { public function fixture(#[Inject(["name" => "a"])] $a, #[Inject] $b) { } }');
     Assert::equals(
-      [['inject' => ['name' => 'a']], ['inject' => null]],
-      [$m->getParameter(0)->getAnnotations(), $m->getParameter(1)->getAnnotations()]
+      ['Inject' => [['name' => 'a']]],
+      $this->annotations($t->method('fixture')->parameter(0))
+    );
+    Assert::equals(
+      ['Inject' => []],
+      $this->annotations($t->method('fixture')->parameter(1))
     );
   }
 
   #[Test]
   public function multiple_class_annotations() {
-    $t= $this->type('#[Resource("/"), Authenticated] class <T> { }');
-    Assert::equals(['resource' => '/', 'authenticated' => null], $t->getAnnotations());
+    Assert::equals(
+      ['Resource' => ['/'], 'Authenticated' => []],
+      $this->annotations($this->declare('#[Resource("/"), Authenticated]'))
+    );
   }
 
   #[Test]
   public function multiple_member_annotations() {
-    $t= $this->type('class <T> { #[Test, Values([1, 2, 3])] public function fixture() { } }');
-    Assert::equals(['test' => null, 'values' => [1, 2, 3]], $t->getMethod('fixture')->getAnnotations());
+    $t= $this->declare('class <T> { #[Test, Values([1, 2, 3])] public function fixture() { } }');
+    Assert::equals(
+      ['Test' => [], 'Values' => [[1, 2, 3]]],
+      $this->annotations($t->method('fixture'))
+    );
   }
 
   #[Test]
   public function multiline_annotations() {
-    $t= $this->type('
+    $annotations= $this->annotations($this->declare('
       #[Authors([
         "Timm",
         "Mr. Midori",
       ])]
       class <T> { }'
-    );
-    Assert::equals(['authors' => ['Timm', 'Mr. Midori']], $t->getAnnotations());
+    ));
+    Assert::equals(['Authors' => [['Timm', 'Mr. Midori']]], $annotations);
   }
 }