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.

Sign up for GitHub

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

Avoid memoizing dependencies #1926

Merged
merged 1 commit into from
Apr 22, 2024
Merged

Avoid memoizing dependencies #1926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 27 additions & 34 deletions lib/ruby_lsp/global_state.rb
View file
Open in desktop
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 || detect_linters
@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
Open in desktop
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
Loading