Details

    • Type: Sub-task Sub-task
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: 2.4.0-beta-4
    • Component/s: groovy-runtime
    • Labels:
      None
    • Testcase included:
      yes
    • Number of attachments :
      1

      Description

      The String setter in the attached example is never called. Instead, the int setter is called when I assign a String with size() == 1. If the size is bigger, I get a GroovyClassCastException

        Issue Links

          Activity

          Hide
          Guillaume Laforge added a comment -

          Single-character strings can be coerced to characters, which can be coerced to ints.
          But strings of length >1 are strings and can't be coerced to numbers that way.

          Show
          Guillaume Laforge added a comment - Single-character strings can be coerced to characters, which can be coerced to ints. But strings of length >1 are strings and can't be coerced to numbers that way.
          Hide
          Aaron Digulla added a comment -

          But why is the String setter ignored? I would expect Groovy to try it before trying to coerce the String into an integer and calling the int setter.

          Show
          Aaron Digulla added a comment - But why is the String setter ignored? I would expect Groovy to try it before trying to coerce the String into an integer and calling the int setter.
          Hide
          Guillaume Laforge added a comment -

          Probably because the property is int i, hence the associated setter is setI(int) and not setI(String).
          I guess if you call o.setI("10"), the string setter is appropriately called.

          Show
          Guillaume Laforge added a comment - Probably because the property is int i, hence the associated setter is setI(int) and not setI(String). I guess if you call o.setI("10"), the string setter is appropriately called.
          Hide
          Aaron Digulla added a comment -

          It would be nice if Groovy would allow to specify "converting setters" How about turning this into an enhancement request for post-1.1?

          I would even prefer a more script-language approach (where '500' == (String)500 and (int)'500' == 500) but that's another issue.

          Show
          Aaron Digulla added a comment - It would be nice if Groovy would allow to specify "converting setters" How about turning this into an enhancement request for post-1.1? I would even prefer a more script-language approach (where '500' == (String)500 and (int)'500' == 500) but that's another issue.
          Hide
          Paul King added a comment -

          We already have:

          assert '500'.toInteger() == 500
          assert 500.toString() == '500'
          
          Show
          Paul King added a comment - We already have: assert '500'.toInteger() == 500 assert 500.toString() == '500'
          Hide
          Paul King added a comment -

          Renaming title to reflect comment that this isn't a bug any longer. Old title was: Overloading doesn't work with int and String setters. But I still think we need further clarification on what the actual enhancement request is.

          Show
          Paul King added a comment - Renaming title to reflect comment that this isn't a bug any longer. Old title was: Overloading doesn't work with int and String setters. But I still think we need further clarification on what the actual enhancement request is.
          Hide
          Aaron Digulla added a comment -

          I need a way to specify what

          a.x = '1'
          

          means: Is '1' the character 0x41, the string "1" with length 1 or the number 1.

          There are two sources of information to determine this:

          1. Any setters for x.
          2. The type of "x"

          Since the user probably doesn't define a setter when she doesn't have to, the setters should take precedence.

          So if there is

          int x;
          void setX(String v) {...}
          

          then '1' is the string "1" with length 1 (and never the int nor the character). In this case, the setter should be called. If I say

          a.x=1
          a.setX(1)
          

          then the value is to be assigned directly or maybe coerced to a String, I'm open to options here. I think it's not too much to ask from the user to write all setters if they allow several types.

          void setX(char v) {...}
          

          Here, int values or single-character strings are ok.

          a.x = 'xxx'
          

          would throw an error ("String 'xxx' can't be coerced to correct type for setter setX(char)").

          Other than that, the only other option I see is to define an annotation which allows to define additional coercion methods for a field:

          @coerceFrom(String.class, coerceX)
          int x;
          
          int coerceX(String value){ ... }
          

          This field would also also accept String values and int's. Single character strings would be strings, not char.

          Really, you should never convert single character Strings to char automatically; ask the user to use toChar(). The single character auto-conversion seems like a good idea at first glance but it is too unpredictable if you can't control the length of your strings (user input, for example).

          Show
          Aaron Digulla added a comment - I need a way to specify what a.x = '1' means: Is '1' the character 0x41, the string "1" with length 1 or the number 1. There are two sources of information to determine this: 1. Any setters for x. 2. The type of "x" Since the user probably doesn't define a setter when she doesn't have to, the setters should take precedence. So if there is int x; void setX( String v) {...} then '1' is the string "1" with length 1 (and never the int nor the character). In this case, the setter should be called. If I say a.x=1 a.setX(1) then the value is to be assigned directly or maybe coerced to a String, I'm open to options here. I think it's not too much to ask from the user to write all setters if they allow several types. void setX( char v) {...} Here, int values or single-character strings are ok. a.x = 'xxx' would throw an error ("String 'xxx' can't be coerced to correct type for setter setX(char)"). Other than that, the only other option I see is to define an annotation which allows to define additional coercion methods for a field: @coerceFrom( String .class, coerceX) int x; int coerceX( String value){ ... } This field would also also accept String values and int's. Single character strings would be strings, not char. Really, you should never convert single character Strings to char automatically; ask the user to use toChar(). The single character auto-conversion seems like a good idea at first glance but it is too unpredictable if you can't control the length of your strings (user input, for example).
          Hide
          blackdrag blackdrag added a comment -

          We have to fight against a general problem here and that is that the bean specification does not really allow more than one setter for one property name. so if you say Groovy should produce two setter for

          int x
          void setX(String s){}
          

          then we could still use only one setter of these, because we use the sun classes to get the bean property information and that will return only one. We could probably extending the MOP in 2.0 to support overloaded setter constructions.. that would solve the problem with a.x=1 and a.x = "xxx" and throw no exception. If you want to suggest to remove the conversion of a one element String to char or int, then I suggest to open a new issue for this and also start a thread on the user list or dev list (possibly both)

          Show
          blackdrag blackdrag added a comment - We have to fight against a general problem here and that is that the bean specification does not really allow more than one setter for one property name. so if you say Groovy should produce two setter for int x void setX( String s){} then we could still use only one setter of these, because we use the sun classes to get the bean property information and that will return only one. We could probably extending the MOP in 2.0 to support overloaded setter constructions.. that would solve the problem with a.x=1 and a.x = "xxx" and throw no exception. If you want to suggest to remove the conversion of a one element String to char or int, then I suggest to open a new issue for this and also start a thread on the user list or dev list (possibly both)
          Hide
          Jon Cox added a comment - - edited

          Given that Groovy already extends Java, what's the problem with providing setter overloads?
          They turn out to be very useful.

          The current behavior of '=' in Groovy is really pretty weird:

          • Explicit calls in Groovy to setMyProperty(...) always work correctly, even when overloaded.
          • If I have a Java getter that returns type X and a non-overloaded Java setter:
            • If the Java setter takes type X, then Groovy's '=' will work properly.
            • If the Java setter takes type Y, then Groovy's '=' will try to convert the type of the Groovy arg proved to the Java getter's return type, and possibly fail.
          • If I have a Java getter that returns type X and the Java setter is overloaded to accept type X and Y:
            • If the Groovy arg provided to '=' is of type X, then the Java setter called will be the one for X.
            • If the Groovy arg provided to '=' is of type Y, Groovy first tries to convert it to an X (possibly failing), then calls the Java setter for type X.

          This behavior is astonishing.
          It is also unhelpful / unproductive.

          Overloaded setters turn out to be very useful. I dont' care if it's the bean spec or not.
          I want to be more productive when working with the JVM – this is why I want to program
          in Groovy to begin with. Why does '=' need to force itself into the tiny box of only working
          with Java beans, when java itself allows you overload setYourProperty(...) and Groovy
          happily such overloads correctly now? I want that happiness to extend to the '=' operator!

          Let's motivate this discussion with a concrete example from real life (my own!):

          In Gradle, the 'javadoc' task is kind of lame as of 0.5.2; it does not take very many of the
          bazillion arguments that the Ant Javadoc task can take.
          (see: http://api.dpml.net/ant/1.6.5/org/apache/tools/ant/taskdefs/Javadoc.html#setAccess(org.apache.tools.ant.taskdefs.Javadoc.AccessType) )

          I wanted to help fix that, because I actually need to use some of these parameters myself and don't like dropping down into
          the much more verbose ant.javadoc(....) syntax because it does not provide many of the nice features the built-in Gradle javadoc
          task has.

          Currently, if I've overloaded the setter for 'overview' with one function that takes a String and another with a File, I could say either:

            setOverview( new File("$srcRoot/overview.html") )      // happily calls  setOverview( File )
          

          Or:

            setOverview( "$srcRoot/overview.html")                        // happily calls  setOverview( String )
          

          However that's rather ugly, and Gradle uses Groovy to be prettier & friendlier.
          I'd like to use the '=' syntax for setter overloads just as nicely as I can use explicit setOverview(...)
          Groovy lets you play fast & loose with types.
          Users expect that:

          • If a user has "Ant XML name/type expectations", then they're prone to provide string values to the task.
          • If a user has Ant API name/type expectations", then they're prone to give the API type to the task.

          So, I want everybody to succeed, by default, as they'd expect (whomever they are).

          Specifically, consider two Ant javadoc properties: 'header' and 'overview'
          With 'header' Ant's API wants you to give it a String (as does the XML)
          With 'overview', Ant's API wants you to give it a File (but the XML wants a String, of course).
          It would be nice Groovy were to be able to support either syntax for Gradle's 'javadoc' task:

          javadoc {
              header = "I want a pony!!!" 
              overview = "$srcRoot/overview.html"         //  call the overload:   setOverview( String ) 
          }
          

          And also this:

          javadoc {
              header = "I want a pony!!!"
              overview = new File("$srcRoot/overview.html")     // call the overload:  setOverview( File )
          }
          

          As things stand, I cannot do this in Groovy 1.5.6.
          When I dug deeper into exactly how '=' works, I came away a sadder Groovy user.

          No pony.

          I hope this motivating case is persuasive enough to show that setter overloads for '=' have
          non-arcane uses, they're consistent with java support for overloaded setMyproperty(...),
          and consistent with Groovy's "just do the right thing" attitude.

          Is there some technical reason why it's difficult to make '=' work nicely
          while explicit calls to an overloaded setMyProperty(....) is easier?
          If so, could you explain it?

          Show
          Jon Cox added a comment - - edited Given that Groovy already extends Java, what's the problem with providing setter overloads? They turn out to be very useful. The current behavior of '=' in Groovy is really pretty weird: Explicit calls in Groovy to setMyProperty(...) always work correctly, even when overloaded. If I have a Java getter that returns type X and a non-overloaded Java setter: If the Java setter takes type X, then Groovy's '=' will work properly. If the Java setter takes type Y, then Groovy's '=' will try to convert the type of the Groovy arg proved to the Java getter's return type, and possibly fail. If I have a Java getter that returns type X and the Java setter is overloaded to accept type X and Y: If the Groovy arg provided to '=' is of type X, then the Java setter called will be the one for X. If the Groovy arg provided to '=' is of type Y, Groovy first tries to convert it to an X (possibly failing), then calls the Java setter for type X. This behavior is astonishing. It is also unhelpful / unproductive. Overloaded setters turn out to be very useful. I dont' care if it's the bean spec or not. I want to be more productive when working with the JVM – this is why I want to program in Groovy to begin with. Why does '=' need to force itself into the tiny box of only working with Java beans, when java itself allows you overload setYourProperty(...) and Groovy happily such overloads correctly now? I want that happiness to extend to the '=' operator! Let's motivate this discussion with a concrete example from real life (my own!): In Gradle, the 'javadoc' task is kind of lame as of 0.5.2; it does not take very many of the bazillion arguments that the Ant Javadoc task can take. (see: http://api.dpml.net/ant/1.6.5/org/apache/tools/ant/taskdefs/Javadoc.html#setAccess(org.apache.tools.ant.taskdefs.Javadoc.AccessType) ) I wanted to help fix that, because I actually need to use some of these parameters myself and don't like dropping down into the much more verbose ant.javadoc(....) syntax because it does not provide many of the nice features the built-in Gradle javadoc task has. Currently, if I've overloaded the setter for 'overview' with one function that takes a String and another with a File, I could say either: setOverview( new File( "$srcRoot/overview.html" ) ) // happily calls setOverview( File ) Or: setOverview( "$srcRoot/overview.html" ) // happily calls setOverview( String ) However that's rather ugly, and Gradle uses Groovy to be prettier & friendlier. I'd like to use the '=' syntax for setter overloads just as nicely as I can use explicit setOverview(...) Groovy lets you play fast & loose with types. Users expect that: If a user has "Ant XML name/type expectations", then they're prone to provide string values to the task. If a user has Ant API name/type expectations", then they're prone to give the API type to the task. So, I want everybody to succeed, by default, as they'd expect (whomever they are). Specifically, consider two Ant javadoc properties: 'header' and 'overview' With 'header' Ant's API wants you to give it a String (as does the XML) With 'overview', Ant's API wants you to give it a File (but the XML wants a String, of course). It would be nice Groovy were to be able to support either syntax for Gradle's 'javadoc' task: javadoc { header = "I want a pony!!!" overview = "$srcRoot/overview.html" // call the overload: setOverview( String ) } And also this: javadoc { header = "I want a pony!!!" overview = new File( "$srcRoot/overview.html" ) // call the overload: setOverview( File ) } As things stand, I cannot do this in Groovy 1.5.6. When I dug deeper into exactly how '=' works, I came away a sadder Groovy user. No pony. I hope this motivating case is persuasive enough to show that setter overloads for '=' have non-arcane uses, they're consistent with java support for overloaded setMyproperty(...), and consistent with Groovy's "just do the right thing" attitude. Is there some technical reason why it's difficult to make '=' work nicely while explicit calls to an overloaded setMyProperty(....) is easier? If so, could you explain it?
          Hide
          Jon Cox added a comment -

          Ouch, the code samples didn't get formatted properly (I think I mises a curly brace).

          Here's more code samples:

          Suppose you have this in Java:

             public String getMyprop()                     // NOTE: return type String
             public void   setMyprop( String )
             public void   setMyprop( File  )
          

          The setMyprop(String) is called when using '=',
          even when the arg is a File:

                 Myprop = "$srcRoot/overview.html"                     // OK:      setMyprop( String )
                 Myprop = new File("$srcRoot/overview.html" )          // WEIRD:   setMyprop( String )
                 setMyprop(  "$srcRoot/overview.html"  )               // OK:      setMyprop( String )
                 setMyprop(  new File("$srcRoot/overview.html")  )     // OK:      setMyprop( File )    
          

          Now change the return type of the getter to File:

                 public File   getMyprop()                     // NOTE: return type File
                 public void   setMyprop( String )
                 public void   setMyprop( File  )
          

          The setMyprop(File) is called and triggers an error when using '=',
          even when the arg is a String an setMyprop(String) should have been called:

                 Myprop = "$srcRoot/overview.html"                          // WEIRD: String -> File casting error
                 Myprop = new File("$srcRoot/overview.html" )               // OK:    setMyprop( File )    
                 setMyprop(  "$srcRoot/overview.html"  )                    // OK:    setMyprop( String )
                 setMyprop(  new File("$srcRoot/overview.html")  )          // OK:    setMyprop(  File  )    
          

          Ok, let's get rid of the getter entirely – we're not even using it.
          So now we have:

                 public void   setMyprop( String )
                 public void   setMyprop( File  )
          

          On the Groovy side, the '=' setter calls setMyprop( String )
          even when the argument is a File (note: this is true even if you
          rearrange the order of the statments in groovy or the method
          definitions in java):

                Myprop = "$srcRoot/overview.html"                          // OK:    setMyprop( String )
                Myprop = new File("$srcRoot/overview.html" )               // WEIRD: setMyprop( String )
                setMyprop(  "$srcRoot/overview.html"  )                    // OK:    setMyprop( String )
                setMyprop(  new File("$srcRoot/overview.html")  )          // OK:    setMyprop( File )
          

          This (mis)behavior is not specific to File and String,
          and does not depend upon whether the Java object has
          a field corresponing to the property (or what its type is).

          Suppose there's ONLY a String settter, but we've got a File getter:

                 public File   getMyprop()                     // NOTE:  return type File
                 public void   setMyprop( String )             // NOTE:  return type String
          

          Again, we get a casting error.
          It seems like there's a simple-minded rule that if you've got a getter return type
          then the setter is assumed to be of that same type, even if if there's no overload,
          and your're handed exactly the right type of the setter function you actually have:

           Myprop = "$srcRoot/overview.html"                   // WEIRD: String -> File casting error
           setMyprop("$srcRoot/overview.html")                 // OK:    SetMyprop(String)
          
          Show
          Jon Cox added a comment - Ouch, the code samples didn't get formatted properly (I think I mises a curly brace). Here's more code samples: Suppose you have this in Java: public String getMyprop() // NOTE: return type String public void setMyprop( String ) public void setMyprop( File ) The setMyprop(String) is called when using '=', even when the arg is a File: Myprop = "$srcRoot/overview.html" // OK: setMyprop( String ) Myprop = new File( "$srcRoot/overview.html" ) // WEIRD: setMyprop( String ) setMyprop( "$srcRoot/overview.html" ) // OK: setMyprop( String ) setMyprop( new File( "$srcRoot/overview.html" ) ) // OK: setMyprop( File ) Now change the return type of the getter to File: public File getMyprop() // NOTE: return type File public void setMyprop( String ) public void setMyprop( File ) The setMyprop(File) is called and triggers an error when using '=', even when the arg is a String an setMyprop(String) should have been called: Myprop = "$srcRoot/overview.html" // WEIRD: String -> File casting error Myprop = new File( "$srcRoot/overview.html" ) // OK: setMyprop( File ) setMyprop( "$srcRoot/overview.html" ) // OK: setMyprop( String ) setMyprop( new File( "$srcRoot/overview.html" ) ) // OK: setMyprop( File ) Ok, let's get rid of the getter entirely – we're not even using it. So now we have: public void setMyprop( String ) public void setMyprop( File ) On the Groovy side, the '=' setter calls setMyprop( String ) even when the argument is a File (note: this is true even if you rearrange the order of the statments in groovy or the method definitions in java): Myprop = "$srcRoot/overview.html" // OK: setMyprop( String ) Myprop = new File( "$srcRoot/overview.html" ) // WEIRD: setMyprop( String ) setMyprop( "$srcRoot/overview.html" ) // OK: setMyprop( String ) setMyprop( new File( "$srcRoot/overview.html" ) ) // OK: setMyprop( File ) This (mis)behavior is not specific to File and String, and does not depend upon whether the Java object has a field corresponing to the property (or what its type is). Suppose there's ONLY a String settter, but we've got a File getter: public File getMyprop() // NOTE: return type File public void setMyprop( String ) // NOTE: return type String Again, we get a casting error. It seems like there's a simple-minded rule that if you've got a getter return type then the setter is assumed to be of that same type, even if if there's no overload, and your're handed exactly the right type of the setter function you actually have: Myprop = "$srcRoot/overview.html" // WEIRD: String -> File casting error setMyprop( "$srcRoot/overview.html" ) // OK: SetMyprop( String )
          Hide
          blackdrag blackdrag added a comment -

          Reordering the occurrence of some methods in file does not always cause a new order in the class. So it is not a proper test.

          Now as of why this can't be done right now. The whole meta class layout is done to represent one getter and one setter. Changing this is a major change and a breaking change too. We said we do no breaking changes to the MOP before 2.0, so this has to wait. I am not saying we won't want to enable this, but it has to wait for 2.0 if we do it

          Show
          blackdrag blackdrag added a comment - Reordering the occurrence of some methods in file does not always cause a new order in the class. So it is not a proper test. Now as of why this can't be done right now. The whole meta class layout is done to represent one getter and one setter. Changing this is a major change and a breaking change too. We said we do no breaking changes to the MOP before 2.0, so this has to wait. I am not saying we won't want to enable this, but it has to wait for 2.0 if we do it
          Hide
          Jon Cox added a comment -

          None of the testing I did indicated that ordering had anything to do
          with the behavior of '='. It all seems to revolve around the types
          and overloads involved.

          It's great news to hear that this bug might get fixed in MOP 2.0.
          What's the rough timeline?

          Cheers,
          -Jon

          Show
          Jon Cox added a comment - None of the testing I did indicated that ordering had anything to do with the behavior of '='. It all seems to revolve around the types and overloads involved. It's great news to hear that this bug might get fixed in MOP 2.0. What's the rough timeline? Cheers, -Jon
          Hide
          Aaron Digulla added a comment -

          A possible solution for this might be an annotation which defines one or more converters:

          File   getMyprop()
          @Converters(ToFile.class)
          void   setMyprop( File )
          

          where ToFile implements "File convert (String)"

          Show
          Aaron Digulla added a comment - A possible solution for this might be an annotation which defines one or more converters: File getMyprop() @Converters(ToFile.class) void setMyprop( File ) where ToFile implements "File convert (String)"
          Hide
          blackdrag blackdrag added a comment -

          as for the timeline... we have not decided yet. Groovy 2.0 will probably bring some fundamental changes to the metaprogramming stuff so it will take a while to work them all out. The next Groovy version will be 1.7 and after that 2.0 is planed.

          As for the Annotation solution... the current problem is not so much that we are unable to detect multiple getters. The problem is more that we need to change the MOP to support that. And not only the resloving part. Since you can get a MetaProperty from the MetaClass and that class has only one setter, a change to the API is needed and one that might not work well with what we already have...

          But looking at it again I was wrong...nice... so I schedule it for 1.7 then

          Show
          blackdrag blackdrag added a comment - as for the timeline... we have not decided yet. Groovy 2.0 will probably bring some fundamental changes to the metaprogramming stuff so it will take a while to work them all out. The next Groovy version will be 1.7 and after that 2.0 is planed. As for the Annotation solution... the current problem is not so much that we are unable to detect multiple getters. The problem is more that we need to change the MOP to support that. And not only the resloving part. Since you can get a MetaProperty from the MetaClass and that class has only one setter, a change to the API is needed and one that might not work well with what we already have... But looking at it again I was wrong...nice... so I schedule it for 1.7 then
          Hide
          Jon Cox added a comment -

          Jochen,

          In your last stament, did you mean that groovy 1.7
          might support setter overloads?

          If so, would special annotations be required, or does
          the idea you've got in mind allow settter overloads without
          having to resort to special annotations?

          It would be really great if they could just work
          without any extra fiddling with annotations.

          Cheers,
          -Jon

          Show
          Jon Cox added a comment - Jochen, In your last stament, did you mean that groovy 1.7 might support setter overloads? If so, would special annotations be required, or does the idea you've got in mind allow settter overloads without having to resort to special annotations? It would be really great if they could just work without any extra fiddling with annotations. Cheers, -Jon
          Hide
          blackdrag blackdrag added a comment -

          I thought about supporting them without any annotation. I think it is doable for 1.7, but 1.7 is a bit away

          Show
          blackdrag blackdrag added a comment - I thought about supporting them without any annotation. I think it is doable for 1.7, but 1.7 is a bit away
          Hide
          Jon Cox added a comment -

          Awesome.
          Thanks for the update.

          -Jon

          Show
          Jon Cox added a comment - Awesome. Thanks for the update. -Jon

            People

            • Assignee:
              CÚdric Champeau
              Reporter:
              Aaron Digulla
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: