rubocop / rails-style-guide

A community-driven Ruby on Rails style guide
http://rails.rubystyle.guide
6.48k stars 1.06k forks source link

Cop suggestion: PreferCreateJoinTable #334

Open connorshea opened 5 years ago

connorshea commented 5 years ago

In a migration you can create a join table in two ways, with create_table and id: false, or create_join_table.

# These are equivalent
create_table :games_genres, id: false do |t|
  t.integer :game_id
  t.integer :genre_id
end

create_join_table :games, :genres do |t|
  t.integer :game_id
  t.integer :genre_id
end

Source: https://guides.rubyonrails.org/association_basics.html#creating-join-tables-for-has-and-belongs-to-many-associations

I'd like a cop that prefers the create_join_table syntax over id: false.

The only problem would be, I'm not sure if there's a valid reason to use id: false besides creating a join table?

mikegee commented 5 years ago

I'm not sure if there's a valid reason to use id: false besides creating a join table?

Not if the Rails developers control the database schema. But, sometimes we write migrations to mirror a an existing DB schema just for local development. In that case, I'd still be ok with an offense here. We're well aware of our deviation from standard practice when we're working in this scenario.

koic commented 1 year ago

Since this is a matter of coding style, I think it can be implemented if the style guide accepts it. I'll transfer it as a style guide issue to the repo.

pirj commented 1 year ago

create_join_table doesn't need t.integer for FKs, the original Rails Guide has t.index.

pirj commented 1 year ago

I'm not sure if there's a valid reason to use id: false besides creating a join table?

Natural PK?