groovy

spread-dot operator problem in 1.5

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Won't Fix
  • Affects Version/s: 1.5
  • Fix Version/s: 1.6-rc-2
  • Component/s: None
  • Labels:
    None
  • Environment:
    Groovy Version: 1.5.0 JVM: 1.6.0_01-b06
    os.name=Windows XP
    os.version=5.1
  • Testcase included:
    yes
  • Number of attachments :
    0

Description

Try this part from Groovy in Action(Listing_7_23_GPath.groovy):

class Invoice {                                          //|#1
    List    items                                        //|#1
    Date    date                                         //|#1
}                                                        //|#1
class LineItem {                                         //|#1
    Product product                                      //|#1
    int     count                                        //|#1
    int total() {                                        //|#1
        return product.dollar * count                    //|#1
    }                                                    //|#1
}                                                        //|#1
class Product {                                          //|#1
    String  name                                         //|#1
    def     dollar                                       //|#1
}                                                        //|#1

def ulcDate = new Date(107,0,1)
def ulc = new Product(dollar:1499, name:'ULC')           //|#2
def ve  = new Product(dollar:499,  name:'Visual Editor') //|#2
                                                         //|#2
def invoices = [                                         //|#2
    new Invoice(date:ulcDate, items: [                   //|#2
        new LineItem(count:5, product:ulc),              //|#2
        new LineItem(count:1, product:ve)                //|#2
    ]),                                                  //|#2
    new Invoice(date:[107,1,2], items: [                 //|#2
        new LineItem(count:4, product:ve)                //|#2
    ])                                                   //|#2
]                                                        //|#2

assert [5*1499, 499, 4*499] == invoices.items*.total()   //#3

Fails at last line //#3 with:
Caught: groovy.lang.MissingMethodException: No signature of method: java.util.ArrayList.total() is applicable for argument types: () values: {}
at Listing_7_23_GPath.run(Listing_7_23_GPath.groovy:31)
at Listing_7_23_GPath.main(Listing_7_23_GPath.groovy)

Hint
If I restore the org.codehaus.groovy.runtime.DefaultGroovyMethods.getAt(Collection coll, String property) from groovy 1.0 code regarding answer like this:

// groovy 1.0
if (value instanceof Collection) {
    answer.addAll((Collection) value);
} else {
    answer.add(value);
}

istead of

// groovy 1.5.0
answer.add(value)

then the Listing_7_23_GPath.groovy works fine

Activity

Hide
Paul King added a comment -

With the current 1.5.0 behavior, the last line becomes:

assert [5 * 1499, 499, 4 * 499] == invoices.items.collect{ it*.total() }.flatten()
Show
Paul King added a comment - With the current 1.5.0 behavior, the last line becomes:
assert [5 * 1499, 499, 4 * 499] == invoices.items.collect{ it*.total() }.flatten()
Hide
Paul King added a comment -

The code in NodeList#getAt(String name) still has:

if (temp instanceof Collection) {
        answer.addAll((Collection) temp);
    } else {
        answer.add(temp);
    }

I suspect we should make these consistent before 1.5.1.

Show
Paul King added a comment - The code in NodeList#getAt(String name) still has:
if (temp instanceof Collection) {
        answer.addAll((Collection) temp);
    } else {
        answer.add(temp);
    }
I suspect we should make these consistent before 1.5.1.
Hide
Dierk Koenig added a comment -

I consider this a major bug and assign to Jochen to prevent that it slips through again. Feel free to assign it to any other committer.
We shouldn't have *any" unassigned major bugs, though.

Show
Dierk Koenig added a comment - I consider this a major bug and assign to Jochen to prevent that it slips through again. Feel free to assign it to any other committer. We shouldn't have *any" unassigned major bugs, though.
Hide
Paul King added a comment -

It is more a case of working out what to do rather than it slipping through unnoticed.

We have changed the semantics of GPath related to this issue in a way which makes one use case look much better using the new semantics but other use cases seem to look better the old way. My issue with the old way is that it applies a level of magic that sometimes you don't want and don't have a away of turning off. The downside is that other scenarios are now more typing. The double downside is we didn't align NodeList which still works the old way. That might be OK but I don't think we have conclusive thoughts on that.

I think it is major too though and really think it should be resolved (including the NodeList implications) before 1.5.2. I would almost be inclined to make it a blocker myself.

Show
Paul King added a comment - It is more a case of working out what to do rather than it slipping through unnoticed. We have changed the semantics of GPath related to this issue in a way which makes one use case look much better using the new semantics but other use cases seem to look better the old way. My issue with the old way is that it applies a level of magic that sometimes you don't want and don't have a away of turning off. The downside is that other scenarios are now more typing. The double downside is we didn't align NodeList which still works the old way. That might be OK but I don't think we have conclusive thoughts on that. I think it is major too though and really think it should be resolved (including the NodeList implications) before 1.5.2. I would almost be inclined to make it a blocker myself.
Hide
blackdrag blackdrag added a comment -

it is not NodeList#getAt, it is Collection#getAt that is causing this problem. I voted against the change, nobody seemed to be interested in keeping the old version so it got changed. Now I can't just go and revert that change. Looking at its history this is not even a normal bug and I would have to close this issue with "won't fix". If it is anything, then maybe a design bug or maybe "error in reasoning". On the other hand I can not simply assign this issue to someone else.. the problem wouldn't change through this.

Show
blackdrag blackdrag added a comment - it is not NodeList#getAt, it is Collection#getAt that is causing this problem. I voted against the change, nobody seemed to be interested in keeping the old version so it got changed. Now I can't just go and revert that change. Looking at its history this is not even a normal bug and I would have to close this issue with "won't fix". If it is anything, then maybe a design bug or maybe "error in reasoning". On the other hand I can not simply assign this issue to someone else.. the problem wouldn't change through this.
Hide
Paul King added a comment -

Actually, thinking about it a bit more, the currently preferred solution would be:

assert [5*1499, 499, 4*499] == invoices.items.sum()*.total()

So, this example isn't as bad as I thought in terms of extra typing. In fact, I think I quite like this.

Show
Paul King added a comment - Actually, thinking about it a bit more, the currently preferred solution would be:
assert [5*1499, 499, 4*499] == invoices.items.sum()*.total()
So, this example isn't as bad as I thought in terms of extra typing. In fact, I think I quite like this.
Hide
Guillaume Laforge added a comment -

We'll have to update GinA with the new behaviour

Show
Guillaume Laforge added a comment - We'll have to update GinA with the new behaviour

People

  • Assignee:
    Unassigned
    Reporter:
    Genrich
Vote (2)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: