Guice, ImplementedBy and order of modules

Question

Suppose we have the following 3 (top-level) classes:

@ImplementedBy(MyClass.class)
public interface MyInterface {}


public class MyClass implements MyInterface {

  @Inject
  MyClass(MySecondClass mySecondClass) {}
}


public class MySecondClass {}

Then the following code results in an exception (A just-in-time binding to MySecondClass was already configured on a parent injector.):

Guice.createInjector(
    binder -> binder.bind(MyInterface.class),
    binder -> binder.bind(MySecondClass.class)
);

However, if you swap around the order of the modules, there is no exception:

Guice.createInjector(
    binder -> binder.bind(MySecondClass.class),
    binder -> binder.bind(MyInterface.class)
);

I would expect the order of the modules not to matter. Also, the exception message is distinctly odd as there is no parent injector. Is this a bug?

I am using Guice version 5.1.0.

UPDATE

I've done a bit more digging and found this open issue, with some very similar code, so I'm now pretty confident that this is something in Guice that needs to be fixed (and has been an issue for at least a decade). The issue isn't limited to the order of modules passed to createInjector; in fact the order of bindings in a single configure method can make a difference.

Answer 1

Explanation

The problem arises because the @ImplementedBy annotation in combination with the call to Binder.bind(...) are causing the resolution of the constructor injection with @Inject to be happen prematurely before all explicit bindings are known.

The yet unknown explicit binding is here (incorrectly) replaced by a just-in-time binding.

Is it a Bug?

I found no explicit documentation about this behaviour so this can be considered a bug.

There is a vague disclaimer about Guice's ability to validate bindings in the Binder API documentation

bind(ServiceImpl.class);

This statement does essentially nothing; it "binds the ServiceImpl class to itself" and does not change Guice's default behavior. You may still want to use this if you prefer your Module class to serve as an explicit manifest for the services it provides. Also, in rare cases, Guice may be unable to validate a binding at injector creation time unless it is given explicitly.

Despite the disclaimer the behaviour you are observing is contradicting the statements "does essentially nothing" and "does not change Guice's default behaviour".

Though one must admit that the description indirectly suggests - by using the name ServiceImpl.class, which is obviously the name of an implementation type - that it is not meant to be used with interface types unless used to create an explicit binding, for example with a following call to .to(...). This detail can be overlooked very easily and could be highlighted more explicitly.

Furthermore the Wiki entry for @ImplementedBy states:

The above annotation is equivalent to the following bind() statement:

bind(CreditCardProcessor.class).to(PayPalCreditCardProcessor.class);

Which is no longer true when it's used the way you are using it.

The severity of the bug is questionable, though, because the statements could be fully omitted without loss of functionality.

Conclusion

Yes, it is a bug with questionable severity.

One would better avoid calling Binder.bind(...) with interface types other than to create explicit bindings.

Final Note

Interestingly the following call sequence works, which could be a hint how to solve this bug in Guice:

        Injector injector = Guice.createInjector(
                binder -> binder.bind(MyClass.class),
                binder -> binder.bind(MyInterface.class),
                binder -> binder.bind(MySecondClass.class)
        );

Update 2022-10-08

I filed a PR: https://github.com/google/guice/pull/1650. Let's see, what happens. 😁