java.lang.RuntimeException: expected call to iterator(), instead saw null #866
Comments
|
Thanks for the report @g-jiannan and for the candidate fix! Before we land any potential fix here, I'd like to try to create a test case that reproduces this issue. Can you share the return type of the
This is surprising to me. We have code that specifically looks for a call to
This code is performance critical, so my instinct is to not use Error Prone matchers here, at least not without careful understanding of the performance implications. If we can reproduce the crash, we may be able to find a more localized fix. Thanks again! |
Here is the API javadoc for
It also surprises me. I guess it may because Set extends Collection, and Collection interface has
FYI: In Google, Error Prone (MethodMatchers) is heavily used when compile Java code.
The reported code is trivial and I tried to add a new test case to |
|
Hi @g-jiannan, I looked though some of the Error Prone code and came up with a narrower fix for this issue in #868. Could you test it out and see if it works for you? I think the potential issue here is that your Java compiler generates a call to Anyway, if you could test out the alternate fix in #868 I would appreciate it. I think even if we don't see performance differences in our benchmarks (which we do have, try running |
|
I see, I'm OK with either solution as long as the issue gets fixed :) Verified that #868 works, thanks! |
|
Great! I'll work on getting #868 reviewed and landed |
…Set (#868) Fixes #866 Before, we would check that an enhanced-for loop includes a call to `Set.iterator()` on the result of calling `Map.keySet()`. However, it is possible and legal that statically, the target of this call is instead `Collection.iterator()`. So, we change our check to test the receiver type passed into the call (which must still be a `Set`). Also, opportunistically switch a couple of places we were throwing `RuntimeException` around this check to throw the more meaningful `VerifyException`. Unfortunately, we have not found a way to add a test in open-source to reproduce the failure from #866 but we have confirmed this change fixes the problem.
|
@g-jiannan the fix for this issue has landed. Would it be helpful to have a new NullAway release containing the fix? If so, we can look into it. |
|
That would be better, please help to create a new release, thanks! |
|
Hi @msridhar Any chance we can have a new release recently? My colleague is waiting for it. Thanks! |
|
@g-jiannan we want to do some internal testing before cutting the release. We'll try to have the release out by the end of the week. /cc @lazaroclapp |
|
Got it, thanks for the update! |
|
@g-jiannan We have just cut a release of NullAway (0.10.18) with this fix. It will likely take a few hours to propagate through Maven Central, but should be ready later today 🙂 |
|
Awesome, thanks for helping! |
When we try to adopt NullAway (version 0.10.16), following exception is thrown unexpectedly sometimes:
The reported code
for (String key : sharedPreferences.getAll().keySet()) {looks trivial and the code pattern is already covered by theNullAwayKeySetIteratorTests. I'm not pretty sure the root cause but it turns out the problem is that AccessPathNullnessPropagation#isCallToMethod is not so robust: it expects Set.iterator() but the the receiver type is resolved as Collection unexpectedly sometimes.A quick fix is leverage errorprone matchers and I'll send pull request soon.
The text was updated successfully, but these errors were encountered: