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 opaque state as a type declaration #354

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

belltoy
Copy link
Contributor

@belltoy belltoy commented Aug 16, 2024

Fix #349

Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test case for this and, if needed, update the corresponding docs?
Thanks.

src/elvis_style.erl Outdated Show resolved Hide resolved
@belltoy
Copy link
Contributor Author

belltoy commented Aug 18, 2024

The previous implementation was incorrect. I've fixed that and rebased with some test cases. And also provided cases to verify rule state_record_and_type + rule export_used_types.

But something looks weird to me. In the pass_state_record_and_type.erl case, the type state() is defined without exported, but the state() is used in the exported function's spec.

@elbrujohalcon
Copy link
Member

But something looks weird to me. In the pass_state_record_and_type.erl case, the type state() is defined without exported, but the state() is used in the exported function's spec.

I'm not 100% sure, but it's likely because we're not running the used_exported_types rule on it.

Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve the PR, in general.
So, if you don't have time or just don't feel like improving the gen_servers, let me know and I'll merge it.
OTOH, if you can improve that code (even when it's just for tests), that would be lovely.

@belltoy
Copy link
Contributor Author

belltoy commented Aug 19, 2024

Of course, I will improve those codes in test cases.

@elbrujohalcon elbrujohalcon merged commit 7d7ac44 into inaka:main Aug 19, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

state_record_and_type not considering opaque as a type declaration
2 participants