Skip to content

Commit

Permalink
Always use workspace URI instead of Dir.pwd
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Aug 20, 2024
1 parent 88b85b8 commit c678b0b
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 67 deletions.
24 changes: 15 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,19 @@ 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
@excluded_patterns << File.join(path, "**", "*.rb") if path

@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 +59,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 +73,11 @@ def indexables
end
end

excluded_patterns = @excluded_patterns.map { |pattern| File.join(@workspace_path, pattern) }

# 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 +128,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 +191,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

0 comments on commit c678b0b

Please sign in to comment.