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

Automatically typed module.required dependencies #1537

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

mtrajano
Copy link
Contributor

Did a quick POC for automatic typing of module resolves. Wondering if something like this would be useful to us. Benefit is that doing local mod = module.required[{dependency}] mod will be automatically typed with the correct class and will give us luals type hints and typechecks without having to do something like this every time:

---@type core.integrations.treesitter
local ts = module.required["core.integrations.treesitter"]

Type hinting:
Screenshot 2024-07-24 at 12 42 50 PM

It already showed some incorrectly type methods in the codebase
Screenshot 2024-07-24 at 12 36 07 PM

We also get automatic suggestions on which modules can be imported as you can see below:
Screenshot 2024-07-24 at 12 34 48 PM

Downside is obviously that any new added modules will have to be updated in that new neorg.module.resolver class

Thoughts/feedback? @vhyrro @benlubas

@max397574
Copy link
Contributor

really like this change

@benlubas
Copy link
Contributor

I also really like this

@vhyrro
Copy link
Member

vhyrro commented Jul 24, 2024

LGTM, thank you! I played around with this and tried my best to make some clever use of generics but couldn't figure it out. Hardcoded it is (for now, until LuaCATS generics get better :p).

@vhyrro
Copy link
Member

vhyrro commented Jul 24, 2024

Are there any other modules that need to be annotated, or can this be marked as ready?

@vhyrro vhyrro marked this pull request as ready for review July 24, 2024 20:01
@vhyrro
Copy link
Member

vhyrro commented Jul 24, 2024

Oops, impulsively pressed the button to let the CI run, made the PR "ready" instead. My bad.

@mtrajano
Copy link
Contributor Author

It pretty much is ready and fully functional. I think I managed to type everything but there were some where the class typing was on the module.private so I can update those. Also wasn't 100% unsure on the syntax, not sure if a class or an indexed table etc..

Other than that just fixing the incorrect typings that will pop up but that feels like a separate PR or we can just fix them as we come across them.

@mtrajano
Copy link
Contributor Author

LGTM, thank you! I played around with this and tried my best to make some clever use of generics but couldn't figure it out. Hardcoded it is (for now, until LuaCATS generics get better :p).

That's interesting, didn't consider the generics route but agree not sure if we're there yet with LuaCATS 😄

@mtrajano mtrajano force-pushed the module-types branch 2 times, most recently from 09654fc to 4b1a60b Compare July 24, 2024 22:24
@mtrajano mtrajano changed the title WIP: automatically typed module.required dependencies Automatically typed module.required dependencies Jul 24, 2024
@mtrajano
Copy link
Contributor Author

Ok I believe I got all of them but maybe worth a second pair of eyes, let me know if I missed anything

@vhyrro
Copy link
Member

vhyrro commented Jul 25, 2024

Looking good! Thanks again. Guess I'll now have to spend some time looking at the type errors :)

@vhyrro vhyrro merged commit 1985f2d into nvim-neorg:main Jul 25, 2024
5 of 6 checks passed
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

Successfully merging this pull request may close these issues.

4 participants