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

Add #[allow(...)] directives to macro-generated entries #1049

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

mivort
Copy link
Contributor

@mivort mivort commented Feb 16, 2025

This patch adds #[allow(non_camel_case_types)] and #[allow(non_upper_case_globals)] directives to generated consts and structs.

While cargo check by default ignores naming style inconsistencies in macro-produced code, rust-analyzer seems to be using a more strict mode. #[allow(...)] annotations for generated entries allows to prevent the display of such warnings by the language server.

Context

Without this patch, rust-analyzer produces the following type of stylistic warnings (tested in Neovim v0.10.4 / rust-analyzer v1.84.1):

When using #[derive(GodotConvert)] macro:

1. Constant `ORD_SharedInventory` should have UPPER_SNAKE_CASE name,
   e.g. `ORD_SHARED_INVENTORY` [non_upper_case_globals]

When using #[derive(GodotClass)] macro:

1. Structure `__gdext_CameraControl_Funcs` should have UpperCamelCase name,
   e.g. `GdextCameraControlFuncs` [non_camel_case_types]

@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1049

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: register Register classes, functions and other symbols to GDScript labels Feb 16, 2025
@Bromeon
Copy link
Member

Bromeon commented Feb 16, 2025

Thank you! 🙂

Do you think there's an easy way how we could detect such cases in CI, without adding rust-analyzer as a dependency? With cargo check (or clippy), this cannot be detected?

@Bromeon Bromeon added this to the 0.2.x milestone Feb 16, 2025
This patch adds `#[allow(non_camel_case_types)]` and
`#[allow(non_upper_case_globals)]` directives to generated `const`s and
`struct`s.

While `cargo check` by default ignores naming style inconsistencies in
macro-produced code, `rust-analyzer` seems to be using a more strict mode.
So annotation of generated entries allows to prevent the display of such
warnings by the language server.
@Bromeon Bromeon force-pushed the rust-analyzer-style-warnings branch from 1a794da to 457bee5 Compare February 16, 2025 13:55
@mivort
Copy link
Contributor Author

mivort commented Feb 16, 2025

@Bromeon I've tried to check the docs for possible options to enable warnings produced during proc macro compilation (inline macros show these warnings), haven't found any. Some linters are disabled explicitly for macros in compiler code, but that's not the case for naming checks (so that's probably why rust-analyzer is able to produce these).

There should be some logic in Rust compiler which discards/doesn't output warnings (errors are still kept, obviously), I wonder if it's externally configurable, though.

@Bromeon Bromeon added this pull request to the merge queue Feb 18, 2025
@Bromeon
Copy link
Member

Bromeon commented Feb 18, 2025

Thank you! I think this is good for now, in case you come across easy ways to prevent this in the first place, let us know 👍

Merged via the queue into godot-rust:master with commit ac8b1da Feb 18, 2025
16 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants