Merge pull request #347 from elabs/rubocop

Add Rubocop.

.rubocop.yml

@@ -0,0 +1,95 @@
+AllCops:
+  Exclude:
+    - "gemfiles/**/*"
+    - "vendor/**/*"
+    - "lib/generators/**/*"
+
+Metrics/MethodLength:
+  Max: 40
+
+Metrics/ModuleLength:
+  Max: 200
+
+Metrics/LineLength:
+  Max: 120
+
+Metrics/AbcSize:
+  Enabled: false
+
+Metrics/CyclomaticComplexity:
+  Enabled: false
+
+Metrics/PerceivedComplexity:
+  Enabled: false
+
+Style/StructInheritance:
+  Enabled: false
+
+Style/AlignParameters:
+  EnforcedStyle: with_fixed_indentation
+
+Style/StringLiterals:
+  EnforcedStyle: double_quotes
+
+Style/StringLiteralsInInterpolation:
+  EnforcedStyle: double_quotes
+
+Style/ClosingParenthesisIndentation:
+  Enabled: false
+
+Style/OneLineConditional:
+  Enabled: false
+
+Style/AndOr:
+  Enabled: false
+
+Style/Not:
+  Enabled: false
+
+Documentation:
+  Enabled: false # TODO: Enable again once we have more docs
+
+Style/CaseIndentation:
+  IndentWhenRelativeTo: case
+  SupportedStyles:
+    - case
+    - end
+  IndentOneStep: true
+
+Style/PercentLiteralDelimiters:
+  PreferredDelimiters:
+    '%w': "[]"
+    '%W': "[]"
+
+Style/AccessModifierIndentation:
+  EnforcedStyle: outdent
+
+Style/SignalException:
+  Enabled: false
+
+Style/IndentationWidth:
+  Enabled: false
+
+Style/TrivialAccessors:
+  ExactNameMatch: true
+
+Lint/EndAlignment:
+  AlignWith: variable
+
+Lint/DefEndAlignment:
+  Enabled: false
+
+Lint/HandleExceptions:
+  Enabled: false
+
+Style/SpecialGlobalVars:
+  Enabled: false
+
+Style/TrivialAccessors:
+  Enabled: false
+
+Style/IndentHash:
+  Enabled: false
+
+Style/DoubleNegation:
+  Enabled: false

Gemfile

@@ -1,5 +1,4 @@
-source 'https://rubygems.org'
+source "https://rubygems.org"
 
-# Specify your gem's dependencies in pundit.gemspec
 gem "rspec", ENV["RSPEC_VERSION"] unless ENV["RSPEC_VERSION"].to_s.empty?
 gemspec

Rakefile

@@ -1,17 +1,19 @@
-require 'rubygems'
-require 'bundler/gem_tasks'
-require 'rspec/core/rake_task'
-require 'yard'
+require "rubygems"
+require "bundler/gem_tasks"
+require "rspec/core/rake_task"
+require "yard"
+require "rubocop/rake_task"
+
+RuboCop::RakeTask.new
 
 desc "Run all examples"
 RSpec::Core::RakeTask.new(:spec) do |t|
-  #t.rspec_path = 'bin/rspec'
   t.rspec_opts = %w[--color]
 end
 
 YARD::Rake::YardocTask.new do |t|
-  t.files   = ['lib/**/*.rb']
-  #t.options = ['--any', '--extra', '--opts'] # optional
+  t.files = ["lib/**/*.rb"]
 end
 
-task :default => [:spec]
+task default: :spec
+task default: :rubocop unless RUBY_ENGINE == "rbx"

lib/pundit.rb

@@ -38,7 +38,7 @@ module Pundit
       policy = policy!(user, record)
 
       unless policy.public_send(query)
-        raise NotAuthorizedError.new(query: query, record: record, policy: policy)
+        raise NotAuthorizedError, query: query, record: record, policy: policy
       end
 
       true
@@ -102,21 +102,22 @@ module Pundit
   end
 
   def verify_authorized
-    raise AuthorizationNotPerformedError.new(self.class) unless pundit_policy_authorized?
+    raise AuthorizationNotPerformedError, self.class unless pundit_policy_authorized?
   end
 
   def verify_policy_scoped
-    raise PolicyScopingNotPerformedError.new(self.class) unless pundit_policy_scoped?
+    raise PolicyScopingNotPerformedError, self.class unless pundit_policy_scoped?
   end
 
-  def authorize(record, query=nil)
+  def authorize(record, query = nil)
     query ||= params[:action].to_s + "?"
 
     @_pundit_policy_authorized = true
 
     policy = policy(record)
+
     unless policy.public_send(query)
-      raise NotAuthorizedError.new(query: query, record: record, policy: policy)
+      raise NotAuthorizedError, query: query, record: record, policy: policy
     end
 
     true
@@ -139,7 +140,7 @@ module Pundit
     policies[record] ||= Pundit.policy!(pundit_user, record)
   end
 
-  def permitted_attributes(record, action=params[:action])
+  def permitted_attributes(record, action = params[:action])
     param_key = PolicyFinder.new(record).param_key
     policy = policy(record)
     method_name = if policy.respond_to?("permitted_attributes_for_#{action}")

lib/pundit/policy_finder.rb

@@ -85,7 +85,7 @@ module Pundit
         object.class.policy_class
       else
         klass = if object.is_a?(Array)
-          object.map { |x| find_class_name(x) }.join('::')
+          object.map { |x| find_class_name(x) }.join("::")
         else
           find_class_name(object)
         end

lib/pundit/rspec.rb

@@ -7,23 +7,29 @@ module Pundit
 
       matcher :permit do |user, record|
         match_proc = lambda do |policy|
-          @violating_permissions = permissions.find_all { |permission| not policy.new(user, record).public_send(permission) }
+          @violating_permissions = permissions.find_all do |permission|
+            not policy.new(user, record).public_send(permission)
+          end
           @violating_permissions.empty?
         end
 
         match_when_negated_proc = lambda do |policy|
-          @violating_permissions = permissions.find_all { |permission| policy.new(user, record).public_send(permission) }
+          @violating_permissions = permissions.find_all do |permission|
+            policy.new(user, record).public_send(permission)
+          end
           @violating_permissions.empty?
         end
 
         failure_message_proc = lambda do |policy|
           was_were = @violating_permissions.count > 1 ? "were" : "was"
-          "Expected #{policy} to grant #{permissions.to_sentence} on #{record} but #{@violating_permissions.to_sentence} #{was_were} not granted"
+          "Expected #{policy} to grant #{permissions.to_sentence} on \
+          #{record} but #{@violating_permissions.to_sentence} #{was_were} not granted"
         end
 
         failure_message_when_negated_proc = lambda do |policy|
           was_were = @violating_permissions.count > 1 ? "were" : "was"
-          "Expected #{policy} not to grant #{permissions.to_sentence} on #{record} but #{@violating_permissions.to_sentence} #{was_were} granted"
+          "Expected #{policy} not to grant #{permissions.to_sentence} on \
+          #{record} but #{@violating_permissions.to_sentence} #{was_were} granted"
         end
 
         if respond_to?(:match_when_negated)
@@ -47,7 +53,7 @@ module Pundit
 
     module DSL
       def permissions(*list, &block)
-        describe(list.to_sentence, :permissions => list, :caller => caller) { instance_eval(&block) }
+        describe(list.to_sentence, permissions: list, caller: caller) { instance_eval(&block) }
       end
     end
 
@@ -65,14 +71,14 @@ end
 
 RSpec.configure do |config|
   if RSpec::Core::Version::STRING.split(".").first.to_i >= 3
-    config.include(Pundit::RSpec::PolicyExampleGroup, {
-      :type => :policy,
-      :file_path => /spec\/policies/,
-    })
+    config.include(Pundit::RSpec::PolicyExampleGroup,
+      type: :policy,
+      file_path: %r{spec/policies}
+    )
   else
-    config.include(Pundit::RSpec::PolicyExampleGroup, {
-      :type => :policy,
-      :example_group => { :file_path => /spec\/policies/ }
-    })
+    config.include(Pundit::RSpec::PolicyExampleGroup,
+      type: :policy,
+      example_group: { file_path: %r{spec/policies} }
+    )
   end
 end

pundit.gemspec

@@ -1,20 +1,20 @@
 # -*- encoding: utf-8 -*-
-lib = File.expand_path('../lib', __FILE__)
+lib = File.expand_path("../lib", __FILE__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
-require 'pundit/version'
+require "pundit/version"
 
 Gem::Specification.new do |gem|
   gem.name          = "pundit"
   gem.version       = Pundit::VERSION
   gem.authors       = ["Jonas Nicklas", "Elabs AB"]
   gem.email         = ["jonas.nicklas@gmail.com", "dev@elabs.se"]
-  gem.description   = %q{Object oriented authorization for Rails applications}
-  gem.summary       = %q{OO authorization for Rails}
+  gem.description   = "Object oriented authorization for Rails applications"
+  gem.summary       = "OO authorization for Rails"
   gem.homepage      = "https://github.com/elabs/pundit"
   gem.license       = "MIT"
 
   gem.files         = `git ls-files`.split($/)
-  gem.executables   = gem.files.grep(%r{^bin/}).map{ |f| File.basename(f) }
+  gem.executables   = gem.files.grep(%r{^bin/}).map { |f| File.basename(f) }
   gem.test_files    = gem.files.grep(%r{^(test|spec|features)/})
   gem.require_paths = ["lib"]
 
@@ -26,4 +26,5 @@ Gem::Specification.new do |gem|
   gem.add_development_dependency "pry"
   gem.add_development_dependency "rake"
   gem.add_development_dependency "yard"
+  gem.add_development_dependency "rubocop"
 end

spec/pundit_spec.rb

@@ -8,7 +8,7 @@ describe Pundit do
   let(:comment) { Comment.new }
   let(:comment_four_five_six) { CommentFourFiveSix.new }
   let(:article) { Article.new }
-  let(:controller) { Controller.new(user, { :action => 'update' }) }
+  let(:controller) { Controller.new(user, action: "update") }
   let(:artificial_blog) { ArtificialBlog.new }
   let(:article_tag) { ArticleTag.new }
   let(:comments_relation) { CommentsRelation.new }
@@ -27,7 +27,10 @@ describe Pundit do
     end
 
     it "raises an error with a query and action" do
-      expect { Pundit.authorize(user, post, :destroy?) }.to raise_error(Pundit::NotAuthorizedError, "not allowed to destroy? this #<Post>") do |error|
+      # rubocop:disable Style/MultilineBlockChain
+      expect do
+        Pundit.authorize(user, post, :destroy?)
+      end.to raise_error(Pundit::NotAuthorizedError, "not allowed to destroy? this #<Post>") do |error|
         expect(error.query).to eq :destroy?
         expect(error.record).to eq post
         expect(error.policy).to eq Pundit.policy(user, post)
@@ -79,7 +82,9 @@ describe Pundit do
     end
 
     it "throws an exception if the given policy scope is nil" do
-      expect { Pundit.policy_scope!(user, nil) }.to raise_error(Pundit::NotDefinedError, "unable to find policy scope of nil")
+      expect do
+        Pundit.policy_scope!(user, nil)
+      end.to raise_error(Pundit::NotDefinedError, "unable to find policy scope of nil")
     end
   end
 
@@ -373,7 +378,7 @@ describe Pundit do
   end
 
   describe "#pundit_user" do
-    it 'returns the same thing as current_user' do
+    it "returns the same thing as current_user" do
       expect(controller.pundit_user).to eq controller.current_user
     end
   end
@@ -416,31 +421,49 @@ describe Pundit do
 
   describe "#permitted_attributes" do
     it "checks policy for permitted attributes" do
-      params = ActionController::Parameters.new({ action: 'update', post: { title: 'Hello', votes: 5, admin: true } })
+      params = ActionController::Parameters.new(action: "update", post: {
+        title: "Hello",
+        votes: 5,
+        admin: true
+      })
 
-      expect(Controller.new(user, params).permitted_attributes(post)).to eq({ 'title' => 'Hello', 'votes' => 5 })
-      expect(Controller.new(double, params).permitted_attributes(post)).to eq({ 'votes' => 5 })
+      expect(Controller.new(user, params).permitted_attributes(post)).to eq("title" => "Hello", "votes" => 5)
+      expect(Controller.new(double, params).permitted_attributes(post)).to eq("votes" => 5)
     end
 
     it "checks policy for permitted attributes for record of a ActiveModel type" do
-      params = ActionController::Parameters.new({ action: 'update', customer_post: { title: 'Hello', votes: 5, admin: true } })
+      params = ActionController::Parameters.new(action: "update", customer_post: {
+        title: "Hello",
+        votes: 5,
+        admin: true
+      })
 
-      expect(Controller.new(user, params).permitted_attributes(customer_post)).to eq({ 'title' => 'Hello', 'votes' => 5 })
-      expect(Controller.new(double, params).permitted_attributes(customer_post)).to eq({ 'votes' => 5 })
+      expect(Controller.new(user, params).permitted_attributes(customer_post)).to eq("title" => "Hello", "votes" => 5)
+      expect(Controller.new(double, params).permitted_attributes(customer_post)).to eq("votes" => 5)
     end
   end
 
   describe "#permitted_attributes_for_action" do
     it "is checked if it is defined in the policy" do
-      params = ActionController::Parameters.new({ action: 'revise', post: { title: 'Hello', body: "blah", votes: 5, admin: true } })
+      params = ActionController::Parameters.new(action: "revise", post: {
+        title: "Hello",
+        body: "blah",
+        votes: 5,
+        admin: true
+      })
 
-      expect(Controller.new(user, params).permitted_attributes(post)).to eq({ 'body' => 'blah' })
+      expect(Controller.new(user, params).permitted_attributes(post)).to eq("body" => "blah")
     end
 
     it "can be explicitly set" do
-      params = ActionController::Parameters.new({ action: 'update', post: { title: 'Hello', body: "blah", votes: 5, admin: true } })
+      params = ActionController::Parameters.new(action: "update", post: {
+        title: "Hello",
+        body: "blah",
+        votes: 5,
+        admin: true
+      })
 
-      expect(Controller.new(user, params).permitted_attributes(post, :revise)).to eq({ 'body' => 'blah' })
+      expect(Controller.new(user, params).permitted_attributes(post, :revise)).to eq("body" => "blah")
     end
   end
 

spec/spec_helper.rb

@@ -26,15 +26,24 @@ RSpec.configure do |config|
 end
 
 class PostPolicy < Struct.new(:user, :post)
+  class Scope < Struct.new(:user, :scope)
+    def resolve
+      scope.published
+    end
+  end
+
   def update?
     post.user == user
   end
+
   def destroy?
     false
   end
+
   def show?
     true
   end
+
   def permitted_attributes
     if post.user == user
       [:title, :votes]
@@ -47,64 +56,84 @@ class PostPolicy < Struct.new(:user, :post)
     [:body]
   end
 end
-class PostPolicy::Scope < Struct.new(:user, :scope)
-  def resolve
-    scope.published
-  end
-end
+
 class Post < Struct.new(:user)
   def self.published
     :published
   end
-  def to_s; "Post"; end
-  def inspect; "#<Post>"; end
+
+  def to_s
+    "Post"
+  end
+
+  def inspect
+    "#<Post>"
+  end
 end
 
 module Customer
   class Post < Post
-    # In ActiveRecord this method is accessible at both object and class level
     def model_name
-      OpenStruct.new(param_key: 'customer_post')
+      OpenStruct.new(param_key: "customer_post")
     end
+
     def policy_class
       PostPolicy
     end
   end
 end
-class CommentPolicy < Struct.new(:user, :comment); end
-class CommentPolicy::Scope < Struct.new(:user, :scope)
-  def resolve
-    scope
+
+class CommentPolicy < Struct.new(:user, :comment)
+  class Scope < Struct.new(:user, :scope)
+    def resolve
+      scope
+    end
   end
 end
-class Comment; extend ActiveModel::Naming; end
 
-# minimum mock for an ActiveRecord Relation returning comments
+class Comment
+  extend ActiveModel::Naming
+end
+
 class CommentsRelation
-  def initialize(empty=false); @empty=empty; end
-  def blank?; @empty; end
-  def model_name; Comment.model_name; end
+  def initialize(empty = false)
+    @empty = empty
+  end
+
+  def blank?
+    @empty
+  end
+
+  def model_name
+    Comment.model_name
+  end
 end
 
 class Article; end
 
 class BlogPolicy < Struct.new(:user, :blog); end
+
 class Blog; end
+
 class ArtificialBlog < Blog
   def self.policy_class
     BlogPolicy
   end
 end
+
+class ArticleTagOtherNamePolicy < Struct.new(:user, :tag)
+  def show?
+    true
+  end
+
+  def destroy?
+    false
+  end
+end
+
 class ArticleTag
   def self.policy_class
-    Struct.new(:user, :tag) do
-      def show?
-        true
-      end
-      def destroy?
-        false
-      end
-    end
+    ArticleTagOtherNamePolicy
   end
 end
 
@@ -146,14 +175,21 @@ class NilClassPolicy
 end
 
 class PostFourFiveSix < Struct.new(:user); end
+
 class CommentFourFiveSix; extend ActiveModel::Naming; end
 
 module ProjectOneTwoThree
   class CommentFourFiveSixPolicy < Struct.new(:user, :post); end
+
   class CriteriaFourFiveSixPolicy < Struct.new(:user, :criteria); end
+
   class PostFourFiveSixPolicy < Struct.new(:user, :post); end
+
   class TagFourFiveSix < Struct.new(:user); end
+
   class TagFourFiveSixPolicy < Struct.new(:user, :tag); end
+
   class AvatarFourFiveSix; extend ActiveModel::Naming; end
+
   class AvatarFourFiveSixPolicy < Struct.new(:user, :avatar); end
 end