From 080c36ba4802d813272dd9737df1772f8943bab8 Mon Sep 17 00:00:00 2001 From: "Jose Fernandez (magec)" Date: Thu, 22 Sep 2016 14:47:31 +0200 Subject: [PATCH] Added remove_index --- .gitignore | 1 + .rspec | 1 + README.md | 61 ++++++++---- lib/rubocop/cop/migrations/remove_index.rb | 27 ++++- lib/rubocop/migrations.rb | 9 +- rubocop-migrations.gemspec | 33 +++---- .../cop/migrations/remove_index_spec.rb | 99 +++++++++++++++++++ spec/spec_helper.rb | 8 +- spec/support/shared_examples.rb | 49 +++++++++ 9 files changed, 242 insertions(+), 46 deletions(-) create mode 100644 spec/rubocop/cop/migrations/remove_index_spec.rb create mode 100644 spec/support/shared_examples.rb diff --git a/.gitignore b/.gitignore index 0cb6eeb..4889cd5 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ /pkg/ /spec/reports/ /tmp/ +*~ diff --git a/.rspec b/.rspec index 8c18f1a..34c5164 100644 --- a/.rspec +++ b/.rspec @@ -1,2 +1,3 @@ --format documentation --color +--require spec_helper diff --git a/README.md b/README.md index 802f4c5..9327b96 100644 --- a/README.md +++ b/README.md @@ -1,41 +1,68 @@ -# Rubocop::Migrations +# RuboCop Migrations -Welcome to your new gem! In this directory, you'll find the files you need to be able to package up your Ruby library into a gem. Put your Ruby code in the file `lib/rubocop/migrations`. To experiment with that code, run `bin/console` for an interactive prompt. - -TODO: Delete this and the text above, and describe your gem +Rails migrations analysis as a extension of [RuboCop](https://github.com/bbatsov/rubocop). Heavily inspired by [`rubocop-cask`](https://github.com/caskroom/rubocop-cask) which in turn is inspired by [`rubocop-rspec`](https://github.com/nevir/rubocop-rspec). ## Installation -Add this line to your application's Gemfile: +Just install the `rubocop-migrations` gem + +```bash +gem install rubocop-migrations +``` + +or if you use bundler put this in your `Gemfile` ```ruby gem 'rubocop-migrations' ``` -And then execute: - $ bundle +## Usage -Or install it yourself as: +You need to tell RuboCop to load the Migrations extension. There are three ways to do this: - $ gem install rubocop-migrations +### RuboCop configuration file -## Usage +Put this into your `.rubocop.yml`: + +```yaml +require: rubocop/migrations +``` -TODO: Write usage instructions here +Now you can run `rubocop` and it will automatically load the RuboCop Migrations cops together with the standard cops. -## Development +### Command line -After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake spec` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. +```bash +rubocop --require rubocop/migrations +``` + +## The Cop + +All cops are located under [`lib/rubocop/cop/migrations`](lib/rubocop/cop/migrations), and contain examples/documentation. + +In your `.rubocop.yml`, you may treat the Cask cops just like any other cop. For example: -To install this gem onto your local machine, run `bundle exec rake install`. To release a new version, update the version number in `version.rb`, and then run `bundle exec rake release`, which will create a git tag for the version, push git commits and tags, and push the `.gem` file to [rubygems.org](https://rubygems.org). +```yaml +Migrations/RemoveIndex: + Enabled: false +``` ## Contributing -Bug reports and pull requests are welcome on GitHub at https://github.com/[USERNAME]/rubocop-migrations. +1. Fork it +2. Create your feature branch (`git checkout -b my-new-feature`) +3. Commit your changes (`git commit -am 'Add some feature'`) +4. Push to the branch (`git push origin my-new-feature`) +5. Create new Pull Request +For running the spec files, this project depends on RuboCop's spec helpers. This means that in order to run the specs locally, you need a (shallow) clone of the RuboCop repository: -## License +```bash +git submodule update --init --depth 1 vendor/rubocop +``` -The gem is available as open source under the terms of the [MIT License](http://opensource.org/licenses/MIT). +## License +`rubocop-migrations` is MIT licensed. [See the accompanying file](MIT-LICENSE.md) for +the full text. diff --git a/lib/rubocop/cop/migrations/remove_index.rb b/lib/rubocop/cop/migrations/remove_index.rb index 41f9035..366a6b6 100644 --- a/lib/rubocop/cop/migrations/remove_index.rb +++ b/lib/rubocop/cop/migrations/remove_index.rb @@ -1,8 +1,31 @@ -module Rubocop +module RuboCop module Cop module Migrations - # Your code goes here... + # Removing indexes is dangerous by nature, if the index you remove + # is used extensively, you could end up with database degradation. + # + # This disallow the use of remove_index at all when. It only allows it + # inside a down method definition. If you actually know what you are doing + # (usually when the index is not being used) you should explicitly disable + # the check by adding a ``rubocop:disable Migrations/RemoveIndex`` comment + # in the offending line class RemoveIndex < Cop + MSG = 'remove_index is disalowed'.freeze + + def_node_matcher :remove_index_found, <<-PATTERN + (:send _ :remove_index ...) + PATTERN + + def on_send(node) + remove_index_found(node) do + node.each_ancestor do |a| + next unless a.def_type? + if a.source =~ /def up/ || a.source =~ /def change/ + add_offense(node, :selector, MSG) + end + end + end + end end end end diff --git a/lib/rubocop/migrations.rb b/lib/rubocop/migrations.rb index 984b368..1f4f164 100644 --- a/lib/rubocop/migrations.rb +++ b/lib/rubocop/migrations.rb @@ -1,7 +1,4 @@ -require "rubocop/migrations/version" +require 'rubocop' -module Rubocop - module Migrations - # Your code goes here... - end -end +# cops +require 'rubocop/cop/migrations/remove_index' diff --git a/rubocop-migrations.gemspec b/rubocop-migrations.gemspec index 676666e..b2343b1 100644 --- a/rubocop-migrations.gemspec +++ b/rubocop-migrations.gemspec @@ -4,32 +4,25 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'rubocop/migrations/version' Gem::Specification.new do |spec| - spec.name = "rubocop-migrations" + spec.name = 'rubocop-migrations' spec.version = Rubocop::Migrations::VERSION - spec.authors = ["enricostano"] - spec.email = ["ocirne.onats@gmail.com"] - spec.summary = "" - spec.description = "" - spec.homepage = "" - spec.license = "MIT" + spec.authors = %w(enricostano magec) + spec.email = %w(ocirne.onats@gmail.com joseferper@gmail.com) - # Prevent pushing this gem to RubyGems.org by setting 'allowed_push_host', or - # delete this section to allow pushing this gem to any host. - if spec.respond_to?(:metadata) - spec.metadata['allowed_push_host'] = "TODO: Set to 'http://mygemserver.com'" - else - raise "RubyGems 2.0 or newer is required to protect against public gem pushes." - end + spec.summary = 'Rails migrations rubocop checks' + spec.description = 'Performs some checks for migration sanity' + spec.homepage = 'https://github.com/redbooth/rubocop-migrations' + spec.license = 'MIT' - spec.files = `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) } - spec.bindir = "exe" + spec.files = `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) } # rubocop:disable Metrics/LineLength + 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 'rubocop', '>= 0.42.0' - spec.add_development_dependency "bundler", "~> 1.11" - spec.add_development_dependency "rake", "~> 10.0" - spec.add_development_dependency "rspec", "3.5.0" + spec.add_development_dependency 'bundler', '~> 1.11' + spec.add_development_dependency 'rake', '~> 10.0' + spec.add_development_dependency 'rspec', '3.5.0' end diff --git a/spec/rubocop/cop/migrations/remove_index_spec.rb b/spec/rubocop/cop/migrations/remove_index_spec.rb new file mode 100644 index 0000000..dddfbd8 --- /dev/null +++ b/spec/rubocop/cop/migrations/remove_index_spec.rb @@ -0,0 +1,99 @@ +describe RuboCop::Cop::Migrations::RemoveIndex do + include SharedExamples + subject(:cop) { described_class.new } + + context 'when there is a remove_index' do + context 'in the up method' do + let(:source) do + <<-MIGRATION + class RemoveSomeIndex < ActiveRecord::Migration + def up + remove_index :some_table, :some_index + end + end + MIGRATION + end + + let(:expected_offenses) do + [ + { + message: 'remove_index is disalowed', + severity: :convention, + line: 3, + column: 14, + source: 'remove_index' + } + ] + end + + include_examples 'reports offenses' + + context 'using method_call syntax' do + let(:source) do + <<-MIGRATION + class RemoveSomeIndex < ActiveRecord::Migration + def up + change_table :some_table do |m| + m.remove_index [:some_index] + end + end + end + MIGRATION + end + let(:expected_offenses) do + [ + { + message: 'remove_index is disalowed', + severity: :convention, + line: 4, + column: 18, + source: 'remove_index' + } + ] + end + include_examples 'reports offenses' + end + end + + context 'in the change method' do + let(:source) do + <<-MIGRATION + class RemoveSomeIndex < ActiveRecord::Migration + def change + remove_index :some_table, :some_index + end + end + MIGRATION + + end + let(:expected_offenses) do + [ + { + message: 'remove_index is disalowed', + severity: :convention, + line: 3, + column: 14, + source: 'remove_index' + } + ] + end + + include_examples 'reports offenses' + end + + context 'in the down method' do + let(:source) do + <<-MIGRATION + class RemoveSomeIndex < ActiveRecord::Migration + def down + remove_index :some_table, :some_index + end + end + MIGRATION + end + + include_examples 'does not report any offenses' + end + + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9a3b2f0..78cb804 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,2 +1,8 @@ -$LOAD_PATH.unshift File.expand_path('../../lib', __FILE__) +$LOAD_PATH.unshift File.expand_path('../lib', __FILE__) +require 'rubocop' +require 'rubocop/rspec/support' + +project_path = File.join(File.dirname(__FILE__), '..') +Dir["#{project_path}/spec/support/**/*.rb"].each { |f| require f } + require 'rubocop/migrations' diff --git a/spec/support/shared_examples.rb b/spec/support/shared_examples.rb new file mode 100644 index 0000000..6f534c1 --- /dev/null +++ b/spec/support/shared_examples.rb @@ -0,0 +1,49 @@ +# inspired on https://github.com/caskroom/rubocop-cask/blob/master/spec/support/cop_shared_examples.rb +# sed 's/inspired on/copied from/g +# +module SharedExamples + shared_examples 'does not report any offenses' do + it 'does not report any offenses' do + expect_no_offenses(cop, source) + end + end + + shared_examples 'reports offenses' do + it 'reports offenses' do + expect_reported_offenses(cop, source, expected_offenses) + end + end + + shared_examples 'autocorrects source' do + it 'autocorrects source' do + expect_autocorrected_source(cop, source, correct_source) + end + end + + def expect_no_offenses(cop, source) + inspect_source(cop, source) + expect(cop.offenses).to be_empty + end + + def expect_reported_offenses(cop, source, expected_offenses) + inspect_source(cop, source) + expect(cop.offenses.size).to eq(expected_offenses.size) + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + # rubocop:disable Metrics/AbcSize + def expect_offense(expected, actual) + expect(actual.message).to eq(expected[:message]) + expect(actual.severity).to eq(expected[:severity]) + expect(actual.line).to eq(expected[:line]) + expect(actual.column).to eq(expected[:column]) + expect(actual.location.source).to eq(expected[:source]) + end + + def expect_autocorrected_source(cop, source, correct_source) + new_source = autocorrect_source(cop, source) + expect(new_source).to eq(Array(correct_source).join("\n")) + end +end