Merge pull request #35887 from kamipo/argument_error

Raise `ArgumentError` for invalid `:limit` and `:precision` like as other options

10 files changed, 40 insertions, 18 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 08f44e0b91..3096f7846a 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,25 @@
+*   Raise `ArgumentError` for invalid `:limit` and `:precision` like as other options.
+
+    Before:
+
+    ```ruby
+    add_column :items, :attr1, :binary,   size: 10      # => ArgumentError
+    add_column :items, :attr2, :decimal,  scale: 10     # => ArgumentError
+    add_column :items, :attr3, :integer,  limit: 10     # => ActiveRecordError
+    add_column :items, :attr4, :datetime, precision: 10 # => ActiveRecordError
+    ```
+
+    After:
+
+    ```ruby
+    add_column :items, :attr1, :binary,   size: 10      # => ArgumentError
+    add_column :items, :attr2, :decimal,  scale: 10     # => ArgumentError
+    add_column :items, :attr3, :integer,  limit: 10     # => ArgumentError
+    add_column :items, :attr4, :datetime, precision: 10 # => ArgumentError
+    ```
+
+    *Ryuta Kamizono*
+
 *   Association loading isn't to be affected by scoping consistently
     whether preloaded / eager loaded or not, with the exception of `unscoped`.
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 9f0335b31d..4840307094 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -1097,7 +1097,7 @@ module ActiveRecord
             if (0..6) === precision
               column_type_sql << "(#{precision})"
             else
-              raise(ActiveRecordError, "No #{native[:name]} type has precision of #{precision}. The allowed range of precision is from 0 to 6")
+              raise ArgumentError, "No #{native[:name]} type has precision of #{precision}. The allowed range of precision is from 0 to 6"
             end
           elsif (type != :primary_key) && (limit ||= native.is_a?(Hash) && native[:limit])
             column_type_sql << "(#{limit})"
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
index a24c432893..25a1fb234a 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
@@ -243,7 +243,7 @@ module ActiveRecord
               when nil, 0x100..0xffff;    nil
               when 0x10000..0xffffff;     "medium"
               when 0x1000000..0xffffffff; "long"
-              else raise ActiveRecordError, "No #{type} type has byte size #{limit}"
+              else raise ArgumentError, "No #{type} type has byte size #{limit}"
               end
             end
           end
@@ -255,7 +255,7 @@ module ActiveRecord
             when 3; "mediumint"
             when nil, 4; "int"
             when 5..8; "bigint"
-            else raise ActiveRecordError, "No integer type has byte size #{limit}. Use a decimal with scale 0 instead."
+            else raise ArgumentError, "No integer type has byte size #{limit}. Use a decimal with scale 0 instead."
             end
           end
       end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
index 5f27cb0793..c412d1f34c 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -548,21 +548,21 @@ module ActiveRecord
               # The hard limit is 1GB, because of a 32-bit size field, and TOAST.
               case limit
               when nil, 0..0x3fffffff; super(type)
-              else raise ActiveRecordError, "No binary type has byte size #{limit}. The limit on binary can be at most 1GB - 1byte."
+              else raise ArgumentError, "No binary type has byte size #{limit}. The limit on binary can be at most 1GB - 1byte."
               end
             when "text"
               # PostgreSQL doesn't support limits on text columns.
               # The hard limit is 1GB, according to section 8.3 in the manual.
               case limit
               when nil, 0..0x3fffffff; super(type)
-              else raise ActiveRecordError, "No text type has byte size #{limit}. The limit on text can be at most 1GB - 1byte."
+              else raise ArgumentError, "No text type has byte size #{limit}. The limit on text can be at most 1GB - 1byte."
               end
             when "integer"
               case limit
               when 1, 2; "smallint"
               when nil, 3, 4; "integer"
               when 5..8; "bigint"
-              else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a numeric with scale 0 instead.")
+              else raise ArgumentError, "No integer type has byte size #{limit}. Use a numeric with scale 0 instead."
               end
             else
               super
diff --git a/activerecord/test/cases/adapters/postgresql/bytea_test.rb b/activerecord/test/cases/adapters/postgresql/bytea_test.rb
index 3988c2adca..531e6b2328 100644
--- a/activerecord/test/cases/adapters/postgresql/bytea_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/bytea_test.rb
@@ -35,7 +35,7 @@ class PostgresqlByteaTest < ActiveRecord::PostgreSQLTestCase
 
   def test_binary_columns_are_limitless_the_upper_limit_is_one_GB
     assert_equal "bytea", @connection.type_to_sql(:binary, limit: 100_000)
-    assert_raise ActiveRecord::ActiveRecordError do
+    assert_raise ArgumentError do
       @connection.type_to_sql(:binary, limit: 4294967295)
     end
   end
diff --git a/activerecord/test/cases/adapters/postgresql/datatype_test.rb b/activerecord/test/cases/adapters/postgresql/datatype_test.rb
index b7535d5c9a..562cf1f2d1 100644
--- a/activerecord/test/cases/adapters/postgresql/datatype_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/datatype_test.rb
@@ -64,7 +64,7 @@ class PostgresqlDataTypeTest < ActiveRecord::PostgreSQLTestCase
 
   def test_text_columns_are_limitless_the_upper_limit_is_one_GB
     assert_equal "text", @connection.type_to_sql(:text, limit: 100_000)
-    assert_raise ActiveRecord::ActiveRecordError do
+    assert_raise ArgumentError do
       @connection.type_to_sql(:text, limit: 4294967295)
     end
   end
diff --git a/activerecord/test/cases/date_time_precision_test.rb b/activerecord/test/cases/date_time_precision_test.rb
index 9d1af9362d..79d63949ca 100644
--- a/activerecord/test/cases/date_time_precision_test.rb
+++ b/activerecord/test/cases/date_time_precision_test.rb
@@ -82,7 +82,7 @@ if subsecond_precision_supported?
     end
 
     def test_invalid_datetime_precision_raises_error
-      assert_raises ActiveRecord::ActiveRecordError do
+      assert_raises ArgumentError do
         @connection.create_table(:foos, force: true) do |t|
           t.timestamps precision: 7
         end
diff --git a/activerecord/test/cases/migration/column_attributes_test.rb b/activerecord/test/cases/migration/column_attributes_test.rb
index 6f9190c110..b6064500ee 100644
--- a/activerecord/test/cases/migration/column_attributes_test.rb
+++ b/activerecord/test/cases/migration/column_attributes_test.rb
@@ -176,9 +176,9 @@ module ActiveRecord
 
       if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter)
         def test_out_of_range_limit_should_raise
-          assert_raise(ActiveRecordError) { add_column :test_models, :integer_too_big, :integer, limit: 10 }
-          assert_raise(ActiveRecordError) { add_column :test_models, :text_too_big, :text, limit: 0xfffffffff }
-          assert_raise(ActiveRecordError) { add_column :test_models, :binary_too_big, :binary, limit: 0xfffffffff }
+          assert_raise(ArgumentError) { add_column :test_models, :integer_too_big, :integer, limit: 10 }
+          assert_raise(ArgumentError) { add_column :test_models, :text_too_big, :text, limit: 0xfffffffff }
+          assert_raise(ArgumentError) { add_column :test_models, :binary_too_big, :binary, limit: 0xfffffffff }
         end
       end
     end
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index f10c26c308..8e8ed494d9 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -583,7 +583,7 @@ class MigrationTest < ActiveRecord::TestCase
 
   if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter)
     def test_out_of_range_integer_limit_should_raise
-      e = assert_raise(ActiveRecord::ActiveRecordError, "integer limit didn't raise") do
+      e = assert_raise(ArgumentError) do
         Person.connection.create_table :test_integer_limits, force: true do |t|
           t.column :bigone, :integer, limit: 10
         end
@@ -595,7 +595,7 @@ class MigrationTest < ActiveRecord::TestCase
     end
 
     def test_out_of_range_text_limit_should_raise
-      e = assert_raise(ActiveRecord::ActiveRecordError, "text limit didn't raise") do
+      e = assert_raise(ArgumentError) do
         Person.connection.create_table :test_text_limits, force: true do |t|
           t.text :bigtext, limit: 0xfffffffff
         end
@@ -607,15 +607,15 @@ class MigrationTest < ActiveRecord::TestCase
     end
 
     def test_out_of_range_binary_limit_should_raise
-      e = assert_raise(ActiveRecord::ActiveRecordError) do
-        Person.connection.create_table :test_text_limits, force: true do |t|
+      e = assert_raise(ArgumentError) do
+        Person.connection.create_table :test_binary_limits, force: true do |t|
           t.binary :bigbinary, limit: 0xfffffffff
         end
       end
 
       assert_includes e.message, "No binary type has byte size #{0xfffffffff}"
     ensure
-      Person.connection.drop_table :test_text_limits, if_exists: true
+      Person.connection.drop_table :test_binary_limits, if_exists: true
     end
   end
 
diff --git a/activerecord/test/cases/time_precision_test.rb b/activerecord/test/cases/time_precision_test.rb
index 2f534ea110..1abd857216 100644
--- a/activerecord/test/cases/time_precision_test.rb
+++ b/activerecord/test/cases/time_precision_test.rb
@@ -75,7 +75,7 @@ if subsecond_precision_supported?
     end
 
     def test_invalid_time_precision_raises_error
-      assert_raises ActiveRecord::ActiveRecordError do
+      assert_raises ArgumentError do
         @connection.create_table(:foos, force: true) do |t|
           t.time :start,  precision: 7
           t.time :finish, precision: 7