-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Ruby: At most one hash-splat ParameterNode per callable
#11461
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
Conversation
3bb5c5a to
9453bb8
Compare
9453bb8 to
c67262a
Compare
|
Feel free to ignore the Swift CI failures. They should be fixed by merging in the recent |
michaelnebel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C#: Looks plausible to me.
c67262a to
b33f592
Compare
|
Rebased to resolve semantic merge conflict (Go data flow libraries now need to be merged as well). |
owen-mc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go 👍🏻
owen-mc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go 👍🏻
michaelnebel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C# LGTM
aibaars
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Would it be possible to add a test case for Ruby?
Rather, we will be adding some consistency checks to the shared data flow library. |
That's even better! |
|
Consistency checks: #11564 |
Similar to the Ruby changes from github#11461 I feel the change to `DataFlowFunciton.getParameter` where we use `not exists(func.getArgByName(_))` is not very great, but I was not allowed to use `not exists(this.getParameter(any(ParameterPosition _).isKeyword(_)))` because of negative recursion.
As discussed with @aschackmull, it is best to have at most one
ParameterNodeperParameterPositionandDataFlowCallable. This PR ensures that eachDataFlowCallablehas at most oneParameterNodewith hash-splatParameterPosition.