Apply suggestions from code review

Co-authored-by: Claire Foster <[email protected]>

Author: mlechu

src/syntax_graph.jl

@@ -22,6 +22,7 @@ function freeze_attrs(graph::SyntaxGraph)
     SyntaxGraph(graph.edge_ranges, graph.edges, frozen_attrs)
 end
 
+# Create a copy of `graph` where the attribute list is mutable
 function unfreeze_attrs(graph::SyntaxGraph)
     unfrozen_attrs = Dict{Symbol,Any}(pairs(graph.attributes)...)
     SyntaxGraph(graph.edge_ranges, graph.edges, unfrozen_attrs)
@@ -38,8 +39,8 @@ function attrnames(graph::SyntaxGraph)
     keys(graph.attributes)
 end
 
-function attrtypes(graph::SyntaxGraph)
-    [(k, typeof(v).parameters[2]) for (k, v) in pairs(graph.attributes)]
+function attrdefs(graph::SyntaxGraph)
+    [(k=>typeof(v).parameters[2]) for (k, v) in pairs(graph.attributes)]
 end
 
 function Base.show(io::IO, ::MIME"text/plain", graph::SyntaxGraph)
@@ -48,19 +49,24 @@ function Base.show(io::IO, ::MIME"text/plain", graph::SyntaxGraph)
     _show_attrs(io, graph.attributes)
 end
 
-function ensure_attributes!(graph::SyntaxGraph{<:Dict}; kws...)
+function ensure_attributes!(graph::SyntaxGraph; kws...)
     for (k,v) in pairs(kws)
         @assert k isa Symbol
         @assert v isa Type
         if haskey(graph.attributes, k)
             v0 = valtype(graph.attributes[k])
             v == v0 || throw(ErrorException("Attribute type mismatch $v != $v0"))
+        elseif graph.attributes isa NamedTuple
+            throw(ErrorException("""
+                ensure_attributes!: $k is not an existing attribute, and the graph's attributes are frozen. \
+                Consider calling non-mutating `ensure_attributes` instead."""))
         else
             graph.attributes[k] = Dict{NodeId,v}()
         end
     end
     graph
 end
+
 function ensure_attributes(graph::SyntaxGraph{<:Dict}; kws...)
     g = unfreeze_attrs(graph)
     ensure_attributes!(g; kws...)

test/syntax_graph.jl

@@ -16,12 +16,12 @@
     @test gf2.attributes isa NamedTuple
     @test gu2.attributes isa Dict
     # does its job
-    @test (:test_attr, Symbol) in JuliaLowering.attrtypes(gf2)
-    @test (:foo, Type) in JuliaLowering.attrtypes(gf2)
+    @test (:test_attr=>Symbol) in JuliaLowering.attrdefs(gf2)
+    @test (:foo=>Type) in JuliaLowering.attrdefs(gf2)
     @test Set(keys(gf2.attributes)) == Set(keys(gu2.attributes))
     # no mutation
-    @test !((:test_attr, Symbol) in JuliaLowering.attrtypes(gf1))
-    @test !((:foo, Type) in JuliaLowering.attrtypes(gf1))
+    @test !((:test_attr=>Symbol) in JuliaLowering.attrdefs(gf1))
+    @test !((:foo=>Type) in JuliaLowering.attrdefs(gf1))
     @test Set(keys(gf1.attributes)) == Set(keys(gu1.attributes))
 
     # delete_attributes
@@ -31,12 +31,12 @@
     @test gf3.attributes isa NamedTuple
     @test gu3.attributes isa Dict
     # does its job
-    @test !((:test_attr, Symbol) in JuliaLowering.attrtypes(gf3))
-    @test !((:foo, Type) in JuliaLowering.attrtypes(gf3))
+    @test !((:test_attr=>Symbol) in JuliaLowering.attrdefs(gf3))
+    @test !((:foo=>Type) in JuliaLowering.attrdefs(gf3))
     @test Set(keys(gf3.attributes)) == Set(keys(gu3.attributes))
     # no mutation
-    @test (:test_attr, Symbol) in JuliaLowering.attrtypes(gf2)
-    @test (:foo, Type) in JuliaLowering.attrtypes(gf2)
+    @test (:test_attr=>Symbol) in JuliaLowering.attrdefs(gf2)
+    @test (:foo=>Type) in JuliaLowering.attrdefs(gf2)
     @test Set(keys(gf2.attributes)) == Set(keys(gu2.attributes))
 end