-
Notifications
You must be signed in to change notification settings - Fork 110
flag abstract base classes with no abstract methods #1748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1175,6 +1175,20 @@ export namespace ClassType { | |
| return !!(classType.shared.flags & ClassTypeFlags.SupportsAbstractMethods); | ||
| } | ||
|
|
||
| export function derivesFromExplicitAbstract(classType: ClassType): boolean { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this name seems misleading. maybe
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a doc string? what on earth is that????? |
||
| // Check if ABC is in the direct base classes | ||
| const derivesDirectlyFromABC = classType.shared.baseClasses.some( | ||
| (baseClass) => isClass(baseClass) && baseClass.shared.fullName === 'abc.ABC' | ||
| ); | ||
| // Check if the class uses ABCMeta as its metaclass | ||
| const hasABCMetaMetaclass = | ||
| classType.shared.declaredMetaclass !== undefined && | ||
| isClass(classType.shared.declaredMetaclass) && | ||
| classType.shared.declaredMetaclass.shared.fullName === 'abc.ABCMeta'; | ||
|
Comment on lines
+1179
to
+1187
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this code is so incomprehensibly confusing if only it could be rewritten to be simple enough that the code was self documenting so that the comments arent needed
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. true, i'll close the pr |
||
|
|
||
| return derivesDirectlyFromABC || hasABCMetaMetaclass; | ||
| } | ||
|
|
||
| export function isDataClass(classType: ClassType) { | ||
| return !!classType.shared.dataClassBehaviors; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # this sample tests the type analyzer's ability to flag attempts | ||
| # to instantiate abstract base classes that have no abstract methods | ||
|
|
||
| from abc import ABC, ABCMeta | ||
|
|
||
|
|
||
| class ExplicitlyAbstract(ABC): | ||
| """an abstract class with no abstract methods""" | ||
|
|
||
| # this should generate an error because `AbstractFoo` | ||
| # is an abstract class even though it has no abstract methods | ||
| a = ExplicitlyAbstract() | ||
|
|
||
| class IndirectlyAbstract(ExplicitlyAbstract): | ||
| """inherits from an explicitly abstract class""" | ||
|
|
||
|
|
||
| # this should not generate an error because IndirectlyAbstract | ||
| # doesn't directly inherit from ABC (only through AbstractFoo) | ||
|
Comment on lines
+10
to
+19
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| f = IndirectlyAbstract() | ||
|
Comment on lines
+14
to
+20
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
|
|
||
| class NotAbstract: | ||
| """a regular class that doesn't derive from ABC""" | ||
|
|
||
| # This should not generate an error because NotAbstract is not abstract | ||
| e = NotAbstract() | ||
|
|
||
| class AbstractWithMetaclass(metaclass=ABCMeta): | ||
| """abstract class using ABCMeta metaclass with no abstract methods""" | ||
|
|
||
| class ConcreteWithMetaclass(AbstractWithMetaclass): | ||
| """concrete subclass of AbstractWithMetaclass""" | ||
|
|
||
|
|
||
| # this should generate an error because AbstractWithMetaclass uses ABCMeta | ||
| i = AbstractWithMetaclass() # pyright: ignore[reportExplicitAbstractUsage] | ||
|
|
||
| # this should not generate an error | ||
| j = ConcreteWithMetaclass() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this variable is only used in one spot and should be moved inside the if statement where its used so its never checked unnecesarily