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

thefloweringash · 2023-04-10T10:45:39Z

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

The text was updated successfully, but these errors were encountered:

GabeIsman · 2023-04-10T15:44:11Z

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

doconnor-clintel · 2023-04-10T23:32:05Z

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

Fix rubocop#979

doconnor-clintel · 2023-04-10T23:38:34Z

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

Fixes rubocop#979. This PR fixes an error for `Rails/ThreeStateBooleanColumn` when using `t.boolean` in `drop_table`.

…boolean [Fix #979] Fix an error for `Rails/ThreeStateBooleanColumn`

koic added the bug Something isn't working label Apr 10, 2023

doconnor-clintel added a commit to doconnor-clintel/rubocop-rails that referenced this issue Apr 10, 2023

Address crashes with three state boolean detection

1a04f06

Fix rubocop#979

doconnor-clintel mentioned this issue Apr 10, 2023

Fix #979 Address crashes with three state boolean detection #980

Closed

8 tasks

koic mentioned this issue Apr 11, 2023

Rails/ThreeStateBooleanColumn undefined method `value' for nil:NilClass #981

Closed

hellcp-work mentioned this issue Apr 11, 2023

[src/api] Update rubocop-rails: 2.18.0 → 2.19.0 (minor) openSUSE/open-build-service#14122

Closed

TastyPi mentioned this issue Apr 11, 2023

Rails/ThreeStateBooleanColumn error when checking Rails drop_table migration with boolean column rubocop/rubocop#11785

Closed

koic added a commit to koic/rubocop-rails that referenced this issue Apr 13, 2023

[Fix rubocop#979] Fix an error for Rails/ThreeStateBooleanColumn

d098903

Fixes rubocop#979. This PR fixes an error for `Rails/ThreeStateBooleanColumn` when using `t.boolean` in `drop_table`.

koic mentioned this issue Apr 13, 2023

[Fix #979] Fix an error for Rails/ThreeStateBooleanColumn #983

Merged

9 tasks

koic closed this as completed in #983 Apr 13, 2023

koic added a commit that referenced this issue Apr 13, 2023

Merge pull request #983 from koic/fix_an_error_for_rails_three_state_…

a889677

…boolean [Fix #979] Fix an error for `Rails/ThreeStateBooleanColumn`

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

thefloweringash commented Apr 10, 2023

GabeIsman commented Apr 10, 2023

doconnor-clintel commented Apr 10, 2023

doconnor-clintel commented Apr 10, 2023

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

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

Comments

thefloweringash commented Apr 10, 2023

Expected behavior

Actual behavior

Steps to reproduce the problem

RuboCop version

GabeIsman commented Apr 10, 2023

Expected behavior

Actual Behavior

Rubocop Version

doconnor-clintel commented Apr 10, 2023

Expected behavior

Actual behavior

Steps to reproduce the problem

RuboCop version

doconnor-clintel commented Apr 10, 2023