Skip to content

Commit

Permalink
Merge pull request #1735 from Shopify/uk-fix-set-compare-by-identity
Browse files Browse the repository at this point in the history
Make sure we use compare by identity Sets properly
  • Loading branch information
paracycle authored Dec 18, 2023
2 parents ffe63f4 + c1333df commit 0bc05d6
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
2 changes: 1 addition & 1 deletion lib/tapioca/dsl/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def gather_constants; end
sig { returns(T::Set[Module]) }
def processable_constants
@processable_constants ||= T.let(
T::Set[Module].new(gather_constants).compare_by_identity,
T::Set[Module].new.compare_by_identity.merge(gather_constants),
T.nilable(T::Set[Module]),
)
end
Expand Down
10 changes: 7 additions & 3 deletions lib/tapioca/dsl/pipeline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ def gather_active_compilers(requested_compilers, excluded_compilers)

sig { params(requested_constants: T::Array[Module], requested_paths: T::Array[Pathname]).returns(T::Set[Module]) }
def gather_constants(requested_constants, requested_paths)
constants = active_compilers.map(&:processable_constants).reduce(Set.new, :union)
constants = Set.new.compare_by_identity
active_compilers.each do |compiler|
constants.merge(compiler.processable_constants)
end
constants = filter_anonymous_and_reloaded_constants(constants)

constants &= requested_constants unless requested_constants.empty? && requested_paths.empty?
Expand Down Expand Up @@ -158,11 +161,12 @@ def filter_anonymous_and_reloaded_constants(constants)

# Look up all the constants back from their names. The resulting constant set will be the
# set of constants that are actually in memory with those names.
constants_by_name
filtered_constants = constants_by_name
.keys
.map { |name| T.cast(Runtime::Reflection.constantize(name), Module) }
.select { |mod| Runtime::Reflection.constant_defined?(mod) }
.to_set

Set.new.compare_by_identity.merge(filtered_constants)
end

sig { params(constant: Module).returns(T.nilable(RBI::File)) }
Expand Down
38 changes: 38 additions & 0 deletions spec/tapioca/cli/dsl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,44 @@ def title=(title); end
end
end

it "generates RBI file for constants that might have overriden the hash method" do
@project.write!("lib/post.rb", <<~RB)
require "smart_properties"
class Post
include SmartProperties
property :title, accepts: String
def self.hash
raise "Cannot call `hash` on `Post`"
end
end
RB

result = @project.tapioca("dsl")

assert_equal(<<~OUT, result.out)
Loading DSL extension classes... Done
Loading Rails application... Done
Loading DSL compiler classes... Done
Compiling DSL RBI files...
create sorbet/rbi/dsl/post.rbi
Done
Checking generated RBI files... Done
No errors found
All operations performed in working directory.
Please review changes and commit them.
OUT

assert_empty_stderr(result)
assert_project_file_exist("sorbet/rbi/dsl/post.rbi")
assert_success_status(result)
end

it "generates RBI files in the correct output directory" do
@project.write!("lib/post.rb", <<~RB)
require "smart_properties"
Expand Down

0 comments on commit 0bc05d6

Please sign in to comment.