From 9d6bf2a48902a8061a0fed8677a602f196a43d4d Mon Sep 17 00:00:00 2001 From: Tim Perkins Date: Thu, 19 May 2016 15:47:21 -0400 Subject: [PATCH] Add RuboCop and Overcommit (#19) --- .overcommit.yml | 13 ++ .rubocop.yml | 5 + Rakefile | 6 +- avro-builder.gemspec | 30 ++--- bin/setup | 3 +- lib/avro/builder/dsl_attributes.rb | 2 +- lib/avro/builder/errors.rb | 6 +- lib/avro/builder/field.rb | 2 +- lib/avro/builder/file_handler.rb | 2 +- lib/avro/builder/type_factory.rb | 5 +- lib/avro/builder/types/configurable_type.rb | 2 +- lib/avro/builder/types/record_type.rb | 4 +- lib/avro/builder/types/type.rb | 2 - lib/avro/builder/version.rb | 2 +- spec/avro/builder_spec.rb | 125 ++++++++++---------- 15 files changed, 113 insertions(+), 96 deletions(-) create mode 100644 .overcommit.yml create mode 100644 .rubocop.yml diff --git a/.overcommit.yml b/.overcommit.yml new file mode 100644 index 0000000..017f51e --- /dev/null +++ b/.overcommit.yml @@ -0,0 +1,13 @@ +PreCommit: + RuboCop: + enabled: true + required: false + on_warn: fail + + HardTabs: + enabled: true + required: false + +CommitMsg: + TrailingPeriod: + enabled: false diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..0444897 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,5 @@ +inherit_gem: + salsify_rubocop: conf/rubocop.yml + +Style/RaiseArgs: + EnforcedStyle: compact diff --git a/Rakefile b/Rakefile index b7e9ed5..4c774a2 100644 --- a/Rakefile +++ b/Rakefile @@ -1,6 +1,6 @@ -require "bundler/gem_tasks" -require "rspec/core/rake_task" +require 'bundler/gem_tasks' +require 'rspec/core/rake_task' RSpec::Core::RakeTask.new(:spec) -task :default => :spec +task default: :spec diff --git a/avro-builder.gemspec b/avro-builder.gemspec index 2f48c90..4e795b7 100644 --- a/avro-builder.gemspec +++ b/avro-builder.gemspec @@ -4,26 +4,28 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'avro/builder/version' Gem::Specification.new do |spec| - spec.name = "avro-builder" + spec.name = 'avro-builder' spec.version = Avro::Builder::VERSION - spec.authors = ["Salsify Engineering"] - spec.email = ["engineering@salsify.com"] + spec.authors = ['Salsify Engineering'] + spec.email = ['engineering@salsify.com'] - spec.summary = %q{Ruby DSL to create Avro schemas} + spec.summary = 'Ruby DSL to create Avro schemas' spec.description = spec.summary - spec.homepage = "https://github.com/salsify/avro-builder.git" - spec.license = "MIT" + spec.homepage = 'https://github.com/salsify/avro-builder.git' + spec.license = 'MIT' spec.files = `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) } - spec.bindir = "exe" + spec.bindir = 'exe' spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } - spec.require_paths = ["lib"] + spec.require_paths = ['lib'] - spec.add_runtime_dependency "avro", ">= 1.7.0" + spec.add_runtime_dependency 'avro', '>= 1.7.0' - spec.add_development_dependency "bundler", "~> 1.11" - spec.add_development_dependency "rake", "~> 10.0" - spec.add_development_dependency "rspec", "~> 3.0" - spec.add_development_dependency "json_spec" - spec.add_development_dependency "simplecov" + spec.add_development_dependency 'bundler', '~> 1.11' + spec.add_development_dependency 'rake', '~> 10.0' + spec.add_development_dependency 'rspec', '~> 3.0' + spec.add_development_dependency 'json_spec' + spec.add_development_dependency 'simplecov' + spec.add_development_dependency 'salsify_rubocop', '~> 0.40.0' + spec.add_development_dependency 'overcommit' end diff --git a/bin/setup b/bin/setup index dce67d8..af08091 100755 --- a/bin/setup +++ b/bin/setup @@ -4,5 +4,4 @@ IFS=$'\n\t' set -vx bundle install - -# Do any other automated setup that you need to do here +overcommit --install diff --git a/lib/avro/builder/dsl_attributes.rb b/lib/avro/builder/dsl_attributes.rb index 08e50f1..a7af769 100644 --- a/lib/avro/builder/dsl_attributes.rb +++ b/lib/avro/builder/dsl_attributes.rb @@ -19,7 +19,7 @@ def self.included(base) base.extend ClassMethods end - def has_dsl_attribute?(name) + def dsl_attribute?(name) self.class.dsl_attribute_names.include?(name.to_sym) end diff --git a/lib/avro/builder/errors.rb b/lib/avro/builder/errors.rb index afa17aa..3788312 100644 --- a/lib/avro/builder/errors.rb +++ b/lib/avro/builder/errors.rb @@ -28,15 +28,13 @@ def to_json(object) class DefinitionNotFoundError < StandardError def initialize(name) - super("definition not found for '#{name.to_s}'.#{suggest_namespace(name)}") + super("definition not found for '#{name}'.#{suggest_namespace(name)}") end private def suggest_namespace(name) - unless name.to_s.index('.') - ' Try specifying the full namespace.' - end + ' Try specifying the full namespace.' unless name.to_s.index('.') end end end diff --git a/lib/avro/builder/field.rb b/lib/avro/builder/field.rb index 9c0ad01..a5d2c24 100644 --- a/lib/avro/builder/field.rb +++ b/lib/avro/builder/field.rb @@ -24,7 +24,7 @@ def initialize(name:, type_name:, record:, cache:, internal: {}, options: {}, &b end options.each do |key, value| - send(key, value) if has_dsl_attribute?(key) + send(key, value) if dsl_attribute?(key) end @type = if builtin_type?(type_name) diff --git a/lib/avro/builder/file_handler.rb b/lib/avro/builder/file_handler.rb index 60eb28c..65babf4 100644 --- a/lib/avro/builder/file_handler.rb +++ b/lib/avro/builder/file_handler.rb @@ -26,7 +26,7 @@ def find_file(name) # and ends with a .rb extension. Additionally, if the name contains # a namespace then ensure that periods (.) are replaced by forward # slashes. E.g. for 'test.example' search for '/test/example.rb'. - file_name = "/#{name.to_s.gsub('.', '/').sub(/^\//, '').sub(/\.rb$/, '')}.rb" + file_name = "/#{name.to_s.tr('.', '/').sub(/^\//, '').sub(/\.rb$/, '')}.rb" matches = self.class.load_paths.flat_map do |load_path| Dir["#{load_path}/**/*.rb"].select do |file_path| file_path.end_with?(file_name) diff --git a/lib/avro/builder/type_factory.rb b/lib/avro/builder/type_factory.rb index bd3a987..8846728 100644 --- a/lib/avro/builder/type_factory.rb +++ b/lib/avro/builder/type_factory.rb @@ -12,10 +12,9 @@ module TypeFactory # Return a new Type instance def create_builtin_type(type_name, field:, cache:) name = type_name.to_s.downcase - case - when Avro::Schema::PRIMITIVE_TYPES.include?(name) + if Avro::Schema::PRIMITIVE_TYPES.include?(name) Avro::Builder::Types::Type.new(name, field: field, cache: cache) - when COMPLEX_TYPES.include?(name) + elsif COMPLEX_TYPES.include?(name) Avro::Builder::Types.const_get("#{name.capitalize}Type").new(field: field, cache: cache) else raise "Invalid builtin type: #{type_name}" diff --git a/lib/avro/builder/types/configurable_type.rb b/lib/avro/builder/types/configurable_type.rb index 07546bc..3261257 100644 --- a/lib/avro/builder/types/configurable_type.rb +++ b/lib/avro/builder/types/configurable_type.rb @@ -7,7 +7,7 @@ module Types module ConfigurableType def configure_options(options = {}) options.each do |key, value| - send(key, value) if has_dsl_attribute?(key) + send(key, value) if dsl_attribute?(key) end end end diff --git a/lib/avro/builder/types/record_type.rb b/lib/avro/builder/types/record_type.rb index df47755..e6a396e 100644 --- a/lib/avro/builder/types/record_type.rb +++ b/lib/avro/builder/types/record_type.rb @@ -1,8 +1,8 @@ module Avro module Builder module Types - # This class represents a record in an Avro schema. Records may be defined - # at the top-level or as the type for a field in a record. + # This class represents a record in an Avro schema. Records may be defined + # at the top-level or as the type for a field in a record. class RecordType < Avro::Builder::Types::NamedType dsl_attributes :doc diff --git a/lib/avro/builder/types/type.rb b/lib/avro/builder/types/type.rb index 3a9c396..feb5692 100644 --- a/lib/avro/builder/types/type.rb +++ b/lib/avro/builder/types/type.rb @@ -58,8 +58,6 @@ def validate_required_attribute!(attribute_name) end end - private - attr_accessor :field, :cache end end diff --git a/lib/avro/builder/version.rb b/lib/avro/builder/version.rb index bafe9d2..80c71d7 100644 --- a/lib/avro/builder/version.rb +++ b/lib/avro/builder/version.rb @@ -1,5 +1,5 @@ module Avro module Builder - VERSION = "0.5.0" + VERSION = '0.5.0'.freeze end end diff --git a/spec/avro/builder_spec.rb b/spec/avro/builder_spec.rb index f16554c..776cd23 100644 --- a/spec/avro/builder_spec.rb +++ b/spec/avro/builder_spec.rb @@ -124,9 +124,9 @@ end it "is invalid" do - expect { subject }. - to raise_error(Avro::Builder::RequiredAttributeError, - "attribute :symbols missing for 'broken_enum' of type :enum") + expect { subject } + .to raise_error(Avro::Builder::RequiredAttributeError, + "attribute :symbols missing for 'broken_enum' of type :enum") end end @@ -140,9 +140,9 @@ end it "is invalid" do - expect { subject }. - to raise_error(Avro::Builder::RequiredAttributeError, - 'attribute :name missing for type :enum') + expect { subject } + .to raise_error(Avro::Builder::RequiredAttributeError, + 'attribute :name missing for type :enum') end end @@ -227,9 +227,9 @@ end it "is invalid" do - expect { subject }. - to raise_error(Avro::Builder::RequiredAttributeError, - "attribute :size missing for 'broken_fixed' of type :fixed") + expect { subject } + .to raise_error(Avro::Builder::RequiredAttributeError, + "attribute :size missing for 'broken_fixed' of type :fixed") end end @@ -243,9 +243,9 @@ end it "is invalid" do - expect { subject }. - to raise_error(Avro::Builder::RequiredAttributeError, - 'attribute :name missing for type :fixed') + expect { subject } + .to raise_error(Avro::Builder::RequiredAttributeError, + 'attribute :name missing for type :fixed') end end @@ -505,7 +505,8 @@ name: :maybe_enum, default: nil, type: [:null, - { name: :e, type: :enum, symbols: %i(A B), namespace: 'com.example' }] }, + { name: :e, type: :enum, symbols: %i(A B), namespace: 'com.example' }] + }, { name: :must_enum, type: 'com.example.e' } ] } @@ -630,9 +631,9 @@ end it "is invalid" do - expect { subject }. - to raise_error(Avro::Builder::RequiredAttributeError, - 'attribute :name missing for type :record') + expect { subject } + .to raise_error(Avro::Builder::RequiredAttributeError, + 'attribute :name missing for type :record') end end @@ -763,9 +764,9 @@ end it "is invalid" do - expect { subject }. - to raise_error(Avro::Builder::RequiredAttributeError, - "attribute :types missing for field 'u' of type :union") + expect { subject } + .to raise_error(Avro::Builder::RequiredAttributeError, + "attribute :types missing for field 'u' of type :union") end end @@ -821,9 +822,9 @@ end it "is invalid" do - expect { subject }. - to raise_error(Avro::Builder::RequiredAttributeError, - "attribute :values missing for field 'm' of type :map") + expect { subject } + .to raise_error(Avro::Builder::RequiredAttributeError, + "attribute :values missing for field 'm' of type :map") end end @@ -879,9 +880,9 @@ end it "is invalid" do - expect { subject }. - to raise_error(Avro::Builder::RequiredAttributeError, - "attribute :items missing for field 'a' of type :array") + expect { subject } + .to raise_error(Avro::Builder::RequiredAttributeError, + "attribute :items missing for field 'a' of type :array") end end @@ -931,13 +932,15 @@ namespace: 'com.example.B', type: :record, fields: [ - { name: :sub, - type: { + { + name: :sub, + type: { name: :sub_rec, namespace: 'com.example.A', type: :record, fields: [{ name: :i, type: :int }] - } } + } + } ] } end @@ -962,14 +965,14 @@ namespace: 'com.example', type: :record, fields: [{ - name: :nested, - type: { - name: :__my_rec_nested_record, - namespace: 'com.example', - type: :record, - fields: [{ name: :s, type: :string }] - } - }] + name: :nested, + type: { + name: :__my_rec_nested_record, + namespace: 'com.example', + type: :record, + fields: [{ name: :s, type: :string }] + } + }] } end it { is_expected.to be_json_eql(expected.to_json) } @@ -994,14 +997,14 @@ namespace: 'com.example', type: :record, fields: [{ - name: :nested, - type: { - name: :__my_rec_nested_record, - namespace: 'com.example.sub', - type: :record, - fields: [{ name: :s, type: :string }] - } - }] + name: :nested, + type: { + name: :__my_rec_nested_record, + namespace: 'com.example.sub', + type: :record, + fields: [{ name: :s, type: :string }] + } + }] } end it { is_expected.to be_json_eql(expected.to_json) } @@ -1031,23 +1034,23 @@ namespace: 'com.example', type: :record, fields: [{ - name: :B, - type: { - name: :__A_B_record, - namespace: 'com.example', - type: :record, - fields: [{ - name: :C, - type: { - name: :__A_B_C_record, - namespace: 'com.example', - type: :record, - fields: [{ name: :s, type: :string }, - { name: :i, type: [:null, :int], default: nil }] - } - }] - } - }, + name: :B, + type: { + name: :__A_B_record, + namespace: 'com.example', + type: :record, + fields: [{ + name: :C, + type: { + name: :__A_B_C_record, + namespace: 'com.example', + type: :record, + fields: [{ name: :s, type: :string }, + { name: :i, type: [:null, :int], default: nil }] + } + }] + } + }, { name: :C, type: {