Skip to content

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Aug 5, 2022

The existing summaries could be simplified using the models-as-data token Argument[hash-splat], which refers to both keyword arguments wrapped in an implicit hash, as well as explicit hash splat arguments.

@github-actions github-actions bot added the Ruby label Aug 5, 2022
@hvitved hvitved marked this pull request as ready for review August 5, 2022 12:35
@hvitved hvitved requested a review from a team as a code owner August 5, 2022 12:35
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Aug 5, 2022
hmac
hmac previously approved these changes Aug 9, 2022
Copy link
Contributor

@hmac hmac left a comment

Choose a reason for hiding this comment

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

Nice. As an optional addition, I thought it might be nice to add these two variants to the existing hash-flow.rb tests. I added them locally to confirm that things were working as expected.

diff --git a/ruby/ql/test/library-tests/dataflow/hash-flow/hash_flow.rb b/ruby/ql/test/library-tests/dataflow/hash-flow/hash_flow.rb
index f9de83dd47..06c57336a4 100644
--- a/ruby/ql/test/library-tests/dataflow/hash-flow/hash_flow.rb
+++ b/ruby/ql/test/library-tests/dataflow/hash-flow/hash_flow.rb
@@ -68,6 +68,14 @@ def m3()
     hash4 = Hash[:a, taint(3.4), :b, 1]
     sink(hash4[:a]) # $ hasValueFlow=3.4
     sink(hash4[:b])
+
+    hash5 = Hash["a" => taint(3.4), "b" => 1]
+    sink(hash5["a"]) # $ hasValueFlow=3.4
+    sink(hash5["b"])
+
+    hash6 = Hash[{"a" => taint(3.4), "b" => 1}]
+    sink(hash6["a"]) # $ hasValueFlow=3.4
+    sink(hash6["b"])
 end
 
 m3()
@@ -746,4 +754,4 @@ def m45()
     sink(hash[:h])
 end
 
-m45()
\ No newline at end of file
+m45()

But it doesn't need to block this PR

@hvitved
Copy link
Contributor Author

hvitved commented Aug 9, 2022

Thanks, I'll add the additional tests.

@hvitved hvitved merged commit 19043bd into github:main Aug 10, 2022
@hvitved hvitved deleted the ruby/hash-literal-summary-simplification branch August 10, 2022 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants