diff --git a/changelog/fix_support_prefixed_constants_on_some_cops.md b/changelog/fix_support_prefixed_constants_on_some_cops.md new file mode 100644 index 0000000000..7bf82592d2 --- /dev/null +++ b/changelog/fix_support_prefixed_constants_on_some_cops.md @@ -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][]) diff --git a/lib/rubocop/cop/mixin/active_record_helper.rb b/lib/rubocop/cop/mixin/active_record_helper.rb index 4e776f90bd..ca5cfd6ab4 100644 --- a/lib/rubocop/cop/mixin/active_record_helper.rb +++ b/lib/rubocop/cop/mixin/active_record_helper.rb @@ -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 diff --git a/lib/rubocop/cop/mixin/migrations_helper.rb b/lib/rubocop/cop/mixin/migrations_helper.rb index 8784a6d98c..76dedda41f 100644 --- a/lib/rubocop/cop/mixin/migrations_helper.rb +++ b/lib/rubocop/cop/mixin/migrations_helper.rb @@ -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) :[] diff --git a/lib/rubocop/cop/rails/action_controller_flash_before_render.rb b/lib/rubocop/cop/rails/action_controller_flash_before_render.rb index 9d1f1cd56e..2e45be2ba9 100644 --- a/lib/rubocop/cop/rails/action_controller_flash_before_render.rb +++ b/lib/rubocop/cop/rails/action_controller_flash_before_render.rb @@ -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 diff --git a/lib/rubocop/cop/rails/action_controller_test_case.rb b/lib/rubocop/cop/rails/action_controller_test_case.rb index 92959c771c..6ea52cdc21 100644 --- a/lib/rubocop/cop/rails/action_controller_test_case.rb +++ b/lib/rubocop/cop/rails/action_controller_test_case.rb @@ -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 diff --git a/lib/rubocop/cop/rails/application_controller.rb b/lib/rubocop/cop/rails/application_controller.rb index 6e67472256..d65fa3d8f7 100644 --- a/lib/rubocop/cop/rails/application_controller.rb +++ b/lib/rubocop/cop/rails/application_controller.rb @@ -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 diff --git a/lib/rubocop/cop/rails/application_job.rb b/lib/rubocop/cop/rails/application_job.rb index d02e32b253..6d55d9786a 100644 --- a/lib/rubocop/cop/rails/application_job.rb +++ b/lib/rubocop/cop/rails/application_job.rb @@ -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 diff --git a/lib/rubocop/cop/rails/application_mailer.rb b/lib/rubocop/cop/rails/application_mailer.rb index c8638da9f2..256dc0385c 100644 --- a/lib/rubocop/cop/rails/application_mailer.rb +++ b/lib/rubocop/cop/rails/application_mailer.rb @@ -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 diff --git a/lib/rubocop/cop/rails/application_record.rb b/lib/rubocop/cop/rails/application_record.rb index ebd46b179f..91ee5e2aaf 100644 --- a/lib/rubocop/cop/rails/application_record.rb +++ b/lib/rubocop/cop/rails/application_record.rb @@ -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 diff --git a/lib/rubocop/cop/rails/dot_separated_keys.rb b/lib/rubocop/cop/rails/dot_separated_keys.rb index 73d8da171c..6121695199 100644 --- a/lib/rubocop/cop/rails/dot_separated_keys.rb +++ b/lib/rubocop/cop/rails/dot_separated_keys.rb @@ -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 diff --git a/lib/rubocop/cop/rails/freeze_time.rb b/lib/rubocop/cop/rails/freeze_time.rb index 40fb6024c3..fff5b37a4e 100644 --- a/lib/rubocop/cop/rails/freeze_time.rb +++ b/lib/rubocop/cop/rails/freeze_time.rb @@ -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) diff --git a/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb b/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb index df752f52b1..9491d47eb0 100644 --- a/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb +++ b/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb @@ -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 diff --git a/lib/rubocop/cop/rails/helper_instance_variable.rb b/lib/rubocop/cop/rails/helper_instance_variable.rb index d390b414fa..92b327c308 100644 --- a/lib/rubocop/cop/rails/helper_instance_variable.rb +++ b/lib/rubocop/cop/rails/helper_instance_variable.rb @@ -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) diff --git a/lib/rubocop/cop/rails/mailer_name.rb b/lib/rubocop/cop/rails/mailer_name.rb index e50ca97513..43abab7bda 100644 --- a/lib/rubocop/cop/rails/mailer_name.rb +++ b/lib/rubocop/cop/rails/mailer_name.rb @@ -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 @@ -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) diff --git a/lib/rubocop/cop/rails/migration_class_name.rb b/lib/rubocop/cop/rails/migration_class_name.rb index d204d450b4..1e895accde 100644 --- a/lib/rubocop/cop/rails/migration_class_name.rb +++ b/lib/rubocop/cop/rails/migration_class_name.rb @@ -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? diff --git a/lib/rubocop/cop/rails/output.rb b/lib/rubocop/cop/rails/output.rb index 9f87c9a7a8..dba0a57dbe 100644 --- a/lib/rubocop/cop/rails/output.rb +++ b/lib/rubocop/cop/rails/output.rb @@ -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} ...) diff --git a/lib/rubocop/cop/rails/short_i18n.rb b/lib/rubocop/cop/rails/short_i18n.rb index 35e02faf2d..dea14b8dd3 100644 --- a/lib/rubocop/cop/rails/short_i18n.rb +++ b/lib/rubocop/cop/rails/short_i18n.rb @@ -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) diff --git a/lib/rubocop/cop/rails/skips_model_validations.rb b/lib/rubocop/cop/rails/skips_model_validations.rb index cbc977eb95..de92ef30c5 100644 --- a/lib/rubocop/cop/rails/skips_model_validations.rb +++ b/lib/rubocop/cop/rails/skips_model_validations.rb @@ -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 diff --git a/lib/rubocop/cop/rails/time_zone_assignment.rb b/lib/rubocop/cop/rails/time_zone_assignment.rb index 0cc54b5b9f..fe16fa3d22 100644 --- a/lib/rubocop/cop/rails/time_zone_assignment.rb +++ b/lib/rubocop/cop/rails/time_zone_assignment.rb @@ -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) diff --git a/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb b/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb index 6cf9dd10e8..b8546a49cf 100644 --- a/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb +++ b/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb @@ -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) diff --git a/spec/rubocop/cop/rails/action_controller_test_case_spec.rb b/spec/rubocop/cop/rails/action_controller_test_case_spec.rb index a9ac35989d..6cc645ed3e 100644 --- a/spec/rubocop/cop/rails/action_controller_test_case_spec.rb +++ b/spec/rubocop/cop/rails/action_controller_test_case_spec.rb @@ -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 diff --git a/spec/rubocop/cop/rails/application_controller_spec.rb b/spec/rubocop/cop/rails/application_controller_spec.rb index 71a38cdc84..3980c487a9 100644 --- a/spec/rubocop/cop/rails/application_controller_spec.rb +++ b/spec/rubocop/cop/rails/application_controller_spec.rb @@ -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 diff --git a/spec/rubocop/cop/rails/application_job_spec.rb b/spec/rubocop/cop/rails/application_job_spec.rb index 40a31c91cd..5315c8c45e 100644 --- a/spec/rubocop/cop/rails/application_job_spec.rb +++ b/spec/rubocop/cop/rails/application_job_spec.rb @@ -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) diff --git a/spec/rubocop/cop/rails/application_mailer_spec.rb b/spec/rubocop/cop/rails/application_mailer_spec.rb index a09f9aba74..4c88db94bd 100644 --- a/spec/rubocop/cop/rails/application_mailer_spec.rb +++ b/spec/rubocop/cop/rails/application_mailer_spec.rb @@ -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 diff --git a/spec/rubocop/cop/rails/application_record_spec.rb b/spec/rubocop/cop/rails/application_record_spec.rb index 39bcc2b86b..ab5b53345f 100644 --- a/spec/rubocop/cop/rails/application_record_spec.rb +++ b/spec/rubocop/cop/rails/application_record_spec.rb @@ -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 diff --git a/spec/rubocop/cop/rails/dot_separated_keys_spec.rb b/spec/rubocop/cop/rails/dot_separated_keys_spec.rb index 0b1540af0c..8d34e837df 100644 --- a/spec/rubocop/cop/rails/dot_separated_keys_spec.rb +++ b/spec/rubocop/cop/rails/dot_separated_keys_spec.rb @@ -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 diff --git a/spec/rubocop/cop/rails/dynamic_find_by_spec.rb b/spec/rubocop/cop/rails/dynamic_find_by_spec.rb index c5824c7b10..1fa9eb202d 100644 --- a/spec/rubocop/cop/rails/dynamic_find_by_spec.rb +++ b/spec/rubocop/cop/rails/dynamic_find_by_spec.rb @@ -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 @@ -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 diff --git a/spec/rubocop/cop/rails/find_each_spec.rb b/spec/rubocop/cop/rails/find_each_spec.rb index 42e3181561..6494abcf14 100644 --- a/spec/rubocop/cop/rails/find_each_spec.rb +++ b/spec/rubocop/cop/rails/find_each_spec.rb @@ -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 @@ -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 diff --git a/spec/rubocop/cop/rails/freeze_time_spec.rb b/spec/rubocop/cop/rails/freeze_time_spec.rb index e54a9a53d7..22ae206e56 100644 --- a/spec/rubocop/cop/rails/freeze_time_spec.rb +++ b/spec/rubocop/cop/rails/freeze_time_spec.rb @@ -17,6 +17,12 @@ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`. travel_to(Time.current.to_time) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`. + travel_to(::Time.now) + ^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`. + travel_to(::DateTime.now) + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`. + travel_to(::Time.zone.now) + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`. RUBY expect_correction(<<~RUBY) @@ -27,6 +33,9 @@ freeze_time freeze_time freeze_time + freeze_time + freeze_time + freeze_time RUBY end diff --git a/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb b/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb index 9f639a755a..1b10b461b7 100644 --- a/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb +++ b/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb @@ -261,6 +261,14 @@ class User < ActiveResource::Base end RUBY end + + it 'does not register an offense when using associations of Active Resource and `::ActiveResource::Base`' do + expect_no_offenses(<<~RUBY) + class User < ::ActiveResource::Base + has_many :projects, class_name: 'API::Project' + end + RUBY + end end context 'when defining `readonly?` method' do diff --git a/spec/rubocop/cop/rails/helper_instance_variable_spec.rb b/spec/rubocop/cop/rails/helper_instance_variable_spec.rb index e47bb53842..52d46c431b 100644 --- a/spec/rubocop/cop/rails/helper_instance_variable_spec.rb +++ b/spec/rubocop/cop/rails/helper_instance_variable_spec.rb @@ -46,6 +46,17 @@ def do_something RUBY end + it 'does not register an offense when a class which inherits `::ActionView::Helpers::FormBuilder`' do + expect_no_offenses(<<~'RUBY') + class MyFormBuilder < ::ActionView::Helpers::FormBuilder + def do_something + @template + @template = do_something + end + end + RUBY + end + it 'registers an offense when using a class which does not inherit `ActionView::Helpers::FormBuilder`' do expect_offense(<<~'RUBY') class Foo diff --git a/spec/rubocop/cop/rails/mailer_name_spec.rb b/spec/rubocop/cop/rails/mailer_name_spec.rb index 211133ad0e..f618cf02c3 100644 --- a/spec/rubocop/cop/rails/mailer_name_spec.rb +++ b/spec/rubocop/cop/rails/mailer_name_spec.rb @@ -1,7 +1,12 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::MailerName, :config do - ['ActionMailer::Base', 'ApplicationMailer'].each do |base| + [ + '::ActionMailer::Base', + '::ApplicationMailer', + 'ActionMailer::Base', + 'ApplicationMailer' + ].each do |base| context 'when regular class definition' do it 'registers an offense and corrects when name without suffix' do expect_offense(<<~RUBY) @@ -42,5 +47,24 @@ class UserMailer < #{base} RUBY end end + + context 'when `::Class.new` definition' do + it 'registers an offense and corrects when name without suffix' do + expect_offense(<<~RUBY) + User = ::Class.new(#{base}) + ^^^^ Mailer should end with `Mailer` suffix. + RUBY + + expect_correction(<<~RUBY) + UserMailer = ::Class.new(#{base}) + RUBY + end + + it 'does not register an offense when name with suffix' do + expect_no_offenses(<<~RUBY) + UserMailer = ::Class.new(#{base}) + RUBY + end + end end end diff --git a/spec/rubocop/cop/rails/migration_class_name_spec.rb b/spec/rubocop/cop/rails/migration_class_name_spec.rb index 3538d2d2f7..a0a7c23302 100644 --- a/spec/rubocop/cop/rails/migration_class_name_spec.rb +++ b/spec/rubocop/cop/rails/migration_class_name_spec.rb @@ -50,6 +50,21 @@ class CreateUsers < ActiveRecord::Migration[7.0] end end + context 'when the class name does not match its file name and class name is prefixed with `::`' do + it 'registers an offense' do + expect_offense(<<~RUBY, filename) + class ::SellBooks < ActiveRecord::Migration[7.0] + ^^^^^^^^^ Replace with `CreateUsers` that matches the file name. + end + RUBY + + expect_correction(<<~RUBY) + class ::CreateUsers < ActiveRecord::Migration[7.0] + end + RUBY + end + end + context 'when the class name contains a dot in its file name' do let(:filename) { 'db/migrate/20220101050505_add_blobs.active_storage.rb' } diff --git a/spec/rubocop/cop/rails/output_spec.rb b/spec/rubocop/cop/rails/output_spec.rb index 4e3077df18..ce501f8409 100644 --- a/spec/rubocop/cop/rails/output_spec.rb +++ b/spec/rubocop/cop/rails/output_spec.rb @@ -45,7 +45,7 @@ RUBY end - it 'registers and corrects an offense for using `$stdout` method without a receiver' do + it 'registers and corrects an offense with `$stdout.write`' do expect_offense(<<~RUBY) $stdout.write "lord wilmore" ^^^^^^^^^^^^^ Do not write to stdout. Use Rails's logger if you want to log. @@ -56,7 +56,7 @@ RUBY end - it 'registers and corrects an offense for using `syswrite` method without a receiver' do + it 'registers and corrects an offense with `$stderr.syswrite`' do expect_offense(<<~RUBY) $stderr.syswrite "faria" ^^^^^^^^^^^^^^^^ Do not write to stdout. Use Rails's logger if you want to log. @@ -67,7 +67,7 @@ RUBY end - it 'registers and corrects an offense for using `write` method without a receiver' do + it 'registers and corrects an offense with `STDOUT.write`' do expect_offense(<<~RUBY) STDOUT.write "bertuccio" ^^^^^^^^^^^^ Do not write to stdout. Use Rails's logger if you want to log. @@ -78,6 +78,39 @@ RUBY end + it 'registers and corrects an offense with `::STDOUT.write`' do + expect_offense(<<~RUBY) + ::STDOUT.write "bertuccio" + ^^^^^^^^^^^^^^ Do not write to stdout. Use Rails's logger if you want to log. + RUBY + + expect_correction(<<~RUBY) + Rails.logger.debug "bertuccio" + RUBY + end + + it 'registers and corrects an offense with `STDERR.write`' do + expect_offense(<<~RUBY) + STDERR.write "bertuccio" + ^^^^^^^^^^^^ Do not write to stdout. Use Rails's logger if you want to log. + RUBY + + expect_correction(<<~RUBY) + Rails.logger.debug "bertuccio" + RUBY + end + + it 'registers and corrects an offense with `::STDERR.write`' do + expect_offense(<<~RUBY) + ::STDERR.write "bertuccio" + ^^^^^^^^^^^^^^ Do not write to stdout. Use Rails's logger if you want to log. + RUBY + + expect_correction(<<~RUBY) + Rails.logger.debug "bertuccio" + RUBY + end + it 'does not record an offense for methods with a receiver' do expect_no_offenses(<<~RUBY) obj.print diff --git a/spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb b/spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb index 55757fa0c0..ebace61127 100644 --- a/spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb +++ b/spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb @@ -23,6 +23,18 @@ def up RUBY end + it 'registers an offense with only an up method and `::` prefixed class name' do + expect_offense(<<~RUBY) + class ::SomeMigration < ActiveRecord::Migration[6.0] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a `change` method, or both an `up` and a `down` method. + + def up + add_column :users, :email, :text, null: false + end + end + RUBY + end + it 'registers an offense with only a down method' do expect_offense(<<~RUBY) class SomeMigration < ActiveRecord::Migration[6.0] diff --git a/spec/rubocop/cop/rails/reversible_migration_spec.rb b/spec/rubocop/cop/rails/reversible_migration_spec.rb index 52a3677de5..445afd42e0 100644 --- a/spec/rubocop/cop/rails/reversible_migration_spec.rb +++ b/spec/rubocop/cop/rails/reversible_migration_spec.rb @@ -332,4 +332,17 @@ def change RUBY end end + + context 'when irreversible operation is used in `::` prefixed class definition' do + it 'registers an offense' do + expect_offense(<<~RUBY, 'db/migrate/20211007000002_remove_animals.rb') + class ::RemoveAnimals < ActiveRecord::Migration[7.0] + def change + drop_table :animals + ^^^^^^^^^^^^^^^^^^^ drop_table(without block) is not reversible. + end + end + RUBY + end + end end diff --git a/spec/rubocop/cop/rails/short_i18n_spec.rb b/spec/rubocop/cop/rails/short_i18n_spec.rb index 291e8a7537..4df15d342a 100644 --- a/spec/rubocop/cop/rails/short_i18n_spec.rb +++ b/spec/rubocop/cop/rails/short_i18n_spec.rb @@ -13,6 +13,17 @@ RUBY end + it 'registers an offense and corrects when using `::I18n.translate`' do + expect_offense(<<~RUBY) + ::I18n.translate :key + ^^^^^^^^^ Use `t` instead of `translate`. + RUBY + + expect_correction(<<~RUBY) + ::I18n.t :key + RUBY + end + it 'registers an offense and corrects when using `I18n.localize`' do expect_offense(<<~RUBY) I18n.localize Time.now @@ -24,6 +35,17 @@ RUBY end + it 'registers an offense and corrects when using `::I18n.localize`' do + expect_offense(<<~RUBY) + ::I18n.localize Time.now + ^^^^^^^^ Use `l` instead of `localize`. + RUBY + + expect_correction(<<~RUBY) + ::I18n.l Time.now + RUBY + end + it 'does not register an offense when using `I18n.t`' do expect_no_offenses('I18n.t :key') end diff --git a/spec/rubocop/cop/rails/skips_model_validations_spec.rb b/spec/rubocop/cop/rails/skips_model_validations_spec.rb index e77c826c47..4e430f23c4 100644 --- a/spec/rubocop/cop/rails/skips_model_validations_spec.rb +++ b/spec/rubocop/cop/rails/skips_model_validations_spec.rb @@ -41,6 +41,10 @@ expect_no_offenses("FileUtils.touch('file')") end + it 'accepts ::FileUtils.touch' do + expect_no_offenses("::FileUtils.touch('file')") + end + it 'accepts touch with literal true' do expect_no_offenses('belongs_to(:user).touch(true)') end diff --git a/spec/rubocop/cop/rails/time_zone_assignment_spec.rb b/spec/rubocop/cop/rails/time_zone_assignment_spec.rb index 24aee3b2b0..2eafbca664 100644 --- a/spec/rubocop/cop/rails/time_zone_assignment_spec.rb +++ b/spec/rubocop/cop/rails/time_zone_assignment_spec.rb @@ -8,6 +8,13 @@ RUBY end + it 'registers an offense for `::Time.zone=`' do + expect_offense(<<~RUBY) + ::Time.zone = 'EST' + ^^^^^^^^^^^^^^^^^^^ Use `Time.use_zone` with block instead of `Time.zone=`. + RUBY + end + it 'accepts `Time.use_zone`' do expect_no_offenses(<<~RUBY) Time.use_zone('EST') do