Velocity
  1. Velocity
  2. VELOCITY-214

References to non-public members (inner classes, fields, etc.) should log a warning rather than failing silently

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 1.3.1
    • Fix Version/s: 1.5
    • Component/s: Engine
    • Labels:
      None
    • Environment:
      Operating System: other
      Platform: Other

      Description

      When I try to access a property on an object through the context and the
      defining class of that object is a non-public inner class the reference fails as
      if there was no object for that key in the context.

      I'd like to at least get a userful error message in the log.

      I have reproduced this with both velocity-1.3.1 and cvs HEAD as of today.

        Activity

        Hide
        Daniel Resare added a comment -

        Created an attachment (id=8480)
        trivial test code to illsutrate my point

        Show
        Daniel Resare added a comment - Created an attachment (id=8480) trivial test code to illsutrate my point
        Hide
        Daniel Resare added a comment -

        Created an attachment (id=8481)
        A test template to go with the test code

        Show
        Daniel Resare added a comment - Created an attachment (id=8481) A test template to go with the test code
        Hide
        Nathan Bubna added a comment -

        Velocity does not provide access to private, package-private, or protected
        methods and classes intentionally. it's a feature, not a bug.

        Show
        Nathan Bubna added a comment - Velocity does not provide access to private, package-private, or protected methods and classes intentionally . it's a feature, not a bug.
        Hide
        Daniel Resare added a comment -

        Ok. I was unclear. I think it's the right behaviour not to give access to
        private/protected methods. However it would be nice to signal a warning or
        notice in the logs when trying, instead of just failing with no debug information.

        It's quite easy to forget adding 'public' to an inner class declaration.

        Show
        Daniel Resare added a comment - Ok. I was unclear. I think it's the right behaviour not to give access to private/protected methods. However it would be nice to signal a warning or notice in the logs when trying, instead of just failing with no debug information. It's quite easy to forget adding 'public' to an inner class declaration.
        Hide
        Nathan Bubna added a comment -

        so are you saying you're not getting the standard "$foo is not a valid
        reference" warning in the logs when $foo is a non-public inner class?

        if so, yeah, that's bug. sorry, i didn't understand what you were getting at.
        if i still didn't get it, well, let me know i guess.

        Show
        Nathan Bubna added a comment - so are you saying you're not getting the standard "$foo is not a valid reference" warning in the logs when $foo is a non-public inner class? if so, yeah, that's bug. sorry, i didn't understand what you were getting at. if i still didn't get it, well, let me know i guess.
        Hide
        Daniel Rall added a comment -

        Daniel, I agree with that – a warning would be very useful, especially for
        newcomers to Velocity. Let's target this for the next minor point release.
        It's much more likely to actually make it into that release if you try your hand
        at a patch.

        Show
        Daniel Rall added a comment - Daniel, I agree with that – a warning would be very useful, especially for newcomers to Velocity. Let's target this for the next minor point release. It's much more likely to actually make it into that release if you try your hand at a patch.
        Hide
        Daniel Rall added a comment -

        Hmm, missed Nathan's comment. Yeah, this walks the tightrope between bug and
        enhancement.

        Show
        Daniel Rall added a comment - Hmm, missed Nathan's comment. Yeah, this walks the tightrope between bug and enhancement.
        Hide
        Henning Schmiedehausen added a comment -

        Well, it does report the

        [info] Null reference [template 'testbed/test.vm', line 1, column 51] : $private.Bar cannot be resolved.

        in the log file with Velocity 1.5. the problem here is, that we cannot really distinguish whether this is just a typo or really a private/protected function and even if we could, we do run chains of method lookups (e.g. for getters, we are looking for getfoo, getFoo, get("foo")) so reporting functions that we could not find would log a number of false positives that might be even more confusing than not explicitly logging non-accessible functions.

        And as we don't want to play games with changing the visiblity of the methods on the fly, we are basically stuck with what we have. That is reporting "null reference, $private.Bar cannot be resolved". We should add a note to the manual, though.

        Show
        Henning Schmiedehausen added a comment - Well, it does report the [info] Null reference [template 'testbed/test.vm', line 1, column 51] : $private.Bar cannot be resolved. in the log file with Velocity 1.5. the problem here is, that we cannot really distinguish whether this is just a typo or really a private/protected function and even if we could, we do run chains of method lookups (e.g. for getters, we are looking for getfoo, getFoo, get("foo")) so reporting functions that we could not find would log a number of false positives that might be even more confusing than not explicitly logging non-accessible functions. And as we don't want to play games with changing the visiblity of the methods on the fly, we are basically stuck with what we have. That is reporting "null reference, $private.Bar cannot be resolved". We should add a note to the manual, though.
        Hide
        Henning Schmiedehausen added a comment -

        Close all resolved issues for Engine 1.5 release.

        Show
        Henning Schmiedehausen added a comment - Close all resolved issues for Engine 1.5 release.

          People

          • Assignee:
            Unassigned
            Reporter:
            Daniel Resare
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development