Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GROOVY-9848: in operator: key set membership for isCase(Map,Object) #1904

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,7 @@ public static boolean isCase(Object caseValue, Object switchValue) {
}

/**
* Special 'Case' implementation for Class, which allows testing
* Special 'case' implementation for Class, which allows testing
* whether some switch value is assignable from the given case class.
*
* If the switch value is an object, {@code isCase} will return true if the
Expand Down Expand Up @@ -1163,9 +1163,8 @@ public static boolean isCase(Class caseValue, Object switchValue) {
}

/**
* 'Case' implementation for collections which tests if the 'switch'
* operand is contained in any of the 'case' values.
* For example:
* Special 'case' implementation for collections which tests if the 'switch'
* operand is contained in any of the 'case' values. For example:
* <pre class="groovyTestCase">switch( 3 ) {
* case [1,3,5]:
* assert true
Expand All @@ -1185,22 +1184,16 @@ public static boolean isCase(Collection caseValue, Object switchValue) {
}

/**
* 'Case' implementation for iterable types which tests if the 'switch'
* operand is contained in any of the 'case' values.
* For example:
* Special 'case' implementation for iterables which tests if the 'switch'
* operand is contained in any of the 'case' values. For example:
* <pre class="groovyTestCase">Iterable it = {[1,3,5].iterator()}
* switch( 3 ) {
* case it:
* assert true
* break
* default:
* assert false
* }
*
* //GROOVY-7919
* assert 1 in it
* assert 2 !in it
* </pre>
* }</pre>
*
* @param caseValue the case value
* @param switchValue the switch value
Expand All @@ -1213,11 +1206,10 @@ public static boolean isCase(Iterable caseValue, Object switchValue) {
}

/**
* 'Case' implementation for maps which tests the groovy truth
* value obtained using the 'switch' operand as key.
* For example:
* Special 'case' implementation for maps which tests if the 'switch' operand
* exists in the key set. For example:
* <pre class="groovyTestCase">switch( 'foo' ) {
* case [foo:true, bar:false]:
* case [foo:true]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make it really clear I would go one step further and change [foo:true] to [foo:false]

* assert true
* break
* default:
Expand All @@ -1226,11 +1218,11 @@ public static boolean isCase(Iterable caseValue, Object switchValue) {
*
* @param caseValue the case value
* @param switchValue the switch value
* @return the groovy truth value from caseValue corresponding to the switchValue key
* @return true if the key set of caseValue contains the switchValue
* @since 1.7.6
*/
public static boolean isCase(Map caseValue, Object switchValue) {
return DefaultTypeTransformation.castToBoolean(caseValue.get(switchValue));
public static boolean isCase(Map<?,?> caseValue, Object switchValue) {
return caseValue.containsKey(switchValue);
}

/**
Expand Down
4 changes: 1 addition & 3 deletions src/spec/test/OperatorsTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,6 @@ class OperatorsTest extends CompilableTestSupport {
// end::nullsafe[]
}

OperatorsTest() {
}

void testDirectFieldAccess() {
assertScript '''
// tag::direct_field_class[]
Expand Down Expand Up @@ -704,6 +701,7 @@ assert (b1 + b2).size == 15 // <1>
// end::operator_overload_op[]
'''
}

void testOperatorOverloadingWithDifferentArgumentType() {
assertScript '''
class Bucket {
Expand Down
71 changes: 63 additions & 8 deletions src/test/groovy/GroovyMethodsTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
package groovy

import groovy.test.GroovyTestCase
import org.codehaus.groovy.util.StringUtil

import java.awt.Dimension
import java.nio.CharBuffer
import java.util.concurrent.LinkedBlockingQueue
import org.codehaus.groovy.util.StringUtil

/**
* Tests various GDK methods
Expand Down Expand Up @@ -445,12 +445,6 @@ class GroovyMethodsTest extends GroovyTestCase {
assert map.size() == 2
}

void testInForLists() {
def list = ['a', 'b', 'c']
assert 'b' in list
assert !('d' in list)
}

void testFirstLastHeadTailInitForLists() {
def list = ['a', 'b', 'c']
assert 'a' == list.first()
Expand Down Expand Up @@ -548,10 +542,71 @@ class GroovyMethodsTest extends GroovyTestCase {
assert list.size() == 3
}

// GROOVY-9848
void testInForMaps() {
assert 'foo' in [foo:true]
assert 'foo' in [foo:null]
assert 'bar' !in [foo:true]
assert 'bar' !in [:].withDefault{ true }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again here I would use false as default to be more clear about the keys being used

}

void testInForSets() {
def set = ['a', 'b', 'c'] as Set
assert 'a' in set
assert 'b' in set
assert 'c' in set
assert 'd' !in set
assert !('d' in set)
assert !(null in set)
assert !(true in set)
}

void testInForLists() {
def list = ['a', 'b', 'c']
assert 'a' in list
assert 'b' in list
assert 'c' in list
assert 'd' !in list
assert !('d' in list)
assert !(null in list)
assert !(true in list)
}

void testInForArrays() {
String[] array = ['a', 'b', 'c']
def array = new String[]{'a', 'b', 'c'}
assert 'a' in array
assert 'b' in array
assert 'c' in array
assert 'd' !in array
assert !('d' in array)
assert !(null in array)
assert !(true in array)
}

// GROOVY-2456
void testInForStrings() {
def string = 'abc'
shouldFail { assert 'a' in string }
shouldFail { assert 'b' in string }
shouldFail { assert 'c' in string }
shouldFail { assert 'ab' in string }
shouldFail { assert 'bc' in string }
assert 'abc' in string
assert !('d' in string)
assert !(null in string)
assert !(true in string)
}

// GROOVY-7919
void testInForIterables() {
Iterable iter = { -> ['a','b','c'].iterator() }
assert 'a' in iter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually it would be nice to do assert 'a' in iter twice to show that the second time it fails

assert 'b' in iter
assert 'c' in iter
assert 'd' !in iter
assert !('d' in iter)
assert !(null in iter)
assert !(true in iter)
}

void testMaxForIterable() {
Expand Down
42 changes: 24 additions & 18 deletions src/test/groovy/MapTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -329,33 +329,39 @@ final class MapTest extends GroovyTestCase {
assert result2 == 'c2 b3 a1 '
}

void testMapWithDefault() {
def m = [:].withDefault {k -> k * 2}
m[1] = 3
assert m[1] == 3
assert m[2] == 4
assert [1: 3, 2: 4] == m
assert m == [1: 3, 2: 4]
}

void testMapIsCaseWithGrep() {
def predicate = [apple:true, banana:true, lemon:false, orange:false, pear:true]
def fruitList = ['apple', 'apple', 'pear', 'orange', 'pear', 'lemon', 'banana']
def expected = ['apple', 'apple', 'pear', 'pear', 'banana']
assert fruitList.grep(predicate) == expected
def predicate = [apple:true, banana:true, lemon:false, orange:false, pear:true]
predicate.retainAll{ e -> e.value } // GROOVY-9848: retain truthy entrys

def nonCitrus = fruitList.grep(predicate)
assert nonCitrus == ['apple', 'apple', 'pear', 'pear', 'banana']
}

void testMapIsCaseWithSwitch() {
switch ('foo') {
case [foo: true, bar: false]: assert true; break
default: assert false
assert switch ('foo') {
case [foo: true, bar: false] -> true
default -> false
}
assert switch ('foo') {
case [foo: false, bar: true] -> true
default -> false
}
switch ('bar') {
case [foo: true, bar: false]: assert false; break
default: assert true
assert switch ('bar') {
case [foo: true] -> false
default -> true
}
}

void testMapWithDefault() {
def m = [:].withDefault { k -> k * 2 }
m[1] = 3
assert m[1] == 3
assert m[2] == 4
assert [1: 3, 2: 4] == m
assert m == [1: 3, 2: 4]
}

void testMapWithDefaultCanBeConfiguredToNotStoreDefaultValue() {
def defaultValue = 0
def m = [:].withDefault(false, true) { defaultValue }
Expand Down