-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add Class Decorator/Metaclass/Base Class plugin #4328
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
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
b84d891
RFC: Add Class Decorator/Metaclass/Base Class plugin
euresti c64cd48
Fix types
euresti f9e049d
Fix old pythons
euresti 757efc8
Allow MemberExpr
euresti c01352e
CR, also remove attrs code
euresti 819b5ba
Support base expressions correctly
euresti 1e5ff80
Comment tweak
euresti da9224c
Simplify IndexExpr lookup
euresti a91559a
Remove comment
euresti 9981756
Fix tests
euresti 1a82e2d
CR
euresti 8ba4932
get_base_class_hook
euresti 32e6e9c
AnalyzerPluginInterface->TypeAnalyzerPluginInterface
euresti File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
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.
Is this the only method that would be potentially used by plugins? Do we need to add some more?
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.
My current explorations also need
parse_bool
andanal_type
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.
fail
could be useful too. Should I add them as needed or add them now?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.
I am not sure why we actually need this class. Can't we just pass everything? Or am I missing something?
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.
I'm following the pattern of the other plugins. Technically the object that's passed in is the full object. I believe this base class is here to limit what methods the plugins should be calling. i.e. if the method isn't listed here then the mypy checker will complain about it.
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.
Then we probably need to think what to add here. My expectation is that many methods of
SemanticAnalyzer
may be useful. You can play more with your implementation forattrs
and add here whatever you ever used.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.
I think we can keep only these methods, and then add more in your PR with the actual plugin (if necessary).
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.
Here is why we are using an ABC instead exposing the entire semantic analyzer interface: