From 41d69b59d198be377b39d7d77341e97dab210d36 Mon Sep 17 00:00:00 2001 From: Louis-Michel Couture Date: Fri, 21 Jun 2024 02:24:24 -0400 Subject: [PATCH 1/6] Make Rails app detection based on Rails::Application superclass This change makes the Rails app detection based on the superclass of the main class being `Rails::Application`. This is a more reliable way to detect Rails apps than looking for the presence of the `rails` gem in the Gemfile. Application can require some of the Rails components without requiring the `rails` gem itself. co-authored-by: Earlopain <14981592+Earlopain@users.noreply.github.com> --- lib/ruby_lsp/setup_bundler.rb | 19 +++++++++++--- test/setup_bundler_test.rb | 49 ++++++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/lib/ruby_lsp/setup_bundler.rb b/lib/ruby_lsp/setup_bundler.rb index 55f5fe059..903f43a3d 100644 --- a/lib/ruby_lsp/setup_bundler.rb +++ b/lib/ruby_lsp/setup_bundler.rb @@ -50,6 +50,7 @@ def initialize(project_path, **options) @last_updated_path = T.let(@custom_dir + "last_updated", Pathname) @dependencies = T.let(load_dependencies, T::Hash[String, T.untyped]) + @rails_app = T.let(rails_app?, T::Boolean) @retry = T.let(false, T::Boolean) end @@ -62,7 +63,7 @@ def setup! # Do not set up a custom bundle if LSP dependencies are already in the Gemfile if @dependencies["ruby-lsp"] && @dependencies["debug"] && - (@dependencies["rails"] ? @dependencies["ruby-lsp-rails"] : true) + (@rails_app ? @dependencies["ruby-lsp-rails"] : true) $stderr.puts( "Ruby LSP> Skipping custom bundle setup since LSP dependencies are already in #{@gemfile}", ) @@ -148,7 +149,7 @@ def write_custom_gemfile parts << 'gem "debug", require: false, group: :development, platforms: :mri' end - if @dependencies["rails"] && !@dependencies["ruby-lsp-rails"] + if @rails_app && !@dependencies["ruby-lsp-rails"] parts << 'gem "ruby-lsp-rails", require: false, group: :development' end @@ -209,7 +210,7 @@ def run_bundle_install(bundle_gemfile = @gemfile) command << " && bundle update " command << "ruby-lsp " unless @dependencies["ruby-lsp"] command << "debug " unless @dependencies["debug"] - command << "ruby-lsp-rails " if @dependencies["rails"] && !@dependencies["ruby-lsp-rails"] + command << "ruby-lsp-rails " if @rails_app && !@dependencies["ruby-lsp-rails"] command << "--pre" if @experimental command.delete_suffix!(" ") command << ")" @@ -244,7 +245,7 @@ def run_bundle_install(bundle_gemfile = @gemfile) def should_bundle_update? # If `ruby-lsp`, `ruby-lsp-rails` and `debug` are in the Gemfile, then we shouldn't try to upgrade them or else it # will produce version control changes - if @dependencies["rails"] + if @rails_app return false if @dependencies.values_at("ruby-lsp", "ruby-lsp-rails", "debug").all? # If the custom lockfile doesn't include `ruby-lsp`, `ruby-lsp-rails` or `debug`, we need to run bundle install @@ -280,5 +281,15 @@ def correct_relative_remote_paths @custom_lockfile.write(content) end + + # Detects if the project is a Rails app by looking if the superclass of the main class is `Rails::Application` + sig { returns(T::Boolean) } + def rails_app? + config = Pathname.new("config/application.rb").expand_path + application_contents = config.read if config.exist? + return false unless application_contents + + /class .* < Rails::Application/.match?(application_contents) + end end end diff --git a/test/setup_bundler_test.rb b/test/setup_bundler_test.rb index 6b9a06fe1..77f3b6059 100644 --- a/test/setup_bundler_test.rb +++ b/test/setup_bundler_test.rb @@ -75,22 +75,33 @@ def test_creates_custom_bundle end def test_creates_custom_bundle_for_a_rails_app - Object.any_instance.expects(:system).with( - bundle_env(".ruby-lsp/Gemfile"), - "(bundle check || bundle install) 1>&2", - ).returns(true) - Bundler::LockfileParser.any_instance.expects(:dependencies).returns({ "rails" => true }).at_least_once - run_script + Dir.mktmpdir do |dir| + Dir.chdir(dir) do + FileUtils.mkdir(File.join(dir, "config")) + File.write(File.join(dir, "config", "application.rb"), <<~RUBY) + module MyApp + class Application < Rails::Application + end + end + RUBY - assert_path_exists(".ruby-lsp") - assert_path_exists(".ruby-lsp/Gemfile") - assert_path_exists(".ruby-lsp/Gemfile.lock") - assert_path_exists(".ruby-lsp/main_lockfile_hash") - assert_match("ruby-lsp", File.read(".ruby-lsp/Gemfile")) - assert_match("debug", File.read(".ruby-lsp/Gemfile")) - assert_match("ruby-lsp-rails", File.read(".ruby-lsp/Gemfile")) - ensure - FileUtils.rm_r(".ruby-lsp") if Dir.exist?(".ruby-lsp") + Object.any_instance.expects(:system).with( + bundle_env(".ruby-lsp/Gemfile"), + "(bundle check || bundle install) 1>&2", + ).returns(true) + Bundler::LockfileParser.any_instance.expects(:dependencies).returns({ "rails" => true }).at_least_once + run_script + + assert_path_exists(".ruby-lsp") + assert_path_exists(".ruby-lsp/Gemfile") + assert_path_exists(".ruby-lsp/Gemfile.lock") + assert_path_exists(".ruby-lsp/main_lockfile_hash") + gemfile_content = File.read(".ruby-lsp/Gemfile") + assert_match("ruby-lsp", gemfile_content) + assert_match("debug", gemfile_content) + assert_match("ruby-lsp-rails", gemfile_content) + end + end end def test_changing_lockfile_causes_custom_bundle_to_be_rebuilt @@ -486,6 +497,14 @@ def test_ruby_lsp_rails_is_automatically_included_in_rails_apps gem "rails" GEMFILE + FileUtils.mkdir(File.join(dir, "config")) + File.write(File.join(dir, "config", "application.rb"), <<~RUBY) + module MyApp + class Application < Rails::Application + end + end + RUBY + capture_subprocess_io do Bundler.with_unbundled_env do # Run bundle install to generate the lockfile From e5ad0da4534ee253e59b2a338c4dc7411432b104 Mon Sep 17 00:00:00 2001 From: Andy Waite <13400+andyw8@users.noreply.github.com> Date: Thu, 8 Aug 2024 11:42:40 -0400 Subject: [PATCH 2/6] Apply earl's suggestion --- lib/ruby_lsp/setup_bundler.rb | 10 ++++--- test/setup_bundler_test.rb | 50 +++++++++++++++++------------------ 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/lib/ruby_lsp/setup_bundler.rb b/lib/ruby_lsp/setup_bundler.rb index 903f43a3d..af354f9a2 100644 --- a/lib/ruby_lsp/setup_bundler.rb +++ b/lib/ruby_lsp/setup_bundler.rb @@ -285,11 +285,13 @@ def correct_relative_remote_paths # Detects if the project is a Rails app by looking if the superclass of the main class is `Rails::Application` sig { returns(T::Boolean) } def rails_app? - config = Pathname.new("config/application.rb").expand_path - application_contents = config.read if config.exist? - return false unless application_contents + /class .* < Rails::Application/.match?(rails_application_content) + end - /class .* < Rails::Application/.match?(application_contents) + sig { returns(T.nilable(String)) } + def rails_application_content + config = Pathname.new("config/application.rb").expand_path + config.read if config.exist? end end end diff --git a/test/setup_bundler_test.rb b/test/setup_bundler_test.rb index 77f3b6059..a65d7b3ca 100644 --- a/test/setup_bundler_test.rb +++ b/test/setup_bundler_test.rb @@ -75,33 +75,33 @@ def test_creates_custom_bundle end def test_creates_custom_bundle_for_a_rails_app - Dir.mktmpdir do |dir| - Dir.chdir(dir) do - FileUtils.mkdir(File.join(dir, "config")) - File.write(File.join(dir, "config", "application.rb"), <<~RUBY) - module MyApp - class Application < Rails::Application - end - end - RUBY - - Object.any_instance.expects(:system).with( - bundle_env(".ruby-lsp/Gemfile"), - "(bundle check || bundle install) 1>&2", - ).returns(true) - Bundler::LockfileParser.any_instance.expects(:dependencies).returns({ "rails" => true }).at_least_once - run_script + Object.any_instance.expects(:system).with( + bundle_env(".ruby-lsp/Gemfile"), + "(bundle check || bundle install) 1>&2", + ).returns(true) - assert_path_exists(".ruby-lsp") - assert_path_exists(".ruby-lsp/Gemfile") - assert_path_exists(".ruby-lsp/Gemfile.lock") - assert_path_exists(".ruby-lsp/main_lockfile_hash") - gemfile_content = File.read(".ruby-lsp/Gemfile") - assert_match("ruby-lsp", gemfile_content) - assert_match("debug", gemfile_content) - assert_match("ruby-lsp-rails", gemfile_content) + # There is some unknown state leak with Bundler which prevents us from creating the + # folder structure ourselves lest we encounter flaky tests + RubyLsp::SetupBundler.any_instance.stubs(:rails_application_content).returns(<<~RUBY) + module MyApp + class Application < Rails::Application + end end - end + RUBY + + Bundler::LockfileParser.any_instance.expects(:dependencies).returns({ "rails" => true }).at_least_once + + run_script + + assert_path_exists(".ruby-lsp") + assert_path_exists(".ruby-lsp/Gemfile") + assert_path_exists(".ruby-lsp/Gemfile.lock") + assert_path_exists(".ruby-lsp/main_lockfile_hash") + assert_match("ruby-lsp", File.read(".ruby-lsp/Gemfile")) + assert_match("debug", File.read(".ruby-lsp/Gemfile")) + assert_match("ruby-lsp-rails", File.read(".ruby-lsp/Gemfile")) + ensure + FileUtils.rm_r(".ruby-lsp") if Dir.exist?(".ruby-lsp") end def test_changing_lockfile_causes_custom_bundle_to_be_rebuilt From f403aeb7adfba6664902d56eca0de573911b75ff Mon Sep 17 00:00:00 2001 From: Andy Waite <13400+andyw8@users.noreply.github.com> Date: Thu, 8 Aug 2024 11:49:52 -0400 Subject: [PATCH 3/6] Create files instead of stubbing --- lib/ruby_lsp/setup_bundler.rb | 10 ++++------ test/fixtures/rails_application.rb | 4 ++++ test/setup_bundler_test.rb | 13 +++---------- 3 files changed, 11 insertions(+), 16 deletions(-) create mode 100644 test/fixtures/rails_application.rb diff --git a/lib/ruby_lsp/setup_bundler.rb b/lib/ruby_lsp/setup_bundler.rb index af354f9a2..903f43a3d 100644 --- a/lib/ruby_lsp/setup_bundler.rb +++ b/lib/ruby_lsp/setup_bundler.rb @@ -285,13 +285,11 @@ def correct_relative_remote_paths # Detects if the project is a Rails app by looking if the superclass of the main class is `Rails::Application` sig { returns(T::Boolean) } def rails_app? - /class .* < Rails::Application/.match?(rails_application_content) - end - - sig { returns(T.nilable(String)) } - def rails_application_content config = Pathname.new("config/application.rb").expand_path - config.read if config.exist? + application_contents = config.read if config.exist? + return false unless application_contents + + /class .* < Rails::Application/.match?(application_contents) end end end diff --git a/test/fixtures/rails_application.rb b/test/fixtures/rails_application.rb new file mode 100644 index 000000000..27eff561a --- /dev/null +++ b/test/fixtures/rails_application.rb @@ -0,0 +1,4 @@ +module MyApp + class Application < Rails::Application + end +end diff --git a/test/setup_bundler_test.rb b/test/setup_bundler_test.rb index a65d7b3ca..dee4edfbe 100644 --- a/test/setup_bundler_test.rb +++ b/test/setup_bundler_test.rb @@ -80,17 +80,9 @@ def test_creates_custom_bundle_for_a_rails_app "(bundle check || bundle install) 1>&2", ).returns(true) - # There is some unknown state leak with Bundler which prevents us from creating the - # folder structure ourselves lest we encounter flaky tests - RubyLsp::SetupBundler.any_instance.stubs(:rails_application_content).returns(<<~RUBY) - module MyApp - class Application < Rails::Application - end - end - RUBY - + FileUtils.mkdir("config") + FileUtils.cp("test/fixtures/rails_application.rb", "config/application.rb") Bundler::LockfileParser.any_instance.expects(:dependencies).returns({ "rails" => true }).at_least_once - run_script assert_path_exists(".ruby-lsp") @@ -102,6 +94,7 @@ class Application < Rails::Application assert_match("ruby-lsp-rails", File.read(".ruby-lsp/Gemfile")) ensure FileUtils.rm_r(".ruby-lsp") if Dir.exist?(".ruby-lsp") + FileUtils.rm_rf("config") if Dir.exist?("config") end def test_changing_lockfile_causes_custom_bundle_to_be_rebuilt From c4288848d554b3e5e418ac19ad6925ab626337c9 Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Thu, 8 Aug 2024 11:55:24 -0400 Subject: [PATCH 4/6] Update lib/ruby_lsp/setup_bundler.rb Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com> --- lib/ruby_lsp/setup_bundler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ruby_lsp/setup_bundler.rb b/lib/ruby_lsp/setup_bundler.rb index 903f43a3d..9f1ba0397 100644 --- a/lib/ruby_lsp/setup_bundler.rb +++ b/lib/ruby_lsp/setup_bundler.rb @@ -289,7 +289,7 @@ def rails_app? application_contents = config.read if config.exist? return false unless application_contents - /class .* < Rails::Application/.match?(application_contents) + /class .* < (::)?Rails::Application/.match?(application_contents) end end end From 8dc23d18ae411874d13b9c2a2bd7c3d5c2c215bd Mon Sep 17 00:00:00 2001 From: Andy Waite <13400+andyw8@users.noreply.github.com> Date: Thu, 8 Aug 2024 12:29:15 -0400 Subject: [PATCH 5/6] Use fixture for test --- test/setup_bundler_test.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/setup_bundler_test.rb b/test/setup_bundler_test.rb index dee4edfbe..05610f995 100644 --- a/test/setup_bundler_test.rb +++ b/test/setup_bundler_test.rb @@ -484,20 +484,14 @@ def test_ensures_lockfile_remotes_are_relative_to_default_gemfile def test_ruby_lsp_rails_is_automatically_included_in_rails_apps Dir.mktmpdir do |dir| + FileUtils.mkdir("#{dir}/config") + FileUtils.cp("test/fixtures/rails_application.rb", "#{dir}/config/application.rb") Dir.chdir(dir) do File.write(File.join(dir, "Gemfile"), <<~GEMFILE) source "https://rubygems.org" gem "rails" GEMFILE - FileUtils.mkdir(File.join(dir, "config")) - File.write(File.join(dir, "config", "application.rb"), <<~RUBY) - module MyApp - class Application < Rails::Application - end - end - RUBY - capture_subprocess_io do Bundler.with_unbundled_env do # Run bundle install to generate the lockfile From 7d2beaefd07536ebaac888eda65d3848f471a3fb Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Thu, 15 Aug 2024 16:01:06 -0400 Subject: [PATCH 6/6] Update test/setup_bundler_test.rb Co-authored-by: Vinicius Stock --- test/setup_bundler_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/setup_bundler_test.rb b/test/setup_bundler_test.rb index 05610f995..09b1f2760 100644 --- a/test/setup_bundler_test.rb +++ b/test/setup_bundler_test.rb @@ -94,7 +94,7 @@ def test_creates_custom_bundle_for_a_rails_app assert_match("ruby-lsp-rails", File.read(".ruby-lsp/Gemfile")) ensure FileUtils.rm_r(".ruby-lsp") if Dir.exist?(".ruby-lsp") - FileUtils.rm_rf("config") if Dir.exist?("config") + FileUtils.rm_r("config") if Dir.exist?("config") end def test_changing_lockfile_causes_custom_bundle_to_be_rebuilt