Skip to content
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

Add typing to scope() #1170

Merged
merged 16 commits into from Sep 15, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion astroid/exceptions.py
Expand Up @@ -14,8 +14,13 @@

"""this module contains exceptions used in the astroid library
"""
from typing import TYPE_CHECKING

from astroid import util

if TYPE_CHECKING:
from astroid import nodes

__all__ = (
"AstroidBuildingError",
"AstroidBuildingException",
Expand Down Expand Up @@ -100,7 +105,7 @@ class TooManyLevelsError(AstroidImportError):
def __init__(
self,
message="Relative import with too many levels " "({level}) for module {name!r}",
**kws
**kws,
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
):
super().__init__(message, **kws)

Expand Down Expand Up @@ -256,6 +261,18 @@ class InferenceOverwriteError(AstroidError):
"""


class ParentMissingError(AstroidError):
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
"""Raised when a node which is expected to have a parent attribute is missing one

Standard attributes:
target: The node for which the parent lookup failed.
"""

def __init__(self, target: "nodes.NodeNG") -> None:
self.target = target
super().__init__(message=f"Parent not found on {target!r}.")


# Backwards-compatibility aliases
OperationError = util.BadOperationMessage
UnaryOperationError = util.BadUnaryOperationMessage
Expand Down
3 changes: 3 additions & 0 deletions astroid/nodes/__init__.py
Expand Up @@ -113,6 +113,7 @@
GeneratorExp,
Lambda,
ListComp,
LocalsDictNodeNG,
Module,
SetComp,
builtin_lookup,
Expand Down Expand Up @@ -176,6 +177,7 @@
Lambda,
List,
ListComp,
LocalsDictNodeNG,
Match,
MatchAs,
MatchCase,
Expand Down Expand Up @@ -268,6 +270,7 @@
"Lambda",
"List",
"ListComp",
"LocalsDictNodeNG",
"Match",
"MatchAs",
"MatchCase",
Expand Down
14 changes: 11 additions & 3 deletions astroid/nodes/node_classes.py
Expand Up @@ -40,7 +40,7 @@
import sys
import typing
from functools import lru_cache
from typing import Callable, Generator, Optional
from typing import TYPE_CHECKING, Callable, Generator, Optional

from astroid import decorators, mixins, util
from astroid.bases import Instance, _infer_stmts
Expand All @@ -51,6 +51,7 @@
AstroidTypeError,
InferenceError,
NoDefault,
ParentMissingError,
)
from astroid.manager import AstroidManager
from astroid.nodes.const import OP_PRECEDENCE
Expand All @@ -61,6 +62,9 @@
else:
from typing_extensions import Literal

if TYPE_CHECKING:
from astroid.nodes import LocalsDictNodeNG


def _is_const(value):
return isinstance(value, tuple(CONST_CLS))
Expand Down Expand Up @@ -2016,13 +2020,17 @@ def postinit(self, nodes: typing.List[NodeNG]) -> None:
"""
self.nodes = nodes

def scope(self):
def scope(self) -> "LocalsDictNodeNG":
"""The first parent node defining a new scope.
These can be Module, FunctionDef, ClassDef, Lambda, or GeneratorExp nodes.

:returns: The first parent scope node.
:rtype: Module or FunctionDef or ClassDef or Lambda or GenExpr
"""
# skip the function node to go directly to the upper level scope
if not self.parent:
raise ParentMissingError(target=self)
if not self.parent.parent:
raise ParentMissingError(target=self.parent)
return self.parent.parent.scope()

def get_children(self):
Expand Down
32 changes: 25 additions & 7 deletions astroid/nodes/node_ng.py
@@ -1,14 +1,32 @@
import pprint
import typing
from functools import singledispatch as _singledispatch
from typing import ClassVar, Iterator, Optional, Tuple, Type, TypeVar, Union, overload
from typing import (
TYPE_CHECKING,
ClassVar,
Iterator,
Optional,
Tuple,
Type,
TypeVar,
Union,
overload,
)

from astroid import decorators, util
from astroid.exceptions import AstroidError, InferenceError, UseInferenceDefault
from astroid.exceptions import (
AstroidError,
InferenceError,
ParentMissingError,
UseInferenceDefault,
)
from astroid.manager import AstroidManager
from astroid.nodes.as_string import AsStringVisitor
from astroid.nodes.const import OP_PRECEDENCE

if TYPE_CHECKING:
from astroid.nodes import LocalsDictNodeNG

# Types for 'NodeNG.nodes_of_class()'
T_Nodes = TypeVar("T_Nodes", bound="NodeNG")
T_Nodes2 = TypeVar("T_Nodes2", bound="NodeNG")
Expand Down Expand Up @@ -251,15 +269,15 @@ def frame(self):
"""
return self.parent.frame()

def scope(self):
def scope(self) -> "LocalsDictNodeNG":
"""The first parent node defining a new scope.
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
These can be Module, FunctionDef, ClassDef, Lambda, or GeneratorExp nodes.

:returns: The first parent scope node.
:rtype: Module or FunctionDef or ClassDef or Lambda or GenExpr
"""
if self.parent:
return self.parent.scope()
return None
if not self.parent:
raise ParentMissingError(target=self)
return self.parent.scope()

def root(self):
"""Return the root node of the syntax tree.
Expand Down
6 changes: 4 additions & 2 deletions astroid/nodes/scoped_nodes.py
Expand Up @@ -44,7 +44,7 @@
import io
import itertools
import typing
from typing import List, Optional
from typing import List, Optional, TypeVar

from astroid import bases
from astroid import decorators as decorators_mod
Expand Down Expand Up @@ -78,6 +78,8 @@
{"classmethod", "staticmethod", "builtins.classmethod", "builtins.staticmethod"}
)

T = TypeVar("T")
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this part, what does T means ?

Copy link
Collaborator Author

@DanielNoord DanielNoord Sep 15, 2021

Choose a reason for hiding this comment

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

I don't know how it works under the hood. But for

def scope(self: T) -> T:
        """The first parent node defining a new scope.

        :returns: The first parent scope node.
        :rtype: Module or FunctionDef or ClassDef or Lambda or GenExpr
        """
        return self

It makes the function return with the correct typing. It sets the return type to the type of self.

Copy link
Member

Choose a reason for hiding this comment

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

Ok I've read the documentation and I think T = TypeVar("T") is the example for TypeVar from the python doc, but I guess we can change the name to something else ? Maybe Scope (Scope = TypeVar("Scope")) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#1170 (comment)

This is something for you to discuss with Marc 😄

Copy link
Member

Choose a reason for hiding this comment

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

@cdce8p what do you think, isn't it better to have an explicit name here ?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the discrepancies @DanielNoord I started to not review some MR that Marc is already reviewing because there is a lot to review :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No worries! With the typing PRs it never hurts to have an additional pair of eyes.

Copy link
Member

Choose a reason for hiding this comment

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

It would be possible to rename the TypeVar, I just wouldn't recommend it in this case. T is the usual convention. Additionally, it doesn't have any constraints or bounds. Thus it could be reused in another function later.

That is, for example, different in the TypeVar we used in pylint-dev/pylint#4978.



def _c3_merge(sequences, cls, context):
"""Merges MROs in *sequences* to a single MRO using the C3 algorithm.
Expand Down Expand Up @@ -238,7 +240,7 @@ def frame(self):
"""
return self

def scope(self):
def scope(self: T) -> T:
"""The first parent node defining a new scope.

:returns: The first parent scope node.
Expand Down
1 change: 1 addition & 0 deletions doc/api/astroid.exceptions.rst
Expand Up @@ -30,6 +30,7 @@ Exceptions
NameInferenceError
NoDefault
NotFoundError
ParentMissingError
ResolveError
SuperArgumentTypeError
SuperError
Expand Down
4 changes: 2 additions & 2 deletions doc/api/base_nodes.rst
Expand Up @@ -12,7 +12,7 @@ These are abstract node classes that :ref:`other nodes <nodes>` inherit from.
astroid.mixins.FilterStmtsMixin
astroid.mixins.ImportFromMixin
astroid.nodes.ListComp
astroid.nodes.scoped_nodes.LocalsDictNodeNG
astroid.nodes.LocalsDictNodeNG
astroid.nodes.node_classes.LookupMixIn
astroid.nodes.NodeNG
astroid.mixins.ParentAssignTypeMixin
Expand All @@ -34,7 +34,7 @@ These are abstract node classes that :ref:`other nodes <nodes>` inherit from.

.. autoclass:: astroid.nodes.ListComp

.. autoclass:: astroid.nodes.scoped_nodes.LocalsDictNodeNG
.. autoclass:: astroid.nodes.LocalsDictNodeNG

.. autoclass:: astroid.nodes.node_classes.LookupMixIn

Expand Down
1 change: 0 additions & 1 deletion tests/unittest_builder.py
Expand Up @@ -551,7 +551,6 @@ def func2(a={}):
"""
builder.parse(code)
nonetype = nodes.const_factory(None)
# pylint: disable=no-member; Infers two potential values
self.assertNotIn("custom_attr", nonetype.locals)
self.assertNotIn("custom_attr", nonetype.instance_attrs)
nonetype = nodes.const_factory({})
Expand Down
1 change: 0 additions & 1 deletion tests/unittest_lookup.py
Expand Up @@ -393,7 +393,6 @@ def test_builtin_lookup(self) -> None:
self.assertEqual(len(intstmts), 1)
self.assertIsInstance(intstmts[0], nodes.ClassDef)
self.assertEqual(intstmts[0].name, "int")
# pylint: disable=no-member; Infers two potential values
self.assertIs(intstmts[0], nodes.const_factory(1)._proxied)

def test_decorator_arguments_lookup(self) -> None:
Expand Down
1 change: 0 additions & 1 deletion tests/unittest_nodes.py
Expand Up @@ -615,7 +615,6 @@ def test_as_string(self) -> None:
class ConstNodeTest(unittest.TestCase):
def _test(self, value: Any) -> None:
node = nodes.const_factory(value)
# pylint: disable=no-member; Infers two potential values
self.assertIsInstance(node._proxied, nodes.ClassDef)
self.assertEqual(node._proxied.name, value.__class__.__name__)
self.assertIs(node.value, value)
Expand Down
2 changes: 1 addition & 1 deletion tests/unittest_scoped_nodes.py
Expand Up @@ -277,7 +277,7 @@ def test_file_stream_api(self) -> None:
path = resources.find("data/all.py")
file_build = builder.AstroidBuilder().file_build(path, "all")
with self.assertRaises(AttributeError):
# pylint: disable=pointless-statement, no-member
# pylint: disable=pointless-statement
file_build.file_stream

def test_stream_api(self) -> None:
Expand Down