Spotbugs: false positive RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT

Created on 11 Feb 2019  路  4Comments  路  Source: spotbugs/spotbugs

Mockito.verify(converter, times(1)).convert(isA(SFDLBean.class));

This code leads to:

Return value of method without side effect is ignored
This code calls a method and ignores the return value. However our analysis shows that the method (including its implementations in subclasses if any) does not produce any effect other than return value. > Thus this call can be removed.
We are trying to reduce the false positives as much as possible, but in some cases this warning might be wrong. Common false-positive cases include:

  • The method is designed to be overridden and produce a side effect in other projects which are out of > the scope of the analysis.
  • The method is called to trigger the class loading which may have a side effect.
  • The method is called just to get some exception.
    If you feel that our assumption is incorrect, you can use a @CheckReturnValue annotation to instruct SpotBugs that ignoring the return value of this method is acceptable.

Bug kind and pattern: RV - RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT

The verify method does effect the code - it breaks the unit test if the method is not called 1 time. Its more or less like an assert.
I will workaround this now with @CheckReturnValue but for me, this is a very common use case. Is it possible to fix that in spotbugs? Thanks!

Most helpful comment

+1

All 4 comments

Are you sure that the bug detector is complaining about the verify call, not the convert call?

My workplace uses Mockito all the time and I haven't seen this cause problems.

Yes it is complaining about the convert() which is called with verify().
A verify() should never require to check the return value, as this is a verify() to check that this method has been called, not if there is any return value to that and if that value has been checked in any way.

I just have another place where i have exact the same problem.

verify(myDatabase).registrationPasswordMatches(eq("sha"));
This is in a class called SecurityHandlerTest.java
It makes absolutely no sense to test here any return value of the registrationPasswordMatches() method from myDatabase, as it is irrelevant. If i would test that (MyDatabase class) inside this test (SecurityHandlerTest), i would do a integration test, not a unit test.

+1

+1
I'm hitting this bug when using Mockito.verify() and an argument captor to test the value passed into a Map.get() call:

ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
verify(mockMap).get(captor.capture());
assertEquals("testValue", captor.getValue());
Was this page helpful?
0 / 5 - 0 ratings

Related issues

dragonjuggler picture dragonjuggler  路  6Comments

alcastic picture alcastic  路  4Comments

jsotuyod picture jsotuyod  路  11Comments

KengoTODA picture KengoTODA  路  5Comments

wrprice picture wrprice  路  3Comments