Issue 208 in mockito: anyMap causes unchecked cast warning in eclipse

[moc...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 208 by bhforumuser: anyMap causes unchecked cast warning in
eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Using anyMap() in eclipse causes a warning. If the code is written like
this:
verify(mockObject).methodWithMap(anyMap());

Then a warning will be issued of the following form:
"Type safety: The expression of type Map needs unchecked conversion to
conform to Map<X,Y>"

If the code is written like this:
verify(mockObject).methodWithMap((Map<X,Y>)anyMap());

Then a warning will be issued of the following form:
"Type safety: Unchecked cast from Map to Map<X,Y>"

Please add a mechanism to allow anyMap() objects to be created with type
safety. Possibilities may include adding generics to anyMap (making the
format "anyMap<X,Y>()") or adding arguments to the method (making the
format "anyMapOf(X.class, Y.class)").

[moc...@googlecode.com]

Comment #1 on issue 208 by szczepiq: anyMap causes unchecked cast warning
in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

We could do this but we prefer to @SupressWarnings in your tests. Why don't
you too? :)

[moc...@googlecode.com]

Comment #2 on issue 208 by bhforumuser: anyMap causes unchecked cast

warning in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

I consider it bad form to add @SuppressWarnings to an entire method; the
purpose of warnings is to appear, and suppressing all of them when I am not
the sole author of the code seems like an invitation to problems. I would
be fine with adding it to a particular line, either across the line as a
whole or as an attribute on a specific variable. I have not yet managed to
do so.

I have tried adding the @SuppressWarnings before the 'verify' statement:
@SuppressWarnings("unchecked")
verify(mockObject).methodWithMap(anyMap())

I have also tried adding the @SuppressWarnings to the anyMap() argument:
verify(mockObject).methodWithMap(@SuppressWarnings("unchecked") anyMap())

Both of these result in compiler errors, rather than warnings, which is
unfortunately a step in the wrong direction. I would greatly appreciate it
if either the anyMap functionality could be changed or if someone could
direct me to the proper usage of @SuppressWarnings in these cases.

[moc...@googlecode.com]

Comment #3 on issue 208 by szczepiq: anyMap causes unchecked cast warning
in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Hey

I put @SuppressWarnings on the entire test class usually and I avoid too
much generic typing to keep the test more compact & readable.

The purpose of warnings is to detect runtime errors earlier, at compilation
time (e.g. CCE). Since you are running the test when you change it you
detect the problem immediately anyway. Therefore the rationale for
type-safety warnings in tests is pretty much null to me. Test readability
wins.

More over, I believe that if you do TDD and reach coverage nearly ~100% it
also deprecates the sense of most bug-spotting warnings in production code.

Do feel I want convince you :) we are entitled to have different opinions.
I will run your idea through other maintainers. However bear in mind that
most of us are freaks around test readability. The more explicit typing,
the less essence is visible in the test. Hence I cannot guarantee we will
change anyMap().

You can create your own anyMap() method and statically import it to the
tests you like.

Cheers ;)

[moc...@googlecode.com]

Comment #4 on issue 208 by david.j.boden: anyMap causes unchecked cast

warning in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

It would be good if "anyMap"could work in exactly the same way as
Collections.emptyMap() and Collections.EMPTY_MAP.

In many situations, the compiler will be able to figure out which generic
types are required. Where the compiler can't figure it out, you can use
this syntax:

Collections.<String, String>emptyMap()

I don't propose that we need an ANY_MAP field seen as Mockito is strictly
Java 5 and above.
anyMap() gets changed to:

public static <Key,Value> Map<Key,Value> anyMap() {
return (Map<Key,Value>)reportMatcher(Any.ANY).returnMap();
}

Because of Type Erasure
(http://java.sun.com/docs/books/tutorial/java/generics/erasure.html) the
same object can be used to represent all empty Maps. However, I notice that
at the moment HandyReturnValues.returnMap returns a "new HashMap()" anyway.

[moc...@googlecode.com]

Comment #5 on issue 208 by szczepiq: anyMap causes unchecked cast warning
in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Hey,

I'm afraid we cannot go for public static <Key,Value> Map<Key,Value>
anyMap() because:

1. it will make some existing tests not-compiling
2. it will force the user to always use casting (or generifying) even if
the user does not like it and prefers cleaner tests + @SuppressWarnings

Cheers

[moc...@googlecode.com]

Comment #6 on issue 208 by szczepiq: anyMap causes unchecked cast warning
in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Perhaps I will create a wiki or blog post on good patterns around naughty
type-safety warnings.

[moc...@googlecode.com]

Comment #7 on issue 208 by david.j.boden: anyMap causes unchecked cast

warning in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Thanks. Anyway, after reading this thread more fully, I'm pretty much sold
on just adding @SuppressWarnings to the whole test class ;)

[moc...@googlecode.com]

Comment #8 on issue 208 by gfotos: anyMap causes unchecked cast warning in
eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Generic friendly aliases (e.g. anyCollectionOf()) have been added in 1.8.

But anyMapOf() is still missing and has been requested in issue #78 as well.

Any chance of adding it? Should I submit a patch or you still reject the
idea of (generic) checked tests?

[moc...@googlecode.com]

Comment #9 on issue 208 by gfotos: anyMap causes unchecked cast warning in
eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Ok here is the patch (with tests) against trunk.

Attachments:
mockito-anyMapOf.patch 2.8 KB

[moc...@googlecode.com]

Comment #10 on issue 208 by szczepiq: anyMap causes unchecked cast warning
in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Thanks for the patch. I'll ask other maintainers how do they feel about
this change.

> you still reject the idea of (generic) checked tests?

sure :)

[moc...@googlecode.com]

Comment #11 on issue 208 by gfo...@gmail.com: anyMap causes unchecked cast

warning in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

The patch still applies cleanly in head. Any news on this one?

[moc...@googlecode.com]

Updates:
Labels: -Type-Defect Type-Enhancement

Comment #12 on issue 208 by brice.du...@gmail.com: anyMap causes unchecked

cast warning in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Done there :
http://code.google.com/p/mockito/source/detail?r=bbefdd03ae7a353cae23f27722935fe456880e68

[moc...@googlecode.com]

Comment #13 on issue 208 by brice.du...@gmail.com: anyMap causes unchecked

cast warning in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

Might be interesting to add anyVarargOf too.

[moc...@googlecode.com]

Updates:
Status: Fixed

Comment #14 on issue 208 by brice.du...@gmail.com: anyMap causes unchecked

cast warning in eclipse
http://code.google.com/p/mockito/issues/detail?id=208

(No comment was entered for this change.)