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

Support "is None" constraints from if statements during inference #1189

Merged
merged 27 commits into from
Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
159af87
Support "is None" constraints from if statements during inference
david-yz-liu Aug 28, 2021
1c5b4b3
Merge branch 'main' into constraint-checking
Pierre-Sassoulas Dec 19, 2021
4dffa4d
Merge branch 'main' of https://github.com/PyCQA/astroid into constrai…
DanielNoord Mar 21, 2022
4828797
Changes
DanielNoord Mar 21, 2022
c39baff
Some typing
DanielNoord Mar 21, 2022
b54d841
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 21, 2022
de4c318
Some changes to typing and removal of dead code
DanielNoord Mar 21, 2022
3da1c83
Fix on python 3.7
DanielNoord Mar 21, 2022
7fe82e6
Fix import
DanielNoord Mar 21, 2022
fc4156e
Add abstractness
DanielNoord Mar 21, 2022
55bb33f
Update ChangeLog
Pierre-Sassoulas Mar 22, 2022
8a3b443
Simplify expression NoneConstraint.match
david-yz-liu Mar 23, 2022
5b86d5a
Merge remote-tracking branch 'upstream/main' into constraint-checking
david-yz-liu Jun 17, 2022
e032c0b
move constraint.py into submodule, and small fixes
david-yz-liu Jun 17, 2022
c1268a2
Fix imports and __all__
david-yz-liu Jun 17, 2022
7d0b6b2
Fix Union usage in type definition
david-yz-liu Jun 17, 2022
f963723
Add __future__ import to unittest_constraint.py
david-yz-liu Jun 17, 2022
21e4cf9
Merge remote-tracking branch 'upstream/main' into constraint-checking
david-yz-liu Jul 25, 2022
65c3c55
Updates based on review
david-yz-liu Jul 25, 2022
f6e001b
Use Self
DanielNoord Nov 20, 2022
5da24fe
Use InferenceResult
DanielNoord Nov 20, 2022
b46f001
Don't check None is
DanielNoord Nov 20, 2022
409543b
Use frozenset
DanielNoord Nov 20, 2022
316ba46
Also accept Proxy
DanielNoord Nov 20, 2022
dabf0e9
Use an Iterator
DanielNoord Nov 20, 2022
f70fbad
Merge branch 'main' into constraint-checking
Pierre-Sassoulas Nov 30, 2022
46c056f
Merge remote-tracking branch 'origin/main' into constraint-checking
Pierre-Sassoulas Jan 6, 2023
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
9 changes: 9 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ Release date: TBA

Closes #1195

* Support "is None" constraints from if statements during inference.

Ref #791
Ref PyCQA/pylint#157
Ref PyCQA/pylint#1472
Ref PyCQA/pylint#2016
Ref PyCQA/pylint#2631
Ref PyCQA/pylint#2880

What's New in astroid 2.11.7?
=============================
Release date: TBA
Expand Down
20 changes: 17 additions & 3 deletions astroid/bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,15 @@ def _infer_stmts(
) -> collections.abc.Generator[InferenceResult, None, None]:
"""Return an iterator on statements inferred by each statement in *stmts*."""
inferred = False
constraint_failed = False
if context is not None:
name = context.lookupname
context = context.clone()
constraints = context.constraints.get(name, {})
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
else:
name = None
context = InferenceContext()
constraints = {}
cdce8p marked this conversation as resolved.
Show resolved Hide resolved

for stmt in stmts:
if stmt is Uninferable:
Expand All @@ -144,16 +147,27 @@ def _infer_stmts(
# 'context' is always InferenceContext and Instances get '_infer_name' from ClassDef
context.lookupname = stmt._infer_name(frame, name) # type: ignore[union-attr]
try:
stmt_constraints = {
constraint
for constraint_stmt, constraint in constraints.items()
if not constraint_stmt.parent_of(stmt)
}
# Mypy doesn't recognize that 'stmt' can't be Uninferable
for inf in stmt.infer(context=context): # type: ignore[union-attr]
yield inf
inferred = True
if all(constraint.satisfied_by(inf) for constraint in stmt_constraints):
yield inf
inferred = True
else:
constraint_failed = True
except NameInferenceError:
continue
except InferenceError:
yield Uninferable
inferred = True
if not inferred:

if not inferred and constraint_failed:
yield Uninferable
elif not inferred:
raise InferenceError(
"Inference failed for all members of {stmts!r}.",
stmts=stmts,
Expand Down
10 changes: 10 additions & 0 deletions astroid/constraint/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Licensed under the LGPL: https://www.gnu.org/licenses/old-licenses/lgpl-2.1.en.html
# For details: https://github.com/PyCQA/astroid/blob/main/LICENSE
# Copyright (c) https://github.com/PyCQA/astroid/blob/main/CONTRIBUTORS.txt

"""Representations of logical constraints on values used during inference.
"""

from .constraint import get_constraints

__all__ = ("get_constraints",)
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
138 changes: 138 additions & 0 deletions astroid/constraint/constraint.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
# Licensed under the LGPL: https://www.gnu.org/licenses/old-licenses/lgpl-2.1.en.html
# For details: https://github.com/PyCQA/astroid/blob/main/LICENSE
# Copyright (c) https://github.com/PyCQA/astroid/blob/main/CONTRIBUTORS.txt

"""Classes representing different types of constraints on inference values."""
from __future__ import annotations

from abc import abstractmethod
from typing import TypeVar

from astroid import nodes, util

__all__ = ("get_constraints",)
cdce8p marked this conversation as resolved.
Show resolved Hide resolved


NameNodes = nodes.AssignAttr | nodes.Attribute | nodes.AssignName | nodes.Name
ConstraintT = TypeVar("ConstraintT", bound="Constraint")


class Constraint:
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
"""Represents a single constraint on a variable."""

def __init__(self, node: nodes.NodeNG, negate: bool) -> None:
self.node = node
"""The node that this constraint applies to."""
self.negate = negate
"""True if this constraint is negated. E.g., "is not" instead of "is"."""

@classmethod
@abstractmethod
def match(
cls: ConstraintT, node: NameNodes, expr: nodes.NodeNG, negate: bool = False
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
) -> ConstraintT | None:
"""Return a new constraint for node matched from expr, if expr matches
the constraint pattern.

If negate is True, negate the constraint.
"""

@abstractmethod
def satisfied_by(self, inferred: nodes.NodeNG | type[util.Uninferable]) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

Haven't checked it yet, can inferred also be of type Proxy here? That's a common result from the inference methods.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah this should be InferenceResult as it is dependent on stmt.infer() in bases._infer_stmts. That alias was added long after this PR was opened.

"""Return True if this constraint is satisfied by the given inferred value."""


class NoneConstraint(Constraint):
"""Represents an "is None" or "is not None" constraint."""

CONST_NONE: nodes.Const = nodes.Const(None)

@classmethod
def match(
cls: ConstraintT, node: NameNodes, expr: nodes.NodeNG, negate: bool = False
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
) -> ConstraintT | None:
"""Return a new constraint for node matched from expr, if expr matches
the constraint pattern.

Negate the constraint based on the value of negate.
"""
if isinstance(expr, nodes.Compare) and len(expr.ops) == 1:
left = expr.left
op, right = expr.ops[0]
if op in {"is", "is not"} and (
matches(left, node)
and matches(right, cls.CONST_NONE)
or matches(left, cls.CONST_NONE)
Copy link
Member

Choose a reason for hiding this comment

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

cls.CONST_NONE is a type error since cls is bound to Constraint. Here it can only ever be a NoneConstraint or a subclass of it. To reflect that, you need to create a new TypeVar unfortunately. At least until mypy supports the new Self typing. I.e.

_NoneConstraintT = TypeVar("_NoneConstraintT", bound="NoneConstraint")

and matches(right, node)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to check both directions? How many people are going to write None is x.y? The constant test is usually last.

This could even be a new pylint checker, to detect the reversed order. Tbh. though I have only ever seen the "correct" one x.y is None.

):
negate = (op == "is" and negate) or (op == "is not" and not negate)
return cls(node=node, negate=negate)

return None

def satisfied_by(self, inferred: nodes.NodeNG | type[util.Uninferable]) -> bool:
"""Return True if this constraint is satisfied by the given inferred value."""
# Assume true if uninferable
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
if inferred is util.Uninferable:
return True

return self.negate ^ matches(inferred, nodes.Const(None))
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
cdce8p marked this conversation as resolved.
Show resolved Hide resolved


def matches(node1: nodes.NodeNG, node2: nodes.NodeNG) -> bool:
"""Returns True if the two nodes match."""
if isinstance(node1, nodes.Name) and isinstance(node2, nodes.Name):
return node1.name == node2.name
if isinstance(node1, nodes.Attribute) and isinstance(node2, nodes.Attribute):
return node1.attrname == node2.attrname and matches(node1.expr, node2.expr)
if isinstance(node1, nodes.Const) and isinstance(node2, nodes.Const):
return node1.value == node2.value

return False


def get_constraints(
expr: NameNodes, frame: nodes.LocalsDictNodeNG
) -> dict[nodes.If, Constraint]:
"""Returns the constraints for the given expression.

The returned dictionary maps the node where the constraint was generated to the
corresponding constraint.

Constraints are computed statically by analysing the code surrounding expr.
Currently this only supports constraints generated from if conditions.
"""
current_node = expr
constraints: dict[nodes.If, Constraint] = {}
while current_node is not None and current_node is not frame:
parent = current_node.parent
if isinstance(parent, nodes.If):
branch, _ = parent.locate_child(current_node)
if branch == "body":
constraint = match_constraint(expr, parent.test)
elif branch == "orelse":
constraint = match_constraint(expr, parent.test, invert=True)
else:
constraint = None
cdce8p marked this conversation as resolved.
Show resolved Hide resolved

if constraint:
constraints[parent] = constraint
current_node = parent

return constraints


ALL_CONSTRAINTS = (NoneConstraint,)
"""All supported constraint types."""
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved


def match_constraint(
node: NameNodes, expr: nodes.NodeNG, invert: bool = False
) -> Constraint | None:
"""Returns a constraint pattern for node, if one matches."""
for constraint_cls in ALL_CONSTRAINTS:
constraint = constraint_cls.match(node, expr, invert)
if constraint:
return constraint

return None
6 changes: 6 additions & 0 deletions astroid/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from typing import TYPE_CHECKING, Dict, Optional, Sequence, Tuple

if TYPE_CHECKING:
from astroid import constraint, nodes
from astroid.nodes.node_classes import Keyword, NodeNG

_InferenceCache = Dict[
Expand All @@ -37,6 +38,7 @@ class InferenceContext:
"callcontext",
"boundnode",
"extra_context",
"constraints",
"_nodes_inferred",
)

Expand Down Expand Up @@ -87,6 +89,9 @@ def __init__(self, path=None, nodes_inferred=None):
for call arguments
"""

self.constraints: dict[str, dict[nodes.If, constraint.Constraint]] = {}
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
"""The constraints on nodes."""

@property
def nodes_inferred(self):
"""
Expand Down Expand Up @@ -137,6 +142,7 @@ def clone(self):
clone.callcontext = self.callcontext
clone.boundnode = self.boundnode
clone.extra_context = self.extra_context
clone.constraints = self.constraints.copy()
return clone

@contextlib.contextmanager
Expand Down
9 changes: 8 additions & 1 deletion astroid/inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from collections.abc import Callable, Generator, Iterable, Iterator
from typing import TYPE_CHECKING, Any, TypeVar

from astroid import bases, decorators, helpers, nodes, protocols, util
from astroid import bases, constraint, decorators, helpers, nodes, protocols, util
from astroid.context import (
CallContext,
InferenceContext,
Expand Down Expand Up @@ -213,6 +213,8 @@ def infer_name(self, context=None):
)
context = copy_context(context)
context.lookupname = self.name
context.constraints[self.name] = constraint.get_constraints(self, frame)

return bases._infer_stmts(stmts, context, frame)


Expand Down Expand Up @@ -317,6 +319,11 @@ def infer_attribute(self, context=None):
old_boundnode = context.boundnode
try:
context.boundnode = owner
if isinstance(owner, (nodes.ClassDef, bases.Instance)):
frame = owner if isinstance(owner, nodes.ClassDef) else owner._proxied
context.constraints[self.attrname] = constraint.get_constraints(
self, frame=frame
)
yield from owner.igetattr(self.attrname, context)
except (
AttributeInferenceError,
Expand Down
Loading