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

[Tapioca Addon] Support gem RBI generation #2081

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jobs:
bundler-cache: true
rubygems: ${{ matrix.rubygems }}
- name: Run tests
run: bin/test
run: bin/test spec/tapioca/ruby_lsp/run_gem_rbi_check_spec.rb
continue-on-error: ${{ !!matrix.experimental }}

buildall:
Expand Down
11 changes: 11 additions & 0 deletions lib/ruby_lsp/tapioca/addon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I noticed is we keep generating RBIs for updated gems. An ideal solution would be like GemSync where we determine that gem RBI is up to date and not generate. It may not be needed yet assuming restarts are rare and folks will stage their lockfile changes. Any thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, it's fine to move forward as is, but I also think it's worth making an attempt at not regenerating if it's not needed.

Maybe we can extract part of the logic from gem sync and check if there's any work to be done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good to me. I mentioned this in the issue as a future improvement.


require "zlib"
require "ruby_lsp/tapioca/run_gem_rbi_check"

module RubyLsp
module Tapioca
Expand All @@ -27,6 +28,7 @@ def initialize
@rails_runner_client = T.let(nil, T.nilable(RubyLsp::Rails::RunnerClient))
@index = T.let(nil, T.nilable(RubyIndexer::Index))
@file_checksums = T.let({}, T::Hash[String, String])
@lockfile_diff = T.let(nil, T.nilable(String))
@outgoing_queue = T.let(nil, T.nilable(Thread::Queue))
end

Expand All @@ -50,6 +52,15 @@ def activate(global_state, outgoing_queue)
request_name: "load_compilers_and_extensions",
workspace_path: @global_state.workspace_path,
)

gem_rbi_check_result = RunGemRbiCheck.new.run
@outgoing_queue << Notification.window_log_message(
gem_rbi_check_result.stdout,
) unless gem_rbi_check_result.stdout.empty?
@outgoing_queue << Notification.window_log_message(
gem_rbi_check_result.stderr,
type: Constant::MessageType::WARNING,
) unless gem_rbi_check_result.stderr.empty?
rescue IncompatibleApiError
# The requested version for the Rails add-on no longer matches. We need to upgrade and fix the breaking
# changes
Expand Down
49 changes: 49 additions & 0 deletions lib/ruby_lsp/tapioca/lockfile_diff_parser.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# typed: true
andyw8 marked this conversation as resolved.
Show resolved Hide resolved
# frozen_string_literal: true

require "bundler"

module RubyLsp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this!

module Tapioca
class LockfileDiffParser
GEM_NAME_PATTERN = /[\w\-]+/
DIFF_LINE_PATTERN = /[+-](.*#{GEM_NAME_PATTERN})\s*\(/
ADDED_LINE_PATTERN = /^\+.*#{GEM_NAME_PATTERN} \(.*\)/
REMOVED_LINE_PATTERN = /^-.*#{GEM_NAME_PATTERN} \(.*\)/

attr_reader :added_or_modified_gems
attr_reader :removed_gems

def initialize(diff_content, direct_dependencies: nil)
@diff_content = diff_content.lines
@current_dependencies = direct_dependencies ||
Bundler::LockfileParser.new(Bundler.default_lockfile.read).dependencies.keys
@added_or_modified_gems = parse_added_or_modified_gems
@removed_gems = parse_removed_gems
end

private

def parse_added_or_modified_gems
@diff_content
.filter_map { |line| extract_gem(line) if line.match?(ADDED_LINE_PATTERN) }
.uniq
end

def parse_removed_gems
@diff_content.filter_map do |line|
next unless line.match?(REMOVED_LINE_PATTERN)

gem = extract_gem(line)
next if @current_dependencies.include?(gem)

gem
end.uniq
end

def extract_gem(line)
line.match(DIFF_LINE_PATTERN)[1].strip
end
end
end
end
128 changes: 128 additions & 0 deletions lib/ruby_lsp/tapioca/run_gem_rbi_check.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
# typed: true
# frozen_string_literal: true

require "ruby_lsp/tapioca/lockfile_diff_parser"

module RubyLsp
module Tapioca
class GemRbiCheckResult < T::Struct
prop :stdout, String
prop :stderr, String
prop :status, T.nilable(Process::Status)
end

class RunGemRbiCheck
extend T::Sig

sig { void }
def initialize
@result = T.let(
GemRbiCheckResult.new(stdout: "", stderr: "", status: nil),
GemRbiCheckResult,
)
end

attr_reader :result

sig { params(project_path: String).returns(GemRbiCheckResult) }
def run(project_path = ".")
FileUtils.chdir(project_path) do
if git_repo?
lockfile_changed? ? generate_gem_rbis : cleanup_orphaned_rbis
else
log_message("Not a git repository")
end
end

@result
end

private

sig { returns(T::Boolean) }
def git_repo?
require "open3"

_, status = Open3.capture2e("git rev-parse --is-inside-work-tree")
T.must(status.success?)
end

sig { returns(T::Boolean) }
def lockfile_changed?
fetch_lockfile_diff
!@lockfile_diff.empty?
end

sig { returns(String) }
def fetch_lockfile_diff
@lockfile_diff = File.exist?("Gemfile.lock") ? %x(git diff Gemfile.lock).strip : ""
end

sig { void }
def generate_gem_rbis
parser = Tapioca::LockfileDiffParser.new(@lockfile_diff)
removed_gems = parser.removed_gems
added_or_modified_gems = parser.added_or_modified_gems

if added_or_modified_gems.any?
log_message("Identified lockfile changes, attempting to generate gem RBIs...")
execute_tapioca_gem_command(added_or_modified_gems)
elsif removed_gems.any?
remove_rbis(removed_gems)
end
end

sig { params(gems: T::Array[String]).void }
def execute_tapioca_gem_command(gems)
Bundler.with_unbundled_env do
# Resetting BUNDLE_GEMFILE to root folder to use the project's Gemfile instead of Ruby LSP's composed Gemfile
stdout, stderr, status = T.unsafe(Open3).capture3(
{ "BUNDLE_GEMFILE" => "Gemfile" },
"bundle",
"exec",
"tapioca",
"gem",
"--lsp_addon",
*gems,
)

log_message(stdout) unless stdout.empty?
log_message(stderr) unless stderr.empty?
@result.status = status
end
end

sig { params(gems: T::Array[String]).void }
def remove_rbis(gems)
FileUtils.rm_f(Dir.glob("sorbet/rbi/gems/{#{gems.join(",")}}@*.rbi"))
log_message("Removed RBIs for: #{gems.join(", ")}")
end

sig { void }
def cleanup_orphaned_rbis
untracked_files = %x(git ls-files --others --exclude-standard sorbet/rbi/gems/).lines.map(&:strip)
deleted_files = %x(git ls-files --deleted sorbet/rbi/gems/).lines.map(&:strip)

delete_files(untracked_files, "Deleted untracked RBIs")
restore_files(deleted_files, "Restored deleted RBIs")
end

sig { params(files: T::Array[String], message: String).void }
def delete_files(files, message)
files.each { |file| File.delete(file) }
log_message("#{message}: #{files.join(", ")}") unless files.empty?
end

sig { params(files: T::Array[String], message: String).void }
def restore_files(files, message)
files.each { |file| %x(git checkout -- #{file}) }
log_message("#{message}: #{files.join(", ")}") unless files.empty?
end

sig { params(message: String).void }
def log_message(message)
@result.stdout += "#{message}\n"
end
end
end
end
2 changes: 1 addition & 1 deletion spec/helpers/mock_gem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class MockGem < Spoom::Context

# The gem's version string such as "1.0.0" or ">= 2.0.5"
sig { returns(String) }
attr_reader :version
attr_accessor :version

# The dependencies to be added to the gem's gemspec
sig { returns(T::Array[String]) }
Expand Down
7 changes: 7 additions & 0 deletions spec/spec_with_project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ def mock_gem(name, version, dependencies: [], path: default_gem_path(name), &blo
gem
end

sig { params(gem: MockGem, version: String).returns(MockGem) }
def update_gem(gem, version)
gem.version = version
gem.gemspec(gem.default_gemspec_contents)
gem
end

# Spec assertions

# Assert that the contents of `path` inside `@project` is equals to `expected`
Expand Down
143 changes: 143 additions & 0 deletions spec/tapioca/ruby_lsp/run_gem_rbi_check_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
# typed: true
# frozen_string_literal: true

require "spec_helper"
require "ruby_lsp/tapioca/run_gem_rbi_check"

module Tapioca
module RubyLsp
class RunGemRbiCheckSpec < SpecWithProject
FOO_RB = <<~RUBY
module Foo
end
RUBY

before do
@project = mock_project
end

after do
@project.destroy!
end

describe "without git" do
before do
@project.bundle_install!
@project.tapioca("configure")
end

it "does nothing if there is no git repo" do
foo = mock_gem("foo", "0.0.1") do
write!("lib/foo.rb", FOO_RB)
end
@project.require_mock_gem(foo)

@project.bundle_install!
check = ::RubyLsp::Tapioca::RunGemRbiCheck.new
check.run(@project.absolute_path)

assert check.result.stdout.include?("Not a git repository")
end
end

describe "with git" do
before do
@project.bundle_install!
@project.tapioca("configure")
@project.exec("git init")
@project.exec("git config commit.gpgsign false")
@project.exec("git add .")
@project.exec("git commit -m 'Initial commit'")
end

it "creates the RBI for a newly added gem" do
foo = mock_gem("foo", "0.0.1") do
write!("lib/foo.rb", FOO_RB)
end
@project.require_mock_gem(foo)
@project.bundle_install!

check = ::RubyLsp::Tapioca::RunGemRbiCheck.new
check.run(@project.absolute_path)

assert_project_file_exist("sorbet/rbi/gems/[email protected]")
end

it "regenerates RBI when a gem version changes" do
foo = mock_gem("foo", "0.0.1") do
write!("lib/foo.rb", FOO_RB)
end
@project.require_mock_gem(foo)
@project.bundle_install!

check = ::RubyLsp::Tapioca::RunGemRbiCheck.new
check.run(@project.absolute_path)

assert_project_file_exist("sorbet/rbi/gems/[email protected]")

# Modify the gem
update_gem foo, "0.0.2"
@project.bundle_install!

check.run(@project.absolute_path)

assert_project_file_exist("sorbet/rbi/gems/[email protected]")
end

it "removes RBI file when a gem is removed" do
foo = mock_gem("foo", "0.0.1") do
write!("lib/foo.rb", FOO_RB)
end
@project.require_mock_gem(foo)
@project.bundle_install!

check1 = ::RubyLsp::Tapioca::RunGemRbiCheck.new
check1.run(@project.absolute_path)

assert_project_file_exist("sorbet/rbi/gems/[email protected]")

@project.exec("git restore Gemfile Gemfile.lock")

check2 = ::RubyLsp::Tapioca::RunGemRbiCheck.new
check2.run(@project.absolute_path)

refute_project_file_exist("sorbet/rbi/gems/[email protected]")
end

it "deletes untracked RBI files" do
@project.bundle_install!
FileUtils.mkdir_p("#{@project.absolute_path}/sorbet/rbi/gems")
# Create an untracked RBI file
FileUtils.touch("#{@project.absolute_path}/sorbet/rbi/gems/[email protected]")

assert_project_file_exist("/sorbet/rbi/gems/[email protected]")

check = ::RubyLsp::Tapioca::RunGemRbiCheck.new
check.run(@project.absolute_path)

refute_project_file_exist("sorbet/rbi/gems/[email protected]")
end

it "restores deleted RBI files" do
@project.bundle_install!
FileUtils.mkdir_p("#{@project.absolute_path}/sorbet/rbi/gems")
# Create and delete a tracked RBI file
FileUtils.touch("#{@project.absolute_path}/sorbet/rbi/gems/[email protected]")
@project.exec("git add sorbet/rbi/gems/[email protected]")
@project.exec("git commit -m 'Add foo RBI'")
FileUtils.rm("#{@project.absolute_path}/sorbet/rbi/gems/[email protected]")

refute_project_file_exist("sorbet/rbi/gems/[email protected]")

check = ::RubyLsp::Tapioca::RunGemRbiCheck.new
check.run(@project.absolute_path)

assert_project_file_exist("sorbet/rbi/gems/[email protected]")

# Clean-up commit
@project.exec("git reset --hard HEAD^")
end
end
end
end
end
Loading
Loading