Fixes #28359

Add stronger assertions to rake migration tasks to make sure the user is providing a numeric VERSION
An empty string was getting converted to version = 0. This would in turn pass the presence check.

Address linting warning

Add test for rake task and refactor code to meet expectations
In particular passing VERSION=0 should not raise an error.

Addressed Comments for PR #28485. Trimmed empty lines + change of wording for error message

Adjust test for change of wording in error message

Change condition to follow rails idioms

4 files changed, 32 insertions, 3 deletions

diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index 1c7206aca4..f3e2df8786 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -77,6 +77,8 @@ db_namespace = namespace :db do
   namespace :migrate do
     # desc  'Rollbacks the database one migration and re migrate up (options: STEP=x, VERSION=x).'
     task redo: [:environment, :load_config] do
+      raise "Empty VERSION provided" if ENV["VERSION"] && ENV["VERSION"].empty?
+
       if ENV["VERSION"]
         db_namespace["migrate:down"].invoke
         db_namespace["migrate:up"].invoke
@@ -91,16 +93,17 @@ db_namespace = namespace :db do
 
     # desc 'Runs the "up" for a given migration VERSION.'
     task up: [:environment, :load_config] do
+      raise "VERSION is required" if ENV["VERSION"] && ENV["VERSION"].empty?
+
       version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil
-      raise "VERSION is required" unless version
       ActiveRecord::Migrator.run(:up, ActiveRecord::Tasks::DatabaseTasks.migrations_paths, version)
       db_namespace["_dump"].invoke
     end
 
     # desc 'Runs the "down" for a given migration VERSION.'
     task down: [:environment, :load_config] do
+      raise "VERSION is required - To go down one migration, use db:rollback" if ENV["VERSION"] && ENV["VERSION"].empty?
       version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil
-      raise "VERSION is required - To go down one migration, run db:rollback" unless version
       ActiveRecord::Migrator.run(:down, ActiveRecord::Tasks::DatabaseTasks.migrations_paths, version)
       db_namespace["_dump"].invoke
     end
diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb
index 82604a915f..46fa8a70a3 100644
--- a/activerecord/lib/active_record/tasks/database_tasks.rb
+++ b/activerecord/lib/active_record/tasks/database_tasks.rb
@@ -162,9 +162,11 @@ module ActiveRecord
       end
 
       def migrate
+        raise "Empty VERSION provided" if ENV["VERSION"] && ENV["VERSION"].empty?
+
         verbose = ENV["VERBOSE"] ? ENV["VERBOSE"] == "true" : true
         version = ENV["VERSION"] ? ENV["VERSION"].to_i : nil
-        scope   = ENV["SCOPE"]
+        scope = ENV["SCOPE"]
         verbose_was, Migration.verbose = Migration.verbose, verbose
         Migrator.migrate(migrations_paths, version) do |migration|
           scope.blank? || scope == migration.scope
diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb
index 5653fd83fd..baa41a3a47 100644
--- a/activerecord/test/cases/tasks/database_tasks_test.rb
+++ b/activerecord/test/cases/tasks/database_tasks_test.rb
@@ -350,6 +350,14 @@ module ActiveRecord
       ENV["VERBOSE"], ENV["VERSION"] = verbose, version
     end
 
+    def test_migrate_raise_error_on_empty_version
+      version = ENV["VERSION"]
+      ENV["VERSION"] = ""
+      assert_raise(RuntimeError, "Empty VERSION provided") { ActiveRecord::Tasks::DatabaseTasks.migrate }
+    ensure
+      ENV["VERSION"] = version
+    end
+
     def test_migrate_clears_schema_cache_afterward
       ActiveRecord::Base.expects(:clear_cache!)
       ActiveRecord::Tasks::DatabaseTasks.migrate
diff --git a/railties/test/application/rake/migrations_test.rb b/railties/test/application/rake/migrations_test.rb
index 76cb302c62..00f6620188 100644
--- a/railties/test/application/rake/migrations_test.rb
+++ b/railties/test/application/rake/migrations_test.rb
@@ -37,6 +37,22 @@ module ApplicationTests
         end
       end
 
+      test "migration with empty version" do
+        Dir.chdir(app_path) do
+          output = `bin/rails db:migrate VERSION= 2>&1`
+          assert_match(/Empty VERSION provided/, output)
+
+          output = `bin/rails db:migrate:redo VERSION= 2>&1`
+          assert_match(/Empty VERSION provided/, output)
+
+          output = `bin/rails db:migrate:up VERSION= 2>&1`
+          assert_match(/VERSION is required/, output)
+
+          output = `bin/rails db:migrate:down VERSION= 2>&1`
+          assert_match(/VERSION is required - To go down one migration, use db:rollback/, output)
+        end
+      end
+
       test "model and migration generator with change syntax" do
         Dir.chdir(app_path) do
           `bin/rails generate model user username:string password:string;