vipx / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Modules.override() loses source information when the current source is skipped #774

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
This test case:

<code language="java">
Guice.createInjector(new AbstractModule() {
    @Override
    protected void configure() {
        bind(String.class).toInstance("foo");
        install(Modules.override(Modules.EMPTY_MODULE).with(new AbstractModule() {
            @Override
            protected void configure() {
                binder().skipSources(getClass()).bind(String.class).toInstance("bar");
            }
        }));
    }
});
</code>

Produces this error:

{{{
1) A binding to java.lang.String was already configured at 
Test$1.configure(Test.java:74).
  at com.google.inject.util.Modules$RealOverriddenModuleBuilder$1.configure(Modules.java:172)
}}}

If the module is installed directly, without Modules.override(), it gives

{{{
1) A binding to java.lang.String was already configured at 
Test$1.configure(Test.java:74).
  at Test$1.configure(Test.java:75)
}}}

instead, which is a lot more helpful.

This seems like a general issue with `Elements.getElements()`: there's no way 
to specify sources to skip, so the bindings it creates can't have sources from 
above the `Elements.getElements()` call.

Original issue reported on code.google.com by tavianator@gmail.com on 7 Oct 2013 at 6:04

GoogleCodeExporter commented 9 years ago
Patches gladly accepted :-)

Original comment by sberlin on 5 Dec 2013 at 10:43

GoogleCodeExporter commented 9 years ago
I was going to cook something up, but I just realised that you can't handle 
multiple nested modules that should all be skipped. The inner module doesn't 
know that the outer one is skipped too.

Another solution is to delay computing the skipped sources until later.  
Element.applyTo() would still call binder.withSource(), but pass it the whole 
stack trace instead of just the top element.  The binder would go through the 
stack trace until it found an element that wasn't skipped.

But that's a huge change for a reasonably corner case.  I'm open to other 
suggestions.

Original comment by tavianator@gmail.com on 6 Dec 2013 at 11:51