Report invalid-abstract-method for @abstractmethod in non-abstract classes#3905
Open
nitishagar wants to merge 1 commit into
Open
Report invalid-abstract-method for @abstractmethod in non-abstract classes#3905nitishagar wants to merge 1 commit into
nitishagar wants to merge 1 commit into
Conversation
This comment has been minimized.
This comment has been minimized.
…asses Summary: Adds a new off-by-default lint rule `invalid-abstract-method` that fires when a method decorated with `@abstractmethod` is defined in a class that is not abstract (does not inherit from `abc.ABC`, use `abc.ABCMeta`, have a transitive ABC base, or be a Protocol). Such a class is directly instantiable at runtime while containing an unimplemented method — a likely programming mistake. The check runs as a class-level diagnostic in `solve_class_checks`, alongside the existing override/variance checks. It reuses the class field map those checks already build and reports at each own method whose `ClassField::is_abstract()` flag is set, guarded by `!extends_abc() && !is_protocol() && !is_new_type()`. Running it here rather than in the abstract-class check means it forces no field resolution for modules that merely import the class, so laziness is preserved. The rule defaults to `Severity::Ignore` to avoid breaking existing code without opt-in. Fixes facebook#3728 Test Plan: - cargo test -p pyrefly (lib 5824 + laziness 18 + integration 301, 0 failed) - cargo test -p pyrefly_config (incl. test_doc_headers, test_doc_severities) - cargo test -p pyrefly --lib abstract_methods (35 passed) - python3 test.py --no-test --no-conformance --no-jsonschema (clean)
nitishagar
force-pushed
the
nitishagar/issue-3728-invalid-abstract-method
branch
from
b897614 to
6e3a633
Compare
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary: Adds a new off-by-default lint rule
invalid-abstract-methodthatfires when a method decorated with
@abstractmethodis defined in a classthat is not abstract (does not inherit from
abc.ABC, useabc.ABCMeta,have a transitive ABC base, or be a Protocol). Such a class is directly
instantiable at runtime while containing an unimplemented method — a likely
programming mistake.
The check runs inside
solve_abstract_members(which already executes forevery class), guarded by
!extends_abc() && !is_protocol() && !is_new_type().It iterates the class's own declared fields (not MRO-inherited ones) and emits
one error per field whose
ClassField::is_abstract()flag is set, pointing atthe method's definition range. The rule defaults to
Severity::Ignoretoavoid breaking existing code without opt-in.
Fixes #3728
Test Plan: