Skip to content

Actually find and warn on private @Listener methods.#1233

Merged
stephan-gh merged 1 commit intoSpongePowered:bleedingfrom
JBYoshi:warn-private-listeners
Apr 13, 2017
Merged

Actually find and warn on private @Listener methods.#1233
stephan-gh merged 1 commit intoSpongePowered:bleedingfrom
JBYoshi:warn-private-listeners

Conversation

@JBYoshi
Copy link
Copy Markdown
Member

@JBYoshi JBYoshi commented Mar 4, 2017

}

for (Method method : invalidMethods) {
SpongeImpl.getLogger().warn("The method {} on {} has @{} but has the wrong signature", method,
Copy link
Copy Markdown
Contributor

@Deamon5550 Deamon5550 Mar 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing as you're changing this, it would probably be good to give a more helpful error message (eg. if its private warn specifically that event listeners cannot be private).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Deamon5550 Fixed.

@JBYoshi JBYoshi force-pushed the warn-private-listeners branch 2 times, most recently from a1973b4 to c1088d7 Compare March 4, 2017 04:36
@ST-DDT
Copy link
Copy Markdown
Member

ST-DDT commented Mar 4, 2017

Nice idea. As an addition/alternative I would suggest using an annotation processor, that checks those things at compile time.
(I might be able to help with that if needed)

@JBYoshi
Copy link
Copy Markdown
Member Author

JBYoshi commented Mar 6, 2017

@ST-DDT SpongePowered/SpongeAPI#1513

@JBYoshi JBYoshi force-pushed the warn-private-listeners branch 2 times, most recently from 2e09bb8 to f179b46 Compare March 26, 2017 22:23
@stephan-gh stephan-gh self-assigned this Apr 13, 2017
@stephan-gh stephan-gh force-pushed the warn-private-listeners branch from f179b46 to 8111124 Compare April 13, 2017 08:52
@stephan-gh stephan-gh merged commit 8111124 into SpongePowered:bleeding Apr 13, 2017
@JBYoshi JBYoshi deleted the warn-private-listeners branch May 17, 2017 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants