Merge pull request #38 from appfolio/APT-AddCopForFilterAccessTo

Add a rubocop for filter_access_to placement

Gemfile

@@ -11,6 +11,7 @@ source 'https://rubygems.org' do
   gem 'minitest-reporters', '>= 1.6', '< 2'
   gem 'mocha', '>= 2.1', '< 3'
   gem 'rake', '>= 13', '< 14'
+  gem 'rubocop', '~> 1.70', groups: %i[development test]
   gem 'simplecov', '>= 0.22', '< 1', group: :test, require: false
   gem 'sprockets', '>= 3.4', '< 4'
   gem 'sqlite3', '>= 1.4', '< 2'

lib/rubocop/cop/decl_auth/before_actions_precede_access_filter.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module DeclAuth
+      # Enforces placing all `before_action` statements prior to the first `filter_access_to` statement.
+      # This ensures that any data required by the access filters is available before the filter is applied.
+      # See the documentation above the `filter_access_to` method in Authorization::Controller::DSL for more information
+      #
+      # @example
+      #   # bad
+      #   before_action: :do_something
+      #   filter_access_to :all
+      #   before_action :find_object
+      #
+      #   # good
+      #   before_action: :do_something
+      #   before_action :find_object
+      #
+      #   filter_access_to :all
+      #
+      class BeforeActionsPrecedeAccessFilter < RuboCop::Cop::Base
+        def_node_search :before_actions, '(send nil? :before_action ...)'
+        def_node_search :access_filters, '(send nil? :filter_access_to ...)'
+
+        MSG = '`:filter_access_to` statements should be placed after all other `:before_action` statements.'
+
+        def on_class(node)
+          before_actions = before_actions(node)
+          access_filters = access_filters(node)
+
+          return if before_actions.count.zero? || access_filters.count.zero?
+
+          last_before_action = before_actions.to_a.last
+          first_access_filter = access_filters.to_a.first
+
+          return if last_before_action.sibling_index < first_access_filter.sibling_index
+
+          add_offense(access_filters.first, message: MSG)
+        end
+      end
+    end
+  end
+end

rubocop-decl-auth.yml

@@ -0,0 +1,7 @@
+require:
+  - ./lib/rubocop/cop/decl_auth/before_actions_precede_access_filter
+
+DeclAuth/BeforeActionsPrecedeAccessFilter:
+  Enabled: true
+  Include:
+      - '**/controllers/**/*controller.rb'

test/rubocop/cop/decl_auth/before_actions_precede_access_filter_test.rb

@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+require 'rubocop'
+require 'rubocop/cop/decl_auth/before_actions_precede_access_filter'
+
+# rubocop:disable Metrics/ MethodLength
+module Rubocop
+  module Cop
+    module DeclAuth
+      class BeforeActionsPrecedeAccessFilterTest < Minitest::Test
+        def setup
+          config = RuboCop::Config.new(
+            { RuboCop::Cop::DeclAuth::BeforeActionsPrecedeAccessFilter.badge.to_s => {} },
+            '/'
+          )
+          @cop = RuboCop::Cop::DeclAuth::BeforeActionsPrecedeAccessFilter.new(config)
+          @commissioner = RuboCop::Cop::Commissioner.new([@cop], [])
+        end
+
+        def test_offense_when_access_filter_precedes_any_before_action
+          source = <<-SOURCE
+            module TestModule
+              class TestController
+                before_action :asdf
+                filter_access_to :all
+                before_action :qwerty
+              end
+            end
+          SOURCE
+
+          offenses = analyze_source(source)
+
+          assert_equal 1, offenses.size
+          assert_equal 'filter_access_to :all', offenses.first.location.source
+        end
+
+        def test_no_offense_when_all_before_actions_precede_access_filter
+          source = <<-SOURCE
+            module TestModule
+              class TestController
+                before_action :asdf
+                before_action :qwerty
+                filter_access_to :all
+              end
+            end
+          SOURCE
+
+          offenses = analyze_source(source)
+
+          assert_equal 0, offenses.size
+        end
+
+        def test_no_offense_when_no_before_actions
+          source = <<-SOURCE
+            module TestModule
+              class TestController
+                filter_access_to :all
+              end
+            end
+          SOURCE
+
+          offenses = analyze_source(source)
+
+          assert_equal 0, offenses.size
+        end
+
+        def test_no_offense_when_no_access_filters
+          source = <<-SOURCE
+            module TestModule
+              class TestController
+                before_action :asdf
+                before_action :qwerty
+              end
+            end
+          SOURCE
+
+          offenses = analyze_source(source)
+
+          assert_equal 0, offenses.size
+        end
+
+        private
+
+        def analyze_source(source)
+          @commissioner.investigate(@cop.parse(source)).offenses
+        end
+
+      end
+    end
+  end
+end
+

test/test_helper.rb

@@ -9,6 +9,7 @@
 end
 
 require 'debug'
+require 'logger'
 require 'minitest/autorun'
 require 'minitest/reporters'
 require 'mocha/minitest'