Skip to content
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

Full type support? #110

Closed
jakkdl opened this issue Jan 28, 2023 · 2 comments
Closed

Full type support? #110

jakkdl opened this issue Jan 28, 2023 · 2 comments

Comments

@jakkdl
Copy link
Member

jakkdl commented Jan 28, 2023

After working more on TRIO232 I kinda wanna look into hooking in[to] a type parser, that would not only make TRIO2XX object handling much simpler and more powerful but also improve a bunch of other checks: detection of nurseries (TRIO101, TRIO112, TRIO113), handling of cancel scopes (TRIO102), better support for imports, etc.

Looking around I'm not seeing a lot of good support for it though, mypy has a plugin system - but that's primarily intended to improve/extend the type inference, although pydantic has a mypy plugin https://docs.pydantic.dev/mypy_plugin/ that adds errors on static analysis - so could maybe do something similar to them and add checks onto a bunch of hooks, moving them out of logic in the flake8 plugin. (did not find any mentions of plugins for pyright, pyre, or pytype)

Another alternative is to run a type checker on startup of flake8-trio, and then presumably the type data is saved somewhere, and it can be inspected, so when traversing the AST we can delve into that data structure and get out the typing data we care for.

This is kind of a meaty addition though, but I kinda feel like I'm reinventing the wheel with subpar tools when I'm writing logic to handle assignments, annotations, and everything. So it might be worth spending some time experimenting with possible alternatives and see if it's not too crazy.

(I also started reinventing libCST's match functionality the other day when I got frustrated with the endless chains of isinstance...)

I don't think we're actually gaining all that much from being a plugin to flake8, as opposed to a standalone checker, so I don't think the limitations from that should weigh too heavily.

@Zac-HD
Copy link
Member

Zac-HD commented Feb 3, 2023

I'm pretty strongly inclined against this.

Engineering-wise, it would be a huge pain to get working - Pydantic's mypy plugin is basically a callback for mypy, not Pydantic calling mypy. LibCST tried that1 against the Pyre query API, which is deprecated and was never ergonomic anyway. Because type-checking and -inference is a global analysis, we also lose the lovely file-level isolation property, and start to get "spooky action at a distance" which could create new errors on apparently-unrelated changes; it's also generally much harder to make this fast and reliable.

Conversely, I think we've got a pretty good solution already, and further improvements probably wouldn't buy us much practical improvement on end-user code.

Footnotes

  1. though I'm generally fond of LibCST, that's another long discussion.

@Zac-HD Zac-HD closed this as completed Feb 3, 2023
@jakkdl
Copy link
Member Author

jakkdl commented Feb 3, 2023

Makes sense, and your arguments are solid. If libCST only managed to implement it through a legacy API it's a pretty bad sign.

But the pyre API does look kinda awesome if it was working well and maintained, shame it isn't. I'll keep hoping an easy solution pops up as typing becomes more prevalent and a service such as this gets wanted by more actors, but you're probably right that the practical improvements aren't too common - and the cross-file level patterns this would catch are likely not all that great to begin with and can be warned on / worked around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants