marcioalmada / annotations Goto Github PK

The majority of php annotation frameworks are based on doctrine/annotations. They support class based annotations only, with constructor, property and setter based data injection in the following format:

@Class("constructor param", "constructor param 2")
@Class(propertyOne = "value one", propertyTwo = "value two")

In order to allow migration from other frameworks, this syntax should be supported.

Implement a way to extend or maybe completely overwrite Minime\Annotations\Parser at runtime.

This package fixes the Nikic PHP parser dependency at 1.* which is ancient. Please upgrade!

This breaks api so this change will happen on version 2.0 only.

Currently there is a method AnnotationBag::getAsArray()
That's very convenient when we want to make sure to retrieve an array.

What's about adding a method
AnnotationBag::getAsString() or AnnotationBag::getAsRawString()
to get the annotation value as a string. Even if it's in array or object annotation,
we sometime want to get this value as the raw original string.

We could do that by changing the parser. But sometime we can need both of the ouput
depending on the annotation name.

What's your opinion ?

Type to allow more dynamic values in annotations. Some examples:

/**
 * @request.token eval md5(uniqid(rand(), true))
 * @my-range      eval range(0, 5)
 * @number        eval base_convert('A37334', 16, 2)
 */

Some people might argue that eval is always evil. I agree with that, but only if untrusted input is involved.

Another idea would be to allow just some simple math calculations. The example below sets one day in milliseconds, much more expressive and maintainable than a 86400000 value.

/**
 * @cache.duration math 1000 * 60 * 60 * 24
 */

I prefer eval type, since it already brings math functionality, or maybe we can have both.

The static Facade class does not allow you to switch out the Parser or ParserRules implementation

There should be an AnnotationReaderInterface and a default implementation which takes a ParserInterface and ParserRulesInterface as constructor parameters.

I would be happy to contribute this as a branch of v2 if you would like.

Internal Types\* classes were renamed due to PHP7 bc breaks.

It's prudent to do a major release when first php7-RC arrives: 8cf541d, 558b0e6, 7d7c489

As of now the string to be parse $raw_doc_block is passed in the constructor. This behaviour leads to having to instantiate the class each time you want to parse a string.

It would be more efficient if the string was directly passed to the parse method, so that one class could be used several times.

$parser = new Parser(new ParserRules);
$data = $parser->parse($string);

This change means:

• Removing the $raw_doc_block protected property from the Parser class;
• Updating the facade and the traits scripts;
• Updating Unit Testing files;

This change if accepted should be schedule for the library v2

Annotations follow a strange versionning which adheres more or less to semver.org
Semver states that a versionning number has 3 parts X.Y.Z
when Y is incremented Z should be resetted to 0. this is not the case for the version 1
Can we just stick to semver starting from version 2. So that the version number is well understood at first glance ?

For project organization reasons I need my concrete annotation classes to reside in a deeply nested namespace, but I don't want to type the whole namespace path every time I use the annotation. It would be very welcome if I could register namespace prefix(es) for concrete annotation classes.

The Parser::parse method used to return an AnnotationsBag instance. But there was a coupling problem that was solved by the introduction of the ParserRulesInterface.

Since then the Parser::parse method return an array which is feed independently to the AnnotationsBag constructor.
Since the reason for further decoupling has vanished would it be more useful to make the Parser::parse method return directly an AnnotationsBag instance like before ?

This changes means:

• Changing the Parser::parse method return signature
• Updating the facade and the traits scripts;
• Updating Unit Testing files;

This change if accepted should be schedule for the library v2

Pass all unit tests on HHVM.

Hi,
Just got an issue while parsing annotation when the line following the annotation does not have a whitespace after the star character :

php code :

$annotations = $reader->getClassAnnotations("someClass");
$foundTypes = $annotations->getAsArray("name");

Annotation with a white space (working as expected) :

/**
 * @name value
 *[WHITESPACE HERE]
 */

output > "value"

Annotation without a white space (issue) :

/**
 * @name value
 *[NO whitespace]
 */

output > "value
/"

This parser will be a specialization of current Minime\Annotations\Parser and will not have types. This will allow anyone to implement annotation APIs without allowing explicit types declaration.

Parser could support inline docblocks, like following:

class FooClass {
    /** @value foo */
    private $inline_docblock_fixture;

    /** @alpha */
    private $inline_docblock_implicit_boolean_fixture;

    /** @alpha @beta @gama */
    private $inline_docblock_multiple_implicit_boolean_fixture;
}

There was an implicit contract between a Parser implementation and AnnotationsBag through the sharing of a common ParserRules object. This was a bad move in the past, so we will get rid of ParserRules.

This will make it easier to implement new annotation parsers with different annotation DSLs.

Refers to #41 #43

The current version of the annotation-parser does not tell between a single JSON-array and multiple simple annotations. But it should!

$parser = new \Minime\Annotations\Parser;
$this->assertNotSame(
    $parser->parse('@foo 1 @foo 2 @foo 3'),
    $parser->parse('@foo [1, 2, 3]')
); // FAIL. Parser errorneously gives the same result for both strings

$this->assertSame(
    ['foo' => [1, 2, 3]],
    $parser->parse('@foo 1 @foo 2 @foo 3') 
); // PASS. Parser works correctly on this

$this->assertSame(
    ['foo' => [[1, 2, 3]]],
    $parser->parse('@foo [1, 2, 3]') 
); // FAIL. Parser fails to preserve the array annotated with @foo

in the V2 channel
the AnnotationsBagInterface seems to require deprecated methods:

• public function grepNamespace($pattern);
• public function merge(AnnotationsBag $bag); //if kept should Typehint to AnnotationsBagInterface instead

Since it's V2 why not just remove them completely ?

tl;dr:
If you merge 2 AnnotationsBag object with different parserRules, the resulting AnnotationsBag will behave incorrectly. the export method will export data that can not be reach using the get or the getAsArray method

Usually I would directly submit a fix using a pull request ... but I'm too busy right now and It may take me 2 weeks to do it properly with the unit testing and all so I'll include here 2 proposals to fix the bug since I think this is urgent.

The problem

Imagine you have 2 AnnotationsBag ($lowerBag and $upperBag) with 2 differents PaserRules.

$lowerBag ParserRules states that arguments must be lowercase
$upperBag ParserRules states that arguments must be uppercase

$lowerBag->export(); // ['toto' => true, 'tata' => true, 'foo' => 'bar'];
$upperBag->export(); // ['TOTO' => true, 'TATA' => true, 'FOO' => 'bar'];
$lowerBag->merge($upperBag);
$lowerBag->export(); // ['toto' => true, 'tata' => true, 'foo' => 'bar', 'TOTO' => true, 'TATA' => true, 'FOO' => 'bar'];
$lowerBag->get('TOTO'); // will throw an InvalidArgumentException !!!

$lowerBag behaviour following the merge call is totally buggy and must be fix!!

Fixing Proposals

Proposal 1: keep AnnotationsBag immutable

Making the merge method behave like grep and useNamespace to keep AnnotationsBag immutable.

public function merge(AnnotationsBag $bag)
{
    $res = array_merge($this->attributes, $bag->export());
    return new static($res, $this->rules);
}

This fix the problem.

Benefits:

• The resulting AnnotationsBag filter out the wrong values on constructor call.
• AnnotationsBag stay immutable

Disadvantages:

• The merge method will no longer affect the parent object like intended (IMHO this is not a disadvantage)

Proposal 2: add a replace method to AnnotationsBag

Create a new method called replace (I'll leave to you to decide this method visibility) making it public would open a whole new range of possibilities :) .

public function replace(array $attributes = array())
{
    foreach ($attributes as $key => $value) {
        if ($this->rules->isKeyValid($key)) {
            $this->attributes[$key] = $attributes[$key];
        }
    }

    return $this;
}

With this function you can then update the __construct and merge methods as follow:

public function __construct(array $attributes, ParserRulesInterface $rules)
{
    $this->rules = $rules;
    $this->replace($attributes);
}

public function merge(AnnotationsBag $bag)
{
    return $this->replace($bar->export());
}

Benefits:

• you get a new method replace
• you don't change merge behaviour (IMHO this is a disadvantage)

Final notes

Keep in mind that those 2 proposals are not exclusive. You can still implement the replace method even if you choose to use proposal 1.

I hope this will help you fix the bug

nyamsprod

https://camo.githubusercontent.com/05ec0993de41940d544b659276480b0f19dc8aa1/68747470733a2f2f646c2e64726f70626f7875736572636f6e74656e742e636f6d2f752f34393534393533302f616e6e6f746174696f6e732f6772616d6d61722e706e67

The objective is to add a KISS plug and play cache implementation for the AnnotationsReader.

Requirements:

• Reader class must allow easy cache handler injection
• The CacheInterface must be as simple as possible
• The CacheInterface should encourage reuse of cache for identical doc block strings even if they come from different sources.
• At least two cache alternatives should be available for immediate use (FileCache, MemoryCache)
• A benchmark should be added for each implemented cache handler

Cheers!

When looking into at the CacheInterface::getKey method and how it is used I think the method should be remove from the CacheInterface.

The way the docblock is transcoded into the cache key should never be exposed to the object using the cache interface. The class should only care about giving a docblock and expecting from the CacheInterface a response. So instead of having:

<?php
public function getAnnotations(\Reflector $Reflection)
{
    $doc = $Reflection->getDocComment();
    if ($this->cache) {
        $key = $this->cache->getKey($doc);
        $ast = $this->cache->get($key);
        if (! $ast) {
            $ast = $this->parser->parse($doc);
            $this->cache->set($key, $ast);
        }
    } else {
        $ast = $this->parser->parse($doc);
    }
    return new AnnotationsBag($ast);
}

you would have

<?php
public function getAnnotations(\Reflector $Reflection)
{
    $doc = $Reflection->getDocComment();
    if (! $this->cache) {
        $ast = $this->parser->parse($doc);
        return new AnnotationsBag($ast);
    }

    $ast = $this->cache->get($doc, $key);
    if (! $ast) {
        $ast = $this->parser->parse($doc);
        $this->cache->set($doc, $ast);
    }
    return new AnnotationsBag($ast);

}

The code becomes simpler and the Cache Adapter more flexible in how it can handles the docblock as a key. Thoughts ?

Nice I see that version 2.0 is out!!
It would be great if you could include in the library a CHANGELOG.md so that users can easily found what's new/changed/deprecated/removed from this new major release. I've added this kind of file in my league/csv library if you need a PHP example of what could be find in it. The format is based on http://keepachangelog.com/

I'm very inclined to use desarrolla2/Cache as the default cache for Minime\Annotations.

Any good idea?

I've been using this library for a couple of days. Yesterday i found what it looks like a parser issue: whenever an annotation value is set to an accepted optional type the parser ignores it. This example class will illustrate the issue:

<?php
class UserModel {
    /**
     * @column user_id
     * @type integer
     **/
    public $id;

    /**
     * @column user_name
     * @type string
     **/
    public $name;
}

While the column annotation is parsed correctly, both type annotations are ignored (actually, they are created but leaved in blank). I figured out that this is caused by scanning the type of an annotation without checking if there's a remaining string to parse.

// Parser.php: 76
$type = $line->scanType($types_pattern, 'dynamic');
$parameters[$key][] = self::parseValue($line->getRemainder(), $type);

I modified these lines in the following way:

$type = $line->scanType($types_pattern, 'dynamic');

if ($line->getRemainder() != '') {
    $parameters[$key][] = self::parseValue($line->getRemainder(), $type);
}
else {
    $parameters[$key][] = $type;
}

Running the tests with this modification worked flawlessly, but i might be ignoring something. I'm developing a library using this code so it would be great if you can check if this is a real issue or not. Bye.

When looking a the AnnotationsBag Interface I can help but wondering if the export method should not be renamed toArray in the version 2 to be more in line with other PHP library. This method basically convert the AnnotationBags data into an array. Which is what the toArray is supposed to do.
This is not just a cosmetic change but also a way to standardized a common pattern by using the commonly method name use in the PHP community. And it helps understand more easily the library.

I have a problem if I used this kind off annotation

/**
 * @export
 * @request.put
 *
 * @param string $idloja
 * @param string $idtransacao
 * @param float $valor
 * @return PaymentStatusClass
 * @throws Exception
 */
public function cancelPayment(string $idloja, string $idtransacao, float $valor = 0.00){

this gave me a error Raw value must be float. Invalid value '' given.
if I change @param float $valor for @param string $valor it's work, but the code is incorrect, because I wait for float and not string.

Given a comment like this:

/**
 * A valid URL is:
 * scheme://user:[email protected]
 *
 * The getReturnType() method will get the return type of the method,
 * by parsing its @return tag in the comment block.
 * 
 * @return void
 */

The parser results in:

array (
  'domain.com' => 'The getReturnType() method will get the return type of the method,
by parsing its',
  'return' => 
  array (
    0 => 'tag in the comment block.',
    1 => 'void',
  ),
)

Would it not be simpler to search for annotations starting with: <any whitespace><asterix character><any whitespace> (\s*\*\s*)

Whenever i try getting all annotations from a class and its properties (without using Minime\Annotations\Traits\Reader) i end up writing something like this:

$rc = new ReflectionClass($classFullName);
$properties = $rc->getProperties();

foreach ($properties as $property) {
    $annotations = Facade::getPropertyAnnotations($classFullName, $property->getName());

    //...
}

In short, i end up creating 2 instances of ReflectionProperty for each class attribute: one is created within the array returned by ReflectionClass::getProperties, the other is created when calling Facade::getPropertyAnnotations.

I think it would be a great idea to allow calling Facade::getAnnotations as a public method. The previous code will look like this:

$rc = new ReflectionClass($classFullName);
$properties = $rc->getProperties();

foreach ($properties as $property) {
    $annotations = Facade::getAnnotations($property);

    //...
}

Actually, i think this could also reduce some overhead and memory consumption for these scenarios.

Parser method is too long and it's getting complicated. CRAP index is currently around 8. This needs a fix before version 2.

The crux of the issue is that if you wanted to add (or replace) a type you couldn't do that without a new Parser right now because of the $types array and the hard coded namespace in parseValue.

Create TypeResolver interface and pass as dependency of Parser

php --version
PHP 5.5.3-1ubuntu2.1 (cli) (built: Dec 12 2013 04:22:11) 
Copyright (c) 1997-2013 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2013 Zend Technologies
    with Zend OPcache v7.0.3-dev, Copyright (c) 1999-2013, by Zend Technologies
    with Xdebug v2.2.3, Copyright (c) 2002-2013, by Derick Rethans

When using this PHP configuration with Ubuntu 13.10 knowing that I'm using
php5-json 1.3.1+dfsg-2 I get a the following error

$ phpunit
PHPUnit 3.7.28 by Sebastian Bergmann.

Configuration read from /var/www/annotations/phpunit.xml.dist

....................................F.................

Time: 1.29 seconds, Memory: 3.75Mb

There was 1 failure:

1) Minime\Annotations\ParserTest::parseFloatFixture
Failed asserting that Array (
    0 => '.45'
    1 => 0.45
    2 => 45.0
    3 => -4.5
) is identical to Array (
    0 => 0.45
    1 => 0.45
    2 => 45.0
    3 => -4.5
).

/path/to/annotations/test/suite/Minime/Annotations/ParserTest.php:102

Code says it all:

class Foo
{
    /**
     * @json_value json [
     *     "x", "y"
     * ]
     * @json_value json {
     *    "x": {
     *        "y": "z"
     *    }
     * }
     */
    private $strong_typed_fixture;
}

the AnnotationsBag::merge method is wrongly named since it creates a new AnnotationsBag which is the union of the two AnnotationsBag instances.

    /**
     * @test
     */
    public function merge()
    {
        $Bag1 = new AnnotationsBag(
            [ 'alpha' => 'a',  'beta'  => 'b',  'gama'  => 'g'],
            $this->Rules
        );

        $Bag2 = new AnnotationsBag(
            ['alpha'  => 'x', 'beta'  => 'x', 'gama' => 'x', 'delta' => 'd', 'epsilon' => 'e'],
            $this->Rules
        );

        $MergedBag = $Bag1->merge($Bag2);
        $this->assertSame('a', $MergedBag->get('alpha'));
        $this->assertSame('b', $MergedBag->get('beta'));
        $this->assertSame('e', $MergedBag->get('epsilon'));
    }

This is misleading since a user without looking at the code may think the method acts in the same way as a PHP array_merge which is not the case.

I propose renaming the method as AnnotationsBag::union and adding a proper AnnotationsBag::merge method.

    /**
     * @test
     */
    public function merge()
    {
        $Bag1 = new AnnotationsBag(
            [ 'alpha' => 'a',  'beta'  => 'b',  'gama'  => 'g'],
            $this->Rules
        );

        $Bag2 = new AnnotationsBag(
            ['alpha'  => 'x', 'beta'  => 'x', 'gama' => 'x', 'delta' => 'd', 'epsilon' => 'e'],
            $this->Rules
        );

        $MergedBag = $Bag1->merge($Bag2);
        $this->assertSame('x', $MergedBag->get('alpha'));
        $this->assertSame('x', $MergedBag->get('beta'));
        $this->assertSame('e', $MergedBag->get('epsilon'));
    }

As this will break an existing functionality. This change should be clearly announced before being introduced.

Its not compatible with current phpunit implementation.
Also the pull requests are not maintained any more.

Is this package abandoned?

It seems ext\json and pecl-json-c have some subtle parsing differences. This affects some edge cases involving floats and integers. Problem happens only when pecl-json-c extension is installed.

There were 2 failures:

1) Minime\Annotations\ParserTest::parseIntegerFixture
Failed asserting that Array (
    1 => '023'
) is identical to Array (
    1 => 23
).

2) Minime\Annotations\ParserTest::parseFloatFixture
Failed asserting that Array (
    0 => '.45'
) is identical to Array (
    0 => 0.45
).

TODO:

• Enable pecl-json-c integer trailing zero support, using JSON_PARSER_NOTSTRICT
• Talk to pecl-json-c maintainer to see if there is a chance to make JSON_PARSER_NOTSTRICT accept trailing dot for floats (makes more sense than to hack Parser).

Ideal solution right now is to split Parser::parseDynamic strategy when encountering edge cases like trailing dot and trailing zeros.

Hi, as says the tilte is there a way to get constant annotations ? As far as I cheked I didnt find such a feature.

Concrete annotations are annotations that map to concrete classes. Ex:

<?php
/**
 * @Entity ->
 * @InheritanceType -> "JOINED"
 * @DiscriminatorColumn -> {"name" : "desc", "type" : "string"}
 * @DiscriminatorMap -> {"person" : "Person", "employee" : "Employee"}
 */

Spec:

• Concrete type is defined by -> symbol
• Syntax should look like this @Annotation -> {valid json}
• Parameters must be only a valid json string
• Namespaces should be supported @App\Annotation
• Concrete class must exist otherwise an exception is throw

Constraints:

• This functionality should not break backwards compatibility

There seems to be an issue regarding the way an annotation type is parsed. Whenever a value is found that begins with one of the accepted types, the parser fails to obtain the complete value. This example illustrates the problem:

<?php
include_once 'vendor/autoload.php';

/**
 * @type stringed
 */ 
class Violin {
    use Minime\Annotations\Traits\Reader;
}

$v = new Violin();
$annotations = $v->getClassAnnotations();

//prints "ed"
echo $annotations->get('type');
?>

I found out that by modifying this line

$this->types_pattern = '/^('.implode('|', $this->types).')(\s)*(\S)+/';

with this

$this->types_pattern = '/^('.implode('|', $this->types).')$/';

seems to fix this problem.

Hello.
Do you planning to support nikic/php-parser versions higher than 4.0 ?

Thanks.