Commit dc545b45 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch 'const_get_false' into 'master'

Lint const_get to not inherit scope (2nd arg false)

Closes #33846

See merge request gitlab-org/gitlab!18048
parents f332ac13 8edc9723
......@@ -178,6 +178,11 @@ Gitlab/ModuleWithInstanceVariables:
- spec/support/**/*.rb
- features/steps/**/*.rb
Gitlab/ConstGetInheritFalse:
Enabled: true
Exclude:
- 'qa/bin/*'
Gitlab/HTTParty:
Enabled: true
Exclude:
......
......@@ -8,13 +8,13 @@ module Clusters
included do
state_machine :status do
before_transition any => [:installed, :updated] do |application|
application.version = application.class.const_get(:VERSION)
application.version = application.class.const_get(:VERSION, false)
end
end
end
def update_available?
version != self.class.const_get(:VERSION)
version != self.class.const_get(:VERSION, false)
end
end
end
......
......@@ -44,7 +44,7 @@ module PrometheusAdapter
end
def query_klass_for(query_name)
Gitlab::Prometheus::Queries.const_get("#{query_name.to_s.classify}Query")
Gitlab::Prometheus::Queries.const_get("#{query_name.to_s.classify}Query", false)
end
def build_query_args(*args)
......
......@@ -24,7 +24,7 @@ class Note < ApplicationRecord
class << self
def values
constants.map {|const| self.const_get(const)}
constants.map {|const| self.const_get(const, false)}
end
def value?(val)
......
......@@ -138,7 +138,7 @@ class Upload < ApplicationRecord
end
def uploader_class
Object.const_get(uploader)
Object.const_get(uploader, false)
end
def identifier
......
......@@ -34,6 +34,7 @@ module Fog
# Gems that have not yet updated with the new fog-core namespace
LEGACY_FOG_PROVIDERS = %w(google rackspace aliyun).freeze
# rubocop:disable Gitlab/ConstGetInheritFalse
def service_provider_constant(service_name, provider_name)
args = service_provider_search_args(service_name, provider_name)
Fog.const_get(args.first).const_get(*const_get_args(args.second))
......@@ -48,5 +49,6 @@ module Fog
[provider_name, service_name]
end
end
# rubocop:enable Gitlab/ConstGetInheritFalse
end
end
......@@ -13,7 +13,7 @@ def instrument_classes(instrumentation)
instrumentation.instrument_methods(Gitlab::Git)
Gitlab::Git.constants.each do |name|
const = Gitlab::Git.const_get(name)
const = Gitlab::Git.const_get(name, false)
next unless const.is_a?(Module)
......@@ -75,7 +75,7 @@ def instrument_classes(instrumentation)
instrumentation.instrument_instance_methods(Rouge::Formatters::HTMLGitlab)
[:XML, :HTML].each do |namespace|
namespace_mod = Nokogiri.const_get(namespace)
namespace_mod = Nokogiri.const_get(namespace, false)
instrumentation.instrument_methods(namespace_mod)
instrumentation.instrument_methods(namespace_mod::Document)
......
......@@ -104,10 +104,10 @@ class Settings < Settingslogic
# check that `current` (string or integer) is a contant in `modul`.
def verify_constant(modul, current, default)
constant = modul.constants.find { |name| modul.const_get(name) == current }
value = constant.nil? ? default : modul.const_get(constant)
constant = modul.constants.find { |name| modul.const_get(name, false) == current }
value = constant.nil? ? default : modul.const_get(constant, false)
if current.is_a? String
value = modul.const_get(current.upcase) rescue default
value = modul.const_get(current.upcase, false) rescue default
end
value
......
......@@ -22,7 +22,7 @@ module EE
validates :"#{type}_access_levels", length: { is: 1 }, if: -> { false }
# Returns access levels that grant the specified access type to the given user / group.
access_level_class = const_get("#{type}_access_level".classify)
access_level_class = const_get("#{type}_access_level".classify, false)
protected_type = self.model_name.singular
scope(
:"#{type}_access_by_user",
......
......@@ -4,7 +4,7 @@ module EE
extend ActiveSupport::Concern
prepended do
EE_CALLOUT_FAILURE_MESSAGES = const_get(:CALLOUT_FAILURE_MESSAGES).merge(
EE_CALLOUT_FAILURE_MESSAGES = const_get(:CALLOUT_FAILURE_MESSAGES, false).merge(
protected_environment_failure: 'The environment this job is deploying to is protected. Only users with permission may successfully run this job.',
insufficient_bridge_permissions: 'This job could not be executed because of insufficient permissions to create a downstream pipeline.',
insufficient_upstream_permissions: 'This job could not be executed because of insufficient permissions to track the upstream project.',
......
......@@ -7,7 +7,7 @@ module EE
include ::Gitlab::Utils::StrongMemoize
prepended do
EE_UPDATE_SERVICES = const_get(:UPDATE_SERVICES).merge(
EE_UPDATE_SERVICES = const_get(:UPDATE_SERVICES, false).merge(
'Epic' => Epics::UpdateService
).freeze
EE::Notes::QuickActionsService.private_constant :EE_UPDATE_SERVICES
......
......@@ -9,7 +9,7 @@ module EE
extend ActiveSupport::Concern
prepended do
EE_TYPES = const_get(:TYPES) + [::EE::Gitlab::Ci::Config::Entry::Bridge]
EE_TYPES = const_get(:TYPES, false) + [::EE::Gitlab::Ci::Config::Entry::Bridge]
end
class_methods do
......
......@@ -9,7 +9,7 @@ module EE
extend ActiveSupport::Concern
prepended do
EE_REASONS = const_get(:REASONS).merge(
EE_REASONS = const_get(:REASONS, false).merge(
protected_environment_failure: 'protected environment failure',
invalid_bridge_trigger: 'downstream pipeline trigger definition is invalid',
downstream_bridge_project_not_found: 'downstream project could not be found',
......
......@@ -9,7 +9,7 @@ module Elastic
def initialize(target)
super(target)
config = version_namespace.const_get('Config')
config = version_namespace.const_get('Config', false)
@index_name = config.index_name
@document_type = config.document_type
......
......@@ -9,7 +9,7 @@ module Elastic
def initialize(target)
super(target)
config = version_namespace.const_get('Config')
config = version_namespace.const_get('Config', false)
@index_name = config.index_name
@document_type = config.document_type
......
......@@ -12,8 +12,8 @@ module Elastic
# @params version [String, Module] can be a string "V12p1" or module (Elastic::V12p1)
def version(version)
version = Elastic.const_get(version) if version.is_a?(String)
version.const_get(proxy_class_name).new(data_target)
version = Elastic.const_get(version, false) if version.is_a?(String)
version.const_get(proxy_class_name, false).new(data_target)
end
private
......
......@@ -159,7 +159,7 @@ describe 'GlobalSearch', :elastic do
# access_level can be :disabled, :enabled or :private
def feature_settings(access_level)
Hash[features.collect { |k| ["#{k}_access_level", ProjectFeature.const_get(access_level.to_s.upcase)] }]
Hash[features.collect { |k| ["#{k}_access_level", ProjectFeature.const_get(access_level.to_s.upcase, false)] }]
end
def expect_no_items_to_be_found(user)
......
......@@ -80,7 +80,7 @@ module API
note = create_note(noteable, opts)
if note.valid?
present note, with: Entities.const_get(note.class.name)
present note, with: Entities.const_get(note.class.name, false)
else
bad_request!("Note #{note.errors.messages}")
end
......
......@@ -49,7 +49,7 @@ module API
resource :todos do
helpers do
def issuable_and_awardable?(type)
obj_type = Object.const_get(type)
obj_type = Object.const_get(type, false)
(obj_type < Issuable) && (obj_type < Awardable)
rescue NameError
......
......@@ -3,7 +3,7 @@
module Banzai
module Filter
def self.[](name)
const_get("#{name.to_s.camelize}Filter")
const_get("#{name.to_s.camelize}Filter", false)
end
end
end
......@@ -4,7 +4,7 @@ module Banzai
module Pipeline
def self.[](name)
name ||= :full
const_get("#{name.to_s.camelize}Pipeline")
const_get("#{name.to_s.camelize}Pipeline", false)
end
end
end
......@@ -10,7 +10,7 @@ module Banzai
#
# This would return the `Banzai::ReferenceParser::IssueParser` class.
def self.[](name)
const_get("#{name.to_s.camelize}Parser")
const_get("#{name.to_s.camelize}Parser", false)
end
end
end
......@@ -30,7 +30,7 @@ module Bitbucket
end
def representation_class(type)
Bitbucket::Representation.const_get(type.to_s.camelize)
Bitbucket::Representation.const_get(type.to_s.camelize, false)
end
end
end
......@@ -30,7 +30,7 @@ module BitbucketServer
end
def representation_class(type)
BitbucketServer::Representation.const_get(type.to_s.camelize)
BitbucketServer::Representation.const_get(type.to_s.camelize, false)
end
end
end
......@@ -78,7 +78,7 @@ module Gitlab
end
def self.migration_class_for(class_name)
const_get(class_name)
const_get(class_name, false)
end
def self.enqueued_job?(queues, migration_class)
......
......@@ -171,7 +171,11 @@ module Gitlab
end
def schedule_retry(project, retry_count)
BackgroundMigrationWorker.perform_in(RETRY_DELAY, self.class::RetryOne.name, [project.id, retry_count])
# Constants provided to BackgroundMigrationWorker must be within the
# scope of Gitlab::BackgroundMigration
retry_class_name = self.class::RetryOne.name.sub('Gitlab::BackgroundMigration::', '')
BackgroundMigrationWorker.perform_in(RETRY_DELAY, retry_class_name, [project.id, retry_count])
end
end
......
......@@ -23,7 +23,7 @@ module Gitlab
end
def request_cache(method_name, &method_key_block)
const_get(:RequestCacheExtension).module_eval do
const_get(:RequestCacheExtension, false).module_eval do
cache_key_method_name = "#{method_name}_cache_key"
define_method(method_name) do |*args|
......
......@@ -6,7 +6,7 @@ module Gitlab
module Policy
def self.fabricate(specs)
specifications = specs.to_h.map do |spec, value|
self.const_get(spec.to_s.camelize).new(value)
self.const_get(spec.to_s.camelize, false).new(value)
end
specifications.compact
......
......@@ -20,7 +20,7 @@ module Gitlab
def core_status
Gitlab::Ci::Status
.const_get(@status.capitalize)
.const_get(@status.capitalize, false)
.new(@subject, @user)
.extend(self.class.common_helpers)
end
......
......@@ -37,7 +37,7 @@ module Gitlab
def self.entry_class(strategy)
if strategy.present?
self.const_get(strategy.name)
self.const_get(strategy.name, false)
else
self::UnknownStrategy
end
......
......@@ -4,7 +4,7 @@ module Gitlab
module CycleAnalytics
module EventFetcher
def self.[](stage_name)
CycleAnalytics.const_get("#{stage_name.to_s.camelize}EventFetcher")
CycleAnalytics.const_get("#{stage_name.to_s.camelize}EventFetcher", false)
end
end
end
......
......@@ -4,7 +4,7 @@ module Gitlab
module CycleAnalytics
module Stage
def self.[](stage_name)
CycleAnalytics.const_get("#{stage_name.to_s.camelize}Stage")
CycleAnalytics.const_get("#{stage_name.to_s.camelize}Stage", false)
end
end
end
......
......@@ -58,13 +58,13 @@ module Gitlab
# Returns true if the given migration can be performed without downtime.
def online?(migration)
migration.const_get(DOWNTIME_CONST) == false
migration.const_get(DOWNTIME_CONST, false) == false
end
# Returns the downtime reason, or nil if none was defined.
def downtime_reason(migration)
if migration.const_defined?(DOWNTIME_REASON_CONST)
migration.const_get(DOWNTIME_REASON_CONST)
migration.const_get(DOWNTIME_REASON_CONST, false)
else
nil
end
......
......@@ -86,7 +86,7 @@ module Gitlab
if name == :health_check
Grpc::Health::V1::Health::Stub
else
Gitaly.const_get(name.to_s.camelcase.to_sym).const_get(:Stub)
Gitaly.const_get(name.to_s.camelcase.to_sym, false).const_get(:Stub, false)
end
end
......
......@@ -8,7 +8,7 @@ module Gitlab
extend ActiveSupport::Concern
included do
attr_accessor(*const_get(:ATTRS))
attr_accessor(*const_get(:ATTRS, false))
end
def initialize(params)
......@@ -26,7 +26,7 @@ module Gitlab
end
def attributes
self.class.const_get(:ATTRS)
self.class.const_get(:ATTRS, false)
end
end
end
......
......@@ -24,7 +24,7 @@ module Gitlab
super
if const_defined?(:ClassMethods)
klass_methods = const_get(:ClassMethods)
klass_methods = const_get(:ClassMethods, false)
base.singleton_class.prepend klass_methods
base.instance_variable_set(:@_prepended_class_methods, klass_methods)
end
......@@ -40,7 +40,7 @@ module Gitlab
super
if instance_variable_defined?(:@_prepended_class_methods)
const_get(:ClassMethods).prepend @_prepended_class_methods
const_get(:ClassMethods, false).prepend @_prepended_class_methods
end
end
......
......@@ -17,7 +17,7 @@ module QA
def constants
@consts ||= @module.constants.map do |const|
@module.const_get(const)
@module.const_get(const, false)
end
end
......
......@@ -65,7 +65,7 @@ module QA
# QA::Runtime::Env.browser.capitalize will work for every driver type except PhantomJS.
# We will have no use to use PhantomJS so this shouldn't be a problem.
options = Selenium::WebDriver.const_get(QA::Runtime::Env.browser.capitalize)::Options.new
options = Selenium::WebDriver.const_get(QA::Runtime::Env.browser.capitalize, false)::Options.new
if QA::Runtime::Env.browser == :chrome
options.add_argument("window-size=1480,2200")
......
......@@ -19,7 +19,7 @@ module QA
end
def strategy
QA.const_get("QA::#{version}::Strategy")
Object.const_get("QA::#{version}::Strategy", false)
end
def self.method_missing(name, *args)
......
# frozen_string_literal: true
module RuboCop
module Cop
module Gitlab
# Cop that encourages usage of inherit=false for 2nd argument when using const_get.
#
# See https://gitlab.com/gitlab-org/gitlab/issues/27678
class ConstGetInheritFalse < RuboCop::Cop::Cop
MSG = 'Use inherit=false when using const_get.'
def_node_matcher :const_get?, <<~PATTERN
(send _ :const_get ...)
PATTERN
def on_send(node)
return unless const_get?(node)
return if second_argument(node)&.false_type?
add_offense(node, location: :selector)
end
def autocorrect(node)
lambda do |corrector|
if arg = second_argument(node)
corrector.replace(arg.source_range, 'false')
else
first_argument = node.arguments[0]
corrector.insert_after(first_argument.source_range, ', false')
end
end
end
private
def second_argument(node)
node.arguments[1]
end
end
end
end
end
require_relative 'cop/gitlab/const_get_inherit_false'
require_relative 'cop/gitlab/module_with_instance_variables'
require_relative 'cop/gitlab/predicate_memoization'
require_relative 'cop/gitlab/httparty'
......
......@@ -15,7 +15,7 @@ FactoryBot.define do
end
path do
uploader_instance = Object.const_get(uploader.to_s).new(model, mount_point)
uploader_instance = Object.const_get(uploader.to_s, false).new(model, mount_point)
File.join(uploader_instance.store_dir, filename)
end
......
......@@ -22,7 +22,7 @@ describe Gitlab::Ci::Status::External::Factory do
end
let(:expected_status) do
Gitlab::Ci::Status.const_get(simple_status.capitalize)
Gitlab::Ci::Status.const_get(simple_status.capitalize, false)
end
it "fabricates a core status #{simple_status}" do
......
......@@ -13,7 +13,7 @@ describe Gitlab::Ci::Status::Factory do
let(:resource) { double('resource', status: simple_status) }
let(:expected_status) do
Gitlab::Ci::Status.const_get(simple_status.capitalize)
Gitlab::Ci::Status.const_get(simple_status.capitalize, false)
end
it "fabricates a core status #{simple_status}" do
......
......@@ -18,7 +18,7 @@ describe Gitlab::Ci::Status::Pipeline::Factory do
let(:pipeline) { create(:ci_pipeline, status: simple_status) }
let(:expected_status) do
Gitlab::Ci::Status.const_get(simple_status.capitalize)
Gitlab::Ci::Status.const_get(simple_status.capitalize, false)
end
it "matches correct core status for #{simple_status}" do
......
......@@ -34,7 +34,7 @@ describe Gitlab::Ci::Status::Stage::Factory do
it "fabricates a core status #{core_status}" do
expect(status).to be_a(
Gitlab::Ci::Status.const_get(core_status.capitalize))
Gitlab::Ci::Status.const_get(core_status.capitalize, false))
end
it 'extends core status with common stage methods' do
......
......@@ -24,9 +24,9 @@ describe Gitlab::Config::Entry::Simplifiable do
let(:unknown) { double('unknown strategy') }
before do
stub_const("#{described_class.name}::Something", first)
stub_const("#{described_class.name}::DifferentOne", second)
stub_const("#{described_class.name}::UnknownStrategy", unknown)
entry::Something = first
entry::DifferentOne = second
entry::UnknownStrategy = unknown
end
context 'when first strategy should be used' do
......
......@@ -72,8 +72,8 @@ describe Gitlab::Patch::Prependable do
expect(subject.ancestors.take(3)).to eq([subject, ee, ce])
expect(subject.singleton_class.ancestors.take(3))
.to eq([subject.singleton_class,
ee.const_get(:ClassMethods),
ce.const_get(:ClassMethods)])
ee.const_get(:ClassMethods, false),
ce.const_get(:ClassMethods, false)])
end
it 'prepends only once even if called twice' do
......@@ -115,8 +115,8 @@ describe Gitlab::Patch::Prependable do
it 'has the expected ancestors' do
expect(subject.ancestors.take(3)).to eq([ee, ce, subject])
expect(subject.singleton_class.ancestors.take(3))
.to eq([ee.const_get(:ClassMethods),
ce.const_get(:ClassMethods),
.to eq([ee.const_get(:ClassMethods, false),
ce.const_get(:ClassMethods, false),
subject.singleton_class])
end
......@@ -152,7 +152,7 @@ describe Gitlab::Patch::Prependable do
it 'has the expected ancestors' do
expect(subject.ancestors.take(2)).to eq([ee, subject])
expect(subject.singleton_class.ancestors.take(2))
.to eq([ee.const_get(:ClassMethods),
.to eq([ee.const_get(:ClassMethods, false),
subject.singleton_class])
end
......
# frozen_string_literal: true
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/gitlab/const_get_inherit_false'
describe RuboCop::Cop::Gitlab::ConstGetInheritFalse do
include CopHelper
subject(:cop) { described_class.new }
context 'Object.const_get' do
it 'registers an offense with no 2nd argument' do
expect_offense(<<~PATTERN.strip_indent)
Object.const_get(:CONSTANT)
^^^^^^^^^ Use inherit=false when using const_get.
PATTERN
end
it 'autocorrects' do
expect(autocorrect_source('Object.const_get(:CONSTANT)')).to eq('Object.const_get(:CONSTANT, false)')
end
context 'inherit=false' do
it 'does not register an offense' do
expect_no_offenses(<<~PATTERN.strip_indent)
Object.const_get(:CONSTANT, false)
PATTERN
end
end
context 'inherit=true' do
it 'registers an offense' do
expect_offense(<<~PATTERN.strip_indent)
Object.const_get(:CONSTANT, true)
^^^^^^^^^ Use inherit=false when using const_get.
PATTERN
end
it 'autocorrects' do
expect(autocorrect_source('Object.const_get(:CONSTANT, true)')).to eq('Object.const_get(:CONSTANT, false)')
end
end
end
context 'const_get for a nested class' do
it 'registers an offense on reload usage' do
expect_offense(<<~PATTERN.strip_indent)
Nested::Blog.const_get(:CONSTANT)
^^^^^^^^^ Use inherit=false when using const_get.
PATTERN
end
it 'autocorrects' do
expect(autocorrect_source('Nested::Blag.const_get(:CONSTANT)')).to eq('Nested::Blag.const_get(:CONSTANT, false)')
end
context 'inherit=false' do
it 'does not register an offense' do
expect_no_offenses(<<~PATTERN.strip_indent)
Nested::Blog.const_get(:CONSTANT, false)
PATTERN
end
end
context 'inherit=true' do
it 'registers an offense if inherit is true' do
expect_offense(<<~PATTERN.strip_indent)
Nested::Blog.const_get(:CONSTANT, true)
^^^^^^^^^ Use inherit=false when using const_get.
PATTERN
end
it 'autocorrects' do
expect(autocorrect_source('Nested::Blag.const_get(:CONSTANT, true)')).to eq('Nested::Blag.const_get(:CONSTANT, false)')
end
end
end
end
......@@ -75,7 +75,7 @@ shared_examples 'cluster application status specs' do |application_name|
subject.reload
expect(subject.version).to eq(subject.class.const_get(:VERSION))
expect(subject.version).to eq(subject.class.const_get(:VERSION, false))
end
context 'application is updating' do
......@@ -104,7 +104,7 @@ shared_examples 'cluster application status specs' do |application_name|
subject.reload
expect(subject.version).to eq(subject.class.const_get(:VERSION))
expect(subject.version).to eq(subject.class.const_get(:VERSION, false))
end
end
end
......
......@@ -12,7 +12,7 @@ shared_examples 'cluster application version specs' do |application_name|
context 'version is the same as VERSION' do
let(:application) { build(application_name) }
let(:version) { application.class.const_get(:VERSION) }
let(:version) { application.class.const_get(:VERSION, false) }
it { is_expected.to be_falsey }
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment