google / gwteventbinder

Making GWT EventBus easy
http://google.github.io/gwteventbinder
Apache License 2.0
140 stars 28 forks source link

event binder generator does not use canonicalname for implements type. #31

Closed michael-conrad closed 9 years ago

michael-conrad commented 9 years ago

Not using full canonical name for implements type will cause issues if the binding code is in different package than the bound code.

michael-conrad commented 9 years ago

Suggested patch:

diff --git a/eventbinder/src/main/java/com/google/web/bindery/event/gwt/rebind/binder/EventBinderGenerator.java b/eventbinder/src/main/java/com/google/web/bindery/event/gwt/rebind/binder/EventBinderGenerator.java
index c54994d..3b6b9a5 100644
--- a/eventbinder/src/main/java/com/google/web/bindery/event/gwt/rebind/binder/EventBinderGenerator.java
+++ b/eventbinder/src/main/java/com/google/web/bindery/event/gwt/rebind/binder/EventBinderGenerator.java
@@ -90,7 +90,7 @@

     composer.setSuperclass(AbstractEventBinder.class.getCanonicalName()
         + "<" + targetType.getQualifiedSourceName() + ">");
-    composer.addImplementedInterface(eventBinderType.getName());
+    composer.addImplementedInterface(eventBinderType.getQualifiedSourceName());

     composer.addImport(EventBinder.class.getCanonicalName());
     composer.addImport(EventBus.class.getCanonicalName());
ekuefler commented 9 years ago

Can you give a more complete example of the problem you're seeing? Binding a class in a different package should work fine under normal circumstances - there's a test at https://github.com/google/gwteventbinder/blob/master/eventbinder/src/test/java/com/google/web/bindery/event/shared/binder/EventBinderTest.java#L133 that confirms this.

The line you're pointing to is where the generated code implements EventBinder. A couple lines down we add EventBinder as an import, so AFAICT this should work fine without having to fully-qualify the name there.

michael-conrad commented 9 years ago

Hrmmm I can't seem to enable strict mode with the gwt maven plugin the project is using. When I run "mvn clean test" in sub-project "eventbinder" I am getting:

Validating units: Ignored 3 units with compilation errors in first pass. Compile with -strict or with -logLevel set to TRACE or DEBUG to see all errors.

Should not this cause a test failure?

I'll create a new gradle project and see if I can reproduce what I was seeing.

(I tried:) <compilerArgs><compilerArg>-strict</compilerArg></compilerArgs> but this seems to be ignored.

michael-conrad commented 9 years ago

Ok, please see:

https://github.com/mjoyner-vbservices-net/eventbinder-magali/tree/master/eventbinder-magali

(A saved copy of the generated output is in the folder "src/gen")

A copy of the error output:

gradle clean compileGwt
:clean
:compileJava
:processResources UP-TO-DATE
:compileGwt
Compiling module test.Test
   Adding '1' new generated units
      Tracing compile failure path for type 'test.shared.BindsEvents_MyEventBinderImpl'
         [ERROR] Errors in '/home/mjoyner/git/eventbinder-magali/eventbinder-magali/build/gwt/gen/test/shared/BindsEvents_MyEventBinderImpl.java'
            [ERROR] Line 11: HandlesEvents cannot be resolved to a type
            [ERROR] Line 11: The type BindsEvents_MyEventBinderImpl must implement the inherited abstract method AbstractEventBinder<HandlesEvents>.doBindEventHandlers(HandlesEvents, EventBus)
            [ERROR] Line 11: The interface EventBinder cannot be implemented more than once with different arguments: EventBinder<HandlesEvents> and EventBinder<HandlesEvents>
      See snapshot: /tmp/test.shared.BindsEvents_MyEventBinderImpl4096437061114992414.java
   [ERROR] Errors in '/home/mjoyner/git/eventbinder-magali/eventbinder-magali/build/gwt/gen/test/shared/BindsEvents_MyEventBinderImpl.java'
      [ERROR] Line 11: The interface EventBinder cannot be implemented more than once with different arguments: EventBinder<HandlesEvents> and EventBinder<HandlesEvents>
      [ERROR] Line 11: The type BindsEvents_MyEventBinderImpl must implement the inherited abstract method AbstractEventBinder<HandlesEvents>.doBindEventHandlers(HandlesEvents, EventBus)
      [ERROR] Line 11: HandlesEvents cannot be resolved to a type
      See snapshot: /tmp/test.shared.BindsEvents_MyEventBinderImpl2504917617312099534.java
   Tracing compile failure path for type 'test.shared.BindsEvents_MyEventBinderImpl'
      [ERROR] Errors in '/home/mjoyner/git/eventbinder-magali/eventbinder-magali/build/gwt/gen/test/shared/BindsEvents_MyEventBinderImpl.java'
         [ERROR] Line 11: HandlesEvents cannot be resolved to a type
         [ERROR] Line 11: The type BindsEvents_MyEventBinderImpl must implement the inherited abstract method AbstractEventBinder<HandlesEvents>.doBindEventHandlers(HandlesEvents, EventBus)
         [ERROR] Line 11: The interface EventBinder cannot be implemented more than once with different arguments: EventBinder<HandlesEvents> and EventBinder<HandlesEvents>
   [ERROR] Hint: Check that the type name 'test.shared.BindsEvents_MyEventBinderImpl' is really what you meant
   [ERROR] Hint: Check that your classpath includes all required source roots
   [ERROR] Errors in 'test/shared/BindsEvents.java'
      [ERROR] Line 12: Rebind result 'test.shared.BindsEvents_MyEventBinderImpl' could not be found
   [ERROR] Errors in 'test/client/HandlesEvents.java'
      [ERROR] Line 11: Rebind result 'test.shared.BindsEvents_MyEventBinderImpl' could not be found
:compileGwt FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileGwt'.
> Process 'command '/usr/lib/jvm/java-8-openjdk-amd64/bin/java'' finished with non-zero exit value 1

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

BUILD FAILED

Total time: 25.244 secs

The relevant error is:

[ERROR] Line 11: HandlesEvents cannot be resolved to a type

"HandlesEvents" is not in the package the generated file is in. It is in a different package.

tbroyer commented 9 years ago

Looks like a dupe of #28 then; already fixed in master but there hasn't been a release since then.

michael-conrad commented 9 years ago

Confirmed. I just compile against 1.1.1-SNAPSHOT and it seems happy.