Skip to content

Commit

Permalink
Use rubocop-disable_syntax
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Sep 26, 2023
1 parent d0c4114 commit a387f96
Show file tree
Hide file tree
Showing 20 changed files with 64 additions and 38 deletions.
20 changes: 19 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
require: rubocop-minitest
require:
- rubocop-minitest
- rubocop-disable_syntax

AllCops:
TargetRubyVersion: 2.3
Expand Down Expand Up @@ -78,6 +80,19 @@ Style/MapToHash:
Style/FetchEnvVar:
Enabled: false

Style/DisableSyntax:
DisableSyntax:
- unless
- safe_navigation
- endless_methods
- arguments_forwarding
- numbered_parameters
- pattern_matching
- shorthand_hash_syntax
- and_or_not
- until
- percent_literals

# we can not use new syntax for older rubies
Lint/ErbNewArguments:
Enabled: false
Expand Down Expand Up @@ -131,6 +146,9 @@ Gemspec/RequiredRubyVersion:
Gemspec/RequireMFA:
Enabled: false

Bundler/OrderedGems:
Enabled: false

Metrics:
Enabled: false

Expand Down
5 changes: 5 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ gem "minitest", "~> 5.0"
gem "rake", "~> 12.0"
gem "rubocop", "< 2"
gem "rubocop-minitest"

if RUBY_VERSION >= "2.7"
gem "rubocop-disable_syntax"
end

gem "yard"

if defined?(@ar_gem_requirement)
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ GEM
unicode-display_width (>= 2.4.0, < 3.0)
rubocop-ast (1.29.0)
parser (>= 3.2.1.0)
rubocop-disable_syntax (0.1.0)
rubocop (>= 1.50)
rubocop-minitest (0.31.0)
rubocop (>= 1.39, < 2.0)
ruby-progressbar (1.13.0)
Expand All @@ -109,6 +111,7 @@ DEPENDENCIES
railties
rake (~> 12.0)
rubocop (< 2)
rubocop-disable_syntax
rubocop-minitest
yard

Expand Down
4 changes: 2 additions & 2 deletions lib/online_migrations/background_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def named(name)
migration = "#{namespace}::#{name}".safe_constantize ||
"#{internal_namespace}::#{name}".safe_constantize

raise NotFoundError.new("Background Migration #{name} not found", name) unless migration
unless migration.is_a?(Class) && migration < self
raise NotFoundError.new("Background Migration #{name} not found", name) if migration.nil?
if !(migration.is_a?(Class) && migration < self)
raise NotFoundError.new("#{name} is not a Background Migration", name)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def validate(record)
relation = record.migration_relation
migration_name = record.migration_name

unless relation.is_a?(ActiveRecord::Relation)
if !relation.is_a?(ActiveRecord::Relation)
record.errors.add(
:migration_name,
"#{migration_name}#relation must return an ActiveRecord::Relation object"
Expand Down
2 changes: 1 addition & 1 deletion lib/online_migrations/background_migrations/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def initialize
end

def throttler=(value)
unless value.respond_to?(:call)
if !value.respond_to?(:call)
raise ArgumentError, "background_migrations throttler must be a callable."
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def initialize(model_name, record_id, association, _options = {})
end

def relation
unless @record.respond_to?(association)
if !@record.respond_to?(association)
raise ArgumentError, "'#{@record.class.name}' has no association called '#{association}'"
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def relation
# https://github.com/rails/rails/pull/34727
associations.inject(model.unscoped) do |relation, association|
reflection = model.reflect_on_association(association)
unless reflection
if reflection.nil?
raise ArgumentError, "'#{model.name}' has no association called '#{association}'"
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ class MigrationJobStatusValidator < ActiveModel::Validator
}

def validate(record)
return unless record.status_changed?
return if !record.status_changed?

previous_status, new_status = record.status_change
valid_new_statuses = VALID_STATUS_TRANSITIONS.fetch(previous_status, [])

unless valid_new_statuses.include?(new_status)
if !valid_new_statuses.include?(new_status)
record.errors.add(
:status,
"cannot transition background migration job from status #{previous_status} to #{new_status}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ class MigrationStatusValidator < ActiveModel::Validator
}

def validate(record)
return unless record.status_changed?
return if !record.status_changed?

previous_status, new_status = record.status_change
valid_new_statuses = VALID_STATUS_TRANSITIONS.fetch(previous_status, [])

unless valid_new_statuses.include?(new_status)
if !valid_new_statuses.include?(new_status)
record.errors.add(
:status,
"cannot transition background migration from status #{previous_status} to #{new_status}"
Expand Down
4 changes: 2 additions & 2 deletions lib/online_migrations/background_migrations/reset_counters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def count
def has_many_association(counter_association) # rubocop:disable Naming/PredicateName
has_many_association = model.reflect_on_association(counter_association)

unless has_many_association
if !has_many_association
has_many = model.reflect_on_all_associations(:has_many)

has_many_association = has_many.find do |association|
Expand All @@ -74,7 +74,7 @@ def has_many_association(counter_association) # rubocop:disable Naming/Predicate

counter_association = has_many_association.plural_name if has_many_association
end
raise ArgumentError, "'#{model.name}' has no association called '#{counter_association}'" unless has_many_association
raise ArgumentError, "'#{model.name}' has no association called '#{counter_association}'" if !has_many_association

if has_many_association.is_a?(ActiveRecord::Reflection::ThroughReflection)
has_many_association = has_many_association.through_reflection
Expand Down
6 changes: 3 additions & 3 deletions lib/online_migrations/batch_iterator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def initialize(relation)
end

def each_batch(of: 1000, column: relation.primary_key, start: nil, finish: nil, order: :asc)
unless [:asc, :desc].include?(order)
if ![:asc, :desc].include?(order)
raise ArgumentError, ":order must be :asc or :desc, got #{order.inspect}"
end

Expand All @@ -26,7 +26,7 @@ def each_batch(of: 1000, column: relation.primary_key, start: nil, finish: nil,

start_row = base_relation.uncached { base_relation.first }

return unless start_row
return if !start_row

start_id = start_row[column]
arel_table = relation.arel_table
Expand Down Expand Up @@ -67,7 +67,7 @@ def each_batch(of: 1000, column: relation.primary_key, start: nil, finish: nil,
# Retaining the results in the query cache would undermine the point of batching.
batch_relation.uncached { yield batch_relation, index }

break unless stop_row
break if !stop_row
end
end

Expand Down
8 changes: 4 additions & 4 deletions lib/online_migrations/change_column_type_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ def initialize_columns_type_change(table_name, columns_and_types, **options)
add_column(table_name, tmp_column_name, new_type,
**old_col_options.merge(column_options).merge(default: old_col.default || 0, null: false))
else
unless old_col.default.nil?
if !old_col.default.nil?
old_col_options = old_col_options.merge(default: old_col.default, null: old_col.null)
end
add_column(table_name, tmp_column_name, new_type, **old_col_options.merge(column_options))
end
else
add_column(table_name, tmp_column_name, new_type, **old_col_options.merge(column_options))
change_column_default(table_name, tmp_column_name, old_col.default) unless old_col.default.nil?
change_column_default(table_name, tmp_column_name, old_col.default) if !old_col.default.nil?
end
end

Expand Down Expand Up @@ -389,7 +389,7 @@ def __options_from_column(column, options)
options.each do |option|
if column.respond_to?(option)
value = column.public_send(option)
result[option] = value unless value.nil?
result[option] = value if !value.nil?
end
end
result
Expand Down Expand Up @@ -417,7 +417,7 @@ def __copy_indexes(table_name, from_column, to_column)
end

# This is necessary as we can't properly rename indexes such as "taggings_idx".
unless index.name.include?(from_column)
if !index.name.include?(from_column)
raise "The index #{index.name} can not be copied as it does not " \
"mention the old column. You have to rename this index manually first."
end
Expand Down
10 changes: 5 additions & 5 deletions lib/online_migrations/command_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def check(command, *args, &block)
check_database_version
check_lock_timeout

unless safe?
if !safe?
do_check(command, *args, &block)

run_custom_checks(command, args)
Expand Down Expand Up @@ -480,7 +480,7 @@ def add_reference(table_name, ref_name, **options)
bad_foreign_key: bad_foreign_key
end

unless options[:polymorphic]
if !options[:polymorphic]
type = (options[:type] || (Utils.ar_version >= 5.1 ? :bigint : :integer)).to_sym
column_name = "#{ref_name}_id"

Expand Down Expand Up @@ -510,9 +510,9 @@ def add_index(table_name, column_name, **options)
existing_indexes = connection.indexes(table_name)

@removed_indexes.each do |removed_index|
next unless removed_index.intersect?(index)
next if !removed_index.intersect?(index)

unless existing_indexes.any? { |existing_index| removed_index.covered_by?(existing_index) }
if existing_indexes.none? { |existing_index| removed_index.covered_by?(existing_index) }
raise_error :replace_index
end
end
Expand Down Expand Up @@ -563,7 +563,7 @@ def validate_foreign_key(*)
end

def add_exclusion_constraint(table_name, _expression, **_options)
unless new_or_small_table?(table_name)
if !new_or_small_table?(table_name)
raise_error :add_exclusion_constraint
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/online_migrations/command_recorder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def invert_revert_initialize_column_rename(args)

def invert_revert_initialize_columns_rename(args)
_table, old_new_column_hash = args
unless old_new_column_hash
if !old_new_column_hash
raise ActiveRecord::IrreversibleMigration,
"revert_initialize_columns_rename is only reversible if given a hash of old and new columns."
end
Expand All @@ -107,15 +107,15 @@ def invert_revert_initialize_columns_rename(args)

def invert_finalize_table_rename(args)
_table_name, new_name = args
unless new_name
if !new_name
raise ActiveRecord::IrreversibleMigration,
"finalize_table_rename is only reversible if given a new_name."
end
[:revert_finalize_table_rename, args]
end

def invert_revert_initialize_column_type_change(args)
unless args[2]
if !args[2]
raise ActiveRecord::IrreversibleMigration,
"revert_initialize_column_type_change is only reversible if given a new_type."
end
Expand All @@ -141,7 +141,7 @@ def invert_add_text_limit_constraint(args)
end

def invert_remove_text_limit_constraint(args)
unless args[2]
if !args[2]
raise ActiveRecord::IrreversibleMigration, "remove_text_limit_constraint is only reversible if given a limit."
end

Expand Down
2 changes: 1 addition & 1 deletion lib/online_migrations/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def add_check(start_after: nil, &block)

private
def ensure_supports_multiple_dbs
unless Utils.supports_multiple_dbs?
if !Utils.supports_multiple_dbs?
raise "OnlineMigrations does not support multiple databases for Active Record < 6.1"
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/online_migrations/schema_statements.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def update_columns_in_batches(table_name, columns_and_values,
value = Arel.sql(value.call.to_s) if value.is_a?(Proc)

# Ignore subqueries in conditions
unless value.is_a?(Arel::Nodes::SqlLiteral) && value.to_s =~ /select\s+/i
if !value.is_a?(Arel::Nodes::SqlLiteral) || value.to_s !~ /select\s+/i
arel_column = model.arel_table[column_name]
if value.nil?
arel_column.not_eq(nil)
Expand Down Expand Up @@ -665,7 +665,7 @@ def add_reference_concurrently(table_name, ref_name, **options)
__ensure_not_in_transaction!

column_name = "#{ref_name}_id"
unless column_exists?(table_name, column_name)
if !column_exists?(table_name, column_name)
type = options[:type] || (Utils.ar_version >= 5.1 ? :bigint : :integer)
allow_null = options.fetch(:null, true)
add_column(table_name, column_name, type, null: allow_null)
Expand Down
2 changes: 1 addition & 1 deletion lib/online_migrations/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def ar_where_not_multiple_conditions(relation, conditions)
private_constant :FUNCTION_CALL_RE

def volatile_default?(connection, type, value)
return false unless value.is_a?(Proc) || (type.to_s == "uuid" && value.is_a?(String))
return false if !(value.is_a?(Proc) || (type.to_s == "uuid" && value.is_a?(String)))

value = value.call if value.is_a?(Proc)
return false if !value.is_a?(String)
Expand Down
8 changes: 4 additions & 4 deletions test/config_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def test_start_after_multiple_dbs_below_6
end

def test_start_after_multiple_dbs
skip unless supports_multiple_dbs?
skip if !supports_multiple_dbs?

with_multiple_dbs do
with_start_after({ primary: 20200101000001 }) do
Expand All @@ -71,7 +71,7 @@ def test_start_after_multiple_dbs
end

def test_start_after_multiple_dbs_unconfigured
skip unless supports_multiple_dbs?
skip if !supports_multiple_dbs?

with_multiple_dbs(connects_to: :primary) do
assert_raises_with_message(StandardError, /OnlineMigrations.config.start_after is not configured for :primary/i) do
Expand Down Expand Up @@ -109,7 +109,7 @@ def test_target_version_multiple_dbs_below_6
end

def test_target_version_multiple_dbs
skip unless supports_multiple_dbs?
skip if !supports_multiple_dbs?

with_multiple_dbs do
with_target_version({ primary: 11 }) do
Expand All @@ -123,7 +123,7 @@ def test_target_version_multiple_dbs
end

def test_target_version_multiple_dbs_unconfigured
skip unless supports_multiple_dbs?
skip if !supports_multiple_dbs?

with_multiple_dbs(connects_to: :primary) do
assert_raises_with_message(StandardError, /OnlineMigrations.config.target_version is not configured for :primary/i) do
Expand Down
2 changes: 1 addition & 1 deletion test/support/minitest_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def assert_raises_in_transaction(&block)

def track_queries(&block)
queries = []
query_cb = ->(*, payload) { queries << payload[:sql] unless ["TRANSACTION"].include?(payload[:name]) }
query_cb = ->(*, payload) { queries << payload[:sql] if !["TRANSACTION"].include?(payload[:name]) }
ActiveSupport::Notifications.subscribed(query_cb, "sql.active_record", &block)
queries
end
Expand Down

0 comments on commit a387f96

Please sign in to comment.