Skip to content
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

Adopt PHP 8 annotation syntax #336

Open
thekid opened this issue Sep 6, 2020 · 11 comments
Open

Adopt PHP 8 annotation syntax #336

thekid opened this issue Sep 6, 2020 · 11 comments

Comments

@thekid
Copy link
Member

thekid commented Sep 6, 2020

Scope of Change

With https://wiki.php.net/rfc/shorter_attribute_syntax_change, PHP 8's annotation syntax has become #[Annotation]. XP Framework has been using a very similar syntax for the past 15 years, #[@annotation]. This RFC suggests changing the annotation functionality to support PHP 8 syntax starting with the next feature release so libraries can start adopting.

Rationale

With the introduction of a native annotation syntax in PHP, the XP Framework's annotations have become syntax errors. The above RFC's authors were aware of the fact that this is a BC break but regarded its outcome neglectable. This may seem frustrating, but on the other hand it's nice the new syntax closely resembles what we've been used to!

Functionality

This is how annotations will look like once we adopt PHP's native syntax:

use unittest\{Assert, Test, Expect, Values};
use lang\ElementNotFoundException;

class ListOfTest {

  #[Test]
  public function size_of_empty_list() {
    Assert::equals(0, (new ListOf())->size());
  }

  #[Expect(class: ElementNotFoundException::class), Values([[[], 0], [[1], 1]], [[1, 2, 3], 100]])]
  public function accessing_non_existant($elements, $offset) {
    (new ListOf($elements))->get($offset);
  }
}

Adoption

A multi-step adoption strategy is outlined below:

Step 1: Allow PHP syntax

Starting in the next 10.X feature release, XP Framework will start allowing PHP native syntax alongside the XP Framework's own syntax. PHP attributes #[Test(true)] will be interpreted the same as #[@test(true)].

Step 2: Deprecate XP syntax

With the next major release (11.0), XP Framework will continue to support its own syntax, however raise deprecation warnings. This will keep production code working but will fail unittests.

Step 3: Remove XP syntax

With the major release following that (12.0), XP Framework will remove support for the deprecated XP syntax and only support PHP 8 native annotations (with some additions, see below).

Values

XP Framework offers support for various expressions inside annotation values that PHP native syntax does not support. This is commonly used in the @values annotation inside unittests:

use unittest\Assert;

class ListOfTest {

  #[@test, @values([[new ListOf()], [ListOf::$EMPTY]])]
  public function size_of_empty($list) {
    Assert::equals(0, $list->size());
  }

  #[@test, @values([
  #  [[1, 2, 3]],
  #  [new \ArrayObject([1, 2, 3])],
  #  [function() { yield 1; yield 2; yield 3; }],
  #])]
  public function can_create_from($elements) {
     Assert::equals([1, 2, 3], (new ListOf($elements))->toArray());
  }
}

In PHP, all of these except for the static array [1, 2, 3] yield Compile error (Constant expression contains invalid operations). While considered as potential future benefit in the PHP RFC here, this is currently not implemented. While these can be rewritten to helper methods (see below), this results in code noise and forces authors to think of names for the helper methods, especially when using multiple value-driven tests in a class.

use unittest\{Assert, Test, Values};

class ListOfTest {

  /** @return iterable */
  private function emptyLists() { return [[new ListOf()], [ListOf::$EMPTY]]; }

  #[Test, Values('emptyLists')]
  public function size_of_empty($list) {
    Assert::equals(0, $list->size());
  }
}

One way to overcome this is to use the special named argument eval and evaluate the expression:

use unittest\{Assert, Test, Values};

class ListOfTest {

  #[Test, Values(eval: '[[new ListOf()], [ListOf::$EMPTY]]')]
  public function size_of_empty($list) {
    Assert::equals(0, $list->size());
  }
}

Name resolution

Another difference is that names are resolved in the PHP native syntax, while XP annotations are simply used as-is. So while #[Test] will refer to the type unittest.Test (while not necessarily requiring it to actually exist!) and resolve to "unittest\Test", the old #[@test] simply yields "test".

This would create a strain on library authors, which would have to refactor their code to support both. By continuing to parse PHP 8 attributes from the codebase, this issue is addressed in backwards-compatible way.

Security considerations

n/a

Speed impact

Slightly slower to be able to support both.

Dependencies

PHP 8 for the new native syntax.

Related documents

@thekid
Copy link
Member Author

thekid commented Sep 6, 2020

XP Compiler has been updated to support PHP 8 attributes via xp-framework/ast#10

@thekid
Copy link
Member Author

thekid commented Sep 12, 2020

Before the name resolution compatiblity issue is not addressed, annotations would need to be rewritten to the PHP 8 syntax with a leading \:

$ git diff
diff --git a/src/main/php/de/thekid/shorturl/Urls.php b/src/main/php/de/thekid/shorturl/Urls.php
index e689a9f..2efca44 100755
--- a/src/main/php/de/thekid/shorturl/Urls.php
+++ b/src/main/php/de/thekid/shorturl/Urls.php
@@ -5,7 +5,7 @@ use rdbms\DriverManager;
 class Urls {
   private $conn;

-  public function __construct(<<inject('db-dsn')>> string $dsn) {
+  public function __construct(#[\inject('db-dsn')] string $dsn) {
     $this->conn= DriverManager::getConnection($dsn);
   }

Update: Solved by parsing PHP 8 attributes manually.

@thekid
Copy link
Member Author

thekid commented Sep 13, 2020

The original idea was to add values alongside annotations, see xp-framework/ast#14:

use unittest\{Assert, Test, Values};

class ListOfTest {

  #$fixtures: [[new ListOf()], [ListOf::$EMPTY]])}
  #[Test, Values(using: '$fixtures')]
  public function size_of_empty($list) {
    Assert::equals(0, $list->size());
  }
}

As an alternative to implementing a new "values" construct, we could simply add an alternative attribute syntax which can easily be converted to native syntax once PHP supports more than just constant expressions, e.g.:

use unittest\{Assert, Test, Values};

class ListOfTest {

  #{Values([[new ListOf()], [ListOf::$EMPTY]])}
  #[Test]
  public function size_of_empty($list) {
    Assert::equals(0, $list->size());
  }
}

This would mean always having to tokenize the source code when fetching annotations, though.

thekid added a commit to xp-framework/ast that referenced this issue Sep 13, 2020
See xp-framework/rfc#336, values are key/value pairs that can be
attached to types and type members. They serve as a complement to
PHP 8 attributes which do not support arbitrary expressions in
their argument lists.
@thekid
Copy link
Member Author

thekid commented Sep 23, 2020

Release

https://github.com/xp-framework/core/releases/tag/v10.2.0 contains step 1 for PHP 8 attribute support. See xp-forge/sequence#51 for an example of a converted library.

thekid added a commit to xp-framework/compiler that referenced this issue Sep 26, 2020
@thekid
Copy link
Member Author

thekid commented Sep 27, 2020

XP meta cache forward and backwards compatibility

By removing namespaces and lowercasing the first letter, annotations can be converted in a backward compatible way:

// Example from your project
class Before {

  #[@test]
  public function test() { }
}

// Converted example from your project
use unittest\Test;

class After {

  #[Test]
  public function test() { }
}

// Inside a library, the code accessing annotations stays the same
foreach ([Before::class, After::class] as $class) {
  (new XPClass($class))->getMethod('test')->getAnnotations(); // ["test" => null]
}

XP Compiler generates the cached xp::$meta as a way of speeding up reflection for conpiled code, which perfectly serves the XP Framework, but cannot be used for the new xp-framework/reflect library, as this expected annotations to be keyed with fully qualified names. It would need to parse the details from the code again, which is unnecessary overhead.

We could add mappings to the exisiting meta information using the PHP qualified class names as seen below:

xp::$meta['fully.qualified.Type']= [
  0 => [
    'property1' => [
      DETAIL_ANNOTATIONS => <annotations>,
      DETAIL_TARGET_ANNO => ['lang\Type' => 'type'], 
    ]
  ],
  1 => [
    'method1' => [
      DETAIL_ARGUMENTS   => ['string', 'int'],
      DETAIL_RETURNS     => ['iterable'],
      DETAIL_THROWS      => [],
      DETAIL_COMMENT     => 'Test'
      DETAIL_ANNOTATIONS => <annotations>,
      DETAIL_TARGET_ANNO => ['unittest\Test' => 'test', '$param1' => <annotations>, ...],
    ]
  ],
  'class' => [
    DETAIL_ANNOTATIONS => <annotations>,
    DETAIL_TARGET_ANNO => ['unittest\Action' => 'action'],
  ]
];

When reading from a property, method or type, the following will yield annotations back in attribute form:

/**
 * Constructs annotations from meta information
 *
 * @param  [:var] $meta
 * @return [:var]
 */
private function annotations($meta) {
  $r= [];
  foreach ($meta[DETAIL_ANNOTATIONS] as $name => $value) {
    $r[$meta[DETAIL_TARGET_ANNO][$name] ?? $name]= (array)$value;
  }
  return $r;
}

@thekid
Copy link
Member Author

thekid commented Oct 1, 2020

XP Compiler, https://github.com/xp-framework/compiler/releases/tag/v5.4.0, supports annotation mapping via TARGET_ANNO.

@thekid
Copy link
Member Author

thekid commented Oct 4, 2020

Migrated all actively developed libraries from xp-lang, xp-forge and xp-framework today ✅

@thekid
Copy link
Member Author

thekid commented Oct 21, 2021

Deprecation warnings will now show up in XP 11

@thekid
Copy link
Member Author

thekid commented Oct 21, 2021

Support for XP annotation syntax will not be removed until XP 12

@thekid
Copy link
Member Author

thekid commented Oct 21, 2021

@thekid
Copy link
Member Author

thekid commented Mar 23, 2024

Step 3 completed in XP 12

https://github.com/xp-framework/core/releases/tag/v12.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant