Navigation Menu

Skip to content

Commit

Permalink
Fix calls to contains(Object) and get(Object) that pass a value of ap…
Browse files Browse the repository at this point in the history
…parently the wrong type.

Tighten up some generics in other cases to make the type, which was correct after all, look more correct.

BUGS:

AbstractBiMapTester:
- Fix inv.entrySet().contains(...) check, which was using the forward entry instead of the reverse.
- Fix getMap().get(v) call to be an inv.get(v) call.
- Use |reversed| instead of |entry| consistently for clarity.

TypeToken:
- Call map.get(K) instead of map.get(TypeCollector).
(Presumably this was just an optimization and not necessary for correctness?)

SIMPLIFICATIONS:

TypeResolver:
- forLookup always returns a (nullable) TypeVariableKey. Declare that return type instead of plain Object.
(benyu@: I feel like we may have talked about this, with your expressing a preference for the Object type. But I can find no record of the discussion, so I could be making that up. If you do prefer Object, I won't push for the change.)
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=108994208
  • Loading branch information
cpovirk committed Nov 30, 2015
1 parent 04ed591 commit 4362a45
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 10 deletions.
Expand Up @@ -63,14 +63,21 @@ protected void expectMissing(Entry<K, V>... entries) {
for (Entry<K, V> entry : entries) { for (Entry<K, V> entry : entries) {
Entry<V, K> reversed = reverseEntry(entry); Entry<V, K> reversed = reverseEntry(entry);
BiMap<V, K> inv = getMap().inverse(); BiMap<V, K> inv = getMap().inverse();
assertFalse("Inverse should not contain entry " + reversed, assertFalse(
inv.entrySet().contains(entry)); "Inverse should not contain entry " + reversed, inv.entrySet().contains(reversed));
assertFalse("Inverse should not contain key " + entry.getValue(), assertFalse(
inv.containsKey(entry.getValue())); "Inverse should not contain key " + reversed.getKey(),
assertFalse("Inverse should not contain value " + entry.getKey(), inv.containsKey(reversed.getKey()));
inv.containsValue(entry.getKey())); assertFalse(
assertNull("Inverse should not return a mapping for key " + entry.getValue(), "Inverse should not contain value " + reversed.getValue(),
getMap().get(entry.getValue())); inv.containsValue(reversed.getValue()));
/*
* TODO(cpovirk): This is a bit stronger than super.expectMissing(), which permits a <key,
* someOtherValue> pair.
*/
assertNull(
"Inverse should not return a mapping for key " + reversed.getKey(),
inv.get(reversed.getKey()));
} }
} }


Expand Down
2 changes: 1 addition & 1 deletion guava/src/com/google/common/reflect/TypeResolver.java
Expand Up @@ -485,7 +485,7 @@ static final class TypeVariableKey {
} }


/** Wraps {@code t} in a {@code TypeVariableKey} if it's a type variable. */ /** Wraps {@code t} in a {@code TypeVariableKey} if it's a type variable. */
static Object forLookup(Type t) { static TypeVariableKey forLookup(Type t) {
if (t instanceof TypeVariable) { if (t instanceof TypeVariable) {
return new TypeVariableKey((TypeVariable<?>) t); return new TypeVariableKey((TypeVariable<?>) t);
} else { } else {
Expand Down
2 changes: 1 addition & 1 deletion guava/src/com/google/common/reflect/TypeToken.java
Expand Up @@ -1167,7 +1167,7 @@ ImmutableList<K> collectTypes(Iterable<? extends K> types) {


/** Collects all types to map, and returns the total depth from T up to Object. */ /** Collects all types to map, and returns the total depth from T up to Object. */
private int collectTypes(K type, Map<? super K, Integer> map) { private int collectTypes(K type, Map<? super K, Integer> map) {
Integer existing = map.get(this); Integer existing = map.get(type);
if (existing != null) { if (existing != null) {
// short circuit: if set contains type it already contains its supertypes // short circuit: if set contains type it already contains its supertypes
return existing; return existing;
Expand Down

0 comments on commit 4362a45

Please sign in to comment.