add new Rails/ImplementedMigration cop

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#457](https://github.com/rubocop/rubocop-rails/pull/457): New cop `Rails/ImplementedMigration` checks for migrations that have a typo in a method name or are half implemented. ([@leonp1991][])
 * [#446](https://github.com/rubocop/rubocop-rails/issues/446): Add new `Rails/RequireDependency` cop. ([@tubaxenor][])
 * [#458](https://github.com/rubocop/rubocop-rails/issues/458): Add new `Rails/TimeZoneAssignment` cop. ([@olivierbuffon][])
 
@@ -353,3 +354,4 @@
 [@ohbarye]: https://github.com/ohbarye
 [@tubaxenor]: https://github.com/tubaxenor
 [@olivierbuffon]: https://github.com/olivierbuffon
+[@leonp1991]: https://github.com/leonp1991

config/default.yml

@@ -362,6 +362,11 @@ Rails/IgnoredSkipActionFilterOption:
   Include:
     - app/controllers/**/*.rb
 
+Rails/ImplementedMigration:
+  Description: 'Checks for migrations that have a typo in a method name or are half implemented.'
+  Enabled: false
+  VersionAdded: '2.10'
+
 Rails/IndexBy:
   Description: 'Prefer `index_by` over `each_with_object`, `to_h`, or `map`.'
   Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -53,6 +53,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
 * xref:cops_rails.adoc#railshttppositionalarguments[Rails/HttpPositionalArguments]
 * xref:cops_rails.adoc#railshttpstatus[Rails/HttpStatus]
 * xref:cops_rails.adoc#railsignoredskipactionfilteroption[Rails/IgnoredSkipActionFilterOption]
+* xref:cops_rails.adoc#railsimplementedmigration[Rails/ImplementedMigration]
 * xref:cops_rails.adoc#railsindexby[Rails/IndexBy]
 * xref:cops_rails.adoc#railsindexwith[Rails/IndexWith]
 * xref:cops_rails.adoc#railsinquiry[Rails/Inquiry]

docs/modules/ROOT/pages/cops_rails.adoc

@@ -1825,6 +1825,68 @@ end
 
 * https://api.rubyonrails.org/classes/AbstractController/Callbacks/ClassMethods.html#method-i-_normalize_callback_options
 
+== Rails/ImplementedMigration
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Disabled
+| Yes
+| No
+| 2.10
+| -
+|===
+
+This cop checks whether the migration implements
+either a `change` method or both an `up` and a `down`
+method.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+class SomeMigration < ActiveRecord::Migration[6.0]
+  def chance # <----- typo
+    # migration
+  end
+end
+
+class SomeMigration < ActiveRecord::Migration[6.0]
+  def up
+    # up migration
+  end
+
+  # <----- missing down method
+end
+
+class SomeMigration < ActiveRecord::Migration[6.0]
+  # <----- missing up method
+
+  def down
+    # down migration
+  end
+end
+
+# good
+class SomeMigration < ActiveRecord::Migration[6.0]
+  def change
+    # migration
+  end
+end
+
+# good
+class SomeMigration < ActiveRecord::Migration[6.0]
+  def up
+    # up migration
+  end
+
+  def down
+    # down migration
+  end
+end
+----
+
 == Rails/IndexBy
 
 |===

lib/rubocop/cop/rails/implemented_migration.rb

@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # This cop checks whether the migration implements
+      # either a `change` method or both an `up` and a `down`
+      # method.
+      #
+      # @example
+      #   # bad
+      #   class SomeMigration < ActiveRecord::Migration[6.0]
+      #     def chance # <----- typo
+      #       # migration
+      #     end
+      #   end
+      #
+      #   class SomeMigration < ActiveRecord::Migration[6.0]
+      #     def up
+      #       # up migration
+      #     end
+      #
+      #     # <----- missing down method
+      #   end
+      #
+      #   class SomeMigration < ActiveRecord::Migration[6.0]
+      #     # <----- missing up method
+      #
+      #     def down
+      #       # down migration
+      #     end
+      #   end
+      #
+      #   # good
+      #   class SomeMigration < ActiveRecord::Migration[6.0]
+      #     def change
+      #       # migration
+      #     end
+      #   end
+      #
+      #   # good
+      #   class SomeMigration < ActiveRecord::Migration[6.0]
+      #     def up
+      #       # up migration
+      #     end
+      #
+      #     def down
+      #       # down migration
+      #     end
+      #   end
+      class ImplementedMigration < Base
+        MSG = 'Migrations must contain either a change method, or ' \
+              'both an up and a down method.'
+
+        def_node_matcher :migration_class?, <<~PATTERN
+          (class
+            (const nil? _)
+            (send
+              (const (const nil? :ActiveRecord) :Migration)
+              :[]
+              (float _))
+            _)
+        PATTERN
+
+        def_node_matcher :change_method?, <<~PATTERN
+          [ #migration_class? `(def :change (args) _) ]
+        PATTERN
+
+        def_node_matcher :up_and_down_methods?, <<~PATTERN
+          [ #migration_class? `(def :up (args) _) `(def :down (args) _) ]
+        PATTERN
+
+        def on_class(node)
+          return if change_method?(node)
+          return if up_and_down_methods?(node)
+
+          add_offense(node)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -42,6 +42,7 @@
 require_relative 'rails/http_positional_arguments'
 require_relative 'rails/http_status'
 require_relative 'rails/ignored_skip_action_filter_option'
+require_relative 'rails/implemented_migration'
 require_relative 'rails/index_by'
 require_relative 'rails/index_with'
 require_relative 'rails/inquiry'

spec/rubocop/cop/rails/implemented_migration_spec.rb

@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::ImplementedMigration, :config do
+  it 'does not register an offense with a change method' do
+    expect_no_offenses(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[6.0]
+        def change
+          add_column :users, :email, :text, null: false
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense with only an up method' do
+    expect_offense(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[6.0]
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a change method, or both an up and a down method.
+
+        def up
+          add_column :users, :email, :text, null: false
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense with only a down method' do
+    expect_offense(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[6.0]
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a change method, or both an up and a down method.
+
+        def down
+          remove_column :users, :email
+        end
+      end
+    RUBY
+  end
+
+  it 'does not register an offense with an up and a down method' do
+    expect_no_offenses(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[6.0]
+        def up
+          add_column :users, :email, :text, null: false
+        end
+
+        def down
+          remove_column :users, :email
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense with a typo\'d change method' do
+    expect_offense(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[6.0]
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a change method, or both an up and a down method.
+
+        def chance
+          add_column :users, :email, :text, null: false
+        end
+      end
+    RUBY
+  end
+
+  it 'does not register an offense with helper methods' do
+    expect_no_offenses(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[6.0]
+        def change
+          add_users_column :email, :text, null: false
+        end
+
+        private
+
+        def add_users_column(column_name, null: false)
+          add_column :users, column_name, type, null: null
+        end
+      end
+    RUBY
+  end
+
+  it 'registers offenses correctly with any migration class' do
+    expect_offense(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[5.1]
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a change method, or both an up and a down method.
+
+        def chance
+          add_column :users, :email, :text, null: false
+        end
+      end
+    RUBY
+
+    expect_offense(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[7.1]
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a change method, or both an up and a down method.
+
+        def chance
+          add_column :users, :email, :text, null: false
+        end
+      end
+    RUBY
+  end
+
+  it 'does not register offenses correctly with any migration class' do
+    expect_no_offenses(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[5.2]
+        def change
+          add_column :users, :email, :text, null: false
+        end
+      end
+    RUBY
+
+    expect_no_offenses(<<~RUBY)
+      class SomeMigration < ActiveRecord::Migration[7.1]
+        def change
+          add_column :users, :email, :text, null: false
+        end
+      end
+    RUBY
+  end
+end