From 92047a028446f64ac3a78083030ff97fd85002ba Mon Sep 17 00:00:00 2001 From: Nikita Shilnikov Date: Thu, 2 Jan 2025 13:49:24 +0100 Subject: [PATCH] Address rubocop offenses --- .rubocop.yml | 3 +++ core/lib/rom/command_registry.rb | 4 ++-- core/lib/rom/memory/dataset.rb | 8 +++---- core/lib/rom/registry.rb | 17 +++++++++----- core/lib/rom/relation.rb | 7 +++--- core/lib/rom/relation/class_interface.rb | 8 +++---- core/lib/rom/relation/curried.rb | 7 +++++- core/lib/rom/relation/loaded.rb | 4 ++-- core/lib/rom/schema.rb | 2 +- core/lib/rom/schema/dsl.rb | 4 ++-- core/lib/rom/setup.rb | 3 ++- core/lib/rom/setup/auto_registration.rb | 8 +++---- .../rom/setup/finalize/finalize_mappers.rb | 22 ++++++++++--------- .../rom/setup/finalize/finalize_relations.rb | 16 +++++++++----- core/lib/rom/struct.rb | 3 ++- core/spec/unit/rom/plugin_spec.rb | 6 ++--- .../rom/relation/class_interface/view_spec.rb | 4 +++- core/spec/unit/rom/relation/map_to_spec.rb | 4 +++- 18 files changed, 78 insertions(+), 52 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 3cc2747d4..2f7f5b323 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -138,6 +138,9 @@ Style/MapIntoArray: Exclude: - "core/spec/**/*.rb" +Style/NumberedParametersLimit: + Max: 2 + Style/OpenStructUse: Exclude: - "core/spec/**/*.rb" diff --git a/core/lib/rom/command_registry.rb b/core/lib/rom/command_registry.rb index 2a0000206..cb0257f73 100644 --- a/core/lib/rom/command_registry.rb +++ b/core/lib/rom/command_registry.rb @@ -91,12 +91,12 @@ def map_with(mapper_name) end # @api private - def set_compiler(compiler) + def set_compiler(compiler) # rubocop:disable Naming/AccessorMethodName options[:compiler] = @compiler = compiler end # @api private - def set_mappers(mappers) + def set_mappers(mappers) # rubocop:disable Naming/AccessorMethodName options[:mappers] = @mappers = mappers end diff --git a/core/lib/rom/memory/dataset.rb b/core/lib/rom/memory/dataset.rb index 57c3cba71..982f5262e 100644 --- a/core/lib/rom/memory/dataset.rb +++ b/core/lib/rom/memory/dataset.rb @@ -41,14 +41,14 @@ def join(*args) # @return [Dataset] # # @api public - def restrict(criteria = nil) - return find_all { |tuple| yield(tuple) } unless criteria + def restrict(criteria = nil, &) + return find_all(&) unless criteria find_all do |tuple| criteria.all? do |k, v| case v - when Array then v.include?(tuple[k]) - when Regexp then tuple[k].match(v) + when ::Array then v.include?(tuple[k]) + when ::Regexp then tuple[k].match(v) else tuple[k].eql?(v) end end diff --git a/core/lib/rom/registry.rb b/core/lib/rom/registry.rb index 3edf56c7f..765624fac 100644 --- a/core/lib/rom/registry.rb +++ b/core/lib/rom/registry.rb @@ -62,18 +62,23 @@ def to_hash end # @api private - def map - new_elements = elements.each_with_object({}) do |(name, element), h| - h[name] = yield(element) - end + def map(&) + new_elements = elements.transform_values(&) self.class.new(new_elements, **options) end # @api private - def each + def each(&) return to_enum unless block_given? - elements.each { |element| yield(element) } + elements.each(&) + end + + # @api private + def each_value(&) + return to_enum(:each_value) unless block_given? + + elements.each_value(&) end # @api private diff --git a/core/lib/rom/relation.rb b/core/lib/rom/relation.rb index 8dc0d536b..6b4471a4a 100644 --- a/core/lib/rom/relation.rb +++ b/core/lib/rom/relation.rb @@ -161,12 +161,13 @@ class Relation } # @!attribute [r] auto_map - # @return [TrueClass,FalseClass] Whether or not a relation and its compositions should be auto-mapped + # @return [true, false] Whether or not a relation and its + # compositions should be auto-mapped # @api private option :auto_map, default: -> { self.class.auto_map } # @!attribute [r] auto_struct - # @return [TrueClass,FalseClass] Whether or not tuples should be auto-mapped to structs + # @return [true, false] Whether or not tuples should be auto-mapped to structs # @api private option :auto_struct, default: -> { self.class.auto_struct } @@ -424,7 +425,7 @@ def new(dataset, **new_opts) if new_opts.empty? options elsif new_opts.key?(:schema) - options.merge(new_opts).reject { |k, _| %i[input_schema output_schema].include?(k) } + options.merge(new_opts).except(:input_schema, :output_schema) else options.merge(new_opts) end diff --git a/core/lib/rom/relation/class_interface.rb b/core/lib/rom/relation/class_interface.rb index c60fe340b..df773245a 100644 --- a/core/lib/rom/relation/class_interface.rb +++ b/core/lib/rom/relation/class_interface.rb @@ -234,10 +234,10 @@ def view(*args, &block) # @api public def forward(*methods) methods.each do |method| - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - new(dataset.__send__(:#{method}, *args, &block)) - end + class_eval(<<-RUBY, __FILE__, __LINE__ + 1) + def #{method}(...) # def super_query(...) + new(dataset.__send__(:#{method}, ...)) # new(dataset.__send__(:super_query, ...)) + end # end RUBY end end diff --git a/core/lib/rom/relation/curried.rb b/core/lib/rom/relation/curried.rb index 5631c626e..f7b5465d2 100644 --- a/core/lib/rom/relation/curried.rb +++ b/core/lib/rom/relation/curried.rb @@ -5,6 +5,9 @@ require 'rom/pipeline' require 'rom/relation/name' require 'rom/relation/materializable' +require 'rom/relation/graph' +require 'rom/relation/wrap' +require 'rom/relation/composite' module ROM class Relation @@ -19,6 +22,8 @@ class Relation class Curried extend Initializer + WRAPS = [Relation, Graph, Wrap, Composite].freeze + include Dry::Equalizer(:relation, :options) include Materializable include Pipeline @@ -112,7 +117,7 @@ def method_missing(meth, ...) super if response.is_a?(self.class) - if response.is_a?(Relation) || response.is_a?(Graph) || response.is_a?(Wrap) || response.is_a?(Composite) + if WRAPS.any? { |klass| response.is_a?(klass) } __new__(response) else response diff --git a/core/lib/rom/relation/loaded.rb b/core/lib/rom/relation/loaded.rb index ece205db2..45616e6c6 100644 --- a/core/lib/rom/relation/loaded.rb +++ b/core/lib/rom/relation/loaded.rb @@ -43,10 +43,10 @@ def initialize(source, collection = source.to_a) # @yield [Hash] # # @api public - def each + def each(&) return to_enum unless block_given? - collection.each { |tuple| yield(tuple) } + collection.each(&) end # Returns a single tuple from the relation if there is one. diff --git a/core/lib/rom/schema.rb b/core/lib/rom/schema.rb index e87f41737..bc58ff31b 100644 --- a/core/lib/rom/schema.rb +++ b/core/lib/rom/schema.rb @@ -39,7 +39,7 @@ class Schema subscribe('configuration.relations.registry.created') do |event| registry = event[:registry] - registry.each do |_, relation| + registry.each_value do |relation| unless relation.schema.frozen? relation.schema.finalize_associations!(relations: registry) relation.schema.finalize! diff --git a/core/lib/rom/schema/dsl.rb b/core/lib/rom/schema/dsl.rb index 5d2f3bfd5..3250de5c2 100644 --- a/core/lib/rom/schema/dsl.rb +++ b/core/lib/rom/schema/dsl.rb @@ -9,7 +9,7 @@ class Schema # Schema DSL exposed as `schema { .. }` in relation classes # # @api public - class DSL < BasicObject + class DSL < ::BasicObject KERNEL_METHODS = %i[extend method].freeze KERNEL_METHODS.each { |m| define_method(m, ::Kernel.instance_method(m)) } @@ -185,7 +185,7 @@ def call(&block) instance_exec(&definition) if definition schema_class.define(relation, **opts) do |schema| - plugins.values.each do |plugin, options| + plugins.each_value do |plugin, options| plugin.apply_to(schema, **options) end end diff --git a/core/lib/rom/setup.rb b/core/lib/rom/setup.rb index 13fea7eea..c567fe5ee 100644 --- a/core/lib/rom/setup.rb +++ b/core/lib/rom/setup.rb @@ -41,7 +41,8 @@ def initialize(notifications) # # @param [String, Pathname] directory The root path to components # @param [Hash] options - # @option options [Boolean, String] :namespace Enable/disable namespace or provide a custom namespace name + # @option options [Boolean, String] :namespace Enable/disable + # namespace or provide a custom namespace name # # @return [Setup] # diff --git a/core/lib/rom/setup/auto_registration.rb b/core/lib/rom/setup/auto_registration.rb index a0782a7c0..3cdf71ed1 100644 --- a/core/lib/rom/setup/auto_registration.rb +++ b/core/lib/rom/setup/auto_registration.rb @@ -44,11 +44,9 @@ class AutoRegistration # @!attribute [r] globs # @return [Hash] File globbing functions for each component dir option :globs, default: lambda { - Hash[ - component_dirs.map { |component, path| - [component, directory.join("#{path}/**/*.rb")] - } - ] + component_dirs.transform_values { |path| + directory.join("#{path}/**/*.rb") + } } # Load relation files diff --git a/core/lib/rom/setup/finalize/finalize_mappers.rb b/core/lib/rom/setup/finalize/finalize_mappers.rb index d5b7f9c49..836602c4f 100644 --- a/core/lib/rom/setup/finalize/finalize_mappers.rb +++ b/core/lib/rom/setup/finalize/finalize_mappers.rb @@ -5,7 +5,9 @@ module ROM class Finalize class FinalizeMappers - attr_reader :mapper_classes, :mapper_objects, :registry_hash + attr_reader :mapper_classes + attr_reader :mapper_objects + attr_reader :registry_hash # @api private def initialize(mapper_classes, mapper_objects) @@ -40,16 +42,16 @@ def run! private def check_duplicate_registered_mappers - mapper_relation_register = mapper_classes.map { |mapper_class| [mapper_class.relation, mapper_class.register_as].compact } - return if mapper_relation_register.uniq.count == mapper_classes.count + duplicates = mapper_classes.map { [_1.relation, _1.register_as] }.tally.select { _2 > 1 } - mapper_relation_register.select { |relation_register_as| mapper_relation_register.count(relation_register_as) > 1 } - .uniq - .each do |duplicated_mappers| - raise MapperAlreadyDefinedError, - "Mapper with `register_as #{duplicated_mappers.last.inspect}` registered more " \ - "than once for relation #{duplicated_mappers.first.inspect}" - end + case duplicates.first + in [rel, as], _ + raise MapperAlreadyDefinedError, + "Mapper with `register_as #{as.inspect}` registered more " \ + "than once for relation #{rel.inspect}" + else + nil + end end def build_mappers(relation_name) diff --git a/core/lib/rom/setup/finalize/finalize_relations.rb b/core/lib/rom/setup/finalize/finalize_relations.rb index 3275e1503..8703ac0ae 100644 --- a/core/lib/rom/setup/finalize/finalize_relations.rb +++ b/core/lib/rom/setup/finalize/finalize_relations.rb @@ -56,16 +56,22 @@ def run! "Relation with name #{key.inspect} registered more than once" end - klass.use(:registry_reader, klass: klass, - relation_readers_module: relation_readers_module) + klass.use( + :registry_reader, + klass: klass, + relation_readers_module: relation_readers_module + ) - notifications.trigger('configuration.relations.class.ready', relation: klass, - adapter: klass.adapter) + notifications.trigger( + 'configuration.relations.class.ready', + relation: klass, + adapter: klass.adapter + ) relations[key] = build_relation(klass, registry) end - registry.each do |_, relation| + registry.each_value do |relation| notifications.trigger( 'configuration.relations.object.registered', relation: relation, registry: registry diff --git a/core/lib/rom/struct.rb b/core/lib/rom/struct.rb index 8fffe17af..4a6aa7426 100644 --- a/core/lib/rom/struct.rb +++ b/core/lib/rom/struct.rb @@ -51,7 +51,8 @@ module ROM # # => #]> # # model.schema[:name] - # # => # | Nominal meta={source: :users}>]> + # # => # | + # # Nominal meta={source: :users}>]> # # @example passing a namespace with an existing parent class # module Entities diff --git a/core/spec/unit/rom/plugin_spec.rb b/core/spec/unit/rom/plugin_spec.rb index 1aecedb9b..3b02217e9 100644 --- a/core/spec/unit/rom/plugin_spec.rb +++ b/core/spec/unit/rom/plugin_spec.rb @@ -16,9 +16,9 @@ def plugged_in end Test::SchemaPlugin = Module.new do def self.apply(schema, **) - schema.attributes.concat( - [ROM::Attribute.new(ROM::Types::Date.meta(source: schema.name), name: :created_at), - ROM::Attribute.new(ROM::Types::Date.meta(source: schema.name), name: :updated_at)] + schema.attributes.push( + ROM::Attribute.new(ROM::Types::Date.meta(source: schema.name), name: :created_at), + ROM::Attribute.new(ROM::Types::Date.meta(source: schema.name), name: :updated_at) ) end end diff --git a/core/spec/unit/rom/relation/class_interface/view_spec.rb b/core/spec/unit/rom/relation/class_interface/view_spec.rb index aab29f5ac..d07afb394 100644 --- a/core/spec/unit/rom/relation/class_interface/view_spec.rb +++ b/core/spec/unit/rom/relation/class_interface/view_spec.rb @@ -136,7 +136,9 @@ } Class.new(ROM::Memory::Relation) do - schema_inferrer ROM::Schema::DEFAULT_INFERRER.with(attributes_inferrer: attributes_inferrer) + schema_inferrer ROM::Schema::DEFAULT_INFERRER.with( + attributes_inferrer: attributes_inferrer + ) schema(:users, infer: true) diff --git a/core/spec/unit/rom/relation/map_to_spec.rb b/core/spec/unit/rom/relation/map_to_spec.rb index c6f313fbb..78c32c30b 100644 --- a/core/spec/unit/rom/relation/map_to_spec.rb +++ b/core/spec/unit/rom/relation/map_to_spec.rb @@ -23,7 +23,9 @@ let(:mappers) { {} } it 'instantiates custom model when auto_struct is enabled' do - expect(relation.with(auto_struct: true).map_to(OpenStruct).first).to be_instance_of(OpenStruct) + expect( + relation.with(auto_struct: true).map_to(OpenStruct).first + ).to be_instance_of(OpenStruct) end it 'instantiates custom model when auto_struct is disabled' do