Emitting Error on Abstract Call

[Adist319]

Resolves #592

Summary

This PR implements error detection for direct calls to abstract methods, preventing runtime errors by catching them during
static analysis. Old PR: #702

[facebook-github-bot]

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D79174475.

[stroxler]

Thanks for the PR!

Unfortunately, I think this might actually be pretty difficult to do correctly (I'm not sure the issue as stated is really one we want, I commented there as well).

A naive approach won't work because of subtyping - this implementation fails on

from abc import ABC, abstractmethod

class Shape(ABC):
    @abstractmethod
    def area(self) -> float:
        pass

class UnitCircle(Shape):
    def area(self) -> float:
        return 3.14159

def f(s: Shape) -> float:
    return s.area()  # E: Cannot call abstract method `Shape.area` - must be implemented in a subclass

f(UnitCircle())

which means we really cannot merge it; the issue is it isn't accounting for subtyping, an instance of Shape as a static type might not be a Shape at runtime, and abstract classes become unusable if we are too aggressive.

I'm not certain that to complaining on abstract method calls is doable in a type checker, it's a very hard problem (some theoretically oriented folks in Hack thought about it and concluded that it's very hard).

What might be easier would be to complain when we create an instance of a class that is abstract - that we can catch syntactically, because we know that Shape() creates a Shape exactly, not some non-abstract subtype of Shape

[stroxler]

Review automatically exported from Phabricator review in Meta.

[Adist319]

@stroxler Thanks for the comments! You're right about the sub-typing issue, that didn't cross my mind. I agree with complaining only when an instance of an abstract class is instantiated. This should be much more straightforward to implement. I can create a new PR as well for this less aggressive checking.

[yangdanny97]

@Adist319 thanks, and apologies about the churn, it was my oversight that I didn't think hard enough before opening the issue

[Adist319]

@yangdanny97 no worries, will get on this soon.

[samwgoldman]

Hi @Adist319, are you still planning to work on this?

[Adist319]

@samwgoldman We should be able to close this PR, I'm working on #807 currently. I'm hoping to get it done by end of next week, where I'll have more time after I'm back from vacation. Apologies for the delay!