Skip to content

Commit

Permalink
Extends Rails/ActionOrder to enforce the order of routes arguments
Browse files Browse the repository at this point in the history
Commit 0ef065a introduced the
Rails/ActionOrder cop to enforce the order of RESTful action methods in
controllers.

These actions also appear in routes configuration through the `resource`
and `resources` methods (docs linked below). The cop should enforce the
order for these actions as well.

https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Resources.html#method-i-resources

Refs #540
  • Loading branch information
jdufresne committed Oct 23, 2022
1 parent 1acb097 commit 3e092d3
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 5 deletions.
1 change: 1 addition & 0 deletions changelog/change_action_order_resources.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#x](https://github.com/rubocop/rubocop-rails/pull/x): Extends Rails/ActionOrder to enforce the order of routes `resource` and `resources` arguments ([@jdufresne][])
2 changes: 0 additions & 2 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ Rails/ActionOrder:
- create
- update
- destroy
Include:
- app/controllers/**/*.rb

Rails/ActiveRecordAliases:
Description: >-
Expand Down
47 changes: 44 additions & 3 deletions lib/rubocop/cop/rails/action_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,38 @@ module Rails
# def index; end
# def show; end
# def destroy; end
#
# @example
# # bad
# resources :books, only: [:show, :index]
# resources :books, except: [:update, :edit]
#
# # good
# resources :books, only: [:index, :show]
# resources :books, except: [:edit, :update]
class ActionOrder < Base
extend AutoCorrector
include VisibilityHelp
include DefNode

RESTRICT_ON_SEND = %i[resource resources].freeze

MSG = 'Action `%<current>s` should appear before `%<previous>s`.'

def_node_search :action_declarations, '(def {%1} ...)'

def_node_matcher :resource, <<~PATTERN
(send nil? {:resource :resources} _ $(hash <(pair (sym {:only :except}) (array ...)) ...>))
PATTERN

def_node_search :resource_argument, <<~PATTERN
(pair (sym {:only :except}) $(array ...))
PATTERN

def_node_search :resource_action_value, <<~PATTERN
$(sym ...)
PATTERN

def on_class(node)
action_declarations(node, actions).each_cons(2) do |previous, current|
next if node_visibility(current) != :public || non_public?(current)
Expand All @@ -49,6 +72,16 @@ def on_class(node)
end
end

def on_send(node)
resource(node) do |hash_node|
resource_argument(hash_node) do |array_node|
resource_action_value(array_node).each_cons(2) do |previous, current|
register_offense(previous, current) if find_index(current) < find_index(previous)
end
end
end
end

private

def expected_order
Expand All @@ -59,16 +92,24 @@ def actions
@actions ||= Set.new(expected_order)
end

def action_name(node)
if node.is_a?(RuboCop::AST::DefNode)
node.method_name
else
node.value
end
end

def find_index(node)
expected_order.find_index(node.method_name)
expected_order.find_index(action_name(node))
end

def register_offense(previous, current)
message = format(
MSG,
expected_order: expected_order.join(', '),
previous: previous.method_name,
current: current.method_name
previous: action_name(previous),
current: action_name(current)
)
add_offense(current, message: message) do |corrector|
corrector.replace(current, previous.source)
Expand Down
40 changes: 40 additions & 0 deletions spec/rubocop/cop/rails/action_order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,46 @@ def show; end
RUBY
end

it 'corrects order of resources only argument' do
expect_offense(<<~RUBY)
resources :books, only: [:show, :index]
^^^^^^ Action `index` should appear before `show`.
RUBY

expect_correction(<<~RUBY)
resources :books, only: [:index, :show]
RUBY
end

it 'corrects order of resources except argument' do
expect_offense(<<~RUBY)
resources :books, except: [:update, :edit]
^^^^^ Action `edit` should appear before `update`.
RUBY

expect_correction(<<~RUBY)
resources :books, except: [:edit, :update]
RUBY
end

it 'corrects order of resources both only and except argument' do
expect_offense(<<~RUBY)
resources :books, only: [:show, :index], except: [:update, :edit]
^^^^^ Action `edit` should appear before `update`.
^^^^^^ Action `index` should appear before `show`.
RUBY

expect_correction(<<~RUBY)
resources :books, only: [:index, :show], except: [:edit, :update]
RUBY
end

it 'has no offenses for resources with arguments in stardard oreder' do
expect_no_offenses(<<~RUBY)
resources :books, only: [:index, :show], except: [:edit, :update]
RUBY
end

context 'with custom ordering' do
it 'enforces custom order' do
cop_config['ExpectedOrder'] = %w[show index new edit create update destroy]
Expand Down

0 comments on commit 3e092d3

Please sign in to comment.