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

rust: Add support for doctest runnables #24806

Merged
merged 3 commits into from
Feb 21, 2025

Conversation

bnjjj
Copy link
Contributor

@bnjjj bnjjj commented Feb 13, 2025

Screenshot:

image

I would be happy to add tests if you point me to the right place to do it please.

Release Notes:

  • Added suuport for doc test in tasks for Rust

Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 13, 2025
@maxdeviant maxdeviant changed the title feat(rust): add support for doc test runnables rust: Add support for doctest runnables Feb 13, 2025
@osiewicz
Copy link
Contributor

osiewicz commented Feb 19, 2025

Thanks for your PR!
Overall it looks good to me, though I've found a few cases where it does not quite work:

  1. It does not seem to work unit structs (struct C;).
  2. It seems to not work in impl blocks. nvm it does
  3. I don't have an example handy, but I don't think it'll work with examples in module-level comments.
  4. It seems that if I swap out /// to //!, it does not work either.

Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
@bnjjj
Copy link
Contributor Author

bnjjj commented Feb 20, 2025

@osiewicz thanks for your review.

It does not seem to work unit structs (struct C;). ✅
It seems that if I swap out /// to //!, it does not work either. ✅

I don't have an example handy, but I don't think it'll work with examples in module-level comments.

For this one I can't think of a good solution, because we wouldn't have @run tag as it's basically the file itself. I could create another query but I can't find a way to tell treesitter to not match function_item, struct_item and so one. That would conflict with existing ones. Do you have any ideas ?

@osiewicz
Copy link
Contributor

We could try to attach runs to the code blocks and not the annotated items. To be fair I think the bigger hurdle is how to spawn a task for one specific doc test.

@bnjjj
Copy link
Contributor Author

bnjjj commented Feb 21, 2025

To be fair I think the bigger hurdle is how to spawn a task for one specific doc test.

Correct me if I'm wrong but with the current implementation it will run one specific doc test because we have the symbol name. Maybe you're talking about your solution (attach the runs to the code blocks).

@osiewicz Could we create an issue as a follow up mentioning that module-level doc tests are not implemented yet ? That's already a good addition I think. What do you think ?

@osiewicz
Copy link
Contributor

osiewicz commented Feb 21, 2025

Sounds good! Yeah, I was talking about a fully robust implementation and that we'd have to "reverse-engineer" rust doctests a bit to know what query to spawn the tests with. You've nerd-sniped me, I will try to improve it a bit in a follow-up PR. :)
But yeah, this PR stands on it's own. Let's get it in!

@osiewicz osiewicz merged commit dff47a8 into zed-industries:main Feb 21, 2025
12 checks passed
@bnjjj
Copy link
Contributor Author

bnjjj commented Feb 21, 2025

Talking about the way RA works @osiewicz , is it the plan to use the experimental runnables coming from RA in the future ? I can see a lot of benefits in the way RA works with runnables but I know it might be very specific to Rust so I wanted to know your thoughts on this.

@osiewicz
Copy link
Contributor

We've considered using runnables from RA when we did an initial task implementation - I'm somewhat against it (at least as the only implementation of tests) as other languages don't usually provide similar experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants