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

fix(rustify_derive): remove derive block expr to fix LSP type inference #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lazulit3
Copy link

@lazulit3 lazulit3 commented Dec 18, 2024

This updates rustify_derive to remove the block expression that the impl Endpoint was wrapped in.

This resolves an issue where rust-analyzer is unable to infer the type of Endpoint::Response in some cases. The issue is described at:

🔧 Changes

  • Removes the block expression const #const_ident: () = {...}; wrapping impl Endpoint in the derive macro
  • Removes the convenience use imports that were contained in this block expression
  • Updates usage of those imported types to use fully qualified type paths e.g. RequestMethod -> rustify::enums::RequestMethod

◀️ Backwards-Compatibility

I believe this fix is backwards-compatible, but I invite others to check me on that.

Since vaultrs uses rustify, I checked vaultrs on master (aaa7ff6) with rustify_derive dependency pointed at this branch.

cargo build succeeds and cargo clippy reports two unused serde::Serialize imports:

warning: unused import: `serde::Serialize`
 --> src/api/identity/entity_alias/requests.rs:5:5
  |
5 | use serde::Serialize;
  |     ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `serde::Serialize`
 --> src/api/identity/entity/requests.rs:2:5
  |
2 | use serde::Serialize;
  |     ^^^^^^^^^^^^^^^^

This is likely due to switching to serde::Serialize in the macro. I think not requiring this use is an improvement, but if we prefer I can adjust the PR.

✅ Testing

  • Verified that these changes allow rust-analyzer to infer response types in my editor.
  • Ran checks/lints/tests/examples locally:
    • cargo test
    • cargo check
    • cargo clippy
    • cargo run --package rustify --example reqres1
    • cargo run --package rustify --example reqres2

This updates `rustify_derive` to remove the block expression that the `impl Endpoint` was wrapped in.

This resolves an issue where `rust-analyzer` is unable to infer the type of `Endpoint::Response` in some cases.
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.

1 participant