Sorry for the messy commits, this was my first github contribution. I'm sure you can squash them into a cleaner commit on your end :)

Hi !

Thanks for the contribution, however I'm not sure I understand what is the problem. You can use either "autowiring" (which completely ignores annotations) or "annotations" (which uses @Inject + @param + autowiring).

If you have a bad @param annotation when you use "annotations", then it makes sense PHP-DI fails (because the annotation is broken). In PHP-DI's language, @param and @Inject are both considered annotations.

The problem with your edit is that:

1. it breaks backward compatibility
2. if you don't have @Inject on the constructor it will not be scanned (if $this->useAutoWiring is false) -> that will lead to failures because then PHP-DI will do new SomeClass() with no parameters while the constructor needs some parameters

Maybe you can tell me what you expected PHP-DI to do with you example:

/**
* @param type $myclass
*/
public function __construct($myclass)
{
    //(...)
}

What do you want it inject in the constructor when annotations are turned on?

Salut Mathieu! Maybe our understanding is wrong, but what we're looking for is a way to be able to auto inject only when the @Inject annotation is present. Our assumption was that if Autowiring is turned off, any constructors/methods/properties without the @Inject annotation would be left alone, but seemingly the only way to leave constructors alone is to turn off both annotations and autowiring - which is not waht we want.

The real problem is that we have a large codebase with a lot of legacy code that has docblocks with bad annotations, and we would like to start using PHP-DI without trying to find these issues scattered throughout our codebase, by only adding @Inject to classes where we want PHP-DI to work. Without the fix I proposed, I don't think it's safe to start using PHP-DI in our production environment.

Is the backwards incompatibility you're concerned about with the possibility of applications having autowiring turned off & annotations turned on; and still counting on autowiring to work?

In the case of the example, if autowiring is off, I would like this bad param annotation to not throw an error. ie I want PHP-DI to completely ignore this constructor.

OK I see but it doesn't make sense to ignore constructors. You can't new a class and not call its constructor (well actually you can with reflection but that's really not a good thing to do here at all).

Sorry - to be more clear, I don't want the code to ignore the constructor, I just don't want PHP-DI to try to auto load the dependency or even try to work with the $param annotations if @Inject is not present. ie. Currently the code calling this class passes in the dependency:

$class2 = new class2($myclass);

but based on the bad annotation in the previous code example, I get an error back, even though I'm passing in the right dependency.

OK sorry but this is still not clear.

Given:

class MyClass {
    /**
    * @param type $myclass
    */
    public function __construct($myclass)
    {
        //(...)
    }
}

$obj = $container->get('MyClass');

What should happen? (i.e. what value should the container pass to the constructor?)

So where I'm running into the issue is in a 'master' class where I added:

class MasterClass {
    public function __construct()
    {
        // Add Dependency Injection Container
        $builder = new \DI\ContainerBuilder();
        $builder->useAutowiring(false);
        $container = $builder->build();
        $container->injectOn($this);
    }
}

A child class of this master class has the offending @param type myclass:

class MyClass extends MasterClass {
    /**
    * @param type $myotherclass
    */
    public function __construct($myotherclass)
    {
        parent::__construct();
        $this->myotherclass = $myotherclass;
        //(...)
    }
}

$obj = new MyClass(new myotherclass());

And that results in the following error:

An Unhandled PhpDocReader\AnnotationException Has Occurred: The @param annotation for parameter "myotherclass" of MyClass::__construct contains a non existent class "type". Did you maybe forget to add a "use" statement for this annotation?

I understand that this is not an optimal setup and that I can (and should) just change @param type $myotherclass to @param myotherclass $myotherclass but that's not a workable solution for our codebase right now..

OK thank you I see now!

What do you think about a global option that would be like "ignore invalid classes in @param annotations" instead?

Because autowiring means using the type-hints. So @param is not autowiring, it's really "annotations" (it doesn't make sense to disable it if "autowiring" is disabled). Having an option for ignoring bad @param phpdoc would be better IMO.

That would require modifying mnapoli/PhpDocReader to have such an option (it would be an option to prevent this exception from being thrown). And then PHP-DI should also have the same option, that is forwarded to PhpDocReader.

For example, at a high level, it could be used like this:

$builder = new ContainerBuilder();
$builder->ignorePhpDocErrors(true);

$container = $builder->build();

That would work great! I'll work on that later today, and I'll revert the other changes.

I added the ignorePhpDocErrors flag, and updated the phpdocreader dependency in composer to 1.3 (the version that has the ignorePhpDocErrors support

Thanks. Just a heads up, I'm in holiday right now for a couple weeks more, so I have random internet access and not much time. If this PR doesn't move a lot it's not because I've forgotten about it ;)

The code looks good (except minor points I added inline), just FYI your last commits are not linked to your GitHub account (for some reason, probably the email address, it happened to me, anyway that's not a problem it's just for you).

Thanks Mathieu, I fixed the indentation, removed the docblock and reverted the inject on constructor comment. Thanks for the heads up on the github email assocation by the way :)

Good I have merged it manually into the 4.4 branch (pull request #187), it will show as "merged" here when 4.4 is merged into master.

Thank you for the contribution.