groovy

Cannot override toString() in a "mapOfClosures as AClass" implementation

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.6-beta-2
  • Fix Version/s: 1.6-rc-1, 1.5.8, 1.7-beta-1
  • Component/s: None
  • Labels:
    None
  • Testcase included:
    yes
  • Number of attachments :
    0

Description

I am unable to override toString() when I coerce a map-of-closures as a class. This works on 1.5.8 but not on 1.6-rc-1 and 1.7-beta-1 branches. The code below highlights the issue:

class GroovyXXXXBug extends GroovyTestCase {
	def void testOverrideToStringInMapOfClosuresAsClass() {
		def subObj = [ toString: { "Sub toString()" } ] as GroovyXXXXBugBase
		assert subObj.toString() == "Sub toString()"
	}
}

class GroovyXXXXBugBase {
   String toString() { "Base toString()" }
}

The code generated by ProxyGenerator for the overridden toString() method looks as below, which is not correct because of the extra args parameter and because of it the toString() gets overloaded and not overridden:

def toString(Object[] args) {
        this.@closureMap['toString'] (*args)
}

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Paul King added a comment -

Spotted the issue. We specifically exclude java.lang.Object methods. It is easy to handle direct case where such methods are overriden. I'll add a change for that. I'll think a little further about such methods further up the inheritance hierarchy.

Show
Paul King added a comment - Spotted the issue. We specifically exclude java.lang.Object methods. It is easy to handle direct case where such methods are overriden. I'll add a change for that. I'll think a little further about such methods further up the inheritance hierarchy.
Hide
Permalink
Paul King added a comment -

Roshan, want to try trunk? See if you think it behaves OK.

Show
Paul King added a comment - Roshan, want to try trunk? See if you think it behaves OK.
Hide
Permalink
Roshan Dawrani added a comment -

Tried the changes done to trunk for this issue and it seems to be working fine now. Can override toString() again.

Show
Roshan Dawrani added a comment - Tried the changes done to trunk for this issue and it seems to be working fine now. Can override toString() again.
Hide
Permalink
Guillaume Laforge added a comment -

So the issue should be closed now?

Show
Guillaume Laforge added a comment - So the issue should be closed now?
Hide
Permalink
Roshan Dawrani added a comment -

The implementation on the trunk fixes the issue but:

1) It has not been implemented on 1.6-rc-1 branch yet.

2) Paul's comments on trunk say "initial fix - perhaps a more elaborate fix and test to come". If he wants to fix it more elaborately, you may want to leave it open for that.

From my side, I see it fixed. As soon as it is marked fixed (indicating that from Paul's side it is done), I can close it from my side.

Show
Roshan Dawrani added a comment - The implementation on the trunk fixes the issue but: 1) It has not been implemented on 1.6-rc-1 branch yet. 2) Paul's comments on trunk say "initial fix - perhaps a more elaborate fix and test to come". If he wants to fix it more elaborately, you may want to leave it open for that. From my side, I see it fixed. As soon as it is marked fixed (indicating that from Paul's side it is done), I can close it from my side.
Hide
Permalink
Guillaume Laforge added a comment -

Okie dokie, thanks Roshan for the clarifications.

Show
Guillaume Laforge added a comment - Okie dokie, thanks Roshan for the clarifications.
Hide
Permalink
Paul King added a comment -

There is still a case not covered. If you have Parent extends Child and Child has an explicit Object method, e.g. toString(), it won't be overridden. I plan to have a look at that tonight.

Show
Paul King added a comment - There is still a case not covered. If you have Parent extends Child and Child has an explicit Object method, e.g. toString() , it won't be overridden. I plan to have a look at that tonight.
Hide
Permalink
Paul King added a comment -

Aligned behavior to be the same as for ConvertedMap behavior. Added test. Ported most of the behavior to the 1.5 branch.

Show
Paul King added a comment - Aligned behavior to be the same as for ConvertedMap behavior. Added test. Ported most of the behavior to the 1.5 branch.

People

  • Assignee:
    Paul King
    Reporter:
    Roshan Dawrani
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: