Velocity
  1. Velocity
  2. VELOCITY-753

IntrospectionUtils.isMethodInvocationConvertible() returns true for NumberFormat.format(double) method with a Float.class argument

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.2
    • Fix Version/s: 1.7, 2.x
    • Component/s: Engine
    • Labels:
      None

      Description

      NumberFormat has both a format(double) and format(Object) method. When evaluated against a Float, IntrospectionUtils is returning true. This causes Method.getBestMatch() to throw an AmbigouusException.

      I have included a failing test case below to show this issue.

      Thanks,
      Tim

      import java.io.StringReader;
      import java.io.StringWriter;
      import java.text.NumberFormat;

      import junit.framework.TestCase;

      import org.apache.velocity.VelocityContext;
      import org.apache.velocity.app.Velocity;

      public class VelocityFormatTest extends TestCase {

      public void test_format_of_float() throws Exception {
      // verify format() outside of Velocity
      NumberFormat numberFormat = NumberFormat.getInstance();
      Float aFloat = new Float(5.23);
      assertEquals("5.23", numberFormat.format(aFloat));

      Velocity.init();
      VelocityContext context = new VelocityContext();
      context.put("numberFormatter", numberFormat);
      context.put("aFloat", aFloat);

      StringWriter sw = new StringWriter();
      StringReader sr = new StringReader(
      "float value is $

      {numberFormatter.format($aFloat)}

      ");
      Velocity.evaluate(context, sw, "name", sr);

      String expectedValue = "float value is 5.23";
      assertEquals(expectedValue, sw.toString());
      }

      }

        Activity

        Mark Thomas made changes -
        Workflow Default workflow, editable Closed status [ 12551709 ] jira [ 12552255 ]
        Mark Thomas made changes -
        Workflow jira [ 12489575 ] Default workflow, editable Closed status [ 12551709 ]
        Nathan Bubna made changes -
        Field Original Value New Value
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 1.7 [ 12313453 ]
        Fix Version/s 2.0 [ 12310291 ]
        Resolution Fixed [ 1 ]
        Hide
        Nathan Bubna added a comment -

        Ok, the key problem here is that $numberFormatter.format($aFloat) should work. The catch is that in reflection-land (where Velocity lives), actual arguments are always objects. In other words, in Java, if you call numberFormat.format(aFloat), it will call the object method, because aFloat doesn't convert to a double. But, in reflection-land, Float is indistinguishable from float. So, format(double) and format(Object) are both applicable. Right now, it considers the preference between those ambiguous, with some abstract validity but ultimately wrongly in practice.

        The good news is that i can fix that. The quasi-bad news is that resolving the ambiguity means identifying the most-specific of the two: format(double). The end result is that numberFormat.format(aFloat) in Java will call a different method than $numberFormat.format($aFloat) in Velocity. In this particular case, that shouldn't matter. However, it certainly could matter in some other corner cases. Still, it would not make sense to call format(Object) more specific than format(double) or even equivalently specific, so i'm pretty sure this is the right thing to do.

        For those interested in the details, when comparing two parameter types in different applicable methods, i am now always considering Object.class parameters to be less specific than anything else, which i think is pretty much the case in reflection-land where things are always Objects. Please yell at me if that is a terrible idea.

        Show
        Nathan Bubna added a comment - Ok, the key problem here is that $numberFormatter.format($aFloat) should work. The catch is that in reflection-land (where Velocity lives), actual arguments are always objects. In other words, in Java, if you call numberFormat.format(aFloat), it will call the object method, because aFloat doesn't convert to a double. But, in reflection-land, Float is indistinguishable from float. So, format(double) and format(Object) are both applicable. Right now, it considers the preference between those ambiguous, with some abstract validity but ultimately wrongly in practice. The good news is that i can fix that. The quasi-bad news is that resolving the ambiguity means identifying the most-specific of the two: format(double). The end result is that numberFormat.format(aFloat) in Java will call a different method than $numberFormat.format($aFloat) in Velocity. In this particular case, that shouldn't matter. However, it certainly could matter in some other corner cases. Still, it would not make sense to call format(Object) more specific than format(double) or even equivalently specific, so i'm pretty sure this is the right thing to do. For those interested in the details, when comparing two parameter types in different applicable methods, i am now always considering Object.class parameters to be less specific than anything else, which i think is pretty much the case in reflection-land where things are always Objects. Please yell at me if that is a terrible idea.
        Tim Kuntz created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Tim Kuntz
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development