Skip to content

Commit

Permalink
Simplified inference on None returns
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 committed Aug 22, 2021
1 parent e1406fc commit d9f1e89
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 d9f1e89

Please sign in to comment.