Skip to content

Fix strong references to mutable objects in context.clone #927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ Release Date: TBA
Closes PyCQA/pylint#3970
Closes PyCQA/pylint#3595

* Fix some spurious cycles detected in ``context.path`` leading to more cases
that can now be inferred

Closes #926


What's New in astroid 2.5.6?
============================
Expand Down
2 changes: 1 addition & 1 deletion astroid/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def clone(self):
starts with the same context but diverge as each side is inferred
so the InferenceContext will need be cloned"""
# XXX copy lookupname/callcontext ?
clone = InferenceContext(self.path, inferred=self.inferred)
clone = InferenceContext(self.path.copy(), inferred=self.inferred.copy())
clone.callcontext = self.callcontext
clone.boundnode = self.boundnode
clone.extra_context = self.extra_context
Expand Down
6 changes: 2 additions & 4 deletions tests/unittest_brain_numpy_core_umath.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
except ImportError:
HAS_NUMPY = False

from astroid import bases, builder, nodes, util
from astroid import bases, builder, nodes


@unittest.skipUnless(HAS_NUMPY, "This test requires the numpy library.")
Expand Down Expand Up @@ -220,9 +220,7 @@ def test_numpy_core_umath_functions_return_type(self):
with self.subTest(typ=func_):
inferred_values = list(self._inferred_numpy_func_call(func_))
self.assertTrue(
len(inferred_values) == 1
or len(inferred_values) == 2
and inferred_values[-1].pytype() is util.Uninferable,
len(inferred_values) == 1,
msg="Too much inferred values ({}) for {:s}".format(
inferred_values[-1].pytype(), func_
),
Expand Down
35 changes: 34 additions & 1 deletion tests/unittest_inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -1704,7 +1704,8 @@ def __init__(self):
"""
ast = extract_node(code, __name__)
expr = ast.func.expr
self.assertIs(next(expr.infer()), util.Uninferable)
with pytest.raises(exceptions.InferenceError):
next(expr.infer())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test has changed behaviour. I assume that it's okay for it to raise InferenceError instead of returning Uninferable as long as it still doesn't leak StopIteration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems fair.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdce8p what is your opinion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with that part of the code base. If the pylint tests continue to pass, I would guess it's fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially this could cause new crash if an inference error is raised in calling code ? I don't think pylint tests cover all possible edge case.


def test_tuple_builtin_inference(self):
code = """
Expand Down Expand Up @@ -6032,5 +6033,37 @@ def test_infer_list_of_uninferables_does_not_crash():
assert not inferred.elts


# https://github.com/PyCQA/astroid/issues/926
def test_issue926_infer_stmts_referencing_same_name_is_not_uninferable():
code = """
pair = [1, 2]
ex = pair[0]
if 1 + 1 == 2:
ex = pair[1]
ex
"""
node = extract_node(code)
inferred = list(node.infer())
assert len(inferred) == 2
assert isinstance(inferred[0], nodes.Const)
assert inferred[0].value == 1
assert isinstance(inferred[1], nodes.Const)
assert inferred[1].value == 2


# https://github.com/PyCQA/astroid/issues/926
def test_issue926_binop_referencing_same_name_is_not_uninferable():
code = """
pair = [1, 2]
ex = pair[0] + pair[1]
ex
"""
node = extract_node(code)
inferred = list(node.infer())
assert len(inferred) == 1
assert isinstance(inferred[0], nodes.Const)
assert inferred[0].value == 3


if __name__ == "__main__":
unittest.main()
2 changes: 1 addition & 1 deletion tests/unittest_regrtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def test_numpy_crash(self):
astroid = builder.string_build(data, __name__, __file__)
callfunc = astroid.body[1].value.func
inferred = callfunc.inferred()
self.assertEqual(len(inferred), 2)
self.assertEqual(len(inferred), 1)

def test_nameconstant(self):
# used to fail for Python 3.4
Expand Down