From c4998bc3f2f1978fbbe3f192f3d692523ee266d3 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Tue, 29 Aug 2023 08:38:33 -0400 Subject: [ruby/yarp] Desugar ||= more accurately Class variables, global variables, constants, and constant paths should actually desugar to `defined?` instead of just reading the value. https://github.com/ruby/yarp/commit/551a59b876 --- lib/yarp/desugar_visitor.rb | 140 +++++++++++++++++++------------------- test/yarp/desugar_visitor_test.rb | 8 +-- 2 files changed, 74 insertions(+), 74 deletions(-) diff --git a/lib/yarp/desugar_visitor.rb b/lib/yarp/desugar_visitor.rb index 83a9bb5336..8fe488368e 100644 --- a/lib/yarp/desugar_visitor.rb +++ b/lib/yarp/desugar_visitor.rb @@ -8,26 +8,16 @@ module YARP # # @@foo && @@foo = bar def visit_class_variable_and_write_node(node) - AndNode.new( - ClassVariableReadNode.new(node.name_loc), - ClassVariableWriteNode.new(node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_and_write_node(node, ClassVariableReadNode, ClassVariableWriteNode) end # @@foo ||= bar # # becomes # - # @@foo || @@foo = bar + # defined?(@@foo) ? @@foo : @@foo = bar def visit_class_variable_or_write_node(node) - OrNode.new( - ClassVariableReadNode.new(node.name_loc), - ClassVariableWriteNode.new(node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_or_write_defined_node(node, ClassVariableReadNode, ClassVariableWriteNode) end # @@foo += bar @@ -36,7 +26,7 @@ module YARP # # @@foo = @@foo + bar def visit_class_variable_operator_write_node(node) - desugar_operator_write_node(node, ClassVariableWriteNode, ClassVariableReadNode) + desugar_operator_write_node(node, ClassVariableReadNode, ClassVariableWriteNode) end # Foo &&= bar @@ -45,12 +35,7 @@ module YARP # # Foo && Foo = bar def visit_constant_and_write_node(node) - AndNode.new( - ConstantReadNode.new(node.name_loc), - ConstantWriteNode.new(node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_and_write_node(node, ConstantReadNode, ConstantWriteNode) end # Foo ||= bar @@ -59,12 +44,7 @@ module YARP # # Foo || Foo = bar def visit_constant_or_write_node(node) - OrNode.new( - ConstantReadNode.new(node.name_loc), - ConstantWriteNode.new(node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_or_write_defined_node(node, ConstantReadNode, ConstantWriteNode) end # Foo += bar @@ -73,7 +53,7 @@ module YARP # # Foo = Foo + bar def visit_constant_operator_write_node(node) - desugar_operator_write_node(node, ConstantWriteNode, ConstantReadNode) + desugar_operator_write_node(node, ConstantReadNode, ConstantWriteNode) end # Foo::Bar &&= baz @@ -96,9 +76,19 @@ module YARP # # Foo::Bar || Foo::Bar = baz def visit_constant_path_or_write_node(node) - OrNode.new( - node.target, - ConstantPathWriteNode.new(node.target, node.value, node.operator_loc, node.location), + IfNode.new( + node.operator_loc, + DefinedNode.new(nil, node.target, nil, node.operator_loc, node.target.location), + StatementsNode.new([node.target], node.location), + ElseNode.new( + node.operator_loc, + StatementsNode.new( + [ConstantPathWriteNode.new(node.target, node.value, node.operator_loc, node.location)], + node.location + ), + node.operator_loc, + node.location + ), node.operator_loc, node.location ) @@ -135,12 +125,7 @@ module YARP # # $foo && $foo = bar def visit_global_variable_and_write_node(node) - AndNode.new( - GlobalVariableReadNode.new(node.name_loc), - GlobalVariableWriteNode.new(node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_and_write_node(node, GlobalVariableReadNode, GlobalVariableWriteNode) end # $foo ||= bar @@ -149,12 +134,7 @@ module YARP # # $foo || $foo = bar def visit_global_variable_or_write_node(node) - OrNode.new( - GlobalVariableReadNode.new(node.name_loc), - GlobalVariableWriteNode.new(node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_or_write_defined_node(node, GlobalVariableReadNode, GlobalVariableWriteNode) end # $foo += bar @@ -163,7 +143,7 @@ module YARP # # $foo = $foo + bar def visit_global_variable_operator_write_node(node) - desugar_operator_write_node(node, GlobalVariableWriteNode, GlobalVariableReadNode) + desugar_operator_write_node(node, GlobalVariableReadNode, GlobalVariableWriteNode) end # @foo &&= bar @@ -172,12 +152,7 @@ module YARP # # @foo && @foo = bar def visit_instance_variable_and_write_node(node) - AndNode.new( - InstanceVariableReadNode.new(node.name, node.name_loc), - InstanceVariableWriteNode.new(node.name, node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_and_write_node(node, InstanceVariableReadNode, InstanceVariableWriteNode, arguments: [node.name]) end # @foo ||= bar @@ -186,12 +161,7 @@ module YARP # # @foo || @foo = bar def visit_instance_variable_or_write_node(node) - OrNode.new( - InstanceVariableReadNode.new(node.name, node.name_loc), - InstanceVariableWriteNode.new(node.name, node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_or_write_node(node, InstanceVariableReadNode, InstanceVariableWriteNode, arguments: [node.name]) end # @foo += bar @@ -200,7 +170,7 @@ module YARP # # @foo = @foo + bar def visit_instance_variable_operator_write_node(node) - desugar_operator_write_node(node, InstanceVariableWriteNode, InstanceVariableReadNode, arguments: [node.name]) + desugar_operator_write_node(node, InstanceVariableReadNode, InstanceVariableWriteNode, arguments: [node.name]) end # foo &&= bar @@ -209,12 +179,7 @@ module YARP # # foo && foo = bar def visit_local_variable_and_write_node(node) - AndNode.new( - LocalVariableReadNode.new(node.name, node.depth, node.name_loc), - LocalVariableWriteNode.new(node.name, node.depth, node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_and_write_node(node, LocalVariableReadNode, LocalVariableWriteNode, arguments: [node.name, node.depth]) end # foo ||= bar @@ -223,12 +188,7 @@ module YARP # # foo || foo = bar def visit_local_variable_or_write_node(node) - OrNode.new( - LocalVariableReadNode.new(node.name, node.depth, node.name_loc), - LocalVariableWriteNode.new(node.name, node.depth, node.name_loc, node.value, node.operator_loc, node.location), - node.operator_loc, - node.location - ) + desugar_or_write_node(node, LocalVariableReadNode, LocalVariableWriteNode, arguments: [node.name, node.depth]) end # foo += bar @@ -237,13 +197,23 @@ module YARP # # foo = foo + bar def visit_local_variable_operator_write_node(node) - desugar_operator_write_node(node, LocalVariableWriteNode, LocalVariableReadNode, arguments: [node.name, node.depth]) + desugar_operator_write_node(node, LocalVariableReadNode, LocalVariableWriteNode, arguments: [node.name, node.depth]) end private + # Desugar `x &&= y` to `x && x = y` + def desugar_and_write_node(node, read_class, write_class, arguments: []) + AndNode.new( + read_class.new(*arguments, node.name_loc), + write_class.new(*arguments, node.name_loc, node.value, node.operator_loc, node.location), + node.operator_loc, + node.location + ) + end + # Desugar `x += y` to `x = x + y` - def desugar_operator_write_node(node, write_class, read_class, arguments: []) + def desugar_operator_write_node(node, read_class, write_class, arguments: []) write_class.new( *arguments, node.name_loc, @@ -263,5 +233,35 @@ module YARP node.location ) end + + # Desugar `x ||= y` to `x || x = y` + def desugar_or_write_node(node, read_class, write_class, arguments: []) + OrNode.new( + read_class.new(*arguments, node.name_loc), + write_class.new(*arguments, node.name_loc, node.value, node.operator_loc, node.location), + node.operator_loc, + node.location + ) + end + + # Don't desugar `x ||= y` to `defined?(x) ? x : x = y` + def desugar_or_write_defined_node(node, read_class, write_class) + IfNode.new( + node.operator_loc, + DefinedNode.new(nil, read_class.new(node.name_loc), nil, node.operator_loc, node.name_loc), + StatementsNode.new([read_class.new(node.name_loc)], node.location), + ElseNode.new( + node.operator_loc, + StatementsNode.new( + [write_class.new(node.name_loc, node.value, node.operator_loc, node.location)], + node.location + ), + node.operator_loc, + node.location + ), + node.operator_loc, + node.location + ) + end end end diff --git a/test/yarp/desugar_visitor_test.rb b/test/yarp/desugar_visitor_test.rb index a893422a7c..de635966a1 100644 --- a/test/yarp/desugar_visitor_test.rb +++ b/test/yarp/desugar_visitor_test.rb @@ -14,10 +14,10 @@ class DesugarVisitorTest < Test::Unit::TestCase end def test_or_write - assert_desugars("(OrNode (ClassVariableReadNode) (ClassVariableWriteNode (CallNode)))", "@@foo ||= bar") - assert_desugars("(OrNode (ConstantPathNode (ConstantReadNode) (ConstantReadNode)) (ConstantPathWriteNode (ConstantPathNode (ConstantReadNode) (ConstantReadNode)) (CallNode)))", "Foo::Bar ||= baz") - assert_desugars("(OrNode (ConstantReadNode) (ConstantWriteNode (CallNode)))", "Foo ||= bar") - assert_desugars("(OrNode (GlobalVariableReadNode) (GlobalVariableWriteNode (CallNode)))", "$foo ||= bar") + assert_desugars("(IfNode (DefinedNode (ClassVariableReadNode)) (StatementsNode (ClassVariableReadNode)) (ElseNode (StatementsNode (ClassVariableWriteNode (CallNode)))))", "@@foo ||= bar") + assert_desugars("(IfNode (DefinedNode (ConstantPathNode (ConstantReadNode) (ConstantReadNode))) (StatementsNode (ConstantPathNode (ConstantReadNode) (ConstantReadNode))) (ElseNode (StatementsNode (ConstantPathWriteNode (ConstantPathNode (ConstantReadNode) (ConstantReadNode)) (CallNode)))))", "Foo::Bar ||= baz") + assert_desugars("(IfNode (DefinedNode (ConstantReadNode)) (StatementsNode (ConstantReadNode)) (ElseNode (StatementsNode (ConstantWriteNode (CallNode)))))", "Foo ||= bar") + assert_desugars("(IfNode (DefinedNode (GlobalVariableReadNode)) (StatementsNode (GlobalVariableReadNode)) (ElseNode (StatementsNode (GlobalVariableWriteNode (CallNode)))))", "$foo ||= bar") assert_desugars("(OrNode (InstanceVariableReadNode) (InstanceVariableWriteNode (CallNode)))", "@foo ||= bar") assert_desugars("(OrNode (LocalVariableReadNode) (LocalVariableWriteNode (CallNode)))", "foo ||= bar") assert_desugars("(OrNode (LocalVariableReadNode) (LocalVariableWriteNode (CallNode)))", "foo = 1; foo ||= bar") -- cgit v1.2.3