Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@MergeMethod annotation should check that a replaced method actually exists #1

Open
Fifty-Nine opened this issue Aug 9, 2016 · 2 comments

Comments

@Fifty-Nine
Copy link

The @MergeMethod annotation allows a user to augment an existing method on a class. However, no checks are done to verify that such a method actually exists. In most cases, this isn't a major issue since the user can use the @Override annotation to ensure such a method exists on the parent class.

However, there are cases where using @Override isn't actually possible. In particular, static methods can't use @Override since you can't use virtual dispatch with a static method. In such a case, it's possible to specify an incorrect signature for a @MergeMethod, and would look like a silent failure from a user perspective: the method wouldn't be overridden, and the default behavior for the overridden method would remain. In most cases this would just be a silent failure, but in some cases if there are multiple merged methods this could lead to crashes or other weird behavior.

All this would be mitigated by issuing an error or warning when a @MergeMethod does not replace something.

@benjholla
Copy link
Member

Good catch. Adding a warning message in the Error Log for now until it can be integrated into the builder.

@benjholla
Copy link
Member

benjholla commented Aug 9, 2016

Warning added in f657a27.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants