From 8ca9464327e7b05eb0318a603726395ffeb8b72f Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Mon, 25 Jul 2022 13:06:19 -0400 Subject: [PATCH] Also check duplicated mixins in shims and TODO files Signed-off-by: Alexandre Terrasa --- lib/tapioca/helpers/rbi_files_helper.rb | 10 ++++ spec/tapioca/cli/check_shims_spec.rb | 67 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/lib/tapioca/helpers/rbi_files_helper.rb b/lib/tapioca/helpers/rbi_files_helper.rb index ef11d8fab..d4d07ef92 100644 --- a/lib/tapioca/helpers/rbi_files_helper.rb +++ b/lib/tapioca/helpers/rbi_files_helper.rb @@ -178,6 +178,9 @@ def shims_or_todos_have_duplicates?(nodes, shim_rbi_dir:, todo_rbi_file:) shims_or_todos_empty_scopes = extract_empty_scopes(shims_or_todos) return true unless shims_or_todos_empty_scopes.empty? + mixins = extract_mixins(shims_or_todos) + return true unless mixins.empty? + props = extract_methods_and_attrs(shims_or_todos) return false if props.empty? @@ -215,6 +218,13 @@ def extract_methods_and_attrs(nodes) end, T::Array[T.any(RBI::Method, RBI::Attr)]) end + sig { params(nodes: T::Array[RBI::Node]).returns(T::Array[T.any(RBI::Mixin, RBI::RequiresAncestor)]) } + def extract_mixins(nodes) + T.cast(nodes.select do |node| + node.is_a?(RBI::Mixin) || node.is_a?(RBI::RequiresAncestor) + end, T::Array[T.all(RBI::Mixin, RBI::RequiresAncestor)]) + end + sig { params(nodes: T::Array[T.any(RBI::Method, RBI::Attr)]).returns(T::Array[T.any(RBI::Method, RBI::Attr)]) } def extract_nodes_with_sigs(nodes) nodes.reject { |node| node.sigs.empty? } diff --git a/spec/tapioca/cli/check_shims_spec.rb b/spec/tapioca/cli/check_shims_spec.rb index c066ec943..edaf4b0ff 100644 --- a/spec/tapioca/cli/check_shims_spec.rb +++ b/spec/tapioca/cli/check_shims_spec.rb @@ -235,6 +235,73 @@ class Foo refute_success_status(result) end + it "detects duplicated includes and extends" do + @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + module Bar; end + + class Foo + include Bar + extend Bar + mixes_in_class_methods Bar + requires_ancestor { Bar } + end + + module Baz; end + + class Qux + # RBI does not attempt to resolve constants used in the mixins, so if the constant duplicated in the shim + # does not use the same namespace, Tapioca won't catch it. The following lines won't raise duplication + # errors. + include Baz + extend Baz + mixes_in_class_methods Baz + requires_ancestor { Baz } + end + RBI + + @project.write("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + include Bar + extend Bar + mixes_in_class_methods Bar + requires_ancestor { Bar } + end + + class Qux + include ::Baz + extend ::Baz + mixes_in_class_methods ::Baz + requires_ancestor { ::Baz } + end + RBI + + result = @project.tapioca("check-shims --no-payload") + + assert_includes(result.err, <<~ERR) + Duplicated RBI for ::Foo.include(Bar): + * sorbet/rbi/shims/foo.rbi:2:2-2:13 + * sorbet/rbi/gems/foo@1.0.0.rbi:4:2-4:13 + + Duplicated RBI for ::Foo.extend(Bar): + * sorbet/rbi/shims/foo.rbi:3:2-3:12 + * sorbet/rbi/gems/foo@1.0.0.rbi:5:2-5:12 + + Duplicated RBI for ::Foo.mixes_in_class_method(Bar): + * sorbet/rbi/shims/foo.rbi:4:2-4:28 + * sorbet/rbi/gems/foo@1.0.0.rbi:6:2-6:28 + + Duplicated RBI for ::Foo.requires_ancestor(Bar): + * sorbet/rbi/shims/foo.rbi:5:2-5:27 + * sorbet/rbi/gems/foo@1.0.0.rbi:7:2-7:27 + ERR + + assert_includes(result.err, <<~ERR) + Please remove the duplicated definitions from sorbet/rbi/shims and sorbet/rbi/todo.rbi + ERR + + refute_success_status(result) + end + it "detects duplicates from same shim file" do @project.write("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) class Foo