Skip to content

Commit

Permalink
Merge pull request #135 from RichardBradley/master
Browse files Browse the repository at this point in the history
fix index required false-positive
  • Loading branch information
flyerhzm committed Oct 25, 2012
2 parents e7fb088 + 053dc91 commit e8db874
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 3 deletions.
21 changes: 18 additions & 3 deletions lib/rails_best_practices/reviews/always_add_db_index_review.rb
Expand Up @@ -42,6 +42,8 @@ def initialize
add_callback :start_command_call do |node|
if %w(integer string).include? node.message.to_s
remember_foreign_key_columns(node)
elsif "index" == node.message.to_s
remember_index_columns_inside_table(node)
end
end

Expand All @@ -57,7 +59,7 @@ def initialize
when "create_table"
remember_table_nodes(node)
when "add_index"
remember_index_columns(node)
remember_index_columns_outside_table(node)
end
end

Expand All @@ -81,15 +83,28 @@ def initialize
end

private
# remember the node as index columns
def remember_index_columns(node)
# remember the node as index columns, when used outside a table
# block, i.e.
# add_index :table_name, :column_name
def remember_index_columns_outside_table(node)
table_name = node.arguments.all.first.to_s
index_column = node.arguments.all[1].to_object

@index_columns[table_name] ||= []
@index_columns[table_name] << index_column
end

# remember the node as index columns, when used inside a table
# block, i.e.
# t.index [:column_name, ...]
def remember_index_columns_inside_table(node)
table_name = @table_name
index_column = node.arguments.all.first.to_object

@index_columns[table_name] ||= []
@index_columns[table_name] << index_column
end

# remember table nodes
def remember_table_nodes(node)
@table_name = node.arguments.all.first.to_s
Expand Down
Expand Up @@ -153,6 +153,24 @@ module Reviews
runner.should have(0).errors
end

it "should not always add db index with t.index" do
# e.g. schema_plus creates indices like this https://github.com/lomba/schema_plus
content = <<-EOF
ActiveRecord::Schema.define(version: 20100603080629) do
create_table "comments", force: true do |t|
t.string "content"
t.integer "post_id"
t.index ["post_id"], :name => "index_comments_on_post_id"
end
create_table "posts", force: true do |t|
end
end
EOF
runner.review('db/schema.rb', content)
runner.after_review
runner.should have(0).errors
end

it "should not always add db index with only _type column" do
content = <<-EOF
ActiveRecord::Schema.define(version: 20100603080629) do
Expand Down

0 comments on commit e8db874

Please sign in to comment.