Handle empty function bodies safely

[JukkaL]

Currently an empty function body is accepted if a function returns a non-None value (even with --strict-optional). This is mostly useful for abstract methods. This is unsound, since we don't prevent anybody from calling these empty functions.

Here are some ideas for making this safe:

• Only allow empty function bodies for abstract methods. Other functions will need something like a return statement, a raise statement or assert False or # type: ignore.
• Tag abstract methods with an empty body with a flag in the AST, and disallow calls to them via super().

This still wouldn't address empty functions defined in stubs, but perhaps can just assume that every function in a stub has a non-empty body.

See also discussion in #2339.

[gvanrossum]

This would only apply to abstract methods whose return value cannot be None, right? I think this should be okay and allowed via super:

class C:
    @abstractmethod
    def f(self) -> Optional[int]:
        pass
class D(C):
    def f(self) -> Optional[int]:
        return super().f()

[JukkaL]

Yeah, we could safely allow those examples.

[rwbarton]

The extensive use of functions with empty body and arbitrary type in the tests was another motivation for allowing empty bodies. Though it isn't a problem now, there would be a huge pain should warn-no-return (and perhaps strict-optional) be turned on by default. Not saying it should block this change, but it would be good to have some plan for dealing with this when it comes up.

[gvanrossum]

I think @ddfisher would like both of these to become the default. David, can you come up with a plan?

[ddfisher]

I think it'd be reasonable not to worry about this too much for now: we're erring on the side of being more permissive (which is the better side to err on when introducing a new default), and I'd expect empty functions to be much more often intentional than not.

[ilevkivskyi]

This keeps coming, so I propose to raise priority at least to normal.

[ilevkivskyi]

Whatever we decide here might require updating thousands of tests cases, so adding the tests label.

[Michael0x2a]

What if we modified the definition of a "trivial" function so that functions containing only pass are no longer considered to be trivial?

One additional benefit is that this change would now make the distinction between ... and pass meaningful, rather than a matter of style. You'll now need to deliberately use the former if you want mypy to ignore the body.

[gvanrossum]

That won't work in Python 2 though -- ... by itself is not valid there.

[ilevkivskyi]

This continues to confuse people, so I am going to fix this. Here is my plan:

• Properly check empty bodies as a default
• Add an --allow-empty-bodies flag, we need it for tests (hundreds would fail otherwise, also in other projects like sqlalchemy-stubs and django-stubs)
• Allow empty bodies in stub files (obviously)
• Allow empty bodies if method is decorated with @abstractmethod
• In protocols make functions with empty bodies implicitly abstract (unless this is a stub)
• Add a flag for empty-bodied abstract methods to prohibit calling them via super() (unless they come from a stub)
• Bonus point: try unifying is_trivial_body() with is_raising_or_empty() (potentially the former can be either a docstring or the latter)

[tusharsadhwani]

Does this need any help?
This has bitten me many times when I get too reliant on mypy:

def some_function() -> int:
   """Does a foo to a bar."""
   # whoops... i got distracted and forgot to write a body

Mypy is happy with this, but I don't think it should be.