Skip to content

Commit

Permalink
Avoid memoizing dependencies
Browse files Browse the repository at this point in the history
  • Loading branch information
@vinistock
vinistock committed Apr 12, 2024
1 parent 1326854 commit e050ed1
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 50 deletions.
61 changes: 27 additions & 34 deletions lib/ruby_lsp/global_state.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ def initialize

@formatter = T.let("auto", String)
@linters = T.let([], T::Array[String])
@test_library = T.let(detect_test_library, String)
@typechecker = T.let(detect_typechecker, T::Boolean)
@test_library = T.let("minitest", String)
@typechecker = T.let(true, T::Boolean)
@index = T.let(RubyIndexer::Index.new, RubyIndexer::Index)
@supported_formatters = T.let({}, T::Hash[String, Requests::Support::Formatter])
@supports_watching_files = T.let(false, T::Boolean)
Expand All @@ -54,15 +54,18 @@ def active_linters

sig { params(options: T::Hash[Symbol, T.untyped]).void }
def apply_options(options)
dependencies = gather_dependencies
workspace_uri = options.dig(:workspaceFolders, 0, :uri)
@workspace_uri = URI(workspace_uri) if workspace_uri

specified_formatter = options.dig(:initializationOptions, :formatter)
@formatter = specified_formatter if specified_formatter
@formatter = detect_formatter if @formatter == "auto"
@formatter = detect_formatter(dependencies) if @formatter == "auto"

specified_linters = options.dig(:initializationOptions, :linters)
@linters = specified_linters ? specified_linters : detect_linters
@linters = specified_linters ? specified_linters : detect_linters(dependencies)
@test_library = detect_test_library(dependencies)
@typechecker = detect_typechecker(dependencies)

encodings = options.dig(:capabilities, :general, :positionEncodings)
@encoding = if !encodings || encodings.empty?
Expand Down Expand Up @@ -98,19 +101,14 @@ def encoding_name
end
end

sig { params(gem_pattern: Regexp).returns(T::Boolean) }
def direct_dependency?(gem_pattern)
dependencies.any?(gem_pattern)
end

private

sig { returns(String) }
def detect_formatter
sig { params(dependencies: T::Array[String]).returns(String) }
def detect_formatter(dependencies)
# NOTE: Intentionally no $ at end, since we want to match rubocop-shopify, etc.
if direct_dependency?(/^rubocop/)
if dependencies.any?(/^rubocop/)
"rubocop"
elsif direct_dependency?(/^syntax_tree$/)
elsif dependencies.any?(/^syntax_tree$/)
"syntax_tree"
else
"none"
Expand All @@ -119,16 +117,16 @@ def detect_formatter

# Try to detect if there are linters in the project's dependencies. For auto-detection, we always only consider a
# single linter. To have multiple linters running, the user must configure them manually
sig { returns(T::Array[String]) }
def detect_linters
sig { params(dependencies: T::Array[String]).returns(T::Array[String]) }
def detect_linters(dependencies)
linters = []
linters << "rubocop" if direct_dependency?(/^rubocop/)
linters << "rubocop" if dependencies.any?(/^rubocop/)
linters
end

sig { returns(String) }
def detect_test_library
if direct_dependency?(/^rspec/)
sig { params(dependencies: T::Array[String]).returns(String) }
def detect_test_library(dependencies)
if dependencies.any?(/^rspec/)
"rspec"
# A Rails app may have a dependency on minitest, but we would instead want to use the Rails test runner provided
# by ruby-lsp-rails. A Rails app doesn't need to depend on the rails gem itself, individual components like
Expand All @@ -137,23 +135,23 @@ def detect_test_library
elsif File.exist?(File.join(workspace_path, "bin/rails"))
"rails"
# NOTE: Intentionally ends with $ to avoid mis-matching minitest-reporters, etc. in a Rails app.
elsif direct_dependency?(/^minitest$/)
elsif dependencies.any?(/^minitest$/)
"minitest"
elsif direct_dependency?(/^test-unit/)
elsif dependencies.any?(/^test-unit/)
"test-unit"
else
"unknown"
end
end

sig { returns(T::Boolean) }
def detect_typechecker
sig { params(dependencies: T::Array[String]).returns(T::Boolean) }
def detect_typechecker(dependencies)
return false if ENV["RUBY_LSP_BYPASS_TYPECHECKER"]

# We can't read the env from within `Bundle.with_original_env` so we need to set it here.
ruby_lsp_env_is_test = (ENV["RUBY_LSP_ENV"] == "test")
Bundler.with_original_env do
sorbet_static_detected = Bundler.locked_gems.specs.any? { |spec| spec.name == "sorbet-static" }
sorbet_static_detected = dependencies.any?(/^sorbet-static/)
# Don't show message while running tests, since it's noisy
if sorbet_static_detected && !ruby_lsp_env_is_test
$stderr.puts("Ruby LSP detected this is a Sorbet project so will defer to Sorbet LSP for some functionality")
Expand All @@ -165,16 +163,11 @@ def detect_typechecker
end

sig { returns(T::Array[String]) }
def dependencies
@dependencies ||= T.let(
begin
Bundler.with_original_env { Bundler.default_gemfile }
Bundler.locked_gems.dependencies.keys + gemspec_dependencies
rescue Bundler::GemfileNotFound
[]
end,
T.nilable(T::Array[String]),
)
def gather_dependencies
Bundler.with_original_env { Bundler.default_gemfile }
Bundler.locked_gems.dependencies.keys + gemspec_dependencies
rescue Bundler::GemfileNotFound
[]
end

sig { returns(T::Array[String]) }
Expand Down
34 changes: 18 additions & 16 deletions test/global_state_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,49 +8,51 @@ class GlobalStateTest < Minitest::Test
def test_detects_no_test_library_when_there_are_no_dependencies
stub_dependencies({})

assert_equal("unknown", GlobalState.new.test_library)
state = GlobalState.new
state.apply_options({})
assert_equal("unknown", state.test_library)
end

def test_detects_minitest
stub_dependencies("minitest" => "1.2.3")

assert_equal("minitest", GlobalState.new.test_library)
state = GlobalState.new
state.apply_options({})
assert_equal("minitest", state.test_library)
end

def test_does_not_detect_minitest_related_gems_as_minitest
stub_dependencies("minitest-reporters" => "1.2.3")

assert_equal("unknown", GlobalState.new.test_library)
state = GlobalState.new
state.apply_options({})
assert_equal("unknown", state.test_library)
end

def test_detects_test_unit
stub_dependencies("test-unit" => "1.2.3")

assert_equal("test-unit", GlobalState.new.test_library)
end

def test_detects_dependencies_in_gemspecs
assert(GlobalState.new.direct_dependency?(/^prism$/))
state = GlobalState.new
state.apply_options({})
assert_equal("test-unit", state.test_library)
end

def test_detects_rails_if_minitest_is_present_and_bin_rails_exists
stub_dependencies("minitest" => "1.2.3")
File.expects(:exist?).with("#{Dir.pwd}/bin/rails").once.returns(true)

assert_equal("rails", GlobalState.new.test_library)
state = GlobalState.new
state.apply_options({})
assert_equal("rails", state.test_library)
end

def test_detects_rspec_if_both_rails_and_rspec_are_present
stub_dependencies("rspec" => "1.2.3")
File.expects(:exist?).never

assert_equal("rspec", GlobalState.new.test_library)
end

def test_direct_dependency_returns_false_outside_of_bundle
File.expects(:file?).at_least_once.returns(false)
stub_dependencies({})
refute(GlobalState.new.direct_dependency?(/^ruby-lsp/))
state = GlobalState.new
state.apply_options({})
assert_equal("rspec", state.test_library)
end

def test_apply_option_selects_formatter
Expand Down

0 comments on commit e050ed1

Please sign in to comment.