-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Issue #4550 XmlConfiguration argument matching #4599
Issue #4550 XmlConfiguration argument matching #4599
Conversation
Improve argument matching by: + rejecting obviously non matches (with allowance for unboxing) + sorting methods so that derived arguments are tried before more generic (eg String before Object) Signed-off-by: Greg Wilkins <gregw@webtide.com>
This change is to address the issue noticed by @joakime that:
I think this is kind of a bad class design, but now at least we will deterministically favour a |
I have the order correct as the following test: private static class TestOrder
{
public void other(Object o)
{
System.err.println("object");
}
public void other(String... s)
{
System.err.println("string varargs");
}
}
@Test
public void testMethodOrdering() throws Exception
{
TestOrder to = new TestOrder();
to.other("foo");
} prints |
Dang! still breaks things... stand by... |
Improve argument matching by: + can unbox from any Number to any Number Signed-off-by: Greg Wilkins <gregw@webtide.com>
jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than a nit in the comparator, LGTM.
if (compare == 0) | ||
{ | ||
// favour primitive type over reference | ||
compare = Boolean.compare(t2.isPrimitive(), t1.isPrimitive()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's subtle that t1
and t2
are inverted here.
I would prefer Boolean.compare(!t1.isPrimitive(), !t2.isPrimitive())
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's how I wrote it initially.... switching....
Improve #4550 argument matching by:
Signed-off-by: Greg Wilkins gregw@webtide.com