From fc38ff6e4417295c870f419f7c164ab5a7dbc4a5 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 15 Jun 2019 22:17:20 +0900 Subject: Remove unused `DepthFirst` visitor We only use `ToSQL` visitors in the our codebase, do not use `DepthFirst` and `Dot` visitors. The `DepthFirst` visitor (which was introduced at c86c37e5f) is used to traverse an Arel (partial) ast with depth first. Is there any worth to keep that undocumented feature with much code and test cases. This removes that unused `DepthFirst` code and test cases. --- activerecord/lib/arel/nodes/node.rb | 8 - activerecord/lib/arel/visitors.rb | 1 - activerecord/lib/arel/visitors/depth_first.rb | 203 --------------- activerecord/test/cases/arel/nodes/node_test.rb | 19 -- .../test/cases/arel/select_manager_test.rb | 10 - .../test/cases/arel/visitors/depth_first_test.rb | 276 --------------------- .../arel/visitors/dispatch_contamination_test.rb | 8 +- 7 files changed, 7 insertions(+), 518 deletions(-) delete mode 100644 activerecord/lib/arel/visitors/depth_first.rb delete mode 100644 activerecord/test/cases/arel/visitors/depth_first_test.rb (limited to 'activerecord') diff --git a/activerecord/lib/arel/nodes/node.rb b/activerecord/lib/arel/nodes/node.rb index 8086102bde..0416ff58de 100644 --- a/activerecord/lib/arel/nodes/node.rb +++ b/activerecord/lib/arel/nodes/node.rb @@ -6,7 +6,6 @@ module Arel # :nodoc: all # Abstract base class for all AST nodes class Node include Arel::FactoryMethods - include Enumerable ### # Factory method to create a Nodes::Not node that has the recipient of @@ -38,13 +37,6 @@ module Arel # :nodoc: all collector = engine.connection.visitor.accept self, collector collector.value end - - # Iterate through AST, nodes will be yielded depth-first - def each(&block) - return enum_for(:each) unless block_given? - - ::Arel::Visitors::DepthFirst.new(block).accept self - end end end end diff --git a/activerecord/lib/arel/visitors.rb b/activerecord/lib/arel/visitors.rb index e350f52e65..a1097f6750 100644 --- a/activerecord/lib/arel/visitors.rb +++ b/activerecord/lib/arel/visitors.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "arel/visitors/visitor" -require "arel/visitors/depth_first" require "arel/visitors/to_sql" require "arel/visitors/sqlite" require "arel/visitors/postgresql" diff --git a/activerecord/lib/arel/visitors/depth_first.rb b/activerecord/lib/arel/visitors/depth_first.rb deleted file mode 100644 index 98c3f92cf1..0000000000 --- a/activerecord/lib/arel/visitors/depth_first.rb +++ /dev/null @@ -1,203 +0,0 @@ -# frozen_string_literal: true - -module Arel # :nodoc: all - module Visitors - class DepthFirst < Arel::Visitors::Visitor - def initialize(block = nil) - @block = block || Proc.new - super() - end - - private - def visit(o, _ = nil) - super - @block.call o - end - - def unary(o) - visit o.expr - end - alias :visit_Arel_Nodes_Else :unary - alias :visit_Arel_Nodes_Group :unary - alias :visit_Arel_Nodes_Cube :unary - alias :visit_Arel_Nodes_RollUp :unary - alias :visit_Arel_Nodes_GroupingSet :unary - alias :visit_Arel_Nodes_GroupingElement :unary - alias :visit_Arel_Nodes_Grouping :unary - alias :visit_Arel_Nodes_Having :unary - alias :visit_Arel_Nodes_Lateral :unary - alias :visit_Arel_Nodes_Limit :unary - alias :visit_Arel_Nodes_Not :unary - alias :visit_Arel_Nodes_Offset :unary - alias :visit_Arel_Nodes_On :unary - alias :visit_Arel_Nodes_Ordering :unary - alias :visit_Arel_Nodes_Ascending :unary - alias :visit_Arel_Nodes_Descending :unary - alias :visit_Arel_Nodes_UnqualifiedColumn :unary - alias :visit_Arel_Nodes_OptimizerHints :unary - alias :visit_Arel_Nodes_ValuesList :unary - - def function(o) - visit o.expressions - visit o.alias - visit o.distinct - end - alias :visit_Arel_Nodes_Avg :function - alias :visit_Arel_Nodes_Exists :function - alias :visit_Arel_Nodes_Max :function - alias :visit_Arel_Nodes_Min :function - alias :visit_Arel_Nodes_Sum :function - - def visit_Arel_Nodes_NamedFunction(o) - visit o.name - visit o.expressions - visit o.distinct - visit o.alias - end - - def visit_Arel_Nodes_Count(o) - visit o.expressions - visit o.alias - visit o.distinct - end - - def visit_Arel_Nodes_Case(o) - visit o.case - visit o.conditions - visit o.default - end - - def nary(o) - o.children.each { |child| visit child } - end - alias :visit_Arel_Nodes_And :nary - - def binary(o) - visit o.left - visit o.right - end - alias :visit_Arel_Nodes_As :binary - alias :visit_Arel_Nodes_Assignment :binary - alias :visit_Arel_Nodes_Between :binary - alias :visit_Arel_Nodes_Concat :binary - alias :visit_Arel_Nodes_DeleteStatement :binary - alias :visit_Arel_Nodes_DoesNotMatch :binary - alias :visit_Arel_Nodes_Equality :binary - alias :visit_Arel_Nodes_FullOuterJoin :binary - alias :visit_Arel_Nodes_GreaterThan :binary - alias :visit_Arel_Nodes_GreaterThanOrEqual :binary - alias :visit_Arel_Nodes_In :binary - alias :visit_Arel_Nodes_InfixOperation :binary - alias :visit_Arel_Nodes_JoinSource :binary - alias :visit_Arel_Nodes_InnerJoin :binary - alias :visit_Arel_Nodes_LessThan :binary - alias :visit_Arel_Nodes_LessThanOrEqual :binary - alias :visit_Arel_Nodes_Matches :binary - alias :visit_Arel_Nodes_NotEqual :binary - alias :visit_Arel_Nodes_NotIn :binary - alias :visit_Arel_Nodes_NotRegexp :binary - alias :visit_Arel_Nodes_IsNotDistinctFrom :binary - alias :visit_Arel_Nodes_IsDistinctFrom :binary - alias :visit_Arel_Nodes_Or :binary - alias :visit_Arel_Nodes_OuterJoin :binary - alias :visit_Arel_Nodes_Regexp :binary - alias :visit_Arel_Nodes_RightOuterJoin :binary - alias :visit_Arel_Nodes_TableAlias :binary - alias :visit_Arel_Nodes_When :binary - - def visit_Arel_Nodes_StringJoin(o) - visit o.left - end - - def visit_Arel_Attribute(o) - visit o.relation - visit o.name - end - alias :visit_Arel_Attributes_Integer :visit_Arel_Attribute - alias :visit_Arel_Attributes_Float :visit_Arel_Attribute - alias :visit_Arel_Attributes_String :visit_Arel_Attribute - alias :visit_Arel_Attributes_Time :visit_Arel_Attribute - alias :visit_Arel_Attributes_Boolean :visit_Arel_Attribute - alias :visit_Arel_Attributes_Attribute :visit_Arel_Attribute - alias :visit_Arel_Attributes_Decimal :visit_Arel_Attribute - - def visit_Arel_Table(o) - visit o.name - end - - def terminal(o) - end - alias :visit_ActiveSupport_Multibyte_Chars :terminal - alias :visit_ActiveSupport_StringInquirer :terminal - alias :visit_Arel_Nodes_Lock :terminal - alias :visit_Arel_Nodes_Node :terminal - alias :visit_Arel_Nodes_SqlLiteral :terminal - alias :visit_Arel_Nodes_BindParam :terminal - alias :visit_Arel_Nodes_Window :terminal - alias :visit_Arel_Nodes_True :terminal - alias :visit_Arel_Nodes_False :terminal - alias :visit_BigDecimal :terminal - alias :visit_Class :terminal - alias :visit_Date :terminal - alias :visit_DateTime :terminal - alias :visit_FalseClass :terminal - alias :visit_Float :terminal - alias :visit_Integer :terminal - alias :visit_NilClass :terminal - alias :visit_String :terminal - alias :visit_Symbol :terminal - alias :visit_Time :terminal - alias :visit_TrueClass :terminal - - def visit_Arel_Nodes_InsertStatement(o) - visit o.relation - visit o.columns - visit o.values - end - - def visit_Arel_Nodes_SelectCore(o) - visit o.projections - visit o.source - visit o.wheres - visit o.groups - visit o.windows - visit o.havings - end - - def visit_Arel_Nodes_SelectStatement(o) - visit o.cores - visit o.orders - visit o.limit - visit o.lock - visit o.offset - end - - def visit_Arel_Nodes_UpdateStatement(o) - visit o.relation - visit o.values - visit o.wheres - visit o.orders - visit o.limit - end - - def visit_Arel_Nodes_Comment(o) - visit o.values - end - - def visit_Array(o) - o.each { |i| visit i } - end - alias :visit_Set :visit_Array - - def visit_Hash(o) - o.each { |k, v| visit(k); visit(v) } - end - - DISPATCH = dispatch_cache - - def get_dispatch_cache - DISPATCH - end - end - end -end diff --git a/activerecord/test/cases/arel/nodes/node_test.rb b/activerecord/test/cases/arel/nodes/node_test.rb index f4f07ef2c5..f1e0ce1ea9 100644 --- a/activerecord/test/cases/arel/nodes/node_test.rb +++ b/activerecord/test/cases/arel/nodes/node_test.rb @@ -18,24 +18,5 @@ module Arel assert klass.ancestors.include?(Nodes::Node), klass.name end end - - def test_each - list = [] - node = Nodes::Node.new - node.each { |n| list << n } - assert_equal [node], list - end - - def test_generator - list = [] - node = Nodes::Node.new - node.each.each { |n| list << n } - assert_equal [node], list - end - - def test_enumerable - node = Nodes::Node.new - assert_kind_of Enumerable, node - end end end diff --git a/activerecord/test/cases/arel/select_manager_test.rb b/activerecord/test/cases/arel/select_manager_test.rb index e6c49cd429..526fe6787a 100644 --- a/activerecord/test/cases/arel/select_manager_test.rb +++ b/activerecord/test/cases/arel/select_manager_test.rb @@ -369,16 +369,6 @@ module Arel mgr = table.from assert mgr.ast end - - it "should allow orders to work when the ast is grepped" do - table = Table.new :users - mgr = table.from - mgr.project Arel.sql "*" - mgr.from table - mgr.orders << Arel::Nodes::Ascending.new(Arel.sql("foo")) - mgr.ast.grep(Arel::Nodes::OuterJoin) - mgr.to_sql.must_be_like %{ SELECT * FROM "users" ORDER BY foo ASC } - end end describe "taken" do diff --git a/activerecord/test/cases/arel/visitors/depth_first_test.rb b/activerecord/test/cases/arel/visitors/depth_first_test.rb deleted file mode 100644 index 106be2311d..0000000000 --- a/activerecord/test/cases/arel/visitors/depth_first_test.rb +++ /dev/null @@ -1,276 +0,0 @@ -# frozen_string_literal: true - -require_relative "../helper" - -module Arel - module Visitors - class TestDepthFirst < Arel::Test - Collector = Struct.new(:calls) do - def call(object) - calls << object - end - end - - def setup - @collector = Collector.new [] - @visitor = Visitors::DepthFirst.new @collector - end - - def test_raises_with_object - assert_raises(TypeError) do - @visitor.accept(Object.new) - end - end - - - # unary ops - [ - Arel::Nodes::Not, - Arel::Nodes::Group, - Arel::Nodes::On, - Arel::Nodes::Grouping, - Arel::Nodes::Offset, - Arel::Nodes::Ordering, - Arel::Nodes::StringJoin, - Arel::Nodes::UnqualifiedColumn, - Arel::Nodes::ValuesList, - Arel::Nodes::Limit, - Arel::Nodes::Else, - ].each do |klass| - define_method("test_#{klass.name.gsub('::', '_')}") do - op = klass.new(:a) - @visitor.accept op - assert_equal [:a, op], @collector.calls - end - end - - # functions - [ - Arel::Nodes::Exists, - Arel::Nodes::Avg, - Arel::Nodes::Min, - Arel::Nodes::Max, - Arel::Nodes::Sum, - ].each do |klass| - define_method("test_#{klass.name.gsub('::', '_')}") do - func = klass.new(:a, "b") - @visitor.accept func - assert_equal [:a, "b", false, func], @collector.calls - end - end - - def test_named_function - func = Arel::Nodes::NamedFunction.new(:a, :b, "c") - @visitor.accept func - assert_equal [:a, :b, false, "c", func], @collector.calls - end - - def test_lock - lock = Nodes::Lock.new true - @visitor.accept lock - assert_equal [lock], @collector.calls - end - - def test_count - count = Nodes::Count.new :a, :b, "c" - @visitor.accept count - assert_equal [:a, "c", :b, count], @collector.calls - end - - def test_inner_join - join = Nodes::InnerJoin.new :a, :b - @visitor.accept join - assert_equal [:a, :b, join], @collector.calls - end - - def test_full_outer_join - join = Nodes::FullOuterJoin.new :a, :b - @visitor.accept join - assert_equal [:a, :b, join], @collector.calls - end - - def test_outer_join - join = Nodes::OuterJoin.new :a, :b - @visitor.accept join - assert_equal [:a, :b, join], @collector.calls - end - - def test_right_outer_join - join = Nodes::RightOuterJoin.new :a, :b - @visitor.accept join - assert_equal [:a, :b, join], @collector.calls - end - - def test_comment - comment = Nodes::Comment.new ["foo"] - @visitor.accept comment - assert_equal ["foo", ["foo"], comment], @collector.calls - end - - [ - Arel::Nodes::Assignment, - Arel::Nodes::Between, - Arel::Nodes::Concat, - Arel::Nodes::DoesNotMatch, - Arel::Nodes::Equality, - Arel::Nodes::GreaterThan, - Arel::Nodes::GreaterThanOrEqual, - Arel::Nodes::In, - Arel::Nodes::LessThan, - Arel::Nodes::LessThanOrEqual, - Arel::Nodes::Matches, - Arel::Nodes::NotEqual, - Arel::Nodes::NotIn, - Arel::Nodes::Or, - Arel::Nodes::TableAlias, - Arel::Nodes::As, - Arel::Nodes::DeleteStatement, - Arel::Nodes::JoinSource, - Arel::Nodes::When, - ].each do |klass| - define_method("test_#{klass.name.gsub('::', '_')}") do - binary = klass.new(:a, :b) - @visitor.accept binary - assert_equal [:a, :b, binary], @collector.calls - end - end - - def test_Arel_Nodes_InfixOperation - binary = Arel::Nodes::InfixOperation.new(:o, :a, :b) - @visitor.accept binary - assert_equal [:a, :b, binary], @collector.calls - end - - # N-ary - [ - Arel::Nodes::And, - ].each do |klass| - define_method("test_#{klass.name.gsub('::', '_')}") do - binary = klass.new([:a, :b, :c]) - @visitor.accept binary - assert_equal [:a, :b, :c, binary], @collector.calls - end - end - - [ - Arel::Attributes::Integer, - Arel::Attributes::Float, - Arel::Attributes::String, - Arel::Attributes::Time, - Arel::Attributes::Boolean, - Arel::Attributes::Attribute - ].each do |klass| - define_method("test_#{klass.name.gsub('::', '_')}") do - binary = klass.new(:a, :b) - @visitor.accept binary - assert_equal [:a, :b, binary], @collector.calls - end - end - - def test_table - relation = Arel::Table.new(:users) - @visitor.accept relation - assert_equal ["users", relation], @collector.calls - end - - def test_array - node = Nodes::Or.new(:a, :b) - list = [node] - @visitor.accept list - assert_equal [:a, :b, node, list], @collector.calls - end - - def test_set - node = Nodes::Or.new(:a, :b) - set = Set.new([node]) - @visitor.accept set - assert_equal [:a, :b, node, set], @collector.calls - end - - def test_hash - node = Nodes::Or.new(:a, :b) - hash = { node => node } - @visitor.accept hash - assert_equal [:a, :b, node, :a, :b, node, hash], @collector.calls - end - - def test_update_statement - stmt = Nodes::UpdateStatement.new - stmt.relation = :a - stmt.values << :b - stmt.wheres << :c - stmt.orders << :d - stmt.limit = :e - - @visitor.accept stmt - assert_equal [:a, :b, stmt.values, :c, stmt.wheres, :d, stmt.orders, - :e, stmt], @collector.calls - end - - def test_select_core - core = Nodes::SelectCore.new - core.projections << :a - core.froms = :b - core.wheres << :c - core.groups << :d - core.windows << :e - core.havings << :f - - @visitor.accept core - assert_equal [ - :a, core.projections, - :b, [], - core.source, - :c, core.wheres, - :d, core.groups, - :e, core.windows, - :f, core.havings, - core], @collector.calls - end - - def test_select_statement - ss = Nodes::SelectStatement.new - ss.cores.replace [:a] - ss.orders << :b - ss.limit = :c - ss.lock = :d - ss.offset = :e - - @visitor.accept ss - assert_equal [ - :a, ss.cores, - :b, ss.orders, - :c, - :d, - :e, - ss], @collector.calls - end - - def test_insert_statement - stmt = Nodes::InsertStatement.new - stmt.relation = :a - stmt.columns << :b - stmt.values = :c - - @visitor.accept stmt - assert_equal [:a, :b, stmt.columns, :c, stmt], @collector.calls - end - - def test_case - node = Arel::Nodes::Case.new - node.case = :a - node.conditions << :b - node.default = :c - - @visitor.accept node - assert_equal [:a, :b, node.conditions, :c, node], @collector.calls - end - - def test_node - node = Nodes::Node.new - @visitor.accept node - assert_equal [node], @collector.calls - end - end - end -end diff --git a/activerecord/test/cases/arel/visitors/dispatch_contamination_test.rb b/activerecord/test/cases/arel/visitors/dispatch_contamination_test.rb index a07a1a050a..36f9eb49a2 100644 --- a/activerecord/test/cases/arel/visitors/dispatch_contamination_test.rb +++ b/activerecord/test/cases/arel/visitors/dispatch_contamination_test.rb @@ -48,7 +48,13 @@ module Arel node = Nodes::Union.new(Nodes::True.new, Nodes::False.new) assert_equal "( TRUE UNION FALSE )", node.to_sql - node.first # from Nodes::Node's Enumerable mixin + visitor = Class.new(Visitor) { + def visit_Arel_Nodes_Union(o); end + alias :visit_Arel_Nodes_True :visit_Arel_Nodes_Union + alias :visit_Arel_Nodes_False :visit_Arel_Nodes_Union + }.new + + visitor.accept(node) assert_equal "( TRUE UNION FALSE )", node.to_sql end -- cgit v1.2.3