From ea8cf7b3f3afa5204203d2a29bdea6e84bd9019e Mon Sep 17 00:00:00 2001 From: Sergio Bayona Date: Thu, 9 Jan 2025 10:07:51 -0500 Subject: [PATCH] rubocop fixes --- Gemfile | 2 +- lib/easy_talk/property.rb | 16 ++--- lib/easy_talk/schema_definition.rb | 9 +-- .../builders/boolean_builder_spec.rb | 46 ++++++------ .../easy_talk/builders/string_builder_spec.rb | 72 +++++++++---------- .../examples/company_employees_spec.rb | 2 +- spec/easy_talk/examples/company_owner_spec.rb | 3 +- 7 files changed, 70 insertions(+), 80 deletions(-) diff --git a/Gemfile b/Gemfile index dadc0fe..ffc30ad 100644 --- a/Gemfile +++ b/Gemfile @@ -4,4 +4,4 @@ source 'https://rubygems.org' gemspec -gem "dartsass-rails", ">= 0.5.0" +gem 'dartsass-rails', '>= 0.5.0' diff --git a/lib/easy_talk/property.rb b/lib/easy_talk/property.rb index e07527d..ad7a052 100644 --- a/lib/easy_talk/property.rb +++ b/lib/easy_talk/property.rb @@ -76,21 +76,15 @@ def initialize(name, type = nil, constraints = {}) # # @return [Object] The built property. def build - # return type.respond_to?(:schema) ? type.schema : 'object' unless builder - - # args = builder.collection_type? ? [name, type, constraints] : [name, constraints] - # builder.new(*args).build if builder args = builder.collection_type? ? [name, type, constraints] : [name, constraints] builder.new(*args).build + elsif type.respond_to?(:schema) + # merge the top-level constraints from *this* property + # e.g. :title, :description, :default, etc + type.schema.merge!(constraints) else - if type.respond_to?(:schema) - # merge the top-level constraints from *this* property - # e.g. :title, :description, :default, etc - type.schema.merge!(constraints) - else - 'object' - end + 'object' end end diff --git a/lib/easy_talk/schema_definition.rb b/lib/easy_talk/schema_definition.rb index b8f6269..ec82450 100644 --- a/lib/easy_talk/schema_definition.rb +++ b/lib/easy_talk/schema_definition.rb @@ -4,13 +4,13 @@ module EasyTalk class InvalidPropertyNameError < StandardError; end + # #= EasyTalk \SchemaDefinition # SchemaDefinition provides the methods for defining a schema within the define_schema block. # The @schema is a hash that contains the unvalidated schema definition for the model. # A SchemaDefinition instanace is the passed to the Builder.build_schema method to validate and compile the schema. class SchemaDefinition - extend T::Sig extend T::AnyOf extend T::OneOf @@ -56,9 +56,10 @@ def property(name, type, constraints = {}, &blk) end def validate_property_name(name) - unless name.to_s.match?(/^[A-Za-z_][A-Za-z0-9_]*$/) - raise InvalidPropertyNameError, "Invalid property name '#{name}'. Must start with letter/underscore and contain only letters, numbers, underscores" - end + return if name.to_s.match?(/^[A-Za-z_][A-Za-z0-9_]*$/) + + raise InvalidPropertyNameError, + "Invalid property name '#{name}'. Must start with letter/underscore and contain only letters, numbers, underscores" end def optional? diff --git a/spec/easy_talk/builders/boolean_builder_spec.rb b/spec/easy_talk/builders/boolean_builder_spec.rb index 1041a5d..c75994e 100644 --- a/spec/easy_talk/builders/boolean_builder_spec.rb +++ b/spec/easy_talk/builders/boolean_builder_spec.rb @@ -37,55 +37,53 @@ it 'combines multiple constraints' do builder = described_class.new(:active, - title: 'Account Status', - description: 'Whether the account is active', - default: true, - enum: [true, false] - ) + title: 'Account Status', + description: 'Whether the account is active', + default: true, + enum: [true, false]) expect(builder.build).to eq({ - type: 'boolean', - title: 'Account Status', - description: 'Whether the account is active', - default: true, - enum: [true, false] - }) + type: 'boolean', + title: 'Account Status', + description: 'Whether the account is active', + default: true, + enum: [true, false] + }) end end context 'with invalid configurations' do it 'raises ArgumentError for unknown constraints' do - expect { + expect do described_class.new(:active, invalid_option: 'value').build - }.to raise_error(ArgumentError, /Unknown key/) + end.to raise_error(ArgumentError, /Unknown key/) end it 'raises TypeError when enum contains non-boolean values' do - expect { + expect do described_class.new(:active, enum: [true, 'false']).build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end it 'raises TypeError when default is not a boolean' do - expect { + expect do described_class.new(:active, default: 'true').build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end it 'raises TypeError when enum is not an array' do - expect { + expect do described_class.new(:active, enum: 'true,false').build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end end context 'with nil values' do it 'excludes constraints with nil values' do builder = described_class.new(:active, - default: nil, - enum: nil, - description: nil - ) + default: nil, + enum: nil, + description: nil) expect(builder.build).to eq({ type: 'boolean' }) end end @@ -98,7 +96,7 @@ it 'excludes optional flag when false' do builder = described_class.new(:subscribed, optional: false) - expect(builder.build).to eq({ type: 'boolean', optional: false}) + expect(builder.build).to eq({ type: 'boolean', optional: false }) end end diff --git a/spec/easy_talk/builders/string_builder_spec.rb b/spec/easy_talk/builders/string_builder_spec.rb index f8724d5..2b32b59 100644 --- a/spec/easy_talk/builders/string_builder_spec.rb +++ b/spec/easy_talk/builders/string_builder_spec.rb @@ -41,8 +41,8 @@ end it 'includes enum constraint' do - builder = described_class.new(:status, enum: ['active', 'inactive', 'pending']) - expect(builder.build).to eq({ type: 'string', enum: ['active', 'inactive', 'pending'] }) + builder = described_class.new(:status, enum: %w[active inactive pending]) + expect(builder.build).to eq({ type: 'string', enum: %w[active inactive pending] }) end it 'includes const constraint' do @@ -57,74 +57,72 @@ it 'combines multiple constraints' do builder = described_class.new(:password, - min_length: 8, - max_length: 32, - pattern: '^(?=.*[A-Za-z])(?=.*\d)[A-Za-z\d]{8,}$', - description: 'Must contain letters and numbers' - ) + min_length: 8, + max_length: 32, + pattern: '^(?=.*[A-Za-z])(?=.*\d)[A-Za-z\d]{8,}$', + description: 'Must contain letters and numbers') expect(builder.build).to eq({ - type: 'string', - minLength: 8, - maxLength: 32, - pattern: '^(?=.*[A-Za-z])(?=.*\d)[A-Za-z\d]{8,}$', - description: 'Must contain letters and numbers' - }) + type: 'string', + minLength: 8, + maxLength: 32, + pattern: '^(?=.*[A-Za-z])(?=.*\d)[A-Za-z\d]{8,}$', + description: 'Must contain letters and numbers' + }) end end context 'with invalid configurations' do it 'raises ArgumentError for unknown constraints' do - expect { + expect do described_class.new(:name, invalid_option: 'value').build - }.to raise_error(ArgumentError, /Unknown key: :invalid_option/) + end.to raise_error(ArgumentError, /Unknown key: :invalid_option/) end it 'raises TypeError when format is not a string' do - expect { + expect do described_class.new(:email, format: 123).build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end it 'raises TypeError when pattern is not a string' do - expect { + expect do described_class.new(:zip, pattern: 123).build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end it 'raises TypeError when minLength is not an integer' do - expect { + expect do described_class.new(:name, min_length: '8').build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end it 'raises TypeError when maxLength is not an integer' do - expect { + expect do described_class.new(:name, max_length: '20').build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end it 'raises TypeError when enum contains non-string values' do - expect { + expect do described_class.new(:status, enum: ['active', 123, 'pending']).build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end it 'raises TypeError when const is not a string' do - expect { + expect do described_class.new(:type, const: 123).build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end end context 'with nil values' do it 'excludes constraints with nil values' do builder = described_class.new(:name, - min_length: nil, - max_length: nil, - pattern: nil, - format: nil - ) + min_length: nil, + max_length: nil, + pattern: nil, + format: nil) expect(builder.build).to eq({ type: 'string' }) end end @@ -132,20 +130,20 @@ context 'with empty values on lenght validators' do it 'raises a type error' do builder = described_class.new(:name, min_length: '') - expect { + expect do builder.build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end it 'raises a type error' do builder = described_class.new(:name, max_length: '') - expect { + expect do builder.build - }.to raise_error(TypeError) + end.to raise_error(TypeError) end end - context "with empty values on pattern" do + context 'with empty values on pattern' do it 'returns empty pattern' do # this is invalid in json schema but there is not practical way to validate non empty strings. builder = described_class.new(:name, pattern: '') diff --git a/spec/easy_talk/examples/company_employees_spec.rb b/spec/easy_talk/examples/company_employees_spec.rb index 4afaffb..ea37c20 100644 --- a/spec/easy_talk/examples/company_employees_spec.rb +++ b/spec/easy_talk/examples/company_employees_spec.rb @@ -151,7 +151,7 @@ def self.name Company.define_schema do title 'Company' property :name, String - property :employees, T::Array[Employee], title: "Company Employees", description: 'A list of company employees' + property :employees, T::Array[Employee], title: 'Company Employees', description: 'A list of company employees' end expect(company.json_schema).to include_json(expected_json_schema) diff --git a/spec/easy_talk/examples/company_owner_spec.rb b/spec/easy_talk/examples/company_owner_spec.rb index 1a96623..3e07d35 100644 --- a/spec/easy_talk/examples/company_owner_spec.rb +++ b/spec/easy_talk/examples/company_owner_spec.rb @@ -39,14 +39,13 @@ def self.name property :owner, Owner, title: 'Owner', description: 'The company owner' end - expected_schema = { 'type' => 'object', 'title' => 'Company', 'properties' => { 'name' => { 'type' => 'string' - }, + }, 'owner' => { 'type' => 'object', 'title' => 'Owner',