Repository navigation
Compilation fails for methods annotated with @ServiceAction that have the same name as one of their parameters #1070
Description
Given an @EIntentService like so:
@EIntentService
public class FooIntentService extends IntentService {
@ServiceAction
public void foo(Object foo) { /* ... */ }
}Compilation fails annotation processing with the following error, which is misleading:
error: method foo(Object) is already defined in class IntentBuilder_
This is because the generated code in FooIntentService_.java has this bit (see comments in code block):
public FooIntentService_.IntentBuilder_ foo(Object foo) {
intent_.setAction(ACTION_FOO);
foo(foo); // <--- Recursive call!!!
return this;
}
public FooIntentService_.IntentBuilder_ foo(Object foo) { // <--- Because of this
intent_.putExtra(FOO_EXTRA, ((Serializable) foo);
return this;
}And when the parameter name is changed, the Extra is properly passed.
public FooIntentService_.IntentBuilder_ foo(Object bar) {
intent_.setAction(ACTION_FOO);
bar(bar); // <--- Happy now
return this;
}
public FooIntentService_.IntentBuilder_ bar(Object bar) { // <--- Because, this.
intent_.putExtra(BAR_EXTRA, ((Serializable) bar);
return this;
}Since the @EIntentService usage is defined as:
FooIntentService_.intent(getContext()).foo(new Whatever()).start();I think having public accessors for the Extras generated from the parameters of the annotated methods is unnecessary. They only pollute the IntentBuilder_s namespace since they'll never be used directly.
I think a better way to do this would be, in this case, to make those Extras private, and use a different naming scheme while generating them so they won't collide with the annotated methods...
Or, maybe changing the usage to something like the following is better?
FooIntentService_.intent(getContext()) //
.foo(/* NO PARAMETERS */).bar(new Whatever()) // .bar() is really the parameter to .foo()
.start();so those Extra methods handling the parameters would make sense.
Either of those, or @ServiceAction annotation's processor might warn users when it's placed on a method that has a parameter name the same as its name.
Activity
[-]Compilation fails for methods annotated with @ServiceAction that have the same name as one of their parameter[/-][+]Compilation fails for methods annotated with @ServiceAction that have the same name as one of their parameters[/+]WonderCsabo commentedon Jul 17, 2014
Actually naming a method as the same as the parameter is not a really good practice. I do not think we should make our code more complex by preparing it for this edge case.
kutsal commentedon Jul 17, 2014
@WonderCsabo, I disagree with you. My problem here is not what's best practice, but the fact that the current implementation pollutes
IntentBuilders namespace with names of the parameters to the methods. This example fails compilation with the same message as well:The error is misleading. In my code, I only defined one
foo(...)and onebar(...), not two of them. The fact that each self-contained method's parameter naming scheme leaking into the generated class is my real problem here. You're imposing a method and parameter naming scheme in my project for me.WonderCsabo commentedon Jul 17, 2014
OK, sorry, i see your point now. @yDelouis you contributed this feature, what do you think?
yDelouis commentedon Jul 17, 2014
I agree with @kutsal. We shouldn't add the parameter as an extra to the
IntentBuilder. (Even if it is added as an extra in theIntentunderneath).Generally, I think an annotation should do one thing and only one. If the developer wants to pass an extra without the action, he still can with
@Extra.WonderCsabo commentedon Jul 17, 2014
Agreed. Can you post a PR to remove the extra methods so the service action method calls
super.extra()directly?yDelouis commentedon Jul 17, 2014
I will I soon as I have enough time.
WonderCsabo commentedon Sep 22, 2014
I think it is rather a defect than a task. :(
WonderCsabo commentedon Nov 5, 2014
@yDelouis can you resolve this?
WonderCsabo commentedon Nov 6, 2014
Fixed.