Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails/ThreeStateBoolean fails when it can't find create_table or change_table #979

Closed
thefloweringash opened this issue Apr 10, 2023 · 3 comments · Fixed by #983
Closed
Labels
bug Something isn't working

Comments

@thefloweringash
Copy link

I have a custom create_table style function for enumerated types that also generates a database type and check constraint. This function accepts a block like create_table. Rails/ThreeStateBoolean fails when checking this.

For example

class AddCurrencies
  def create
    create_enum :currencies do |t|
      t.boolean :symbol_first, null: false
    end
  end
end
Scanning [...]/db/migrate/tsb_test.rb
An error occurred while Rails/ThreeStateBooleanColumn cop was inspecting [...]/db/migrate/tsb_test.rb:4:6.
undefined method `value' for nil:NilClass

            return if def_node && change_column_null?(def_node, table_node.value, column_node.value)
                                                                          ^^^^^^
rubocop-rails-2.19.0/lib/rubocop/cop/rails/three_state_boolean_column.rb:49:in `block in on_send'
rubocop-rails-2.19.0/lib/rubocop/cop/rails/three_state_boolean_column.rb:85:in `three_state_boolean?'
rubocop-rails-2.19.0/lib/rubocop/cop/rails/three_state_boolean_column.rb:42:in `on_send'

Expected behavior

RuboCop Rails shouldn't fail, and should not report a problem with the example code.

Actual behavior

1 error occurred:
An error occurred while Rails/ThreeStateBooleanColumn cop was inspecting [...]/db/migrate/tsb_test.rb:4:6.

Steps to reproduce the problem

Run RuboCop Rails on the example code with the Rails/ThreeStateBooleanColumn cop enabled (which seems to be the default).

RuboCop version

❯ bundle exec rubocop -V
1.49.0 (using Parser 3.2.2.0, rubocop-ast 1.28.0, running on ruby 3.1.4) [arm64-darwin22]
  - rubocop-rails 2.19.0
@GabeIsman
Copy link

I've encountered the same issue in a different context. Example:

# rubocop:disable Rails/SkipsModelValidations
class MakeNewslettersPublishable < ActiveRecord::Migration[7.0]
  class Newsletter < ApplicationRecord
  end

  class WeeklyNewsletter < ApplicationRecord
  end

  class MonthlyNewsletter < ApplicationRecord
  end

  def up_alteration(model)
    ->(t) {
      t.string :status
      t.index :status

      model.where(public: true).update_all(status: "published")
      model.where(public: false).update_all(status: "draft")

      t.remove :public
    }
  end

  def down_alteration(model)
    ->(t) {
      t.boolean :public

      model.where(status: "draft").update_all(public: false)
      model.where(status: "published").update_all(public: true)

      t.remove :status
    }
  end

  def up
    change_table :newsletters, &up_alteration(Newsletter)
    change_table :weekly_newsletters, &up_alteration(WeeklyNewsletter)
    change_table :monthly_newsletters, &up_alteration(MonthlyNewsletter)
  end

  def down
    change_table :newsletters, &down_alteration(Newsletter)
    change_table :weekly_newsletters, &down_alteration(WeeklyNewsletter)
    change_table :monthly_newsletters, &down_alteration(MonthlyNewsletter)
  end
end
# rubocop:enable Rails/SkipsModelValidations

Expected behavior


Rubocop shouldn't fail, but should report the three-state problem with the code. Although this cop is a bit awkward for legacy projects. We're never going to change these migration files, even if we do change the column default. So these files are guaranteed to sit in our rubocop_todo file forever, or we'll have to manually disable them. Maybe some way of configuring "migrations created after x date" would be a solution there?

Actual Behavior


An error occurred while Rails/ThreeStateBooleanColumn cop was inspecting db/migrate/20220713144748_make_newsletters_publishable.rb:27:6.
undefined method `value' for nil:NilClass

            return if def_node && change_column_null?(def_node, table_node.value, column_node.value)
                                                                          ^^^^^^

Rubocop Version


be rubocop -V
1.48.1 (using Parser 3.2.2.0, rubocop-ast 1.28.0, running on ruby 3.1.3) [x86_64-darwin22]
  - rubocop-performance 1.16.0
  - rubocop-rails 2.19.0
  - rubocop-rspec 2.18.1

@koic koic added the bug Something isn't working label Apr 10, 2023
@doconnor-clintel
Copy link

Third variant with drop_table:

undefined method 'value' for nil:NilClass in lib/rubocop/cop/rails/three_state_boolean_column.rb

Expected behavior

No crash

Actual behavior

Describe here what actually happened.

An error occurred while Rails/ThreeStateBooleanColumn cop was inspecting (redacted)/db/migrate/20200713024056_drop_global_codes.rb:6:6.
undefined method `value' for nil:NilClass
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.19.0/lib/rubocop/cop/rails/three_state_boolean_column.rb:49:in `block in on_send'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.19.0/lib/rubocop/cop/rails/three_state_boolean_column.rb:85:in `three_state_boolean?'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.19.0/lib/rubocop/cop/rails/three_state_boolean_column.rb:42:in `on_send'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-1.49.0/lib/rubocop/cop/commissioner.rb:143:in `public_send'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-1.49.0/lib/rubocop/cop/commissioner.rb:143:in `block (2 levels) in trigger_restricted_cops'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-1.49.0/lib/rubocop/cop/commissioner.rb:171:in `with_cop_error_handling'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-1.49.0/lib/rubocop/cop/commissioner.rb:142:in `block in trigger_restricted_cops'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-1.49.0/lib/rubocop/cop/commissioner.rb:141:in `each'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-1.49.0/lib/rubocop/cop/commissioner.rb:141:in `trigger_restricted_cops'
/home/clintel/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/rubocop-1.49.0/lib/rubocop/cop/commissioner.rb:70:in `on_send'

Steps to reproduce the problem

class DropGlobalCodes < ActiveRecord::Migration[4.2]
  def change
    drop_table :global_codes do |t|
      t.string   :code_name
      t.integer  :int_value
      t.boolean  :bool_value
      t.string   :string_value
      t.date     :date_value
      t.float    :float_value
      t.binary   :binary_value
      t.timestamps
    end
  end
end

RuboCop version

$ bundle exec rubocop -V
1.49.0 (using Parser 3.2.2.0, rubocop-ast 1.28.0, running on ruby 2.6.6) [x86_64-linux]
  - rubocop-performance 1.16.0
  - rubocop-rails 2.19.0
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.19.0

@doconnor-clintel
Copy link

Started #980 but need someone to take it over/review the approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants