godot-rust / gdnative

Rust bindings for Godot 3
https://godot-rust.github.io
MIT License
3.61k stars 210 forks source link

Add `#[non_exhaustive]` to most public enums #1015

Closed chitoyuu closed 1 year ago

chitoyuu commented 1 year ago

Additionally, utilizes the clippy::exhaustive_enums lint to prevent new exhaustive ones from appearing in the public interface.

This makes it non-breaking to add new variants to enums that aren't intended to be exhaustively matched on.

Close #987

chitoyuu commented 1 year ago

Marking as draft until 0.11.3 is released.

chitoyuu commented 1 year ago

I agree that #[non_exhaustive] does have a drawback as well, that's why it's important to consider whether the enums are supposed to be exhaustively pattern matched on.

The Margin enum, for example, is used mainly as the index in indexed setters. It isn't something often returned from APIs and consequently pattern matched on, or perhaps at all. Of course, users can abuse it to represent something like movement direction, but that's exactly what it is: abusing the type, so it isn't a particularly interesting use case to support.

I'd argue that it's mostly the same for Axis, although it's less clear cut because it's much more general semantically. I think it's fine to make it exhaustive for now. Perhaps it's more useful that way.

chitoyuu commented 1 year ago

Made Axis exhaustive.

bors try

bors[bot] commented 1 year ago

try

Build succeeded:

chitoyuu commented 1 year ago

bors r+

bors[bot] commented 1 year ago

Build succeeded: