-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Implement generic AST pattern matcher helper #699
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
Comments
Something like this could be useful when implementing #698. |
@JukkaL I felt this need too - I wanted to suggest that, but now I see there's an issue for this. I think I'd like to work on it, if nobody else did it already. |
Go for it! |
I have something. The matcher looks like
Where A code excerpt (note that it uses def check_namedtuple(self, node: Node, var_name: str = None) -> TypeInfo:
...
_typename, _names, _module, _types = Identifier(), Identifier(), Identifier(), WildCard()
if callee.fullname == 'collections.namedtuple':
def namedtuple(typename, field_names, *, verbose = ..., rename = ...): ...
args = call.bind(namedtuple)
match = Or(StringOfIds(_names), MatchList(_names, etc))
bad_fields = match.find_mismatch(args.get('field_names'))
bad_module = None
elif callee.fullname == 'typing.NamedTuple':
def NamedTuple(typename, fields, *, verbose = ..., rename = ..., module = ...): ...
args = call.bind(NamedTuple)
match = MatchList(MatchTuple(_names, _types), etc)
bad_fields = match.find_mismatch(args.get('fields'))
bad_module = _module.find_mismatch(args.get('module'))
else:
return None
if not args:
return self.build_namedtuple_typeinfo('namedtuple', [], [])
if _typename.find_mismatch(args['typename']):
self.fail("namedtuple() expects a string literal as the first argument", call)
... It could have been cleaner if it need not be typechecked... Together with the inspection, it saves about 40 lines in the main logic. It's paid back with interest, but mostly in a dedicated file, and it should be reusable (although I only tried it on namedtuple so far). |
Sure! I'm going to PyCon UK tomorrow so I may not have much time until next week. |
The proposed syntax feels too magical for me. It's shorter than the current code but I had to read through it several times before I understood what is going on. The real challenge seems to be to come up with something that is both easy to understand and compact (and not too difficult to implement, of course). Each line of code will be read many times but written only once, so ease of understanding is the key thing. Here's another idea (again, not convinced it's a good idea, but here goes anyway):
Automating the generation of meaningful error messages would also be nice. |
Your API is indeed more readable. On first impression: one problem with The reasons for making the mutation-based patterns was to be able to find and report the exact point of error, differentiate between |
Another decision is what to leave to the type checker. I don't like the idea of rejecting valid code just because it's not a string literal. |
I don't think that is worth implementing, since we aren't adding new special forms that often any more, and just reviewing (not to mention implementing) a generic solution could take more effort than we'd save using it. |
Currently the mypy implementation has a lot of ad-hoc code that checks whether some part of an AST looks like something specific. For example, we have code that detects various special forms:
It would be better to have a generic utility that makes it easy to write code to detect cases like the above.
A straw man proposal:
The pattern syntax would be a subset of Python syntax to make the implementation easier (but with different semantics).
The text was updated successfully, but these errors were encountered: