Skip to content

Fix another non module root #2744

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.

Sign up for GitHub

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 4 commits into from
May 13, 2025
Merged

Fix another non module root #2744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26f72e6
add parents to Unknowns in the objectmodel
temyurchenko May 13, 2025
14d565a
restore the invariant of root
temyurchenko May 13, 2025
0553256
add xfailing "__eval__.get(1)" test
temyurchenko May 13, 2025
db6a898
enforce the correct parent on Unknown nodes
temyurchenko May 13, 2025
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
12 changes: 6 additions & 6 deletions astroid/interpreter/objectmodel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,17 +228,17 @@ def attr___package__(self):
@property
def attr___spec__(self):
# No handling for now.
return node_classes.Unknown()
return node_classes.Unknown(parent=self._instance)

@property
def attr___loader__(self):
# No handling for now.
return node_classes.Unknown()
return node_classes.Unknown(parent=self._instance)

@property
def attr___cached__(self):
# No handling for now.
return node_classes.Unknown()
return node_classes.Unknown(parent=self._instance)


class FunctionModel(ObjectModel):
Expand Down Expand Up @@ -462,7 +462,7 @@ def test(self):
# These are here just for completion.
@property
def attr___ne__(self):
return node_classes.Unknown()
return node_classes.Unknown(parent=self._instance)

attr___subclasshook__ = attr___ne__
attr___str__ = attr___ne__
Expand Down Expand Up @@ -493,8 +493,8 @@ def __init__(self):
super().__init__()

@property
def attr___annotations__(self) -> node_classes.Unkown:
return node_classes.Unknown()
def attr___annotations__(self) -> node_classes.Unknown:
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch, strange that this typo was not a problem anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My local checker (not mypy, pyright) found other typing issues in that file. Maybe mypy isn't running on this file for whatever reason.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, mypy doesn't run on a lot of file at the moment. Lots of dynamic things that are very hard to type. I would have thought that actual import of something that does not exist would have been an error, but it seems it's not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's due to from __future__ import annotations at the top. It makes python treat every annotation as a string. Without it, it would have been checked.

return node_classes.Unknown(parent=self._instance)

@property
def attr___module__(self):
Expand Down
5 changes: 4 additions & 1 deletion astroid/nodes/node_classes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4963,9 +4963,9 @@ class Unknown(_base_nodes.AssignTypeNode):

def __init__(
self,
parent: NodeNG,
lineno: None = None,
col_offset: None = None,
parent: None = None,
*,
Comment on lines 4964 to 4969
Copy link
Member

Choose a reason for hiding this comment

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

Should we consider this a breaking change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the procedure for handling this?

Copy link
Member

Choose a reason for hiding this comment

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

We'll wait for 4.0 to release it, and in astroid we just add a mildly strongly worded changelog entry. Or maybe we're not going to, I'm sure @DanielNoord will have an opinion about it :)

end_lineno: None = None,
end_col_offset: None = None,
Expand All @@ -4986,6 +4986,9 @@ def _infer(self, context: InferenceContext | None = None, **kwargs):
yield util.Uninferable


UNATTACHED_UNKNOWN = Unknown(parent=SYNTHETIC_ROOT)


class EvaluatedObject(NodeNG):
"""Contains an object that has already been inferred

Expand Down
4 changes: 2 additions & 2 deletions astroid/nodes/node_ng.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,13 +309,13 @@ def scope(self) -> nodes.LocalsDictNodeNG:
raise ParentMissingError(target=self)
return self.parent.scope()

def root(self) -> nodes.Module | nodes.Unknown:
def root(self) -> nodes.Module:
"""Return the root node of the syntax tree.

:returns: The root node.
"""
if not (parent := self.parent):
assert isinstance(self, (nodes.Module, nodes.Unknown))
assert isinstance(self, nodes.Module)
return self

while parent.parent:
Expand Down
4 changes: 2 additions & 2 deletions astroid/protocols.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,13 @@ def _filter_uninferable_nodes(
) -> Iterator[SuccessfulInferenceResult]:
for elt in elts:
if isinstance(elt, util.UninferableBase):
yield nodes.Unknown()
yield node_classes.UNATTACHED_UNKNOWN
else:
for inferred in elt.infer(context):
if not isinstance(inferred, util.UninferableBase):
yield inferred
else:
yield nodes.Unknown()
yield node_classes.UNATTACHED_UNKNOWN


@decorators.yes_if_nothing_inferred
Expand Down
3 changes: 2 additions & 1 deletion tests/test_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from astroid.builder import AstroidBuilder
from astroid.const import IS_PYPY
from astroid.exceptions import _NonDeducibleTypeHierarchy
from astroid.nodes.node_classes import UNATTACHED_UNKNOWN
from astroid.nodes.scoped_nodes import ClassDef


Expand Down Expand Up @@ -269,7 +270,7 @@ def test_uninferable_for_safe_infer() -> None:

def test_safe_infer_shim() -> None:
with pytest.warns(DeprecationWarning) as records:
helpers.safe_infer(nodes.Unknown())
helpers.safe_infer(UNATTACHED_UNKNOWN)

assert (
"Import safe_infer from astroid.util; this shim in astroid.helpers will be removed."
Expand Down
15 changes: 9 additions & 6 deletions tests/test_nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
StatementMissing,
)
from astroid.nodes.node_classes import (
UNATTACHED_UNKNOWN,
AssignAttr,
AssignName,
Attribute,
Expand Down Expand Up @@ -281,8 +282,10 @@ def test_f_strings(self):

@staticmethod
def test_as_string_unknown() -> None:
assert nodes.Unknown().as_string() == "Unknown.Unknown()"
assert nodes.Unknown(lineno=1, col_offset=0).as_string() == "Unknown.Unknown()"
unknown1 = nodes.Unknown(parent=SYNTHETIC_ROOT)
unknown2 = nodes.Unknown(lineno=1, col_offset=0, parent=SYNTHETIC_ROOT)
assert unknown1.as_string() == "Unknown.Unknown()"
assert unknown2.as_string() == "Unknown.Unknown()"

@staticmethod
@pytest.mark.skipif(
Expand Down Expand Up @@ -1231,9 +1234,9 @@ def test_starred_store(self) -> None:

def test_unknown() -> None:
"""Test Unknown node."""
assert isinstance(next(nodes.Unknown().infer()), type(util.Uninferable))
assert isinstance(nodes.Unknown().name, str)
assert isinstance(nodes.Unknown().qname(), str)
assert isinstance(next(UNATTACHED_UNKNOWN.infer()), type(util.Uninferable))
assert isinstance(UNATTACHED_UNKNOWN.name, str)
assert isinstance(UNATTACHED_UNKNOWN.qname(), str)


def test_type_comments_with() -> None:
Expand Down Expand Up @@ -1963,7 +1966,7 @@ def test_str_repr_no_warnings(node):
"NodeNG" in param_type.annotation
or "SuccessfulInferenceResult" in param_type.annotation
):
args[name] = nodes.Unknown()
args[name] = UNATTACHED_UNKNOWN
elif "str" in param_type.annotation:
args[name] = ""
else:
Expand Down
10 changes: 9 additions & 1 deletion tests/test_regrtest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ def _get_option(self, option):

def test_regression_root_is_not_a_module() -> None:
"""Regression test for #2672."""
node: nodes.Attribute = _extract_single_node(
node: nodes.ClassDef = _extract_single_node(
textwrap.dedent(
"""
a=eval.__get__(1).__gt__
Expand All @@ -515,6 +515,14 @@ class c: ...
assert node.name == "c"


@pytest.mark.xfail(reason="Not fixed yet")
def test_regression_eval_get_of_arg() -> None:
"""Regression test for #2743"""
node = _extract_single_node("eval.__get__(1)")
with pytest.raises(InferenceError):
next(node.infer())


def test_regression_no_crash_during_build() -> None:
node: nodes.Attribute = extract_node("__()")
assert node.args == []
Expand Down
Loading