Skip to content

Commit

Permalink
Simplified inference on None returns (#45)
Browse files Browse the repository at this point in the history
This patch simplifies how we infer that the function returns `None`
since the previous procedure caused problems related to type annotations
with generic standard collections (see PEP 585).

Related to issue #44.
  • Loading branch information
mristin authored Aug 28, 2021
1 parent e1406fc commit ddc7f68
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 13 deletions.
27 changes: 15 additions & 12 deletions icontract_lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import re
import sys
import traceback
from typing import Set, List, Optional, TextIO, cast, Tuple
from typing import Set, List, Optional, TextIO, cast, Tuple, Any

if sys.version_info >= (3, 8):
from typing import Final, TypedDict
Expand Down Expand Up @@ -410,17 +410,20 @@ def visit_FunctionDef(self, node: astroid.nodes.FunctionDef) -> None: # pylint:
# annotated with None.
func_has_result = True

if node.returns is not None:
try:
inferred_returns = next(node.returns.infer())

if isinstance(inferred_returns, astroid.nodes.Const):
if inferred_returns.value is None:
func_has_result = False

except astroid.exceptions.NameInferenceError:
# Ignore uninferrable returns
pass
if node.returns is None:
# If no return annotation is specified, we assume that it has a result.
# We could use ``node.infer_call_result()`` to infer the result (see
# https://github.com/PyCQA/astroid/issues/1134), but that *could* lead to quite
# confusing false positives.
assert func_has_result, "Assume there is a result if there is no returns annotation."
else:
# Do not use ``node.returns.infer()`` as it causes problems with Python <3.10.
# See: https://github.com/Parquery/pyicontract-lint/issues/44 and
# https://github.com/PyCQA/astroid/issues/1134.
#
# We use ``node.returns.value`` instead, since it is the easiest way to check for
# ``... -> None`` annotation.
func_has_result = not (isinstance(node.returns, astroid.Const) and node.returns.value is None)

# Infer the decorator instances

Expand Down
43 changes: 42 additions & 1 deletion tests/test_postcondition.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,48 @@ def some_func(x: int):
with tests.common.sys_path_with(tmp_path):
self.assertListEqual([], icontract_lint.check_file(path=pth))

def test_result_none(self):
def test_that_it_accepts_result_annotated_with_pep_585(self):
text = textwrap.dedent("""\
from icontract import ensure
@ensure(lambda result: len(result) > 0)
def some_func() -> list[int]:
return [1, 2, 3]
""")

with tempfile.TemporaryDirectory() as tmp:
tmp_path = pathlib.Path(tmp)

pth = tmp_path / "some_module.py"
pth.write_text(text)

with tests.common.sys_path_with(tmp_path):
self.assertListEqual([], icontract_lint.check_file(path=pth))

def test_result_none_without_annotation_ignored_though_invalid(self):
# If the annotation is missing, we err on the side of the false negatives.
#
# Though we could use astroid to infer that the result is None, the error messages
# would be very confusing in the cases of false positives. Hence we ignore functions
# without the explicit annotation for the return value.
text = textwrap.dedent("""\
from icontract import ensure
@ensure(lambda result: result > 0)
def some_func(x: int):
pass
""")

with tempfile.TemporaryDirectory() as tmp:
tmp_path = pathlib.Path(tmp)

pth = tmp_path / "some_module.py"
pth.write_text(text)

with tests.common.sys_path_with(tmp_path):
self.assertListEqual([], icontract_lint.check_file(path=pth))

def test_result_none_with_explicit_annotation(self):
text = textwrap.dedent("""\
from icontract import ensure
Expand Down

0 comments on commit ddc7f68

Please sign in to comment.