You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
@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.
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.
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
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.
This PR adds
nilstate parameter to the type signature for allto_jsonmethods.cState_from_state_swill return an unconfigured instance ifnilis passed. A lot of implementations ofto_jsonhavenilas the default value.