Details

    • Type: Sub-task Sub-task
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.0-RC-1
    • Fix Version/s: None
    • Component/s: groovy-runtime
    • Labels:
      None

      Description

      If I declare a private field like this...

      Person.groovy
      class Person {
         private firstName
      }
      

      I can still access this field from another class like this...

      Foo.groovy
      class Foo {
         def doIt() {
             def p = new Person(firstName:'Jeff')
             println p.firstName
         }
         static void main(args) {
             new Foo().doIt()
         }
      }
      

      I don't think that I should be able to access p.firstName inside of the Foo class.

        Issue Links

          Activity

          Hide
          Stig Lau added a comment -

          "Groovy recipies" has dedicated a whole page (70) to "Groovy ignores the private modifier", and portray this as a functionality of Groovy.
          Can you comment on whether Groovy wants to remove this feature?

          Show
          Stig Lau added a comment - "Groovy recipies" has dedicated a whole page (70) to "Groovy ignores the private modifier", and portray this as a functionality of Groovy. Can you comment on whether Groovy wants to remove this feature?
          Hide
          Kai G. Schwebke added a comment -

          As I do not own "Groovy recipies", what's the rationale in ignoring private? Data hiding is one of the basic concepts of OO programming. A feature would be if it's the other way round: everything's private, public is ignored and access is only possible using get/set methodes (properties).

          Show
          Kai G. Schwebke added a comment - As I do not own "Groovy recipies", what's the rationale in ignoring private? Data hiding is one of the basic concepts of OO programming. A feature would be if it's the other way round: everything's private, public is ignored and access is only possible using get/set methodes (properties).
          Hide
          Jochen Theodorou added a comment -

          I always wonder how there can be OO programming in languages that have no concept of private. Ah, well... to answer Stig Lau: As you can see the issue is targeted for 2.0, so 2.0 is the version we are going to fix this by introducing several breaking changes. and the breaking changes are the reason why this is targeted for 2.0.

          Show
          Jochen Theodorou added a comment - I always wonder how there can be OO programming in languages that have no concept of private. Ah, well... to answer Stig Lau: As you can see the issue is targeted for 2.0, so 2.0 is the version we are going to fix this by introducing several breaking changes. and the breaking changes are the reason why this is targeted for 2.0.
          Hide
          Kirk Rasmussen added a comment -

          So what happened now that 2.0 is out? This is a pretty serious flaw in the language.

          +1 to Kai. If you are developing big systems with Groovy respecting data hiding is a MUST. You can also run into bizarre Java interop situations, e.g. enhancing a groovy class with CGLIB/Javassist when private starts getting enforced.

          groovy.lang.MissingPropertyException: No such property: selectedOrg for class: DUMMY
          Method threw 'java.lang.UnsupportedOperationException' exception.
          

          Turns out, the root cause was the logger being declared as private/final and the closures not having access, since switching to CGLIB. Easing the private scope solve the issue.

          Show
          Kirk Rasmussen added a comment - So what happened now that 2.0 is out? This is a pretty serious flaw in the language. +1 to Kai. If you are developing big systems with Groovy respecting data hiding is a MUST. You can also run into bizarre Java interop situations, e.g. enhancing a groovy class with CGLIB/Javassist when private starts getting enforced. groovy.lang.MissingPropertyException: No such property: selectedOrg for class: DUMMY Method threw 'java.lang.UnsupportedOperationException' exception. Turns out, the root cause was the logger being declared as private/final and the closures not having access, since switching to CGLIB. Easing the private scope solve the issue.
          Hide
          Jochen Theodorou added a comment -

          Since 1.9 suddenly became Groovy 2, it does not yet contain the new MOP. Unlike the situation in 2008 the new MOP is now gaining shape and I already started working on it. In the new concept, that I started writing in http://docs.codehaus.org/display/GroovyJSR/GEP+11+-+Groovy+3+semantics+and+new+MOP (not much there yet, I really only started formalizing things a little) the invocation of that private method, as in the original description will be possible, but needs to be enabled first. So no longer by default. I think there should be a not too difficult way for testing, but as default there is no need for this indeed. I personally am not worried by private being visible, but there are enough user requesting this. What annoys me more is the problem of accessing a private member of the class your closure is defined in, from that closure, once there is a subclass of that class. The access then is suddenly not possible anymore. Currently Groovy sees private like this: It is always accessible on the instance with the class it was declared with. It is not inherited and not visible from a sub class. This leads to problems for Closures, when you write a java style equals and some other things. All these are a "has to be fixed" for the new MOP. In the old MOP this was mainly not possible because of the logic involved with invokeMethod and the way it is called... which is loosing the call sender information, making it impossible to find out if you are allowed to make the call to this private method or not.

          Show
          Jochen Theodorou added a comment - Since 1.9 suddenly became Groovy 2, it does not yet contain the new MOP. Unlike the situation in 2008 the new MOP is now gaining shape and I already started working on it. In the new concept, that I started writing in http://docs.codehaus.org/display/GroovyJSR/GEP+11+-+Groovy+3+semantics+and+new+MOP (not much there yet, I really only started formalizing things a little) the invocation of that private method, as in the original description will be possible, but needs to be enabled first. So no longer by default. I think there should be a not too difficult way for testing, but as default there is no need for this indeed. I personally am not worried by private being visible, but there are enough user requesting this. What annoys me more is the problem of accessing a private member of the class your closure is defined in, from that closure, once there is a subclass of that class. The access then is suddenly not possible anymore. Currently Groovy sees private like this: It is always accessible on the instance with the class it was declared with. It is not inherited and not visible from a sub class. This leads to problems for Closures, when you write a java style equals and some other things. All these are a "has to be fixed" for the new MOP. In the old MOP this was mainly not possible because of the logic involved with invokeMethod and the way it is called... which is loosing the call sender information, making it impossible to find out if you are allowed to make the call to this private method or not.
          Hide
          Mauro Molinari added a comment -

          Isn't this a duplicate of GROOVY-1875 (which is about private methods too)?

          Show
          Mauro Molinari added a comment - Isn't this a duplicate of GROOVY-1875 (which is about private methods too)?
          Hide
          Cédric Champeau added a comment -

          It is also worth mentionning here that this bug is also considered a feature by many. And even in Java, it is so easy to change a private value that making Groovy disallowing private access wouldn't make the language more secure than it is now. It's just hiding a little more what you can already do without much work.

          Show
          Cédric Champeau added a comment - It is also worth mentionning here that this bug is also considered a feature by many. And even in Java, it is so easy to change a private value that making Groovy disallowing private access wouldn't make the language more secure than it is now. It's just hiding a little more what you can already do without much work.
          Hide
          Mauro Molinari added a comment -

          Yes, but it doesn't make sense to have "private" keyword if it does nothing, does it? Personally I agree with those who think that "private" keyword should be honoured by default and ignored only if explicitly requested. The fact there are workarounds to violate the private visibility even in Java doesn't make "private" keyword useless.

          Show
          Mauro Molinari added a comment - Yes, but it doesn't make sense to have "private" keyword if it does nothing, does it? Personally I agree with those who think that "private" keyword should be honoured by default and ignored only if explicitly requested. The fact there are workarounds to violate the private visibility even in Java doesn't make "private" keyword useless.
          Hide
          Cédric Champeau added a comment -

          At least, private documents the fact that the field or method is supposed to be internal and that it's better to avoid using it directly unless you have no other choice.

          I'm not saying that Groovy should not honor private modifiers semantics, I'm just saying that it's not a big deal, since there are always options, and easy ones, to bypass it. So it's definitely not a critical issue IMHO. The compiler could produce warnings, for example, that's already one level. You can ask it to throw errors only if you are using static type checking, because there's no way to know at compile time if a field is private or not. At runtime, using a new MOP, we could also enforce the semantics, but if you make it an option to be able to use private fields or not, then you are just making it a little more complicated to use that "feature", but you're still not enforcing anything.

          Last but not least, this "feature" is probably used in a lot of Groovy programs, so enforcing the semantics by default would probably break a lot of code.

          So, in the end, whatever we decide, we have to keep all this in mind.

          Show
          Cédric Champeau added a comment - At least, private documents the fact that the field or method is supposed to be internal and that it's better to avoid using it directly unless you have no other choice. I'm not saying that Groovy should not honor private modifiers semantics, I'm just saying that it's not a big deal, since there are always options, and easy ones, to bypass it. So it's definitely not a critical issue IMHO. The compiler could produce warnings, for example, that's already one level. You can ask it to throw errors only if you are using static type checking, because there's no way to know at compile time if a field is private or not. At runtime, using a new MOP, we could also enforce the semantics, but if you make it an option to be able to use private fields or not, then you are just making it a little more complicated to use that "feature", but you're still not enforcing anything. Last but not least, this "feature" is probably used in a lot of Groovy programs, so enforcing the semantics by default would probably break a lot of code. So, in the end, whatever we decide, we have to keep all this in mind.
          Hide
          Jochen Theodorou added a comment -

          GROOVY-1875 is about private properties, this here about a private field. So no duplicate but very related

          As for private not having an effect... that is not true, a subclass cannot access a private member. So it has an effect, even if it is only partially of what Java has/allows

          Show
          Jochen Theodorou added a comment - GROOVY-1875 is about private properties, this here about a private field. So no duplicate but very related As for private not having an effect... that is not true, a subclass cannot access a private member. So it has an effect, even if it is only partially of what Java has/allows
          Hide
          Kenneth Endfinger added a comment -

          Duplicate of GROOVY-3010

          Show
          Kenneth Endfinger added a comment - Duplicate of GROOVY-3010
          Hide
          Jeff Brown added a comment -

          Kenneth,

          This issue is currently categorized as a subtask of GROOVY-3010.

          Show
          Jeff Brown added a comment - Kenneth, This issue is currently categorized as a subtask of GROOVY-3010 .

            People

            • Assignee:
              Unassigned
              Reporter:
              Jeff Brown
            • Votes:
              11 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:

                Development