Skip to content

Commit

Permalink
Ban use of default_scope
Browse files Browse the repository at this point in the history
  • Loading branch information
Nick Thomas committed Jun 10, 2020
1 parent dee19c9 commit 4077687
Show file tree
Hide file tree
Showing 13 changed files with 85 additions and 10 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ Rails/ApplicationRecord:
- ee/db/**/*.rb
- ee/spec/**/*.rb

Cop/DefaultScope:
Enabled: true

Rails/FindBy:
Enabled: true
Include:
Expand Down
2 changes: 1 addition & 1 deletion app/models/badge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Badge < ApplicationRecord
# This regex will build the new PLACEHOLDER_REGEX with the new information
PLACEHOLDERS_REGEX = /(#{PLACEHOLDERS.keys.join('|')})/.freeze

default_scope { order_created_at_asc }
default_scope { order_created_at_asc } # rubocop:disable Cop/DefaultScope

scope :order_created_at_asc, -> { reorder(created_at: :asc) }

Expand Down
2 changes: 1 addition & 1 deletion app/models/ci/freeze_period.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class FreezePeriod < ApplicationRecord
include StripAttribute
self.table_name = 'ci_freeze_periods'

default_scope { order(created_at: :asc) }
default_scope { order(created_at: :asc) } # rubocop:disable Cop/DefaultScope

belongs_to :project, inverse_of: :freeze_periods

Expand Down
2 changes: 1 addition & 1 deletion app/models/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Event < ApplicationRecord
include Gitlab::Utils::StrongMemoize
include UsageStatistics

default_scope { reorder(nil) }
default_scope { reorder(nil) } # rubocop:disable Cop/DefaultScope

ACTIONS = HashWithIndifferentAccess.new(
created: 1,
Expand Down
2 changes: 1 addition & 1 deletion app/models/label.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class Label < ApplicationRecord
validates :title, uniqueness: { scope: [:group_id, :project_id] }
validates :title, length: { maximum: 255 }

default_scope { order(title: :asc) }
default_scope { order(title: :asc) } # rubocop:disable Cop/DefaultScope

scope :templates, -> { where(template: true, type: [Label.name, nil]) }
scope :with_title, ->(title) { where(title: title) }
Expand Down
2 changes: 1 addition & 1 deletion app/models/members/group_member.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class GroupMember < Member
# Make sure group member points only to group as it source
default_value_for :source_type, SOURCE_TYPE
validates :source_type, format: { with: /\ANamespace\z/ }
default_scope { where(source_type: SOURCE_TYPE) }
default_scope { where(source_type: SOURCE_TYPE) } # rubocop:disable Cop/DefaultScope

scope :of_groups, ->(groups) { where(source_id: groups.select(:id)) }
scope :count_users_by_group_id, -> { joins(:user).group(:source_id).count }
Expand Down
2 changes: 1 addition & 1 deletion app/models/members/project_member.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ProjectMember < Member
default_value_for :source_type, SOURCE_TYPE
validates :source_type, format: { with: /\AProject\z/ }
validates :access_level, inclusion: { in: Gitlab::Access.values }
default_scope { where(source_type: SOURCE_TYPE) }
default_scope { where(source_type: SOURCE_TYPE) } # rubocop:disable Cop/DefaultScope

scope :in_project, ->(project) { where(source_id: project.id) }
scope :in_namespaces, ->(groups) do
Expand Down
2 changes: 1 addition & 1 deletion app/models/releases/evidence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Evidence < ApplicationRecord

belongs_to :release, inverse_of: :evidences

default_scope { order(created_at: :asc) }
default_scope { order(created_at: :asc) } # rubocop:disable Cop/DefaultScope

sha_attribute :summary_sha
alias_attribute :collected_at, :created_at
Expand Down
2 changes: 1 addition & 1 deletion app/models/repository_language.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class RepositoryLanguage < ApplicationRecord
belongs_to :project
belongs_to :programming_language

default_scope { includes(:programming_language) }
default_scope { includes(:programming_language) } # rubocop:disable Cop/DefaultScope

validates :project, presence: true
validates :share, inclusion: { in: 0..100, message: "The share of a language is between 0 and 100" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class BackfillDeploymentClustersFromDeployments < ActiveRecord::Migration[6.0]
class Deployment < ActiveRecord::Base
include EachBatch

default_scope { where('cluster_id IS NOT NULL') }
default_scope { where('cluster_id IS NOT NULL') } # rubocop:disable Cop/DefaultScope

self.table_name = 'deployments'
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def self.has_prometheus_application?
class PrometheusService < ActiveRecord::Base
self.inheritance_column = :_type_disabled
self.table_name = 'services'
default_scope { where(type: type) }
default_scope { where(type: type) } # rubocop:disable Cop/DefaultScope

def self.type
'PrometheusService'
Expand Down
24 changes: 24 additions & 0 deletions rubocop/cop/default_scope.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

module RuboCop
module Cop
# Cop that blacklists the use of `default_scope`.
class DefaultScope < RuboCop::Cop::Cop
MSG = <<~EOF
Do not use `default_scope`, as it does not follow the principle of
least surprise. See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33847
for more details.
EOF

def_node_matcher :default_scope?, <<~PATTERN
(send {nil? (const nil? ...)} :default_scope ...)
PATTERN

def on_send(node)
return unless default_scope?(node)

add_offense(node, location: :expression)
end
end
end
end
48 changes: 48 additions & 0 deletions spec/rubocop/cop/default_scope_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true

require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/default_scope'

describe RuboCop::Cop::DefaultScope do
include CopHelper

subject(:cop) { described_class.new }

it 'does not flag the use of default_scope with a send receiver' do
inspect_source('foo.default_scope')

expect(cop.offenses.size).to eq(0)
end

it 'flags the use of default_scope with a constant receiver' do
inspect_source('User.default_scope')

expect(cop.offenses.size).to eq(1)
end

it 'flags the use of default_scope with a nil receiver' do
inspect_source('class Foo ; default_scope ; end')

expect(cop.offenses.size).to eq(1)
end

it 'flags the use of default_scope when passing arguments' do
inspect_source('class Foo ; default_scope(:foo) ; end')

expect(cop.offenses.size).to eq(1)
end

it 'flags the use of default_scope when passing a block' do
inspect_source('class Foo ; default_scope { :foo } ; end')

expect(cop.offenses.size).to eq(1)
end

it 'ignores the use of default_scope with a local variable receiver' do
inspect_source('users = User.all ; users.default_scope')

expect(cop.offenses.size).to eq(0)
end
end

0 comments on commit 4077687

Please sign in to comment.