diff --git a/lib/tapioca/helpers/rbi_files_helper.rb b/lib/tapioca/helpers/rbi_files_helper.rb index ac2bdcd05..d8d99bb2d 100644 --- a/lib/tapioca/helpers/rbi_files_helper.rb +++ b/lib/tapioca/helpers/rbi_files_helper.rb @@ -217,7 +217,9 @@ def has_duplicated_methods_and_attrs?(nodes, shims_or_todos) shims_or_todos_props.each do |shim_or_todo_prop| other_nodes = extract_methods_and_attrs(nodes) - [shim_or_todo_prop] - if shim_or_todo_prop.sigs.empty? + shim_rbs_comments = extract_rbs_comments(shim_or_todo_prop) + + if shim_or_todo_prop.sigs.empty? && shim_rbs_comments.empty? # If the node doesn't have a signature and is an attribute accessor, we have a duplicate return true if shim_or_todo_prop.is_a?(RBI::Attr) @@ -235,6 +237,11 @@ def has_duplicated_methods_and_attrs?(nodes, shims_or_todos) other_nodes.each do |node| # Another prop has the same sig, we have a duplicate return true if shim_or_todo_prop.sigs.any? { |sig| node.sigs.include?(sig) } + + # Another prop has the same RBS comment, we have a duplicate + # Note: can't use `intersect?` here because RBI::RBSComment doesn't implement `eql?`/`hash` + other_rbs_comments = extract_rbs_comments(node) + return true if shim_rbs_comments.any? { |rbs| other_rbs_comments.include?(rbs) } end end end @@ -259,6 +266,11 @@ def extract_methods_and_attrs(nodes) ) end + #: ((RBI::Method | RBI::Attr) node) -> Array[RBI::RBSComment] + def extract_rbs_comments(node) + node.comments.grep(RBI::RBSComment) + end + #: (Array[Spoom::Sorbet::Errors::Error] errors, String gem_dir) -> void def update_gem_rbis_strictnesses(errors, gem_dir) files = [] diff --git a/spec/tapioca/cli/check_shims_spec.rb b/spec/tapioca/cli/check_shims_spec.rb index b57645e0c..57e82e90f 100644 --- a/spec/tapioca/cli/check_shims_spec.rb +++ b/spec/tapioca/cli/check_shims_spec.rb @@ -159,6 +159,87 @@ def foo; end assert_success_status(result) end + it "ignores duplicates that have an RBS 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 + #: () -> void + def foo; end + end + RBI + + result = @project.tapioca("check-shims --no-payload") + assert_success_status(result) + end + + it "ignores duplicates that have a different RBS signature" do + @project.write!("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + #: () -> void + def foo; end + end + RBI + + @project.write!("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + #: () -> Integer + def foo; end + end + RBI + + result = @project.tapioca("check-shims --no-payload") + assert_success_status(result) + end + + it "detects duplicates that have the same RBS signature" do + @project.write!("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) + class Foo + #: (Integer x, String y) -> String + def foo(x, y); end + + #: (Integer x, Integer y) -> String + def bar(x, y); end + + #: (Integer x, Integer y) -> Integer + def baz(x, y); end + end + RBI + + @project.write!("sorbet/rbi/shims/foo.rbi", <<~RBI) + class Foo + #: (Integer x, String y) -> String + def foo(x, y); end + + #: (String x, Integer y) -> String + def bar(x, y); end + + #: (Integer x, Integer y) -> String + def baz(x, y); end + end + RBI + + result = @project.tapioca("check-shims --no-payload") + + assert_stderr_equals(<<~ERR, result) + + 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 + + Please remove the duplicated definitions from sorbet/rbi/shims and sorbet/rbi/todo.rbi + 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 that have the same signature" do @project.write!("sorbet/rbi/gems/foo@1.0.0.rbi", <<~RBI) class Foo