From 03266889ca71a1114cd91a3cc9c3a0dd435c51d2 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Mon, 14 Mar 2022 16:14:25 -0400 Subject: [PATCH 1/7] Check shims for duplicates from Sorbet's payload Signed-off-by: Alexandre Terrasa Co-authored-by: Kaan Ozkan Co-authored-by: Vinicius Stock --- lib/tapioca/cli.rb | 32 ++++++++++++++++++++- lib/tapioca/helpers/shims_helper.rb | 43 ++++++++++++++++++++++------ lib/tapioca/helpers/sorbet_helper.rb | 4 +-- 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/lib/tapioca/cli.rb b/lib/tapioca/cli.rb index 09db90bf4..09caf629a 100644 --- a/lib/tapioca/cli.rb +++ b/lib/tapioca/cli.rb @@ -5,6 +5,7 @@ module Tapioca class Cli < Thor include CliHelper include ConfigHelper + include SorbetHelper include ShimsHelper FILE_HEADER_OPTION_DESC = "Add a \"This file is generated\" header on top of each generated RBI file" @@ -242,6 +243,7 @@ def gem(*gems) option :gem_rbi_dir, type: :string, desc: "Path to gem RBIs", default: DEFAULT_GEM_DIR option :dsl_rbi_dir, type: :string, desc: "Path to DSL RBIs", default: DEFAULT_DSL_DIR option :shim_rbi_dir, type: :string, desc: "Path to shim RBIs", default: DEFAULT_SHIM_DIR + option :payload, type: :boolean, desc: "Check shims against Sorbet's payload", default: true def check_shims index = RBI::Index.new @@ -251,6 +253,30 @@ def check_shims exit(0) end + payload_path = T.let(nil, T.nilable(String)) + + if options[:payload] + if sorbet_supports?(:print_payload_sources) + Dir.mktmpdir do |dir| + payload_path = dir + result = sorbet("--no-config --print=payload-sources:#{payload_path}") + + unless result.status + say_error("Sorbet failed to dump payload") + say_error(result.err) + exit(1) + end + + index_payload(index, payload_path) + end + else + say_error("The version of Sorbet used in your Gemfile.lock does not support `--print=payload-sources`") + say_error("Current: v#{SORBET_GEM_SPEC.version}") + say_error("Required: #{FEATURE_REQUIREMENTS[:print_payload_sources]}") + exit(1) + end + end + index_rbis(index, "shim", shim_rbi_dir) index_rbis(index, "gem", options[:gem_rbi_dir]) index_rbis(index, "dsl", options[:dsl_rbi_dir]) @@ -260,7 +286,11 @@ def check_shims duplicates.each do |key, nodes| say_error("\nDuplicated RBI for #{key}:", :red) nodes.each do |node| - say_error(" * #{node.loc}", :red) + node_loc = node.loc + next unless node_loc + + loc_string = location_to_payload_url(node_loc, path_prefix: payload_path) + say_error(" * #{loc_string}", :red) end end say_error("\nPlease remove the duplicated definitions from the #{shim_rbi_dir} directory.", :red) diff --git a/lib/tapioca/helpers/shims_helper.rb b/lib/tapioca/helpers/shims_helper.rb index aad2eeaa2..375f60282 100644 --- a/lib/tapioca/helpers/shims_helper.rb +++ b/lib/tapioca/helpers/shims_helper.rb @@ -8,20 +8,25 @@ module ShimsHelper requires_ancestor { Thor::Shell } + SORBET_PAYLOAD_URL = "https://github.com/sorbet/sorbet/tree/master/rbi" + + sig { params(index: RBI::Index, dir: String).void } + def index_payload(index, dir) + return unless Dir.exist?(dir) + + say("Loading Sorbet payload... ") + files = Dir.glob("#{dir}/**/*.rbi").sort + parse_and_index_files(index, files) + say(" Done", :green) + end + sig { params(index: RBI::Index, kind: String, dir: String).void } def index_rbis(index, kind, dir) return unless Dir.exist?(dir) && !Dir.empty?(dir) say("Loading #{kind} RBIs from #{dir}... ") files = Dir.glob("#{dir}/**/*.rbi").sort - - trees = files.map do |file| - RBI::Parser.parse_file(file) - rescue RBI::ParseError => e - say_error("\nWarning: #{e} (#{e.location})", :yellow) - end.compact - - index.visit_all(trees) + parse_and_index_files(index, files) say(" Done", :green) end @@ -39,8 +44,30 @@ def duplicated_nodes_from_index(index, shim_rbi_dir) duplicates end + sig { params(loc: RBI::Loc, path_prefix: T.nilable(String)).returns(String) } + def location_to_payload_url(loc, path_prefix:) + return loc.to_s unless path_prefix + + url = loc.file || "" + return loc.to_s unless url.start_with?(path_prefix) + + url = url.sub(path_prefix, SORBET_PAYLOAD_URL) + url = "#{url}#L#{loc.begin_line}" + url + end + private + sig { params(index: RBI::Index, files: T::Array[String]).void } + def parse_and_index_files(index, files) + trees = files.map do |file| + RBI::Parser.parse_file(file) + rescue RBI::ParseError => e + say_error("\nWarning: #{e} (#{e.location})", :yellow) + end.compact + index.visit_all(trees) + end + sig { params(nodes: T::Array[RBI::Node], shim_rbi_dir: String).returns(T::Boolean) } def shims_have_duplicates?(nodes, shim_rbi_dir) return false if nodes.size == 1 diff --git a/lib/tapioca/helpers/sorbet_helper.rb b/lib/tapioca/helpers/sorbet_helper.rb index a32eddc8d..6e918b4ce 100644 --- a/lib/tapioca/helpers/sorbet_helper.rb +++ b/lib/tapioca/helpers/sorbet_helper.rb @@ -21,8 +21,8 @@ module SorbetHelper SORBET_EXE_PATH_ENV_VAR = "TAPIOCA_SORBET_EXE" FEATURE_REQUIREMENTS = T.let({ - # First tag that includes https://github.com/sorbet/sorbet/pull/4706 - to_ary_nil_support: ::Gem::Requirement.new(">= 0.5.9220"), + to_ary_nil_support: ::Gem::Requirement.new(">= 0.5.9220"), # https://github.com/sorbet/sorbet/pull/4706 + print_payload_sources: ::Gem::Requirement.new(">= 0.5.9818"), # https://github.com/sorbet/sorbet/pull/5504 }.freeze, T::Hash[Symbol, ::Gem::Requirement]) sig { params(sorbet_args: String).returns(Spoom::ExecResult) } From 99f51bc746d7fa4a2cdb9dcf5bfa4902b003f659 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 31 Mar 2022 11:10:34 -0400 Subject: [PATCH 2/7] Nest `check-shims` tests under an additional `describe` Extracted in this commit to avoid a bigger, harder to read change in the next one. No functional change here. Signed-off-by: Alexandre Terrasa --- spec/tapioca/cli/check_shims_spec.rb | 636 ++++++++++++++------------- 1 file changed, 319 insertions(+), 317 deletions(-) diff --git a/spec/tapioca/cli/check_shims_spec.rb b/spec/tapioca/cli/check_shims_spec.rb index 2085e1acf..68a5ea8ee 100644 --- a/spec/tapioca/cli/check_shims_spec.rb +++ b/spec/tapioca/cli/check_shims_spec.rb @@ -6,327 +6,329 @@ module Tapioca class CleanShimsTest < SpecWithProject describe "cli::clean-shims" do - before(:all) do - @project.bundle_install - end - after do @project.remove("sorbet/rbi") end - it "does nothing when there is no shims to check" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - attr_reader :foo - end - RBI - - @project.write("sorbet/rbi/dsl/foo.rbi", <<~RBI) - class Foo - attr_reader :bar - end - RBI - - result = @project.tapioca("check-shims") - - assert_equal(<<~OUT, result.out) - No shim RBIs to check - OUT - - assert_success_status(result) - end - - it "does nothing when there is no duplicates" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - attr_reader :foo - end - RBI - - @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) - class Foo - attr_reader :bar - end - RBI - - result = @project.tapioca("check-shims") - - assert_equal(<<~OUT, result.out) - Loading shim RBIs from sorbet/rbi/shims... Done - Loading gem RBIs from sorbet/rbi/gems... Done - Looking for duplicates... Done - - No duplicates found in shim RBIs - OUT - - assert_success_status(result) - end - - it "detects duplicated definitions between shim and generated RBIs" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - attr_reader :foo - end - RBI - - @project.write("sorbet/rbi/dsl/bar.rbi", <<~RBI) - module Bar - def bar; end - end - RBI - - @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) - class Foo - attr_reader :foo - end - RBI - - @project.write("sorbet/rbi/shims/bar.rbi", <<~RBI) - module Bar - def bar; end - end - RBI - - result = @project.tapioca("check-shims") - - assert_includes(result.err, <<~ERR) - Duplicated RBI for ::Bar#bar: - * sorbet/rbi/shims/bar.rbi:2:2-2:14 - * sorbet/rbi/dsl/bar.rbi:2:2-2:14 - ERR - - assert_includes(result.err, <<~ERR) - Duplicated RBI for ::Foo#foo: - * sorbet/rbi/shims/foo.rbi:2:2-2:18 - * sorbet/rbi/gems/foo@1.0.0.rbi:2:2-2:18 - ERR - - assert_includes(result.err, <<~ERR) - Please remove the duplicated definitions from the sorbet/rbi/shims directory. - ERR - - refute_success_status(result) - end - - it "ignores duplicates that have a signature" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - def foo; end - end - RBI - - @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) - class Foo - sig { void } - def foo; end - end - RBI - - result = @project.tapioca("check-shims") - assert_success_status(result) - end - - it "ignores duplicates that have a different signature" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - sig { void } - def foo; end - end - RBI - - @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) - class Foo - sig { returns(Integer) } - def foo; end - end - RBI - - result = @project.tapioca("check-shims") - assert_success_status(result) - end - - it "detects duplicates that have the same signature" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - sig { params(x: Integer, y: String).returns(String) } - def foo(x, y); end - - sig { params(x: Integer, y: Integer).returns(String) } - def bar(x, y); end - - sig { params(x: Integer, y: Integer).returns(Integer) } - def baz(x, y); end - end - RBI - - @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) - class Foo - sig { params(x: Integer, y: String).returns(String) } - def foo(x, y); end - - sig { params(x: String, y: Integer).returns(String) } - def bar(x, y); end - - sig { params(x: Integer, y: Integer).returns(String) } - def baz(x, y); end - end - RBI - - result = @project.tapioca("check-shims") - - assert_includes(result.err, <<~ERR) - Duplicated RBI for ::Foo#foo: - * sorbet/rbi/shims/foo.rbi:3:2-3:20 - * sorbet/rbi/gems/foo@1.0.0.rbi:3:2-3:20 - ERR - - assert_includes(result.err, <<~ERR) - Please remove the duplicated definitions from the sorbet/rbi/shims directory. - ERR - - refute_includes(result.err, "Duplicated RBI for ::Foo#bar") - refute_includes(result.err, "Duplicated RBI for ::Foo#baz") - - refute_success_status(result) - end - - it "detects duplicates from nodes with multiple definitions" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - attr_reader :foo - end - RBI - - @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) - class Foo - attr_reader :foo, :bar - end - RBI - - result = @project.tapioca("check-shims") - - assert_includes(result.err, <<~ERR) - Duplicated RBI for ::Foo#foo: - * sorbet/rbi/shims/foo.rbi:2:2-2:24 - * sorbet/rbi/gems/foo@1.0.0.rbi:2:2-2:18 - ERR - - assert_includes(result.err, <<~ERR) - Please remove the duplicated definitions from the sorbet/rbi/shims directory. - ERR - - refute_success_status(result) - end - - it "detects duplicates from the same shim file" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - attr_reader :foo - end - RBI - - @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) - class Foo - attr_reader :foo, :bar - def foo; end - end - RBI - - result = @project.tapioca("check-shims") - - assert_includes(result.err, <<~ERR) - Duplicated RBI for ::Foo#foo: - * sorbet/rbi/shims/foo.rbi:2:2-2:24 - * sorbet/rbi/shims/foo.rbi:3:2-3:14 - * sorbet/rbi/gems/foo@1.0.0.rbi:2:2-2:18 - ERR - - assert_includes(result.err, <<~ERR) - Please remove the duplicated definitions from the sorbet/rbi/shims directory. - ERR - - refute_success_status(result) - end - - it "checks shims with custom rbi dirs" do - @project.write("rbi/gem/foo@1.0.0.rbi", <<~RBI) - class Foo - def foo; end - end - RBI - - @project.write("rbi/dsl/foo.rbi", <<~RBI) - class Foo - def bar; end - end - RBI - - @project.write("rbi/shim/foo.rbi", <<~RBI) - class Foo - def foo; end - def bar; end - end - RBI - - result = @project.tapioca( - "check-shims --gem-rbi-dir=rbi/gem --dsl-rbi-dir=rbi/dsl --shim-rbi-dir=rbi/shim" - ) - - assert_includes(result.err, <<~ERR) - Duplicated RBI for ::Foo#bar: - * rbi/shim/foo.rbi:3:2-3:14 - * rbi/dsl/foo.rbi:2:2-2:14 - ERR - - assert_includes(result.err, <<~ERR) - Duplicated RBI for ::Foo#foo: - * rbi/shim/foo.rbi:2:2-2:14 - * rbi/gem/foo@1.0.0.rbi:2:2-2:14 - ERR - - assert_includes(result.err, <<~ERR) - Please remove the duplicated definitions from the rbi/shim directory. - ERR - - refute_success_status(result) - end - - it "skips files with parse errors" do - @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) - class Foo - attr_reader :foo - end - RBI - - @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) - class Foo - foo { bar } - end - RBI - - @project.write("sorbet/rbi/shims/bar.rbi", <<~RBI) - module Foo - def foo; end - end - RBI - - result = @project.tapioca("check-shims") - - assert_includes(result.err, <<~ERR) - Warning: Unsupported block node type `foo` (sorbet/rbi/shims/foo.rbi:2:2-2:13) - ERR - - assert_includes(result.err, <<~ERR) - Duplicated RBI for ::Foo#foo: - * sorbet/rbi/shims/bar.rbi:2:2-2:14 - * sorbet/rbi/gems/foo@1.0.0.rbi:2:2-2:18 - ERR - - assert_includes(result.err, <<~ERR) - Please remove the duplicated definitions from the sorbet/rbi/shims directory. - ERR - - refute_success_status(result) + describe "when Sorbet version is >= 0.5.9818" do + before(:all) do + @project.bundle_install + end + + it "does nothing when there is no shims to check" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + attr_reader :foo + end + RBI + + @project.write("sorbet/rbi/dsl/foo.rbi", <<~RBI) + class Foo + attr_reader :bar + end + RBI + + result = @project.tapioca("check-shims") + + assert_equal(<<~OUT, result.out) + No shim RBIs to check + OUT + + assert_success_status(result) + end + + it "does nothing when there is no duplicates" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + attr_reader :foo + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + attr_reader :bar + end + RBI + + result = @project.tapioca("check-shims") + + assert_equal(<<~OUT, result.out) + Loading shim RBIs from sorbet/rbi/shims... Done + Loading gem RBIs from sorbet/rbi/gems... Done + Looking for duplicates... Done + + No duplicates found in shim RBIs + OUT + + assert_success_status(result) + end + + it "detects duplicated definitions between shim and generated RBIs" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + attr_reader :foo + end + RBI + + @project.write("sorbet/rbi/dsl/bar.rbi", <<~RBI) + module Bar + def bar; end + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + attr_reader :foo + end + RBI + + @project.write("sorbet/rbi/shims/bar.rbi", <<~RBI) + module Bar + def bar; end + end + RBI + + result = @project.tapioca("check-shims") + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Bar#bar: + * sorbet/rbi/shims/bar.rbi:2:2-2:14 + * sorbet/rbi/dsl/bar.rbi:2:2-2:14 + ERR + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Foo#foo: + * sorbet/rbi/shims/foo.rbi:2:2-2:18 + * sorbet/rbi/gems/foo@1.0.0.rbi:2:2-2:18 + ERR + + assert_includes(result.err, <<~ERR) + Please remove the duplicated definitions from the sorbet/rbi/shims directory. + ERR + + refute_success_status(result) + end + + it "ignores duplicates that have a signature" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + def foo; end + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + sig { void } + def foo; end + end + RBI + + result = @project.tapioca("check-shims") + assert_success_status(result) + end + + it "ignores duplicates that have a different signature" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + sig { void } + def foo; end + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + sig { returns(Integer) } + def foo; end + end + RBI + + result = @project.tapioca("check-shims") + assert_success_status(result) + end + + it "detects duplicates that have the same signature" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + sig { params(x: Integer, y: String).returns(String) } + def foo(x, y); end + + sig { params(x: Integer, y: Integer).returns(String) } + def bar(x, y); end + + sig { params(x: Integer, y: Integer).returns(Integer) } + def baz(x, y); end + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + sig { params(x: Integer, y: String).returns(String) } + def foo(x, y); end + + sig { params(x: String, y: Integer).returns(String) } + def bar(x, y); end + + sig { params(x: Integer, y: Integer).returns(String) } + def baz(x, y); end + end + RBI + + result = @project.tapioca("check-shims") + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Foo#foo: + * sorbet/rbi/shims/foo.rbi:3:2-3:20 + * sorbet/rbi/gems/foo@1.0.0.rbi:3:2-3:20 + ERR + + assert_includes(result.err, <<~ERR) + Please remove the duplicated definitions from the sorbet/rbi/shims directory. + ERR + + refute_includes(result.err, "Duplicated RBI for ::Foo#bar") + refute_includes(result.err, "Duplicated RBI for ::Foo#baz") + + refute_success_status(result) + end + + it "detects duplicates from nodes with multiple definitions" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + attr_reader :foo + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + attr_reader :foo, :bar + end + RBI + + result = @project.tapioca("check-shims") + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Foo#foo: + * sorbet/rbi/shims/foo.rbi:2:2-2:24 + * sorbet/rbi/gems/foo@1.0.0.rbi:2:2-2:18 + ERR + + assert_includes(result.err, <<~ERR) + Please remove the duplicated definitions from the sorbet/rbi/shims directory. + ERR + + refute_success_status(result) + end + + it "detects duplicates from the same shim file" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + attr_reader :foo + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + attr_reader :foo, :bar + def foo; end + end + RBI + + result = @project.tapioca("check-shims") + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Foo#foo: + * sorbet/rbi/shims/foo.rbi:2:2-2:24 + * sorbet/rbi/shims/foo.rbi:3:2-3:14 + * sorbet/rbi/gems/foo@1.0.0.rbi:2:2-2:18 + ERR + + assert_includes(result.err, <<~ERR) + Please remove the duplicated definitions from the sorbet/rbi/shims directory. + ERR + + refute_success_status(result) + end + + it "checks shims with custom rbi dirs" do + @project.write("rbi/gem/foo@1.0.0.rbi", <<~RBI) + class Foo + def foo; end + end + RBI + + @project.write("rbi/dsl/foo.rbi", <<~RBI) + class Foo + def bar; end + end + RBI + + @project.write("rbi/shim/foo.rbi", <<~RBI) + class Foo + def foo; end + def bar; end + end + RBI + + result = @project.tapioca( + "check-shims --gem-rbi-dir=rbi/gem --dsl-rbi-dir=rbi/dsl --shim-rbi-dir=rbi/shim" + ) + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Foo#bar: + * rbi/shim/foo.rbi:3:2-3:14 + * rbi/dsl/foo.rbi:2:2-2:14 + ERR + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Foo#foo: + * rbi/shim/foo.rbi:2:2-2:14 + * rbi/gem/foo@1.0.0.rbi:2:2-2:14 + ERR + + assert_includes(result.err, <<~ERR) + Please remove the duplicated definitions from the rbi/shim directory. + ERR + + refute_success_status(result) + end + + it "skips files with parse errors" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + attr_reader :foo + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + foo { bar } + end + RBI + + @project.write("sorbet/rbi/shims/bar.rbi", <<~RBI) + module Foo + def foo; end + end + RBI + + result = @project.tapioca("check-shims") + + assert_includes(result.err, <<~ERR) + Warning: Unsupported block node type `foo` (sorbet/rbi/shims/foo.rbi:2:2-2:13) + ERR + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Foo#foo: + * sorbet/rbi/shims/bar.rbi:2:2-2:14 + * sorbet/rbi/gems/foo@1.0.0.rbi:2:2-2:18 + ERR + + assert_includes(result.err, <<~ERR) + Please remove the duplicated definitions from the sorbet/rbi/shims directory. + ERR + + refute_success_status(result) + end end end end From c083f7628ac13ec7b053950be8e0c654b974a2ce Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 31 Mar 2022 11:15:34 -0400 Subject: [PATCH 3/7] Add test for `check-shims --payload` Signed-off-by: Alexandre Terrasa --- spec/tapioca/cli/check_shims_spec.rb | 44 ++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/spec/tapioca/cli/check_shims_spec.rb b/spec/tapioca/cli/check_shims_spec.rb index 68a5ea8ee..c1a870b8d 100644 --- a/spec/tapioca/cli/check_shims_spec.rb +++ b/spec/tapioca/cli/check_shims_spec.rb @@ -249,6 +249,50 @@ def foo; end refute_success_status(result) end + it "detects duplicates from Sorbet's payload" do + @project.write("sorbet/rbi/shims/core/string.rbi", <<~RBI) + class String + sig { returns(String) } + def capitalize(); end + + def some_method_that_is_not_defined_in_the_payload; end + end + RBI + + @project.write("sorbet/rbi/shims/stdlib/base64.rbi", <<~RBI) + module Base64 + sig { params(str: String).returns(String) } + def self.decode64(str); end + + def self.some_method_that_is_not_defined_in_the_payload; end + end + RBI + + result = @project.tapioca("check-shims") + + assert_includes(result.out, <<~OUT) + Loading Sorbet payload... Done + Loading shim RBIs from sorbet/rbi/shims... Done + Looking for duplicates... Done + OUT + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::String#capitalize: + * https://github.com/sorbet/sorbet/tree/master/rbi/core/string.rbi#L406 + * sorbet/rbi/shims/core/string.rbi:3:2-3:23 + + Duplicated RBI for ::Base64::decode64: + * https://github.com/sorbet/sorbet/tree/master/rbi/stdlib/base64.rbi#L37 + * sorbet/rbi/shims/stdlib/base64.rbi:3:2-3:29 + ERR + + assert_includes(result.err, <<~ERR) + Please remove the duplicated definitions from the sorbet/rbi/shims directory. + ERR + + refute_success_status(result) + end + it "checks shims with custom rbi dirs" do @project.write("rbi/gem/foo@1.0.0.rbi", <<~RBI) class Foo From 35bb67c5b3102dc17a66a5c561d20973d2e3050e Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 31 Mar 2022 11:16:22 -0400 Subject: [PATCH 4/7] Add test checking the version gate on `check-shims --payload` Signed-off-by: Alexandre Terrasa --- spec/tapioca/cli/check_shims_spec.rb | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/tapioca/cli/check_shims_spec.rb b/spec/tapioca/cli/check_shims_spec.rb index c1a870b8d..24c9b7e6a 100644 --- a/spec/tapioca/cli/check_shims_spec.rb +++ b/spec/tapioca/cli/check_shims_spec.rb @@ -374,6 +374,34 @@ def foo; end refute_success_status(result) end end + + describe "when Sorbet version is too old" do + before(:all) do + @project.require_real_gem("sorbet", "=0.5.9760") + @project.bundle_install + end + + it "does not check shims against payload for older Sorbet versions" do + @project.write("sorbet/rbi/shims/core/string.rbi", <<~RBI) + class String + sig { returns(String) } + def capitalize(); end + + def some_method_that_is_not_defined_in_the_payload; end + end + RBI + + result = @project.tapioca("check-shims") + + assert_includes(result.err, <<~ERR) + The version of Sorbet used in your Gemfile.lock does not support `--print=payload-sources` + Current: v0.5.9760 + Required: >= 0.5.9818 + ERR + + refute_success_status(result) + end + end end end end From c9cb6b9a4dd29fac5b6844c4e7b4d5c4b5a41d7a Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 31 Mar 2022 11:17:14 -0400 Subject: [PATCH 5/7] Speed up default tests on `check-shims --no-payload` Dumping, parsing and indexing Sorbet's payload takes around ~5s. We don't need to waste this time in most tests here. Signed-off-by: Alexandre Terrasa --- spec/tapioca/cli/check_shims_spec.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/tapioca/cli/check_shims_spec.rb b/spec/tapioca/cli/check_shims_spec.rb index 24c9b7e6a..6bce97037 100644 --- a/spec/tapioca/cli/check_shims_spec.rb +++ b/spec/tapioca/cli/check_shims_spec.rb @@ -28,7 +28,7 @@ class Foo end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_equal(<<~OUT, result.out) No shim RBIs to check @@ -50,7 +50,7 @@ class Foo end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_equal(<<~OUT, result.out) Loading shim RBIs from sorbet/rbi/shims... Done @@ -88,7 +88,7 @@ def bar; end end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_includes(result.err, <<~ERR) Duplicated RBI for ::Bar#bar: @@ -123,7 +123,7 @@ def foo; end end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_success_status(result) end @@ -142,7 +142,7 @@ def foo; end end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_success_status(result) end @@ -173,7 +173,7 @@ def baz(x, y); end end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_includes(result.err, <<~ERR) Duplicated RBI for ::Foo#foo: @@ -204,7 +204,7 @@ class Foo end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_includes(result.err, <<~ERR) Duplicated RBI for ::Foo#foo: @@ -233,7 +233,7 @@ def foo; end end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_includes(result.err, <<~ERR) Duplicated RBI for ::Foo#foo: @@ -314,7 +314,7 @@ def bar; end RBI result = @project.tapioca( - "check-shims --gem-rbi-dir=rbi/gem --dsl-rbi-dir=rbi/dsl --shim-rbi-dir=rbi/shim" + "check-shims --gem-rbi-dir=rbi/gem --dsl-rbi-dir=rbi/dsl --shim-rbi-dir=rbi/shim --no-payload" ) assert_includes(result.err, <<~ERR) @@ -355,7 +355,7 @@ def foo; end end RBI - result = @project.tapioca("check-shims") + result = @project.tapioca("check-shims --no-payload") assert_includes(result.err, <<~ERR) Warning: Unsupported block node type `foo` (sorbet/rbi/shims/foo.rbi:2:2-2:13) From c30a8bb1fd51bd8501ce842077d4963d1b0da647 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 31 Mar 2022 11:20:39 -0400 Subject: [PATCH 6/7] Remove duplicated shim for Bundler This definition is duplicated from https://github.com/sorbet/sorbet/blob/master/rbi/stdlib/bundler.rbi#L9958. Signed-off-by: Alexandre Terrasa --- sorbet/rbi/shims/bundler.rbi | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 sorbet/rbi/shims/bundler.rbi diff --git a/sorbet/rbi/shims/bundler.rbi b/sorbet/rbi/shims/bundler.rbi deleted file mode 100644 index 51a94dec9..000000000 --- a/sorbet/rbi/shims/bundler.rbi +++ /dev/null @@ -1,6 +0,0 @@ -# typed: true - -class Bundler::StubSpecification - sig { returns(T::Boolean) } - def default_gem?; end -end From 91dcde9e83d76847bbe0147121e3c0302ec9fec2 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Fri, 1 Apr 2022 10:27:35 -0400 Subject: [PATCH 7/7] Update README with new `--payload` option Signed-off-by: Alexandre Terrasa --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 7e3b6f815..7045ecdc6 100644 --- a/README.md +++ b/README.md @@ -257,6 +257,7 @@ check_shims: gem_rbi_dir: sorbet/rbi/gems dsl_rbi_dir: sorbet/rbi/dsl shim_rbi_dir: sorbet/rbi/shims + payload: true ```