Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-77

PHP Warning when trying to skip non-existaint Annotations

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0-ALPHA2
    • Fix Version/s: 2.0-ALPHA3
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      Description

      If you have a docblock that ends with an annotation, for example:

      /**
       * @return
       */
      

      it throws a PHP warnings, because is_subclass_of() requires the $class to be existent.

      Fix: Add class_exists($name) before.

        Activity

        Hide
        Benjamin Eberlei added a comment -

        Fixed in r6583, first commit \o/

        However, I cant change the assignee here now anymore.

        Show
        Benjamin Eberlei added a comment - Fixed in r6583, first commit \o/ However, I cant change the assignee here now anymore.
        Hide
        Roman S. Borschel added a comment -

        Would class_exists(..., false) work, too? Because by default class_exists might trigger autoloading and this is sometimes not wanted for performance reasons (if a class does not exist, each call to class_exists might trigger autoload again for that class, over and over). So if possible, always do class_exists(...., false) unless you really have the intention that autoloading is invoked, then use class_exists(..., true) to make that clear.

        Show
        Roman S. Borschel added a comment - Would class_exists(..., false) work, too? Because by default class_exists might trigger autoloading and this is sometimes not wanted for performance reasons (if a class does not exist, each call to class_exists might trigger autoload again for that class, over and over). So if possible, always do class_exists(...., false) unless you really have the intention that autoloading is invoked, then use class_exists(..., true) to make that clear.
        Hide
        Benjamin Eberlei added a comment -

        Isn't that what the caching is for in the first place?

        But yes, there could be another layer that caches the class_exists calls, however then the complete condition has to be extracted into a method (which would probably be good anyhow, since its not readable).

        Show
        Benjamin Eberlei added a comment - Isn't that what the caching is for in the first place? But yes, there could be another layer that caches the class_exists calls, however then the complete condition has to be extracted into a method (which would probably be good anyhow, since its not readable).
        Hide
        Roman S. Borschel added a comment -

        What caching? Probably you misunderstood me. The second parameter of class_exists defaults to true which can have unwanted consequences with ugly autoloaders.

        I changed it to class_exists($name, false) and your test still runs fine, so I guess its ok.

        Even when you want autoload to be invoked I would recommend using explicitly: class_exists($className, true) because that makes it clear that it is intended. I've seen many times that people did not even know that autoload was invoked on class_exists

        Show
        Roman S. Borschel added a comment - What caching? Probably you misunderstood me. The second parameter of class_exists defaults to true which can have unwanted consequences with ugly autoloaders. I changed it to class_exists($name, false) and your test still runs fine, so I guess its ok. Even when you want autoload to be invoked I would recommend using explicitly: class_exists($className, true) because that makes it clear that it is intended. I've seen many times that people did not even know that autoload was invoked on class_exists
        Hide
        Benjamin Eberlei added a comment -

        Ah ok, well my intention was to use autoloading, because otherwise the code behaves differently when the annotations are loaded and when they are not which is not obvious from the outside I think.

        Show
        Benjamin Eberlei added a comment - Ah ok, well my intention was to use autoloading, because otherwise the code behaves differently when the annotations are loaded and when they are not which is not obvious from the outside I think.
        Hide
        Kirill chEbba Chebunin added a comment -

        With class_exists(..., false) it's hard to use this part of library somewhere with custom annotations and autoloading. If we create some custom annotations we always need to load them all before we can use reader.
        May be it will be better to autoload annotations with class_exists(..., true), but filter PhpDoc tags? or separate tags and annotations in some way?

        Show
        Kirill chEbba Chebunin added a comment - With class_exists(..., false) it's hard to use this part of library somewhere with custom annotations and autoloading. If we create some custom annotations we always need to load them all before we can use reader. May be it will be better to autoload annotations with class_exists(..., true), but filter PhpDoc tags? or separate tags and annotations in some way?

          People

          • Assignee:
            Roman S. Borschel
            Reporter:
            Benjamin Eberlei
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: