add new Rails/ImplementedMigration cop

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#457](https://github.com/rubocop/rubocop-rails/pull/457): Add new `Rails/ReversibleMigrationMethodDefinition` cop. ([@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

@@ -620,6 +620,13 @@ Rails/ReversibleMigration:
  Include:
  - db/migrate/*.rb
 
+Rails/ReversibleMigrationMethodDefinition:
+ Description: 'Checks whether the migration implements either a `change` method or both an `up` and a `down` method.'
+ Enabled: false
+ VersionAdded: '2.10'
+ include:
+ - db/migrate/*.rb
+
 Rails/SafeNavigation:
  Description: "Use Ruby's safe navigation operator (`&.`) instead of `try!`."
  Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -86,6 +86,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
 * xref:cops_rails.adoc#railsrequestreferer[Rails/RequestReferer]
 * xref:cops_rails.adoc#railsrequiredependency[Rails/RequireDependency]
 * xref:cops_rails.adoc#railsreversiblemigration[Rails/ReversibleMigration]
+* xref:cops_rails.adoc#railsreversiblemigrationmethoddefinition[Rails/ReversibleMigrationMethodDefinition]
 * xref:cops_rails.adoc#railssafenavigation[Rails/SafeNavigation]
 * xref:cops_rails.adoc#railssafenavigationwithblank[Rails/SafeNavigationWithBlank]
 * xref:cops_rails.adoc#railssavebang[Rails/SaveBang]

docs/modules/ROOT/pages/cops_rails.adoc

@@ -3694,6 +3694,72 @@ end
 * https://rails.rubystyle.guide#reversible-migration
 * https://api.rubyonrails.org/classes/ActiveRecord/Migration/CommandRecorder.html
 
+== Rails/ReversibleMigrationMethodDefinition
+
+|===
+| 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 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
+ # reversible migration
+ end
+end
+
+# good
+class SomeMigration < ActiveRecord::Migration[6.0]
+ def up
+ # up migration
+ end
+
+ def down
+ # down migration
+ end
+end
+----
+
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| include
+| `db/migrate/*.rb`
+| Array
+|===
+
 == Rails/SafeNavigation
 
 |===

lib/rubocop/cop/rails/reversible_migration_method_definition.rb

@@ -0,0 +1,75 @@
+# 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 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
+ # # reversible migration
+ # end
+ # end
+ #
+ # # good
+ # class SomeMigration < ActiveRecord::Migration[6.0]
+ # def up
+ # # up migration
+ # end
+ #
+ # def down
+ # # down migration
+ # end
+ # end
+ class ReversibleMigrationMethodDefinition < 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) || up_and_down_methods?(node)
+
+ add_offense(node)
+ end
+ end
+ end
+ end
+end

lib/rubocop/cop/rails_cops.rb

@@ -75,6 +75,7 @@
 require_relative 'rails/request_referer'
 require_relative 'rails/require_dependency'
 require_relative 'rails/reversible_migration'
+require_relative 'rails/reversible_migration_method_definition'
 require_relative 'rails/safe_navigation'
 require_relative 'rails/safe_navigation_with_blank'
 require_relative 'rails/save_bang'

spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb

@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::ReversibleMigrationMethodDefinition, :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.2]
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
+ end
+end