Fix Bug in the hashing of empty tuples (#49)

The new parser doesn't correctly parse "{}", which occurs in e.g. an empty tuple type. This adds tests and fixes the parsing of this edge case.
beacon-biosignals · Jan 19, 2024 · 1724ffa · 1724ffa · haberdashPI · Jan 19, 2024
1 parent 6c7ce8f
commit 1724ffa
Show file tree

Hide file tree

Showing 9 changed files with 19 additions and 4 deletions.
diff --git a/Project.toml b/Project.toml
@@ -1,7 +1,7 @@
 name = "StableHashTraits"
 uuid = "c5dd0088-6c3f-4803-b00e-f31a60c170fa"
 authors = ["Beacon Biosignals, Inc."]
-version = "1.1.5"
+version = "1.1.6"
 
 [deps]
 Compat = "34da2185-b29b-5c13-b0c7-acf172513d20"

diff --git a/src/StableNames.jl b/src/StableNames.jl
@@ -70,7 +70,8 @@ end
                      :space => P.satisfy(isspace),
                      :sep => P.first(P.seq(P.many(:space), P.token(','), P.many(:space)),
                                      P.some(:space)),
-                     :brackets => P.seq(P.token('{'), :clause, P.token('}')),
+                     :brackets => P.first(P.seq(P.token('{'), P.many(:sep), P.token('}')),
+                                          P.seq(P.token('{'), :clause, P.token('}'))),
                      :head_brackets => P.seq(:element, :brackets),
                      :inclause => P.first(:head_brackets, :brackets, :element),
                      :clause => P.seq(P.many(:sep),
@@ -95,7 +96,7 @@ end
         return if match.rule ∈ (:element, :space, :sep)
             String(match.view)
         elseif match.rule == :brackets
-            Parsed(:Brackets, vals[2]...)
+            Parsed(:Brackets, (isnothing(vals[1][2]) ? [""] : vals[1][2])...)
         elseif match.rule == :clause
             reduce(vcat, filter(!isnothing, vals); init=[])
         elseif match.rule == :head_brackets
@@ -171,7 +172,7 @@ end
         if parsed isa Parsed && parsed.name == :Head &&
            endswith(parsed.args[1], "@NamedTuple")
             symbols_and_types = split_symbol_and_type.(parsed.args[2].args)
-            symbol_tuple = join(":" .* first.(symbols_and_types), ",")
+            symbol_tuple = join(":" .* filter(!isempty, first.(symbols_and_types)), ",")
             types = map(t -> parse_walker(fn, t), last.(symbols_and_types))
             types_tuple = join(types, ",")
             prefix = replace(parsed.args[1], "@NamedTuple" => "")

diff --git a/test/references/ref22_1_crc.txt b/test/references/ref22_1_crc.txt
@@ -0,0 +1 @@
+2427942825
diff --git a/test/references/ref22_1_sha1.txt b/test/references/ref22_1_sha1.txt
@@ -0,0 +1 @@
+54a26391c9b89dfd2bea64d847ddf4375bd3f0de
diff --git a/test/references/ref22_1_sha256.txt b/test/references/ref22_1_sha256.txt
@@ -0,0 +1 @@
+39821b69f3416b69ab37bf2144578236b5b07afae616dd07e8ecbb46813a3bf4
diff --git a/test/references/ref22_2_crc32c.txt b/test/references/ref22_2_crc32c.txt
@@ -0,0 +1 @@
+2606483572
diff --git a/test/references/ref22_2_sha1.txt b/test/references/ref22_2_sha1.txt
@@ -0,0 +1 @@
+f2f60b99477939b30b470b8718f7ef38cbb2431a
diff --git a/test/references/ref22_2_sha256.txt b/test/references/ref22_2_sha256.txt
@@ -0,0 +1 @@
+1ca343199ba59bb3c36204421371ba45be23af7a1b3b2e4d56f4c1e2f6a48659
diff --git a/test/runtests.jl b/test/runtests.jl
@@ -78,6 +78,8 @@ include("setup_tests.jl")
                                 bytes2hex_(test_hash(==("test"))))
                 @test_reference("references/ref21_$(V)_$(nameof(hashfn)).txt",
                                 bytes2hex_(test_hash((1, (a=1, b=(x=1, y=2), c=(1, 2))))))
+                @test_reference("references/ref22_$(V)_$(nameof(hashfn)).txt",
+                                bytes2hex_(test_hash((;))))
             end
             # verifies that transform can be called recursively
 
@@ -340,6 +342,10 @@ include("setup_tests.jl")
             @test parse_brackets("bob, joe, ") ==
                   ["bob", Parsed(:SepClause, ", ", "joe"), ", "]
             @test parse_brackets("bob,joe") == ["bob", Parsed(:SepClause, ",", "joe")]
+            @test parse_brackets("{}") == Any[Parsed(:Brackets, "")]
+            @test parse_brackets("{,,}") == Any[Parsed(:Brackets, ",", ",")]
+            @test parse_brackets("{ }") == Any[Parsed(:Brackets, " ")]
+            @test parse_brackets("{, joe}") == Any[Parsed(:Brackets, ", ", "joe")]
             @test parse_brackets("{bob, joe}") ==
                   Any[Parsed(:Brackets, "bob", Parsed(:SepClause, ", ", "joe"))]
             @test parse_brackets("foo{bob, joe}") ==
@@ -363,6 +369,7 @@ include("setup_tests.jl")
             @test_throws ParseError parse_brackets("{{ joe{bob, bill} }")
             @test_throws ParseError parse_brackets("{ joe{bob,} bill} }")
             @test_throws ParseError parse_brackets("{ joe{bob, {bill} }")
+            @test_throws ParseError parse_brackets("")
 
             # verify parser round-trip
             round_trips(str) = parse_walker((fn, p) -> nothing, parse_brackets(str)) == str
@@ -390,6 +397,7 @@ include("setup_tests.jl")
             @test replace_bob("foo{bar{baz, bob}, boz}") == "foo{bar{baz, BOB}, boz}"
 
             # validate the named tuple replaceer
+            @test cleanup_named_tuple_type(string(typeof((;)))) == "NamedTuple{(),Tuple{}}"
             @test cleanup_named_tuple_type("@NamedTuple{x::Int, y::Int}") ==
                   "NamedTuple{(:x,:y),Tuple{Int,Int}}"
             @test cleanup_named_tuple_type("FooBar{Baz{Float64, (custom, display(}, " *