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

godot_api attribute proc macro hits new rust-analyzer orphan trait implementation checks #490

Closed
DamnWidget opened this issue Nov 20, 2023 · 6 comments
Labels
c: register Register classes, functions and other symbols to GDScript quality-of-life No new functionality, but improves ergonomics/internals status: upstream Depending on upstream fix (typically Godot)

Comments

@DamnWidget
Copy link

DamnWidget commented Nov 20, 2023

Abstract

The rust-analyzer LSP made a new release recently introducing a new feature for diagnose orphan trait implementation cases rust-lang/rust-analyzer#15895

Godot crate hits this new check wherever #[godot_api] attribute proc macro is being used

image

Workaround

I did not found a setting in rust-analyzer to disable the check so the only workaround for now is to downgrade your rust-analyzer version to a lower one.

P.S: if there is a setting to disable this check, please, let me know

@PgBiel
Copy link
Contributor

PgBiel commented Nov 20, 2023

Is it not this check? rust-lang/rust-analyzer#15895

Also, are you using the latest rust-analyzer? Have you tried using the latest nightly/preview version of it as well?

@DamnWidget
Copy link
Author

Is it not this check? rust-lang/rust-analyzer#15895

I was browsing their issues tracker and looks like I messed up the copy and paste, it was exactly that pull request the one that I wanted to refer to, fixed my original post. Thanks for pointing out.

Also, are you using the latest rust-analyzer? Have you tried using the latest nightly/preview version of it as well?

I am using the latest released version https://github.com/rust-lang/rust-analyzer/releases/tag/2023-11-20 , I just tried their nightly version and hit the same check

@lilizoey
Copy link
Member

oh is that what the issue is. it's a pretty simple fix at least. just gotta change this

impl ::godot::private::Cannot_export_without_godot_api_impl for #class_name {}

to

impl ::godot::private::Cannot_export_without_godot_api_impl for #class_name {
  const EXISTS: () = ();
}

but it also seems like RA is fixing it here rust-lang/rust-analyzer#15911

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: register Register classes, functions and other symbols to GDScript labels Nov 20, 2023
@PgBiel
Copy link
Contributor

PgBiel commented Nov 20, 2023

I think we can just consider it an upstream problem then, since that would probably cause tons of warnings in several other codebases (and the fix will probably be in the next rust-analyzer release)...

Let's wait for the next rust-analyzer release to confirm.

Edit: Although we could consider "fixing" it on our side anyway if the next release takes too long.

@Young-Flash
Copy link

Hi, rust-analyzer PR rust-lang/rust-analyzer#15911 got merged, which fix the problem. The next nightly release will be avaliable in a few days (actually there just make a release yesterday but we miss the patched PR to merge)

@PgBiel
Copy link
Contributor

PgBiel commented Nov 27, 2023

Thanks @Young-Flash !

This can probably be closed now, seems like the patch has hit stable: https://rust-analyzer.github.io/thisweek/2023/11/27/changelog-209.html

@Bromeon Bromeon closed this as completed Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: register Register classes, functions and other symbols to GDScript quality-of-life No new functionality, but improves ergonomics/internals status: upstream Depending on upstream fix (typically Godot)
Projects
None yet
Development

No branches or pull requests

5 participants