Deprecate passing ranges to `#in` and `#not_in`

The goal of these methods should be to generate in nodes, not handle every possible permutation of more than one value. The `#between` and `#not_between` methods have been extracted, which better represent the semantics of handling ranges in SQL.
author: Sean Griffin <sean@thoughtbot.com> 2014-10-25 07:38:56 -0500
committer: Sean Griffin <sean@thoughtbot.com> 2014-10-25 07:38:56 -0500
commit: f8d85cf24bca522a04edc5cc48f8716e65efb107 (patch)
tree: 400a9d564be166d15cb6566dd78cd251f8aeaef5
parent: df5723dfbe856abc8c91cc87c609156b432e97d3 (diff)
download: rails-f8d85cf24bca522a04edc5cc48f8716e65efb107.tar.gz
rails-f8d85cf24bca522a04edc5cc48f8716e65efb107.tar.bz2
rails-f8d85cf24bca522a04edc5cc48f8716e65efb107.zip
4 files changed, 192 insertions, 180 deletions
diff --git a/lib/arel/predications.rb b/lib/arel/predications.rb
index 4079b4830e..a0b6728943 100644
--- a/lib/arel/predications.rb
+++ b/lib/arel/predications.rb
@@ -25,9 +25,23 @@ module Arel
     end
 
     def between other
-      left = Nodes.build_quoted(other.begin, self)
-      right = Nodes.build_quoted(other.end, self)
-      Nodes::Between.new(self, left.and(right))
+      if other.begin == -Float::INFINITY
+        if other.end == Float::INFINITY
+          not_in([])
+        elsif other.exclude_end?
+          lt(other.end)
+        else
+          lteq(other.end)
+        end
+      elsif other.end == Float::INFINITY
+        gteq(other.begin)
+      elsif other.exclude_end?
+        gteq(other.begin).and(lt(other.end))
+      else
+        left = Nodes.build_quoted(other.begin, self)
+        right = Nodes.build_quoted(other.end, self)
+        Nodes::Between.new(self, left.and(right))
+      end
     end
 
     def in other
@@ -35,21 +49,12 @@ module Arel
       when Arel::SelectManager
         Arel::Nodes::In.new(self, other.ast)
       when Range
-        if other.begin == -Float::INFINITY
-          if other.end == Float::INFINITY
-            not_in([])
-          elsif other.exclude_end?
-            lt(other.end)
-          else
-            lteq(other.end)
-          end
-        elsif other.end == Float::INFINITY
-          gteq(other.begin)
-        elsif other.exclude_end?
-          gteq(other.begin).and(lt(other.end))
-        else
-          between(other)
+        if $VERBOSE
+          warn <<-eowarn
+Passing a range to `#in` is deprecated. Call `#between`, instead.
+          eowarn
         end
+        between(other)
       when Array
         Nodes::In.new self, other.map { |x| Nodes.build_quoted(x, self) }
       else
@@ -65,30 +70,39 @@ module Arel
       grouping_all :in, others
     end
 
+    def not_between other
+      if other.begin == -Float::INFINITY # The range begins with negative infinity
+        if other.end == Float::INFINITY
+          self.in([])
+        elsif other.exclude_end?
+          gteq(other.end)
+        else
+          gt(other.end)
+        end
+      elsif other.end == Float::INFINITY
+        lt(other.begin)
+      else
+        left = lt(other.begin)
+        right = if other.exclude_end?
+          gteq(other.end)
+        else
+          gt(other.end)
+        end
+        left.or(right)
+      end
+    end
+
     def not_in other
       case other
       when Arel::SelectManager
         Arel::Nodes::NotIn.new(self, other.ast)
       when Range
-        if other.begin == -Float::INFINITY # The range begins with negative infinity
-          if other.end == Float::INFINITY
-            self.in([])
-          elsif other.exclude_end?
-            gteq(other.end)
-          else
-            gt(other.end)
-          end
-        elsif other.end == Float::INFINITY
-          lt(other.begin)
-        else
-          left = lt(other.begin)
-          right = if other.exclude_end?
-            gteq(other.end)
-          else
-            gt(other.end)
-          end
-          left.or(right)
+        if $VERBOSE
+          warn <<-eowarn
+Passing a range to `#not_in` is deprecated. Call `#not_between`, instead.
+          eowarn
         end
+        not_between(other)
       when Array
         Nodes::NotIn.new self, other.map { |x| Nodes.build_quoted(x, self) }
       else
diff --git a/test/attributes/test_attribute.rb b/test/attributes/test_attribute.rb
index 0851a07258..500f9385f8 100644
--- a/test/attributes/test_attribute.rb
+++ b/test/attributes/test_attribute.rb
@@ -548,84 +548,84 @@ module Arel
         end
       end
 
-      describe '#in' do
-        it 'can be constructed with a subquery' do
-          relation = Table.new(:users)
-          mgr = relation.project relation[:id]
-          mgr.where relation[:name].does_not_match_all(['%chunky%','%bacon%'])
+      describe 'with a range' do
+        it 'can be constructed with a standard range' do
           attribute = Attribute.new nil, nil
+          node = attribute.between(1..3)
 
-          node = attribute.in(mgr)
-
-          node.must_equal Nodes::In.new(attribute, mgr.ast)
+          node.must_equal Nodes::Between.new(
+            attribute,
+            Nodes::And.new([
+              Nodes::Casted.new(1, attribute),
+              Nodes::Casted.new(3, attribute)
+            ])
+          )
         end
 
-        describe 'with a range' do
-          it 'can be constructed with a standard range' do
-            attribute = Attribute.new nil, nil
-            node = attribute.in(1..3)
+        it 'can be constructed with a range starting from -Infinity' do
+          attribute = Attribute.new nil, nil
+          node = attribute.between(-::Float::INFINITY..3)
 
-            node.must_equal Nodes::Between.new(
-              attribute,
-              Nodes::And.new([
-                Nodes::Casted.new(1, attribute),
-                Nodes::Casted.new(3, attribute)
-              ])
-            )
-          end
+          node.must_equal Nodes::LessThanOrEqual.new(
+            attribute,
+            Nodes::Casted.new(3, attribute)
+          )
+        end
 
-          it 'can be constructed with a range starting from -Infinity' do
-            attribute = Attribute.new nil, nil
-            node = attribute.in(-::Float::INFINITY..3)
+        it 'can be constructed with an exclusive range starting from -Infinity' do
+          attribute = Attribute.new nil, nil
+          node = attribute.between(-::Float::INFINITY...3)
 
-            node.must_equal Nodes::LessThanOrEqual.new(
-              attribute,
-              Nodes::Casted.new(3, attribute)
-            )
-          end
+          node.must_equal Nodes::LessThan.new(
+            attribute,
+            Nodes::Casted.new(3, attribute)
+          )
+        end
 
-          it 'can be constructed with an exclusive range starting from -Infinity' do
-            attribute = Attribute.new nil, nil
-            node = attribute.in(-::Float::INFINITY...3)
+        it 'can be constructed with an infinite range' do
+          attribute = Attribute.new nil, nil
+          node = attribute.between(-::Float::INFINITY..::Float::INFINITY)
 
-            node.must_equal Nodes::LessThan.new(
-              attribute,
-              Nodes::Casted.new(3, attribute)
-            )
-          end
+          node.must_equal Nodes::NotIn.new(attribute, [])
+        end
 
-          it 'can be constructed with an infinite range' do
-            attribute = Attribute.new nil, nil
-            node = attribute.in(-::Float::INFINITY..::Float::INFINITY)
+        it 'can be constructed with a range ending at Infinity' do
+          attribute = Attribute.new nil, nil
+          node = attribute.between(0..::Float::INFINITY)
 
-            node.must_equal Nodes::NotIn.new(attribute, [])
-          end
+          node.must_equal Nodes::GreaterThanOrEqual.new(
+            attribute,
+            Nodes::Casted.new(0, attribute)
+          )
+        end
 
-          it 'can be constructed with a range ending at Infinity' do
-            attribute = Attribute.new nil, nil
-            node = attribute.in(0..::Float::INFINITY)
+        it 'can be constructed with an exclusive range' do
+          attribute = Attribute.new nil, nil
+          node = attribute.between(0...3)
 
-            node.must_equal Nodes::GreaterThanOrEqual.new(
+          node.must_equal Nodes::And.new([
+            Nodes::GreaterThanOrEqual.new(
               attribute,
               Nodes::Casted.new(0, attribute)
+            ),
+            Nodes::LessThan.new(
+              attribute,
+              Nodes::Casted.new(3, attribute)
             )
-          end
-
-          it 'can be constructed with an exclusive range' do
-            attribute = Attribute.new nil, nil
-            node = attribute.in(0...3)
-
-            node.must_equal Nodes::And.new([
-              Nodes::GreaterThanOrEqual.new(
-                attribute,
-                Nodes::Casted.new(0, attribute)
-              ),
-              Nodes::LessThan.new(
-                attribute,
-                Nodes::Casted.new(3, attribute)
-              )
-            ])
-          end
+          ])
+        end
+      end
+
+      describe '#in' do
+        it 'can be constructed with a subquery' do
+          relation = Table.new(:users)
+          mgr = relation.project relation[:id]
+          mgr.where relation[:name].does_not_match_all(['%chunky%','%bacon%'])
+          attribute = Attribute.new nil, nil
+
+          node = attribute.in(mgr)
+
+          node.must_equal Nodes::In.new(attribute, mgr.ast)
         end
 
         it 'can be constructed with a list' do
@@ -695,87 +695,87 @@ module Arel
         end
       end
 
-      describe '#not_in' do
-        it 'can be constructed with a subquery' do
-          relation = Table.new(:users)
-          mgr = relation.project relation[:id]
-          mgr.where relation[:name].does_not_match_all(['%chunky%','%bacon%'])
+      describe 'with a range' do
+        it 'can be constructed with a standard range' do
           attribute = Attribute.new nil, nil
+          node = attribute.not_between(1..3)
 
-          node = attribute.not_in(mgr)
-
-          node.must_equal Nodes::NotIn.new(attribute, mgr.ast)
-        end
-
-        describe 'with a range' do
-          it 'can be constructed with a standard range' do
-            attribute = Attribute.new nil, nil
-            node = attribute.not_in(1..3)
-
-            node.must_equal Nodes::Grouping.new(Nodes::Or.new(
-              Nodes::LessThan.new(
-                attribute,
-                Nodes::Casted.new(1, attribute)
-              ),
-              Nodes::GreaterThan.new(
-                attribute,
-                Nodes::Casted.new(3, attribute)
-              )
-            ))
-          end
-
-          it 'can be constructed with a range starting from -Infinity' do
-            attribute = Attribute.new nil, nil
-            node = attribute.not_in(-::Float::INFINITY..3)
-
-            node.must_equal Nodes::GreaterThan.new(
+          node.must_equal Nodes::Grouping.new(Nodes::Or.new(
+            Nodes::LessThan.new(
+              attribute,
+              Nodes::Casted.new(1, attribute)
+            ),
+            Nodes::GreaterThan.new(
               attribute,
               Nodes::Casted.new(3, attribute)
             )
-          end
+          ))
+        end
 
-          it 'can be constructed with an exclusive range starting from -Infinity' do
-            attribute = Attribute.new nil, nil
-            node = attribute.not_in(-::Float::INFINITY...3)
+        it 'can be constructed with a range starting from -Infinity' do
+          attribute = Attribute.new nil, nil
+          node = attribute.not_between(-::Float::INFINITY..3)
 
-            node.must_equal Nodes::GreaterThanOrEqual.new(
-              attribute,
-              Nodes::Casted.new(3, attribute)
-            )
-          end
+          node.must_equal Nodes::GreaterThan.new(
+            attribute,
+            Nodes::Casted.new(3, attribute)
+          )
+        end
+
+        it 'can be constructed with an exclusive range starting from -Infinity' do
+          attribute = Attribute.new nil, nil
+          node = attribute.not_between(-::Float::INFINITY...3)
+
+          node.must_equal Nodes::GreaterThanOrEqual.new(
+            attribute,
+            Nodes::Casted.new(3, attribute)
+          )
+        end
 
-          it 'can be constructed with an infinite range' do
-            attribute = Attribute.new nil, nil
-            node = attribute.not_in(-::Float::INFINITY..::Float::INFINITY)
+        it 'can be constructed with an infinite range' do
+          attribute = Attribute.new nil, nil
+          node = attribute.not_between(-::Float::INFINITY..::Float::INFINITY)
 
-            node.must_equal Nodes::In.new(attribute, [])
-          end
+          node.must_equal Nodes::In.new(attribute, [])
+        end
 
-          it 'can be constructed with a range ending at Infinity' do
-            attribute = Attribute.new nil, nil
-            node = attribute.not_in(0..::Float::INFINITY)
+        it 'can be constructed with a range ending at Infinity' do
+          attribute = Attribute.new nil, nil
+          node = attribute.not_between(0..::Float::INFINITY)
 
-            node.must_equal Nodes::LessThan.new(
+          node.must_equal Nodes::LessThan.new(
+            attribute,
+            Nodes::Casted.new(0, attribute)
+          )
+        end
+
+        it 'can be constructed with an exclusive range' do
+          attribute = Attribute.new nil, nil
+          node = attribute.not_between(0...3)
+
+          node.must_equal Nodes::Grouping.new(Nodes::Or.new(
+            Nodes::LessThan.new(
               attribute,
               Nodes::Casted.new(0, attribute)
+            ),
+            Nodes::GreaterThanOrEqual.new(
+              attribute,
+              Nodes::Casted.new(3, attribute)
             )
-          end
-
-          it 'can be constructed with an exclusive range' do
-            attribute = Attribute.new nil, nil
-            node = attribute.not_in(0...3)
-
-            node.must_equal Nodes::Grouping.new(Nodes::Or.new(
-              Nodes::LessThan.new(
-                attribute,
-                Nodes::Casted.new(0, attribute)
-              ),
-              Nodes::GreaterThanOrEqual.new(
-                attribute,
-                Nodes::Casted.new(3, attribute)
-              )
-            ))
-          end
+          ))
+        end
+      end
+
+      describe '#not_in' do
+        it 'can be constructed with a subquery' do
+          relation = Table.new(:users)
+          mgr = relation.project relation[:id]
+          mgr.where relation[:name].does_not_match_all(['%chunky%','%bacon%'])
+          attribute = Attribute.new nil, nil
+
+          node = attribute.not_in(mgr)
+
+          node.must_equal Nodes::NotIn.new(attribute, mgr.ast)
         end
 
         it 'can be constructed with a list' do
diff --git a/test/test_select_manager.rb b/test/test_select_manager.rb
index 1ffb56fd9f..78b3a25928 100644
--- a/test/test_select_manager.rb
+++ b/test/test_select_manager.rb
@@ -307,13 +307,11 @@ module Arel
         table = Table.new :users
         @m1 = Arel::SelectManager.new Table.engine, table
         @m1.project Arel.star
-        @m1.where(table[:age].in(18..60))
+        @m1.where(table[:age].between(18..60))
 
         @m2 = Arel::SelectManager.new Table.engine, table
         @m2.project Arel.star
-        @m2.where(table[:age].in(40..99))
-
-
+        @m2.where(table[:age].between(40..99))
       end
 
       it 'should except two managers' do
diff --git a/test/visitors/test_to_sql.rb b/test/visitors/test_to_sql.rb
index 62e1c57c73..9c18d74827 100644
--- a/test/visitors/test_to_sql.rb
+++ b/test/visitors/test_to_sql.rb
@@ -375,33 +375,33 @@ module Arel
         end
 
         it 'can handle two dot ranges' do
-          node = @attr.in 1..3
+          node = @attr.between 1..3
           compile(node).must_be_like %{
             "users"."id" BETWEEN 1 AND 3
           }
         end
 
         it 'can handle three dot ranges' do
-          node = @attr.in 1...3
+          node = @attr.between 1...3
           compile(node).must_be_like %{
             "users"."id" >= 1 AND "users"."id" < 3
           }
         end
 
         it 'can handle ranges bounded by infinity' do
-          node = @attr.in 1..Float::INFINITY
+          node = @attr.between 1..Float::INFINITY
           compile(node).must_be_like %{
             "users"."id" >= 1
           }
-          node = @attr.in(-Float::INFINITY..3)
+          node = @attr.between(-Float::INFINITY..3)
           compile(node).must_be_like %{
             "users"."id" <= 3
           }
-          node = @attr.in(-Float::INFINITY...3)
+          node = @attr.between(-Float::INFINITY...3)
           compile(node).must_be_like %{
             "users"."id" < 3
           }
-          node = @attr.in(-Float::INFINITY..Float::INFINITY)
+          node = @attr.between(-Float::INFINITY..Float::INFINITY)
           compile(node).must_be_like %{1=1}
         end
 
@@ -479,33 +479,33 @@ module Arel
         end
 
         it 'can handle two dot ranges' do
-          node = @attr.not_in 1..3
+          node = @attr.not_between 1..3
           compile(node).must_equal(
             %{("users"."id" < 1 OR "users"."id" > 3)}
           )
         end
 
         it 'can handle three dot ranges' do
-          node = @attr.not_in 1...3
+          node = @attr.not_between 1...3
           compile(node).must_equal(
             %{("users"."id" < 1 OR "users"."id" >= 3)}
           )
         end
 
         it 'can handle ranges bounded by infinity' do
-          node = @attr.not_in 1..Float::INFINITY
+          node = @attr.not_between 1..Float::INFINITY
           compile(node).must_be_like %{
             "users"."id" < 1
           }
-          node = @attr.not_in(-Float::INFINITY..3)
+          node = @attr.not_between(-Float::INFINITY..3)
           compile(node).must_be_like %{
             "users"."id" > 3
           }
-          node = @attr.not_in(-Float::INFINITY...3)
+          node = @attr.not_between(-Float::INFINITY...3)
           compile(node).must_be_like %{
             "users"."id" >= 3
           }
-          node = @attr.not_in(-Float::INFINITY..Float::INFINITY)
+          node = @attr.not_between(-Float::INFINITY..Float::INFINITY)
           compile(node).must_be_like %{1=0}
         end
author	Sean Griffin <sean@thoughtbot.com>	2014-10-25 07:38:56 -0500
committer	Sean Griffin <sean@thoughtbot.com>	2014-10-25 07:38:56 -0500
commit	f8d85cf24bca522a04edc5cc48f8716e65efb107 (patch)
tree	400a9d564be166d15cb6566dd78cd251f8aeaef5
parent	df5723dfbe856abc8c91cc87c609156b432e97d3 (diff)
download	rails-f8d85cf24bca522a04edc5cc48f8716e65efb107.tar.gz rails-f8d85cf24bca522a04edc5cc48f8716e65efb107.tar.bz2 rails-f8d85cf24bca522a04edc5cc48f8716e65efb107.zip