Regression in v.1.63.3

[AmShaegar13]

#12848 (comment) mentions a problem with bundler not being used anymore if rubocop is run globally. This indeed causes a regression with e.g. rubocop-rails, see https://github.com/rubocop/rubocop-rails/blob/28b274bcd14da7d8c861aaf9dcdbe165d9fc28f2/lib/rubocop/cop/rails/reversible_migration.rb#L293C1-L293C84

I suspect target_rails_version to not return the installed Rails version anymore. Which makes sense because the Gemfile.lock is not parsed anymore. However, we use rubocop globally in CI. Why would I want to install the bundle everytime?

I don't think that this change should be made quietly in a patch version update. If this is intentional, this is a big thing that should at least be announced.

---

Expected behavior

sh-5.2$ rubocop _1.63.1_ db/migrate/20220915105426_remove_columns_from_order_items.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

sh-5.2$ bundle exec rubocop db/migrate/20220915105426_remove_columns_from_order_items.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

Actual behavior

sh-5.2$ rubocop _1.63.3_ db/migrate/20220915105426_remove_columns_from_order_items.rb
Inspecting 1 file
C

Offenses:

db/migrate/20220915105426_remove_columns_from_order_items.rb:5:7: C: Rails/ReversibleMigration: t.remove is not reversible.
      t.remove :delivery_date, type: :datetime
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

Steps to reproduce the problem

sh-5.2$ cat db/migrate/20220915105426_remove_columns_from_order_items.rb
class RemoveColumnsFromOrderItems < ActiveRecord::Migration[7.0]
  def change
    change_table :order_items, bulk: true do |t|
      t.remove :delivery_date, type: :datetime
    end
  end
end

RuboCop version

sh-5.2$ rubocop _1.63.3_ -V
1.63.3 (using Parser 3.3.0.5, rubocop-ast 1.31.2, running on ruby 3.3.0) [x86_64-linux]
  - rubocop-factory_bot 2.25.1
  - rubocop-performance 1.21.0
  - rubocop-rails 2.24.1
  - rubocop-rspec 2.29.1

sh-5.2$ bundle exec rubocop -V
1.63.3 (using Parser 3.3.0.5, rubocop-ast 1.31.2, running on ruby 3.3.0) [x86_64-linux]
  - rubocop-factory_bot 2.25.1
  - rubocop-performance 1.21.0
  - rubocop-rails 2.24.1
  - rubocop-rspec 2.29.1

[bbatsov]

Likely it was a regression from the fix of another regression. We've been plagued by different forms of the same problem since 1.63 was released and it seems that every fix creates another problem.

//cc @amomchilov @koic

[AmShaegar13]

Yea, I thought so yesterday when we faced the 'uninitialized constant Bundler' problem. I hope this helps.

[amomchilov]

Oh boy, a fractal of regressions. Sorry for causing this trouble, I had done a pretty through job at manually testing these changes at different times, but there's so many scenarios to consider, and it needs to be retested after even the smallest tweaks.

Is there a way we can add tests to characterize all the various ways that Rubocop can be called? Off the top of my head:

1. With a Gemfile, running outside bundler
2. With a Gemfile, running with bundler
3. Without a Gemfile, running outside bundler
4. Without a Gemfile, running with bundler

---

As for what OP is describing here, I think the correct approach would be to always try to require "bundler/lockfile_parser" whenever possible. However, this file can't be required on its own, and needs a bunch of supporting files. Which files are needed, and what order they need to be required in, is completely implementation dependant. This is why I ended up just doing require "bundler", but that's didn't work because of the side-effects that causes. (Update, not true! See my comment below)

[amomchilov]

Interestingly, the Bundler docs confirm the idea that it can be used "as a library":

# As a standard library inside project, Bundler could be used for introspection
# of loaded and required modules.

[amomchilov]

OOOOOHHHHHHhhhhhh. The issue isn't with require "Bundler". That always succeeds, even in a folder without a Gemfile(.lock).

The issue in #12846 was from trying to call Bundler.default_lockfile outside of a context that has a lockfile. See #12846 (comment) for details.

I think a simple rescue Bundler::GemfileNotFound can fix this. I've opened a PR, but will need assistance from others to test and merge it. #12866

[AmShaegar13]

Thanks a lot!