pylint-dev / astroid

A common base representation of python source code for pylint and other projects
https://pylint.readthedocs.io/projects/astroid/en/latest/
GNU Lesser General Public License v2.1
533 stars 276 forks source link

Problem with partial and pickle #925

Open renefritze opened 3 years ago

renefritze commented 3 years ago

I'm not using astroid directly, but discovered this issue downstream using sphinx-autoapi. It seems I've run into an edge case with partial and the pickle module. I've added a new test case to astroid to showcase my problem: https://github.com/PyCQA/astroid/compare/master...renefritze:conditional_test_pickle

With the last commit on that branch the test suite succeeds: https://travis-ci.com/github/renefritze/astroid/jobs/487475875 Dropping the last commit, the suite fails: https://travis-ci.com/github/renefritze/astroid/jobs/487452325 The difference being https://github.com/PyCQA/astroid/commit/307f3acbfaa8bac3e7e4f1f0d0f38a965e54c53d

hippo91 commented 3 years ago

@renefritze thanks for your report. Would you mind opening a PR on astroid please?

renefritze commented 3 years ago

Sure thing.

Pierre-Sassoulas commented 3 years ago

Reopening because I did not understand that the merge request was only illustrative

renefritze commented 3 years ago

Is there anything more I can do to help that doesn't require in-depth astroid knowledge?

DanielNoord commented 2 years ago

I have investigated this a little.

 def test_conditional_definition_partial_pickle(self) -> None:
        """Regression test for a conditional defintion of a partial function wrapping pickle.

        Reported in https://github.com/PyCQA/astroid/issues/925
        """
        module = resources.build_file(
            "data/conditional_pickle_import/conditional_importer.py"
        )

        dump_nodes = module.lookup("dump")[1]
        assert len(dump_nodes) == 2

        # Check the function defintion node
        func_def_nodes = list(dump_nodes[0].infer())
        assert len(func_def_nodes) == 1
        assert isinstance(func_def_nodes[0], FunctionDef)

        # Check the partial node
        partial_node = list(dump_nodes[1].infer())
        assert len(partial_node) == 1
        assert isinstance(partial_node[0], objects.PartialFunction)

This should pass with:

# conditional_importer
import pickle

if False:

    def dump(obj, file, protocol=None):
        pass

else:
    from functools import partial

    dump = partial(pickle.dump, protocol=0)

The problem is that pickle.dump can be either pickle.dump or _pickle.dump. https://github.com/PyCQA/astroid/blob/0d1211558670cfefd95b39984b8d5f7f34837f32/astroid/brain/brain_functools.py#L77

On that line in the functools brain we take the first value, which is the _pickle one (I believe). We don't infer its arguments and therefore we fail here: https://github.com/PyCQA/astroid/blob/0d1211558670cfefd95b39984b8d5f7f34837f32/astroid/brain/brain_functools.py#L98-L99

We will need a tip for a pickle brain I think to solve this. I don't have much experience with adding brains so I probably won't get to this very soon but for anybody looking to fix this: I believe this is where we should be looking.