From 5abeb29808a3c2ba373c88cc48f85c33cb97ad88 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 8 Apr 2020 20:34:12 +0900 Subject: [PATCH] [Fix #227] Make `Rails/UniqueValidationWithoutIndex` aware of updating schema.rb Fixes #227. This PR makes `Rails/UniqueValidationWithoutIndex` aware of updating db/schema.rb `Rails/UniqueValidationWithoutIndex` cop needs to know both model and db/schema.rb changes to register an offense. However, with default RuboCop, only changes to the model affect cache behavior. This PR ensures that changes to db/schema.rb affect the cache by overriding the following method: ```ruby # This method should be overridden when a cop's behavior depends # on state that lives outside of these locations: # # (1) the file under inspection # (2) the cop's source code # (3) the config (eg a .rubocop.yml file) # # For example, some cops may want to look at other parts of # the codebase being inspected to find violations. A cop may # use the presence or absence of file `foo.rb` to determine # whether a certain violation exists in `bar.rb`. # # Overriding this method allows the cop to indicate to RuboCop's # ResultCache system when those external dependencies change, # ie when the ResultCache should be invalidated. def external_dependency_checksum nil end ``` https://github.com/rubocop-hq/rubocop/blob/v0.81.0/lib/rubocop/cop/cop.rb#L222-L239 --- CHANGELOG.md | 4 ++ lib/rubocop/cop/mixin/active_record_helper.rb | 17 +++++++ .../rails/unique_validation_without_index.rb | 4 -- lib/rubocop/rails/schema_loader.rb | 20 ++++---- spec/rubocop/cop/active_record_helper_spec.rb | 49 +++++++++++++++++++ 5 files changed, 80 insertions(+), 14 deletions(-) create mode 100644 spec/rubocop/cop/active_record_helper_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 85d326203b..4b94c41a3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### Bug fixes + +* [#227](https://github.com/rubocop-hq/rubocop-rails/issues/227): Make `Rails/UniqueValidationWithoutIndex` aware of updating db/schema.rb. ([@koic][]) + ## 2.5.1 (2020-04-02) ### Bug fixes diff --git a/lib/rubocop/cop/mixin/active_record_helper.rb b/lib/rubocop/cop/mixin/active_record_helper.rb index 90b86dd815..2dd044c0a9 100644 --- a/lib/rubocop/cop/mixin/active_record_helper.rb +++ b/lib/rubocop/cop/mixin/active_record_helper.rb @@ -14,6 +14,23 @@ module ActiveRecordHelper (send nil? :belongs_to {str sym} ...) PATTERN + def external_dependency_checksum + if defined?(@external_dependency_checksum) + return @external_dependency_checksum + end + + schema_path = RuboCop::Rails::SchemaLoader.db_schema_path + return nil if schema_path.nil? + + schema_code = File.read(schema_path) + + @external_dependency_checksum ||= Digest::SHA1.hexdigest(schema_code) + end + + def schema + RuboCop::Rails::SchemaLoader.load(target_ruby_version) + end + def table_name(class_node) table_name = find_set_table_name(class_node).to_a.last&.first_argument return table_name.value.to_s if table_name diff --git a/lib/rubocop/cop/rails/unique_validation_without_index.rb b/lib/rubocop/cop/rails/unique_validation_without_index.rb index 8cbf4759c9..ebd1b1087a 100644 --- a/lib/rubocop/cop/rails/unique_validation_without_index.rb +++ b/lib/rubocop/cop/rails/unique_validation_without_index.rb @@ -149,10 +149,6 @@ def array_node_to_array(node) end end end - - def schema - RuboCop::Rails::SchemaLoader.load(target_ruby_version) - end end end end diff --git a/lib/rubocop/rails/schema_loader.rb b/lib/rubocop/rails/schema_loader.rb index 7f59f19867..8025496d6a 100644 --- a/lib/rubocop/rails/schema_loader.rb +++ b/lib/rubocop/rails/schema_loader.rb @@ -24,16 +24,6 @@ def reset! remove_instance_variable(:@schema) end - private - - def load!(target_ruby_version) - path = db_schema_path - return unless path - - ast = parse(path, target_ruby_version) - Schema.new(ast) - end - def db_schema_path path = Pathname.pwd until path.root? @@ -46,6 +36,16 @@ def db_schema_path nil end + private + + def load!(target_ruby_version) + path = db_schema_path + return unless path + + ast = parse(path, target_ruby_version) + Schema.new(ast) + end + def parse(path, target_ruby_version) klass_name = :"Ruby#{target_ruby_version.to_s.sub('.', '')}" klass = ::Parser.const_get(klass_name) diff --git a/spec/rubocop/cop/active_record_helper_spec.rb b/spec/rubocop/cop/active_record_helper_spec.rb new file mode 100644 index 0000000000..8162d946b1 --- /dev/null +++ b/spec/rubocop/cop/active_record_helper_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::ActiveRecordHelper, :config do + include FileHelper + + module RuboCop + module Cop + class Test < Cop + include ActiveRecordHelper + end + end + end + + let(:cop) do + RuboCop::Cop::Test.new + end + + let(:schema_path) { 'db/schema.rb' } + + after { FileUtils.rm_rf(schema_path) } + + describe '#external_dependency_checksum' do + subject { cop.external_dependency_checksum } + + context 'with db/schema.rb' do + before do + create_file(schema_path, <<~RUBY) + ActiveRecord::Schema.define(version: 2020_04_08_082625) do + create_table "tests" do |t| + t.string "foo" + end + end + RUBY + end + + it { is_expected.to eq 'eb198d7aa709c189fec9f41bc74da07843b4d67a' } + end + + context 'with empty db/schema.rb' do + before { create_empty_file(schema_path) } + + it { is_expected.to eq 'adc83b19e793491b1c6ea0fd8b46cd9f32e592fc' } + end + + context 'without db/schema.rb' do + it { is_expected.to be_nil } + end + end +end