Skip to content

Conversation

@vpellan
Copy link

@vpellan vpellan commented Oct 29, 2025

This PR adds nil state parameter to the type signature for all to_json methods. cState_from_state_s will return an unconfigured instance if nil is passed. A lot of implementations of to_json have nil as the default value.

@ksss
Copy link
Collaborator

ksss commented Oct 30, 2025

@vpellan Thank you for your request.
Please tell me the specific use cases that will be improved by this change.
This is a breaking change, so we need information to make a decision.

@vpellan
Copy link
Author

vpellan commented Oct 30, 2025

Hey @ksss, thanks for your quick reply ! We're currently trying to improve typing in dd-trace-rb, and one of the first step is to re-enable the disabled rules in our Steepfile. When re-enabling Ruby::IncompatibleAssignment, we stumbled on an error on the parameter of a to_json method that we've implemented.
If we change its signature to def to_json: (?JSON::State? state) -> String, a new Ruby::ArgumentTypeMismatch error will appear at line 140 saying that Hash#to_json does not accept nil. Of course we can skip it by changing the parameter to arg = _ = nil or adding a # steep:ignore comment (and even if this PR gets merged, we'll still have to do it until the next release anyway) but I thought that fixing the root cause could be a great overall improvement.

@ksss
Copy link
Collaborator

ksss commented Nov 5, 2025

@vpellan
Thank you for the use case.
We plan to discuss it at our next developer meeting.

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.

2 participants