[GROOVY-6144] Confusing treatment of m['foo'], m.get('foo'), m.foo, m.getFoo() for Maps

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 5.0.0-alpha-1
• Component/s: groovy-runtime
• Labels: breaking
• Assignee: Eric Milles
• Reporter: Richard Rattigan
• Created: 04/May/13 13:47
• Updated: 23/Aug/23 09:11
• Resolved: 25/Jan/22 18:21

Description

See the following example for some Map-related quirks that have bitten me a few times. I think it's crucial to iron out these inconsistencies in order to prevent the grooviness of maps from being a liability.

I think the rules should be:

1. subscript operator always uses map interface (for both get and set/put)
2. property access always prefers setFoo(bar) over set('foo', bar)
3. existence of setFoo() or getFoo() prevents use of map interface for property access, whether reading or writing (to avoid inconsistency between x = m.foo and m.foo = x)

I guess that accessing m.'foo' should be equivalent to accessing m.foo, but maybe it should be equivalent to m['foo']. I can see arguments for both.

def m = [:]
// 1. as expected, accessing a map by subscript
assert m['class'] == null
assert m['metaClass'] == null

// 2. which of these should pass? I think the current behaviour is questionable:
// getters should be preferred over map members for this syntax.
// I would expect these to pass instead: 
// assert m.class == m.getClass()
// assert m.metaClass == m.getMetaClass()
assert m.class == null // WRONG
assert m.'class' == null // WRONG (?)
assert m.metaClass == null // WRONG
assert m.'metaClass' == null // WRONG (?)
 
// 3. as expected, using map interface method
assert m.get('class') == null

// 4. as expected, using getter
assert m.getClass() instanceof Class

// 5. as expected, accessing map by subscript
assert (m['class'] = 'foo') == 'foo'

// 6. this is unexpected, and it is inconsistent with (1) and (5). I would expect the following to pass, 
// since we're using the subscript operator:
// assert (m['metaClass'] = 'foo') == 'foo'
try { m['metaClass'] = 'foo'; assert false } catch (ClassCastException e) { } // WRONG

// 7. this is consistent with (2), but I would argue that this should fail by the same logic: 
// Maps have a getClass(), therefore it has a property 'class'. 
// Attempting to set the property should attempt to invoke setClass(), which doesn't exist.
// It should therefore throw a MissingMethodException for setClass()
assert (m.class = 'foo') == 'foo' // WRONG
assert (m.'class' = 'foo') == 'foo' // WRONG (?)

// 8. I think this is doing the right thing - preferring the property over map semantics - but it is inconsistent with (2)
try { m.metaClass = 'foo'; assert false } catch (ClassCastException e) { }
try { m.'metaClass' = 'foo'; assert false } catch (ClassCastException e) { }

// 9. as expected, looking for setter, none exists
try { m.setClass('foo'); assert false } catch (MissingMethodException e) { }

Issue Links

• is related to: GROOVY-5001 Map access is given higher precedence when trying to access fields/properties in classes which implement java.util.Map or extend java.util.HashMap or java.util.Properties