Rule to detect map.get(wrongType)

[TheBeruriahIncident]

What happened?

It's currently easy to write code like:

Map<Foo, Object> map;
Bar key;
Object value = map.get(key)

and not realize you have a bug. This is particularly insidious with things like wrapped strings, where you get with the right "thing", you just have to wrap or unwrap it.

For example, see commit 331070 in the major internal repo I work on.

What did you want to happen?

Errorprone to flag that this can't work. IntelliJ does, so I expect this is doable.

CC @carterkozak

[carterkozak]

👍 I'll take a look

[iamdanfox]

I think this is already implemented: https://errorprone.info/bugpattern/CollectionIncompatibleType

I wrote the following code and error-prone refused to compile it:

        ImmutableMap<String, String> of = ImmutableMap.of("foo", "bar");
        System.out.println(of.get(1));

> Task :conjure-backcompat:compileTestJava
/Volumes/git/conjure-backcompat/conjure-backcompat/src/test/java/com/palantir/conjure/backcompat/ConjureBackCompatTest.java:49: error: [CollectionIncompatibleType] Argument '1' should not be passed to this method; its type int is not compatible with its collection's type argument String
        System.out.println(of.get(1));
                                 ^
    (see https://errorprone.info/bugpattern/CollectionIncompatibleType)

[carterkozak]

I don't think CollectionIncompatibleType is quite as strict as we would like, it appears to catch impossible cases, but not those that are unlikely to be correct, for example:

interface FirstIface {}
interface SecondIface {}

String read(Map<FirstIface, String> map, SecondIface key) {
  // it's possible the SecondIface happens to implement FirstIface and this works correctly,
  // but not likely. Idea warns in this case. It would be better to leverage the type system.
  return map.get(key);
}

[carterkozak]

To clarify, we enforce CollectionIncompatibleType in the project where @akroy discovered the bug which prompted this issue!

[markelliot]

Maybe it’d make sense for the fix should be pushed upstream, then?

[carterkozak]

Absolutely, though I’ve had less engagement on the upstream error-prone project than others like guava, conscrypt, etc.

…

-ck

On Nov 6, 2019, at 8:39 AM, Mark Elliot ***@***.***> wrote: Maybe it’d make sense for the fix should be pushed upstream, then? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or unsubscribe.