search

LemmyNet / lemmy

🐀 A link aggregator and forum for the fediverse
https://join-lemmy.org
GNU Affero General Public License v3.0
13.3k stars 882 forks source link

Use visibility on enums #5201

Closed jayvdb closed 2 weeks ago

jayvdb commented 2 weeks ago

c.f. https://stackoverflow.com/questions/75244498/is-it-possible-to-find-unused-enum-variants/78562603

Nutomic commented 2 weeks ago

Does this actually give you an error message for unused enums or variants? I tried to add a new, unused enum in your code and run the tests, but there was no error/warning at all.

jayvdb commented 2 weeks ago

It doesnt work if the enum is used somewhere where it is pub , so it requires marking lots of the usages as pub(crate).

So for example adding "Other" to PersonOrGroupType doesnt get caught because PersonOrGroupType is used in public member AttributedToPeertube.kind. I tried to get errors for the enums in lemmy_apub , but couldnt figure it out - the enums are very very deeply nested in structs that are pub - I added #[cfg_attr(test, visibility::make(pub(crate)))] to a lot of the structs, but must have missed one.

Adding unused members to LemmyErrorType also does not trigger compile errors because it is used in public member LemmyError.error_type. Again I tried to get this to be private, but failed. FederationError and LemmyErrorType are almost identical to how I am using visibility to catch unused enum variants, so it definitely works, but the LemmyError wrapper means it wont work.

However e.g. EndpointType is not used in tests, so it already causes a compile error when visibility is applied to it.

Sorry I cant throw more time at this. I will take the learnings from this PR and add them to the stackoverflow answer to explain these scenarios where it is probably too difficult to use visibility.

dessalines commented 2 weeks ago

Ah dang, thanks for the try anyway.

Ideally someone would make a crate that could scan a workspace / codebase for unused enums, that we and other projects could add to their CI.

It'd probably have to be based on text searching alone since rust / clippy can't help us detect these.

Nutomic commented 2 weeks ago

I think this approach only works if all the code is within a single crate, but not across a workspace.