False positive C2801 (unnecessary-dunder-call) with unittest.mock.call

[jamesbraza]

Bug description

Here is a minimal repro:

# pylint: disable=missing-module-docstring, disable=missing-class-docstring

from unittest import TestCase
from unittest.mock import MagicMock, call, patch

class HasDunder:  # pylint: disable=too-few-public-methods
    def __setitem__(self, key: int, value: int) -> None:
        pass

class TestClass(TestCase):
    def test___setitem__(self) -> None:  # pylint: disable=missing-function-docstring
        all_mocks = MagicMock()

        has_dunder = HasDunder()
        with patch.object(HasDunder, "__setitem__") as mock__setitem__:
            all_mocks.attach_mock(mock__setitem__, "HasDunder.__setitem__")
            has_dunder[1] = 2
        all_mocks.assert_has_calls([call.HasDunder.__setitem__(1, 2)])

Configuration

No response

Command used

pylint a.py

Pylint output

************* Module a
a.py:20:36: C2801: Unnecessarily calls dunder method __setitem__. Set item via subscript. (unnecessary-dunder-call)

Expected behavior

I expect pylint to now throw an unnecessary-dunder-call here.

Pylint version

pylint 2.14.5
astroid 2.11.7
Python 3.10.4 (main, May 22 2022, 14:36:56) [Clang 13.0.0 (clang-1300.0.29.30)]

OS / Environment

macOS Big Sur v11.5.1

Additional dependencies

No response

[DanielNoord]

So basically, we shouldn't emit this warning if the call is within unittest.MagicMock.assert_has_calls?

[jamesbraza]

Yeah that is a good question, and I was sort of hoping you all'd know how to better generalize this 😅

The call.HasDunder.__setitem__ that's causing the pylint error is set up to match the string passed in the all_mocks.attach_mock(mock__setitem__, "HasDunder.__setitem__").

This:

all_mocks.assert_has_calls([call.HasDunder.__setitem__(1, 2)])

Can be re-written as:

call_ = call.HasDunder.__setitem__(1, 2)
all_mocks.assert_has_calls([call_])

And it would move the error message to be on the first call_ line. Two options:

• Don't emit the message universally for unittest.mock.call?
• Trace the call.XYZ.__setitem__ back to a attach_mock call, and then not emit a message if there's a __setitem__ within the string inside attach_mock?

[DanielNoord]

• Don't emit the message universally for unittest.mock.call?

I think this makes the most sense and is relatively easy to fix knowing how pylint deals with stuff like this. I like this proposal, thanks!

[jontwo]

I've just hit this same issue, as a result of following previous pylint advice!

Originally my code included this:

response = request.urlopen(request_obj)
retcode = response.getcode()

and in the test (using pytest-mock, which is a wrapper for unittest.mock)

mock_open = mocker.MagicMock()
mock_open.getcode.return_value = 418

Then pylint warned that I should be using urlopen as a context manager, so I changed the code to this:

with request.urlopen(request_obj) as response:
    retcode = response.getcode()

and setting the mock return value had to change to this, which triggered a C2801 warning:

mock_open.__enter__().getcode.return_value = 418

There may be a better way to set the return value on the mock, but this is how I've always done it. If pylint could allow dunder calls on mock objects that would be great!