-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Implement component name completion inside html!
macro for IntelliJ Rust plugin
#2972
Conversation
Visit the preview URL for this PR (updated for commit a219e2f): https://yew-rs-api--pr2972-intellij-rust-comple-a5oa472j.web.app (expires Thu, 26 Jan 2023 15:58:56 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Interesting approach. Before reviewing, can you perhaps clarify
html! {
<MyC/*caret*/>
</MyComponent>
} Would this be expanded as html! {
<MyC INTELLIJ_DUMMY_SYMBOL>
</MyComponent>
} or html! {
<INTELLIJ_DUMMY_SYMBOL>
</MyComponent>
} In rust-lang/rust-analyzer#7402 (comment) you mention the possibility to communicate "valid" expansions back to intellisense. Is this sort of thing possible for the IntelliJ plugin? |
Benchmark - SSRYew Master
Pull Request
|
Size Comparison
✅ None of the examples has changed their size significantly. |
html! {
<MyC/*caret*/>
</MyComponent>
} Guessed wrong :D html! {
<MyCINTELLIJ_DUMMY_SYMBOL>
</MyComponent>
}
Of course! If a macro expands normally and correctly sets spans for output tokens, IntelliJ Rust looks through the macro call at the expansion and provide some IDE features there (like highlighting and, of course, completion). It the case of |
7469f37
to
cf2f960
Compare
Great to see this. There's something I would like to clarify: what about the closing tag? If you try using a component in JSX (in a .jsx or .tsx file in IntelliJ), it automatically completes the closing tag when Same goes for editing. If I start typing in opening tag, it automatically matches what's in the closing tag. In Rust, those two are separate
I didn't even know that option existed. @siku2 can you make the change? |
@hamza1311 done! |
@hamza1311, well, at the moment I don't see how such advanced features could be achieved using such an approach, unfortunately. We likely need a special IntelliJ plugin with |
Exactly. Writing plugins specifically for proc macros would need to be supported by IntelliJ Rust |
I realize this is a bit off-topic for this issue in this repository, but the ideally, ide integration shouldn't not need to have special plugin for each possible proc-maco. What I was discussing with matklat that lead to rust-lang/rust-analyzer#7402 (comment) was specifically that the yew html! macro, and when run through an ide, would look for the Of course, the whole protocol needs to be well defined so we can do completion items, highlight, gotodefinition, and so on. But for sure the first thing we can do now as macro authors is to be a bit more resilient to errors in macro implementation, and still try to produce the most possible rust code before throwing a |
@ogoffart The features that you describe can be done in the scope of this PR, I guess (even without a special |
@ogoffart Let me explain. With this PR, when you invoke completion with such a source: html! {
<MyC/*caret*/>
} the macro will see this input:
Since the macro understands it's invoked during a completion inside an IDE, it uses a less strict parser and eventually expands to something where
Of course, the macro sets correct You don't need Let's take a look at a more complex example with auto-import: Peek.2022-11-23.14-58.mp4This is unachievable with Such tricks works well with closing tag just because they're correctly mapped with expansion as well: What is not done in this PR is completion for standard html elements: html! {
</*caret*/>
} You'd likely want
And the IDE will pick these definitions up and show them in the completion list just because these names are now in the scope. No need for special What @hamza1311 suggests is automatic insertion of the corresponding closing tag when a user types Peek.2022-11-23.15-11.mp4Or automatically adjust a closing tag when editing the open one: Peek.2022-11-23.15-14.mp4Completion is just not involved here. So I think the only way to fulfill these feature is implementing a special plugin |
if !is_ide_completion() { | ||
return match close { |
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.
Would it make sense to move is_ide_completion
call to root of the macro where syn::Error
is converted to compile_err!
? That way all the cases of compilation failure will covered, while avoiding adding the if conditional at every error return
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.
Hmm, but what can we do with syn::Error
except converting it to compile_err!
? Here we skip returning with an error and continuing the parsing. If we return with an error, the parsing is abandoned, so we can do nothing except returning compile_err!
, I guess
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.
The macro needs to expand to actual results (even if incomplete)? I was thinking maybe we could return an empty TokenStream and such so the output doesn't return a compile error. Html::default()
is valid return type for every macro (the return type must be Html
- more accurately VNode
- for it to pass type checking) . Perhaps that can be returned?
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.
The macro needs to expand to actual results (even if incomplete)?
Yes, definitely. If not for this, then no changes in yew
would be required at all.
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.
Sorry, it took me so long to properly try this out. It works well. Great work!
I did run into issues with error reporting though. See intellij-rust/intellij-rust#9898
Side note: I'm not sure how IntelliJ is able to figure out the prop definitions for components with props, considering it's impossible for the macro to know about the props struct. It would be great if it could auto complete too. You can try it out by jumping to definition of seed
here:
https://github.com/vlad20012/yew/blob/5355b65ff5f9747cbad801d4b337a5ac7a94d0f4/examples/function_router/src/app.rs#L92
d0a8e6c
to
a219e2f
Compare
a219e2f
to
3e39615
Compare
I'm sorry for being so late with the response. I was on vacation without a PC 😅
I'm going to try making it in a separate PR right after this. I think in this case the macro should expand to a completely different code that puts the dummy identifier in such specific context where the completion will work as expected. struct FooComponent {
attribute: i32
}
...
html! {
<FooComponent attr/*caret*/>
} In this case, we could expand the macro to let _ = FooComponent {
attr/*caret*/: ()
} Then, when an IDE will invoke completion in such expanded fragment, it will show only fields of the |
FYI: this would need to show fields of |
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.
Thank you for the contribution.
I am going to follow @hamza1311's review and approve this as I do not use IntelliJ.
CI all passes so I assume it will not have impacts when this feature is not enabled.
Description
These changes bring completion of component names inside an
html!
macro invocation:Peek.2022-11-21.16-38.mp4
The behavior of
html
macro has been leaved absolutely unchanged in the case of usual usage during compilation.How to check it out
You need CLion or IntelliJ IDE with Rust plugin version not lower than 0.4.184 (which includes intellij-rust/intellij-rust#9711)
Implementation details
When a user performs code completion, IntelliJ Rust duplicates the current file, inserts a dummy identifier at the current caret position and then expands a macro invocation at the current caret position. After that, it maps the caret position to the expansion (based on spans) and then performs code completion on the expansion.
If the macro expansion fails (expands to
compile_err!(...)
), which is usual when a user is typing, IntelliJ Rust can't provide any completion.yew::html!
expansion fails in any cases of invalid syntax. For example, unmatched tag/component names:In such cases
html!
expands tocompile_err!(...)
, hence the user is left without completion :(This PR is based on the fact that since intellij-rust/intellij-rust#9711 IntelliJ Rust sets these environment variables when invoking a proc macro during completion:
RUST_IDE_PROC_MACRO_COMPLETION=1
RUST_IDE_PROC_MACRO_COMPLETION_DUMMY_IDENTIFIER=actual dummy identifier
So, I changed the behavior of the parser if
RUST_IDE_PROC_MACRO_COMPLETION
env is set. Namely:Note that this behavior only works when the macro is invoked from an IDE during completion. When an IDE invokes a macro in other cases, it doesn't set these envs and hence usual parser works.
Also, with these env vars, it is possible to make fully custom completion. For example, it's possible complete standard tag names like
div
(but this PR does not include this).Inspired by rust-lang/rust-analyzer#7402 (comment) (thanks to @ogoffart & @matklad)