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.

Sign up for GitHub

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

Jump to bottom

state_record_and_type not considering opaque as a type declaration #349

Closed
paulo-ferraz-oliveira opened this issue Jun 30, 2024 · 3 comments · Fixed by #354
Closed

state_record_and_type not considering opaque as a type declaration #349

paulo-ferraz-oliveira opened this issue Jun 30, 2024 · 3 comments · Fixed by #354
Labels
bug

Comments

@paulo-ferraz-oliveira
Copy link
Collaborator

Bug Description

I have a state, in a gen_server declared as a record.

I have to export it for export_used_types (it's in the public interface).

If I try to make it opaque, I get This module implements an OTP behavior and has a 'state' record but is missing a 'state()' type.

Should it be "wrong" (for Elvis) to have

-record(state, {
}).
-opaque state() :: #state{}.
-export_type([state/0]).

?

To Reproduce

Check Description.

Expected Behavior

opaque should be handled the same as type.

rebar3 Logs

Not relevant.

Additional Context

Not relevant.
@elbrujohalcon
Copy link
Member

I have to export it for export_used_types (it's in the public interface).

This is weird. Why do you need to export a gen_server internal state? 🤔

I think this all comes down to this ☝🏻 . It's not usual to export the internal state of a behaviour implementation. With that in mind, if there is a good reason to do so… That (in my mind) would be the perfect example for when -elvis is needed. It's not that the rules are wrong, it's just that this is a very very uncommon situation.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

Why do you need to export a gen_server internal state?

I don't, but https://github.com/inaka/elvis_core/blob/main/doc_rules/elvis_style/export_used_types.md does 😄 So the bug is basically that the rules are "incompatible" if applied at the same time.

@elbrujohalcon
Copy link
Member

I see… The type needs to be exported because it's used in the spec of a callback function.
So, yeah… I agree… state_record_and_type should not warn you if you declare an opaque type, instead of a public one.

@belltoy belltoy mentioned this issue Aug 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add opaque state as a type declaration belltoy/elvis_core
2 participants
@elbrujohalcon @paulo-ferraz-oliveira