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.

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

Rely always on workspace URI instead of Dir.pwd #2424

Merged
merged 1 commit into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 28 additions & 9 deletions lib/ruby_indexer/lib/ruby_indexer/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,24 @@ class Configuration
T::Hash[String, T.untyped],
)

sig { params(workspace_path: String).void }
attr_writer :workspace_path

sig { void }
def initialize
@workspace_path = T.let(Dir.pwd, String)
@excluded_gems = T.let(initial_excluded_gems, T::Array[String])
@included_gems = T.let([], T::Array[String])
@excluded_patterns = T.let([File.join("**", "*_test.rb"), File.join("**", "tmp", "**", "*")], T::Array[String])
@excluded_patterns = T.let([File.join("**", "*_test.rb"), File.join("tmp", "**", "*")], T::Array[String])

path = Bundler.settings["path"]
@excluded_patterns << File.join(File.expand_path(path, Dir.pwd), "**", "*.rb") if path
if path
# Substitute Windows backslashes into forward slashes, which are used in glob patterns
glob = path.gsub(/[\\]+/, "/")
@excluded_patterns << File.join(glob, "**", "*.rb")
end

@included_patterns = T.let([File.join(Dir.pwd, "**", "*.rb")], T::Array[String])
@included_patterns = T.let([File.join("**", "*.rb")], T::Array[String])
@excluded_magic_comments = T.let(
[
"frozen_string_literal:",
Expand Down Expand Up @@ -55,12 +64,12 @@ def indexables
indexables = @included_patterns.flat_map do |pattern|
load_path_entry = T.let(nil, T.nilable(String))

Dir.glob(pattern, File::FNM_PATHNAME | File::FNM_EXTGLOB).map! do |path|
Dir.glob(File.join(@workspace_path, pattern), File::FNM_PATHNAME | File::FNM_EXTGLOB).map! do |path|
path = File.expand_path(path)
# All entries for the same pattern match the same $LOAD_PATH entry. Since searching the $LOAD_PATH for every
# entry is expensive, we memoize it until we find a path that doesn't belong to that $LOAD_PATH. This happens
# on repositories that define multiple gems, like Rails. All frameworks are defined inside the Dir.pwd, but
# each one of them belongs to a different $LOAD_PATH entry
# on repositories that define multiple gems, like Rails. All frameworks are defined inside the current
# workspace directory, but each one of them belongs to a different $LOAD_PATH entry
if load_path_entry.nil? || !path.start_with?(load_path_entry)
load_path_entry = $LOAD_PATH.find { |load_path| path.start_with?(load_path) }
end
Expand All @@ -69,9 +78,19 @@ def indexables
end
end

# If the patterns are relative, we make it relative to the workspace path. If they are absolute, then we shouldn't
# concatenate anything
excluded_patterns = @excluded_patterns.map do |pattern|
if File.absolute_path?(pattern)
pattern
else
File.join(@workspace_path, pattern)
end
end

# Remove user specified patterns
indexables.reject! do |indexable|
@excluded_patterns.any? do |pattern|
excluded_patterns.any? do |pattern|
File.fnmatch?(pattern, indexable.full_path, File::FNM_PATHNAME | File::FNM_EXTGLOB)
end
end
Expand Down Expand Up @@ -122,7 +141,7 @@ def indexables
# When working on a gem, it will be included in the locked_gems list. Since these are the project's own files,
# we have already included and handled exclude patterns for it and should not re-include or it'll lead to
# duplicates or accidentally ignoring exclude patterns
next if spec.full_gem_path == Dir.pwd
next if spec.full_gem_path == @workspace_path

indexables.concat(
spec.require_paths.flat_map do |require_path|
Expand Down Expand Up @@ -185,7 +204,7 @@ def initial_excluded_gems
# If the dependency is prerelease, `to_spec` may return `nil` due to a bug in older version of Bundler/RubyGems:
# https://github.com/Shopify/ruby-lsp/issues/1246
this_gem = Bundler.definition.dependencies.find do |d|
d.to_spec&.full_gem_path == Dir.pwd
d.to_spec&.full_gem_path == @workspace_path
rescue Gem::MissingSpecError
false
end
Expand Down
48 changes: 41 additions & 7 deletions lib/ruby_indexer/test/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ module RubyIndexer
class ConfigurationTest < Minitest::Test
def setup
@config = Configuration.new
@workspace_path = File.expand_path(File.join("..", "..", ".."), __dir__)
@config.workspace_path = @workspace_path
end

def test_load_configuration_executes_configure_block
@config.apply_config({ "excluded_patterns" => ["**/test/fixtures/**/*.rb"] })
@config.apply_config({ "excluded_patterns" => ["**/fixtures/**/*.rb"] })
indexables = @config.indexables

assert(indexables.none? { |indexable| indexable.full_path.include?("test/fixtures") })
Expand All @@ -25,7 +27,7 @@ def test_indexables_have_expanded_full_paths
indexables = @config.indexables

# All paths should be expanded
assert(indexables.none? { |indexable| indexable.full_path.start_with?("lib/") })
assert(indexables.all? { |indexable| File.absolute_path?(indexable.full_path) })
end

def test_indexables_only_includes_gem_require_paths
Expand Down Expand Up @@ -71,17 +73,20 @@ def test_indexables_avoids_duplicates_if_bundle_path_is_inside_project
Bundler.settings.temporary(path: "vendor/bundle") do
config = Configuration.new

assert_includes(config.instance_variable_get(:@excluded_patterns), "#{Dir.pwd}/vendor/bundle/**/*.rb")
assert_includes(config.instance_variable_get(:@excluded_patterns), "vendor/bundle/**/*.rb")
end
end

def test_indexables_does_not_include_gems_own_installed_files
indexables = @config.indexables
indexables_inside_bundled_lsp = indexables.select do |indexable|
indexable.full_path.start_with?(Bundler.bundle_path.join("gems", "ruby-lsp").to_s)
end

assert(
indexables.none? do |indexable|
indexable.full_path.start_with?(Bundler.bundle_path.join("gems", "ruby-lsp").to_s)
end,
assert_empty(
indexables_inside_bundled_lsp,
"Indexables should not include files from the gem currently being worked on. " \
"Included: #{indexables_inside_bundled_lsp.map(&:full_path)}",
)
end

Expand Down Expand Up @@ -126,5 +131,34 @@ def test_magic_comments_regex
assert_match(regex, comment)
end
end

def test_indexables_respect_given_workspace_path
Dir.mktmpdir do |dir|
FileUtils.mkdir(File.join(dir, "ignore"))
FileUtils.touch(File.join(dir, "ignore", "file0.rb"))
FileUtils.touch(File.join(dir, "file1.rb"))
FileUtils.touch(File.join(dir, "file2.rb"))

@config.apply_config({ "excluded_patterns" => ["ignore/**/*.rb"] })
@config.workspace_path = dir
indexables = @config.indexables

assert(indexables.none? { |indexable| indexable.full_path.start_with?(File.join(dir, "ignore")) })

# After switching the workspace path, all indexables will be found in one of these places:
# - The new workspace path
# - The Ruby LSP's own code (because Bundler is requiring the dependency from source)
# - Bundled gems
# - Default gems
assert(
indexables.all? do |i|
i.full_path.start_with?(dir) ||
i.full_path.start_with?(File.join(Dir.pwd, "lib")) ||
i.full_path.start_with?(Bundler.bundle_path.to_s) ||
i.full_path.start_with?(RbConfig::CONFIG["rubylibdir"])
end,
)
end
end
end
end
2 changes: 1 addition & 1 deletion lib/ruby_lsp/listeners/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def handle_require_definition(node, message)
when :require_relative
required_file = "#{node.content}.rb"
path = @uri.to_standardized_path
current_folder = path ? Pathname.new(CGI.unescape(path)).dirname : Dir.pwd
current_folder = path ? Pathname.new(CGI.unescape(path)).dirname : @global_state.workspace_path
candidate = File.expand_path(File.join(current_folder, required_file))

@response_builder << Interface::Location.new(
Expand Down
6 changes: 3 additions & 3 deletions lib/ruby_lsp/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1000,10 +1000,10 @@ def process_indexing_configuration(indexing_options)

return unless indexing_options

configuration = @global_state.index.configuration
configuration.workspace_path = @global_state.workspace_path
# The index expects snake case configurations, but VS Code standardizes on camel case settings
@global_state.index.configuration.apply_config(
indexing_options.transform_keys { |key| key.to_s.gsub(/([A-Z])/, "_\\1").downcase },
)
configuration.apply_config(indexing_options.transform_keys { |key| key.to_s.gsub(/([A-Z])/, "_\\1").downcase })
end
end
end
6 changes: 3 additions & 3 deletions lib/ruby_lsp/setup_bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def initialize(project_path, **options)
@gemfile_name = T.let(@gemfile&.basename&.to_s || "Gemfile", String)

# Custom bundle paths
@custom_dir = T.let(Pathname.new(".ruby-lsp").expand_path(Dir.pwd), Pathname)
@custom_dir = T.let(Pathname.new(".ruby-lsp").expand_path(@project_path), Pathname)
@custom_gemfile = T.let(@custom_dir + @gemfile_name, Pathname)
@custom_lockfile = T.let(@custom_dir + (@lockfile&.basename || "Gemfile.lock"), Pathname)
@lockfile_hash_path = T.let(@custom_dir + "main_lockfile_hash", Pathname)
Expand Down Expand Up @@ -183,14 +183,14 @@ def run_bundle_install(bundle_gemfile = @gemfile)
# `.ruby-lsp` folder, which is not the user's intention. For example, if the path is configured as `vendor`, we
# want to install it in the top level `vendor` and not `.ruby-lsp/vendor`
path = Bundler.settings["path"]
expanded_path = File.expand_path(path, Dir.pwd) if path
expanded_path = File.expand_path(path, @project_path) if path

# Use the absolute `BUNDLE_PATH` to prevent accidentally creating unwanted folders under `.ruby-lsp`
env = {}
env["BUNDLE_GEMFILE"] = bundle_gemfile.to_s
env["BUNDLE_PATH"] = expanded_path if expanded_path

local_config_path = File.join(Dir.pwd, ".bundle")
local_config_path = File.join(@project_path, ".bundle")
env["BUNDLE_APP_CONFIG"] = local_config_path if Dir.exist?(local_config_path)

# If `ruby-lsp` and `debug` (and potentially `ruby-lsp-rails`) are already in the Gemfile, then we shouldn't try
Expand Down
8 changes: 8 additions & 0 deletions sorbet/rbi/shims/file.rbi
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# typed: true

class File
class << self
sig { params(path: String).returns(T::Boolean) }
def absolute_path?(path); end
end
end
Loading
Loading