Skip to content

Commit

Permalink
Update check of implicit return to ignore abstract methods
Browse files Browse the repository at this point in the history
Ref pylint-dev#485. To avoid additional breakage, we optionally extend is_abstract
to consider functions whose body is any raise statement (not just raise
NotImplementedError)
  • Loading branch information
nelfin committed May 24, 2021
1 parent 826d0e9 commit c801ca6
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
10 changes: 8 additions & 2 deletions astroid/scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1661,11 +1661,12 @@ def is_bound(self):
"""
return self.type == "classmethod"

def is_abstract(self, pass_is_abstract=True):
def is_abstract(self, pass_is_abstract=True, any_raise_is_abstract=False):
"""Check if the method is abstract.
A method is considered abstract if any of the following is true:
* The only statement is 'raise NotImplementedError'
* The only statement is 'raise <SomeException>' and any_raise_is_abstract is True
* The only statement is 'pass' and pass_is_abstract is True
* The method is annotated with abc.astractproperty/abc.abstractmethod
Expand All @@ -1686,6 +1687,8 @@ def is_abstract(self, pass_is_abstract=True):

for child_node in self.body:
if isinstance(child_node, node_classes.Raise):
if any_raise_is_abstract:
return True
if child_node.raises_not_implemented():
return True
return pass_is_abstract and isinstance(child_node, node_classes.Pass)
Expand Down Expand Up @@ -1745,7 +1748,10 @@ def infer_call_result(self, caller=None, context=None):
first_return = next(returns, None)
if not first_return:
if self.body:
yield node_classes.Const(None)
if self.is_abstract(pass_is_abstract=True, any_raise_is_abstract=True):
yield util.Uninferable
else:
yield node_classes.Const(None)
return

raise exceptions.InferenceError(
Expand Down
37 changes: 37 additions & 0 deletions tests/unittest_scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,43 @@ def f():
assert isinstance(inferred, nodes.Const)
assert inferred.value is None

def test_only_raises_is_not_implicitly_none(self):
code = """
def f():
raise SystemExit()
f()
"""
node = builder.extract_node(code) # type: nodes.Call
inferred = next(node.infer())
assert inferred is util.Uninferable

def test_abstract_methods_are_not_implicitly_none(self):
code = """
from abc import ABCMeta, abstractmethod
class Abstract(metaclass=ABCMeta):
@abstractmethod
def foo(self):
pass
def bar(self):
print('non-empty, non-pass, no return statements')
Abstract().foo() #@
Abstract().bar() #@
class Concrete(Abstract):
def foo(self):
return 123
Concrete().foo() #@
Concrete().bar() #@
"""
afoo, abar, cfoo, cbar = builder.extract_node(code)

assert next(afoo.infer()) is util.Uninferable
for node, value in [(abar, None), (cfoo, 123), (cbar, None)]:
inferred = next(node.infer())
assert isinstance(inferred, nodes.Const)
assert inferred.value == value

def test_func_instance_attr(self):
"""test instance attributes for functions"""
data = """
Expand Down

0 comments on commit c801ca6

Please sign in to comment.