Uploaded image for project: 'Velocity'
  1. Velocity
  2. VELOCITY-404

Uberspect - info is created wrong / premature blocking of invalid methods

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 1.4
    • 1.5
    • Engine
    • None

    Description

      Reported and patch submitted by Llwellyn Falco & Dan Powell. Email below:

      Sorry,

      apparently i am being a bit confusing.

      the patch was rather large (six lines of code) so i can understand why i
      should have explained it more clearly,

      The patch is against 3 issues, 1 of them i submitted when the old
      bugzilla, the other 2 don't exist. but there is a unit test per each issue
      submitted with the patch.
      don't quite understand why i would create an issue merely to close it. i
      mean if i needed to show my boss i was working the busy work would make
      sense, but as we are all doing this
      in our free time.... if you feel you need notes that better explain the
      patch than the unit tests, by all means fell free, but i personally can't
      think what i would write that is more precise than actual code.

      issue 1 (this is Velocity-381)

      [ see Velocity-381 ]

      issue 2 (no jira issue)
      the info is created wrong.
      code change


      • method = rsvc.getUberspect().getMethod(o, methodName,
        params, new Info("",1,1));
        + method = rsvc.getUberspect().getMethod(o, methodName,
        params, new Info(context.getCurrentTemplateName(), getLine(), getColumn()));

        the test checks against an uberspect that throws exceptions with it can't
        find stuff.
        (this is also useful for development, but i am trying to start with small
        changes as to keep things simple, so kept it to testing)

        public Info getInfoFor(String velocity) throws Exception

        Unknown macro: { try { parseString(velocity, this); fail("Uberspect Should have thrown an exception"); throw new Error("Shouldn't be able to reach this point"); } catch (VelocityParsingError t) { return t.getInfo(); } }

      public void testInfoForField() throws Exception

      { Info i = getInfoFor("$main.unknownField"); assertInfoEqual(i, "$main.unknownField", 1, 7); }

      public void testInfoForMethod() throws Exception

      { Info i = getInfoFor("$main.unknownMethod()"); assertInfoEqual(i, "$main.unknownMethod()", 1, 7); }

      private void assertInfoEqual(Info i, String name, int line, int column)

      { assertEquals("Template Name", name, i.getTemplateName()); assertEquals("Template Line", line, i.getLine()); assertEquals("Template Column", column, i.getColumn()); }

      The third test also deals with the uberspect. design indicates that the
      uberspect should be asked to find the method, and therefore if you wanted a
      to write an uberspect to check when you are calling methods on null's you
      could write one. but the parser blocked it.
      the test is

      public void testNullPointer()

      { assertErrorThrown("$main.getNull().callMethod()"); }

      private void assertErrorThrown(String string) {
      try

      { String result = parseString(string, this); Assert.fail("parsing '" + string + "' did not fail but returned '" + result + "'"); }

      catch (Throwable t)

      { return; }

      }


      the code change is the removal of premature exit

      • if (result == null)
      • { - return null; - }

      Attachments

        1. patch1.txt
          3 kB
          William Glass-Husain
        2. InfoOnExceptionUberspect.java
          1 kB
          Llewellyn Falco
        3. InfoOnExceptionUberspect.java
          1 kB
          Llewellyn Falco
        4. InfoOnExceptionTest.java
          3 kB
          Llewellyn Falco
        5. StringResourceLoader.java
          0.8 kB
          Llewellyn Falco

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            wglass William Glass-Husain
            wglass William Glass-Husain
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment