Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support :: prefixed constants on some cops #868

Merged
merged 1 commit into from
Nov 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_support_prefixed_constants_on_some_cops.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#868](https://github.com/rubocop/rubocop-rails/pull/868): Support `::` prefixed constants on `Rails/ActionControllerFlashBeforeRender`, `Rails/ActionControllerTestCase`, `Rails/ApplicationController`, `Rails/ApplicationJob`, `Rails/ApplicationMailer`, `Rails/ApplicationRecord`, `Rails/DotSeparatedKeys`, `Rails/DynamicFindBy`, `Rails/FindEach`, `Rails/FreezeTime`, `Rails/HasManyOrHasOneDependent`, `Rails/HelperInstanceVariable`, `Rails/MailerName`, `Rails/MigrationClassName`, `Rails/Output`, `Rails/ReversibleMigrationMethodDefinition`, `Rails/ReversibleMigration`, `Rails/ShortI18n`, `Rails/SkipsModelValidations`, and `Rails/TimeZoneAssignment`. ([@r7kamura][])
4 changes: 2 additions & 2 deletions lib/rubocop/cop/mixin/active_record_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ module ActiveRecordHelper

def_node_matcher :active_record?, <<~PATTERN
{
(const nil? :ApplicationRecord)
(const (const nil? :ActiveRecord) :Base)
(const {nil? cbase} :ApplicationRecord)
(const (const {nil? cbase} :ActiveRecord) :Base)
}
PATTERN

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/migrations_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module MigrationsHelper

def_node_matcher :migration_class?, <<~PATTERN
(class
(const nil? _)
(const {nil? cbase} _)
(send
(const (const {nil? cbase} :ActiveRecord) :Migration)
:[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ class ActionControllerFlashBeforeRender < Base

def_node_search :action_controller?, <<~PATTERN
{
(const nil? :ApplicationController)
(const (const nil? :ActionController) :Base)
(const {nil? cbase} :ApplicationController)
(const (const {nil? cbase} :ActionController) :Base)
}
PATTERN

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/action_controller_test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ActionControllerTestCase < Base

def_node_matcher :action_controller_test_case?, <<~PATTERN
(class
(const nil? _)
(const {nil? cbase} _)
(const (const {nil? cbase} :ActionController) :TestCase) _)
PATTERN

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class ApplicationController < Base

MSG = 'Controllers should subclass `ApplicationController`.'
SUPERCLASS = 'ApplicationController'
BASE_PATTERN = '(const (const nil? :ActionController) :Base)'
BASE_PATTERN = '(const (const {nil? cbase} :ActionController) :Base)'

# rubocop:disable Layout/ClassStructure
include RuboCop::Cop::EnforceSuperclass
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/application_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ApplicationJob < Base

MSG = 'Jobs should subclass `ApplicationJob`.'
SUPERCLASS = 'ApplicationJob'
BASE_PATTERN = '(const (const nil? :ActiveJob) :Base)'
BASE_PATTERN = '(const (const {nil? cbase} :ActiveJob) :Base)'

# rubocop:disable Layout/ClassStructure
include RuboCop::Cop::EnforceSuperclass
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/application_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ApplicationMailer < Base

MSG = 'Mailers should subclass `ApplicationMailer`.'
SUPERCLASS = 'ApplicationMailer'
BASE_PATTERN = '(const (const nil? :ActionMailer) :Base)'
BASE_PATTERN = '(const (const {nil? cbase} :ActionMailer) :Base)'

# rubocop:disable Layout/ClassStructure
include RuboCop::Cop::EnforceSuperclass
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/application_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class ApplicationRecord < Base

MSG = 'Models should subclass `ApplicationRecord`.'
SUPERCLASS = 'ApplicationRecord'
BASE_PATTERN = '(const (const nil? :ActiveRecord) :Base)'
BASE_PATTERN = '(const (const {nil? cbase} :ActiveRecord) :Base)'

# rubocop:disable Layout/ClassStructure
include RuboCop::Cop::EnforceSuperclass
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/dot_separated_keys.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class DotSeparatedKeys < Base
TRANSLATE_METHODS = %i[translate t].freeze

def_node_matcher :translate_with_scope?, <<~PATTERN
(send {nil? (const nil? :I18n)} {:translate :t} ${sym_type? str_type?}
(send {nil? (const {nil? cbase} :I18n)} {:translate :t} ${sym_type? str_type?}
(hash <$(pair (sym :scope) ${array_type? sym_type?}) ...>)
)
PATTERN
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/rails/freeze_time.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ class FreezeTime < Base

# @!method time_now?(node)
def_node_matcher :time_now?, <<~PATTERN
(const nil? {:Time :DateTime})
(const {nil? cbase} {:Time :DateTime})
PATTERN

# @!method zoned_time_now?(node)
def_node_matcher :zoned_time_now?, <<~PATTERN
(send (const nil? :Time) :zone)
(send (const {nil? cbase} :Time) :zone)
PATTERN

def on_send(node)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class HasManyOrHasOneDependent < Base
RESTRICT_ON_SEND = %i[has_many has_one].freeze

def_node_search :active_resource_class?, <<~PATTERN
(const (const nil? :ActiveResource) :Base)
(const (const {nil? cbase} :ActiveResource) :Base)
PATTERN

def_node_matcher :association_without_options?, <<~PATTERN
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/helper_instance_variable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class HelperInstanceVariable < Base
def_node_matcher :form_builder_class?, <<~PATTERN
(const
(const
(const nil? :ActionView) :Helpers) :FormBuilder)
(const {nil? cbase} :ActionView) :Helpers) :FormBuilder)
PATTERN

def on_ivar(node)
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/rails/mailer_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class MailerName < Base

def_node_matcher :mailer_base_class?, <<~PATTERN
{
(const (const nil? :ActionMailer) :Base)
(const nil? :ApplicationMailer)
(const (const {nil? cbase} :ActionMailer) :Base)
(const {nil? cbase} :ApplicationMailer)
}
PATTERN

Expand All @@ -44,7 +44,7 @@ class MailerName < Base
PATTERN

def_node_matcher :class_new_definition?, <<~PATTERN
(send (const nil? :Class) :new #mailer_base_class?)
(send (const {nil? cbase} :Class) :new #mailer_base_class?)
PATTERN

def on_class(node)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/migration_class_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def on_class(node)

basename = basename_without_timestamp_and_suffix(processed_source.file_path)

class_identifier = node.identifier
class_identifier = node.identifier.location.name
camelized_basename = camelize(basename)
return if class_identifier.source.casecmp(camelized_basename).zero?

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/output.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Output < Base
(send
{
(gvar #match_gvar?)
{(const nil? :STDOUT) (const nil? :STDERR)}
(const {nil? cbase} {:STDOUT :STDERR})
}
{:binwrite :syswrite :write :write_nonblock}
...)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/short_i18n.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ShortI18n < Base
RESTRICT_ON_SEND = PREFERRED_METHODS.keys.freeze

def_node_matcher :long_i18n?, <<~PATTERN
(send {nil? (const nil? :I18n)} ${:translate :localize} ...)
(send {nil? (const {nil? cbase} :I18n)} ${:translate :localize} ...)
PATTERN

def on_send(node)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/skips_model_validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class SkipsModelValidations < Base

def_node_matcher :good_touch?, <<~PATTERN
{
(send (const nil? :FileUtils) :touch ...)
(send (const {nil? cbase} :FileUtils) :touch ...)
(send _ :touch {true false})
}
PATTERN
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/time_zone_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class TimeZoneAssignment < Base
RESTRICT_ON_SEND = %i[zone=].freeze

def_node_matcher :time_zone_assignment?, <<~PATTERN
(send (const nil? :Time) :zone= ...)
(send (const {nil? cbase} :Time) :zone= ...)
PATTERN

def on_send(node)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
RSpec.describe RuboCop::Cop::Rails::ActionControllerFlashBeforeRender, :config do
context 'when using `flash` before `render`' do
context 'within an instance method' do
%w[ActionController::Base ApplicationController].each do |parent_class|
%w[
::ActionController::Base
::ApplicationController
ActionController::Base
ApplicationController
].each do |parent_class|
context "within a class inherited from #{parent_class}" do
it 'registers an offense and corrects when the render call is explicit' do
expect_offense(<<~RUBY)
Expand Down
13 changes: 13 additions & 0 deletions spec/rubocop/cop/rails/action_controller_test_case_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@ class MyControllerTest < ActionDispatch::IntegrationTest
RUBY
end

it 'adds offense when defining `::MyControllerTest`' do
expect_offense(<<~RUBY)
class ::MyControllerTest < ActionController::TestCase
^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActionDispatch::IntegrationTest` instead.
end
RUBY

expect_correction(<<~RUBY)
class ::MyControllerTest < ActionDispatch::IntegrationTest
end
RUBY
end

it 'adds offense when extending `ActionController::TestCase` and having a method definition' do
expect_offense(<<~RUBY)
class MyControllerTest < ActionController::TestCase
Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/rails/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ class MyController < ApplicationController; end
RUBY
end

it 'corrects controllers that subclass `::ActionController::Base`' do
expect_offense(<<~RUBY)
class MyController < ::ActionController::Base; end
^^^^^^^^^^^^^^^^^^^^^^^^ Controllers should subclass `ApplicationController`.
RUBY

expect_correction(<<~RUBY)
class MyController < ApplicationController; end
RUBY
end

it 'corrects controllers defined in module namespaces' do
expect_offense(<<~RUBY)
module Nested
Expand Down
13 changes: 13 additions & 0 deletions spec/rubocop/cop/rails/application_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ class MyJob < ApplicationJob; end
end
end

context 'when subclassing `::ActiveJob::Base`' do
it 'registers an offense' do
expect_offense(<<~RUBY)
class MyJob < ::ActiveJob::Base; end
^^^^^^^^^^^^^^^^^ Jobs should subclass `ApplicationJob`.
RUBY

expect_correction(<<~RUBY)
class MyJob < ApplicationJob; end
RUBY
end
end

context 'when subclassing `ActiveJob::Base` in a module namespace' do
it 'registers an offense' do
expect_offense(<<~RUBY)
Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/rails/application_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@ class MyMailer < ApplicationMailer; end
RUBY
end

it 'corrects mailers that subclass `::ActionMailer::Base`' do
expect_offense(<<~RUBY)
class MyMailer < ::ActionMailer::Base; end
^^^^^^^^^^^^^^^^^^^^ Mailers should subclass `ApplicationMailer`.
RUBY

expect_correction(<<~RUBY)
class MyMailer < ApplicationMailer; end
RUBY
end

it 'corrects mailers defined in module namespaces' do
expect_offense(<<~RUBY)
module Nested
Expand Down
13 changes: 13 additions & 0 deletions spec/rubocop/cop/rails/application_record_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ class MyModel < ApplicationRecord
RUBY
end

it 'corrects models that subclass ::ActiveRecord::Base' do
expect_offense(<<~RUBY)
class MyModel < ::ActiveRecord::Base
^^^^^^^^^^^^^^^^^^^^ Models should subclass `ApplicationRecord`.
end
RUBY

expect_correction(<<~RUBY)
class MyModel < ApplicationRecord
end
RUBY
end

it 'corrects single-line class definitions' do
expect_offense(<<~RUBY)
class MyModel < ActiveRecord::Base; end
Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/rails/dot_separated_keys_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@
RUBY
end

it 'registers an offense and corrects when translating keys with convertible scopes are used with `::I18n`' do
expect_offense(<<~RUBY)
::I18n.t :key, scope: [:one, :two]
^^^^^^^^^^^^^^^^^^^ Use the dot-separated keys instead of specifying the `:scope` option.
RUBY

expect_correction(<<~RUBY)
::I18n.t 'one.two.key'
RUBY
end

it 'does not register an offense when key is an array' do
expect_no_offenses(<<~RUBY)
t [:key1, :key2], scope: :one
Expand Down
22 changes: 22 additions & 0 deletions spec/rubocop/cop/rails/dynamic_find_by_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,17 @@ def do_something
RUBY
end

it 'registers an offense when inheriting `::ApplicationRecord`' do
expect_offense(<<~RUBY)
class C < ::ApplicationRecord
def do_something
find_by_name(name)
^^^^^^^^^^^^^^^^^^ Use `find_by` instead of dynamic `find_by_name`.
end
end
RUBY
end

it 'registers an offense when inheriting `ActiveRecord::Base`' do
expect_offense(<<~RUBY)
class C < ActiveRecord::Base
Expand All @@ -188,6 +199,17 @@ def do_something
end
RUBY
end

it 'registers an offense when inheriting `::ActiveRecord::Base`' do
expect_offense(<<~RUBY)
class C < ::ActiveRecord::Base
def do_something
find_by_name(name)
^^^^^^^^^^^^^^^^^^ Use `find_by` instead of dynamic `find_by_name`.
end
end
RUBY
end
end

context 'with allowed receiver name' do
Expand Down
18 changes: 18 additions & 0 deletions spec/rubocop/cop/rails/find_each_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ class C < ApplicationRecord
RUBY
end

it 'registers an offense when inheriting `::ApplicationRecord`' do
expect_offense(<<~RUBY)
class C < ::ApplicationRecord
all.each { |u| u.x }
^^^^ Use `find_each` instead of `each`.
end
RUBY
end

it 'registers an offense when inheriting `ActiveRecord::Base`' do
expect_offense(<<~RUBY)
class C < ActiveRecord::Base
Expand All @@ -116,6 +125,15 @@ class C < ActiveRecord::Base
end
RUBY
end

it 'registers an offense when inheriting `::ActiveRecord::Base`' do
expect_offense(<<~RUBY)
class C < ::ActiveRecord::Base
all.each { |u| u.x }
^^^^ Use `find_each` instead of `each`.
end
RUBY
end
end

context 'allowed methods' do
Expand Down
Loading