Add inference of Compare nodes #979
Conversation
nelfin
force-pushed
the
feature/846-literal-compare
branch
2 times, most recently
from
50545b9 to
eca9202
Compare
|
Updated to reflect latest changes in astroid API and to catch an edge case I found in later testing. Since this is a new feature (even if it's small) this might be better as part of a minor release instead of a point release so I'm happy to wait for 2.8.x before merging. |
Pierre-Sassoulas
added
Enhancement ✨
There was a problem hiding this comment.
Hey, thank you for this, it looks awesome and will definitely go in 2.8.0. I don't have time for a full review yet, but could you add the typing for new or modified function please ? Circular import might appear, you can use if TYPE_CHECKING: to import if that's the case.
nelfin
force-pushed
the
feature/846-literal-compare
branch
from
eca9202 to
154f60c
Compare
|
Bleeeeergh, I set |
Ref pylint-dev#846. Identity checks are currently Uninferable as there is no sensible way to infer that two Instances refer to the same object without accurately modelling control flow.
nelfin
force-pushed
the
feature/846-literal-compare
branch
from
154f60c to
030f4d0
Compare
Pierre-Sassoulas
removed
the
Needs review 🔍
| @@ -790,6 +792,98 @@ def infer_binop(self, context=None): | |||
| nodes.BinOp._infer_binop = _infer_binop | |||
| nodes.BinOp._infer = infer_binop | |||
|
|
|||
| COMPARE_OPS = { | |||
There was a problem hiding this comment.
COMPARE_OPS: Dict[str, Callable[[Any, Any], bool]] = { ... }That would help inferring op_func and expr correctly.
| UNINFERABLE_OPS = { | ||
| "is", | ||
| "is not", | ||
| } |
| # Is this the stupidest idea or the simplest idea? | ||
| return ast.literal_eval(node.as_string()) |
There was a problem hiding this comment.
I like that! Something like this here would also work, but isn't as clean
if isinstance(node, nodes.Const):
return node.value
if isinstance(node, nodes.Tuple):
return tuple(_to_literal(val) for val in node.elts)
if isinstance(node, nodes.List):
return [_to_literal(val) for val in node.elts]
if isinstance(node, nodes.Set):
return set(_to_literal(val) for val in node.elts)
if isinstance(node, nodes.Dict):
return {_to_literal(k): _to_literal(v) for k, v in node.items}| return retval # it was all the same value | ||
|
|
||
|
|
||
| def _infer_compare(self: nodes.Compare, context: InferenceContext) -> Any: |
There was a problem hiding this comment.
In almost all _infer_xxx methods context is annotated as Optional[InferenceContext] with None as default. It works either way, but it might be nice to use the current default.
Steps
Description
This change adds a first attempt at inferring the boolean value of
Comparenodes.Type of Changes
Related Issue
Closes #846