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 compatibility with astroid inference cache changes #8872

Merged
merged 14 commits into from
Aug 5, 2023
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
1 change: 1 addition & 0 deletions doc/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ sphinx-reredirects<1
towncrier~=23.6
furo==2023.5.20
-e .
astroid @ git+https://github.com/pylint-dev/astroid.git@cf8763a2b8e897ec7c8389906f3cb13714300cd2
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/529.performance
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
``pylint`` runs (at least) ~5% faster after improvements to ``astroid``
that make better use of the inference cache.

Refs pylint-dev/astroid#529
13 changes: 11 additions & 2 deletions pylint/checkers/design_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,11 +440,20 @@ def visit_classdef(self, node: nodes.ClassDef) -> None:
args=(nb_parents, self.linter.config.max_parents),
)

if len(node.instance_attrs) > self.linter.config.max_attributes:
# Something at inference time is modifying instance_attrs to add
# properties from parent classes. Given how much we cache inference
# results, mutating instance_attrs can become a real mess. Filter
# them out here until the root cause is solved.
# https://github.com/pylint-dev/astroid/issues/2273
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be a release blocker?

Copy link
Member Author

Choose a reason for hiding this comment

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

I doubt it, since the current situation is better (more deterministic).

Copy link
Member Author

Choose a reason for hiding this comment

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

The primer results are looking a lot better.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's with the deprecated-typing-alias messages?

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like the junk from #8790 is finally going away!

Copy link
Member Author

Choose a reason for hiding this comment

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

Btw, I made a stab at the root cause in astroid and made a bit of progress, but there is still a way to go before mergeable.

root = node.root()
filtered_attrs = [
k for (k, v) in node.instance_attrs.items() if v[0].root() is root
]
if len(filtered_attrs) > self.linter.config.max_attributes:
self.add_message(
"too-many-instance-attributes",
node=node,
args=(len(node.instance_attrs), self.linter.config.max_attributes),
args=(len(filtered_attrs), self.linter.config.max_attributes),
)

@only_required_for_messages("too-few-public-methods", "too-many-public-methods")
Expand Down
38 changes: 7 additions & 31 deletions pylint/checkers/typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@
import re
import shlex
import sys
import types
from collections.abc import Callable, Iterable, Iterator, Sequence
from collections.abc import Callable, Iterable
from functools import cached_property, singledispatch
from re import Pattern
from typing import TYPE_CHECKING, Any, Literal, TypeVar, Union
from typing import TYPE_CHECKING, Any, Literal, Union

import astroid
import astroid.exceptions
Expand Down Expand Up @@ -64,8 +63,6 @@
nodes.ClassDef,
]

_T = TypeVar("_T")

STR_FORMAT = {"builtins.str.format"}
ASYNCIO_COROUTINE = "asyncio.coroutines.coroutine"
BUILTIN_TUPLE = "builtins.tuple"
Expand Down Expand Up @@ -105,24 +102,6 @@ class VERSION_COMPATIBLE_OVERLOAD:
VERSION_COMPATIBLE_OVERLOAD_SENTINEL = VERSION_COMPATIBLE_OVERLOAD()


def _unflatten(iterable: Iterable[_T]) -> Iterator[_T]:
for index, elem in enumerate(iterable):
if isinstance(elem, Sequence) and not isinstance(elem, str):
yield from _unflatten(elem)
elif elem and not index:
# We're interested only in the first element.
yield elem # type: ignore[misc]


def _flatten_container(iterable: Iterable[_T]) -> Iterator[_T]:
# Flatten nested containers into a single iterable
for item in iterable:
if isinstance(item, (list, tuple, types.GeneratorType)):
yield from _flatten_container(item)
else:
yield item


def _is_owner_ignored(
owner: SuccessfulInferenceResult,
attrname: str | None,
Expand Down Expand Up @@ -1899,16 +1878,13 @@ def visit_with(self, node: nodes.With) -> None:

# Retrieve node from all previously visited nodes in the
# inference history
context_path_names: Iterator[Any] = filter(
None, _unflatten(context.path)
)
inferred_paths = _flatten_container(
safe_infer(path) for path in context_path_names
)
for inferred_path in inferred_paths:
for inferred_path, _ in context.path:
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
if not inferred_path:
continue
scope = inferred_path.scope()
if isinstance(inferred_path, nodes.Call):
scope = safe_infer(inferred_path.func)
else:
scope = inferred_path.scope()
if not isinstance(scope, nodes.FunctionDef):
continue
if decorated_with(
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ dependencies = [
# Also upgrade requirements_test_min.txt.
# Pinned to dev of second minor update to allow editable installs and fix primer issues,
# see https://github.com/pylint-dev/astroid/issues/1341
"astroid>=3.0.0a8,<=3.1.0-dev0",
"astroid @ git+https://github.com/pylint-dev/astroid.git@cf8763a2b8e897ec7c8389906f3cb13714300cd2",
Copy link
Member Author

Choose a reason for hiding this comment

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

We can then immediately go up to 3.0.0a9 next, but I wanted to do baby steps first in case more changes are needed.

"isort>=4.2.5,<6",
"mccabe>=0.6,<0.8",
"tomli>=1.1.0;python_version<'3.11'",
Expand Down
2 changes: 1 addition & 1 deletion requirements_test_min.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-e .[testutils,spelling]
# astroid dependency is also defined in pyproject.toml
astroid==3.0.0a8 # Pinned to a specific version for tests
astroid @ git+https://github.com/pylint-dev/astroid.git@cf8763a2b8e897ec7c8389906f3cb13714300cd2
typing-extensions~=4.7
py~=1.11.0
pytest~=7.4
Expand Down
Loading