fix(grpc): remove specs and enforce compiler warnings#552
Merged
polvalente merged 1 commit intoJul 1, 2026
Conversation
3 tasks
polvalente
approved these changes
Jul 1, 2026
|
|
||
| defp push_message(%{messages: messages} = state, message, terminal?) do | ||
| {:noreply, %{state | messages: :queue.in(message, messages), done: state.done or terminal?}} | ||
| {:noreply, %{state | messages: :queue.in(message, messages), done: terminal?}} |
Contributor
There was a problem hiding this comment.
Gotta double-check this change and the one above it
Contributor
There was a problem hiding this comment.
Ok, this change makes sense! There's a clause above catching done: true, so the type checker knows done is false in all clauses below
Contributor
|
Thanks for pivoting into this! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Dialyzer reports
GRPC.Stub.connect/2as returning a value outside its own spec.connect/2returns aGRPC.Channelstruct, butGRPC.Channel.t()is stricter than the channel states built by the client connection code. In our project, running dialyzer errors on calls toconnect/2which sees a broken public type:GRPC.Client.Stream.t()has the same issue for fields that start asniland are filled later.Root cause
The
ChannelandStreamstructs support incrementally populating their values, but the public types describe only fully populated state.Fix
Project direction is to stop supporting Dialyzer and instead leverage Elixir compiler warnings:
@specannotations from source and README examplesReview attention
Elixir's type system did not catch the issue related to stream trailer parsing flagged by Dialyzer, so I'm deferring that to a future PR to avoid conflating things:
Test plan
grpcandgrpc_corefrom this branch in our project and successfully ran dialyzer