From 93815378a4fc58b093c9e20b1a8f66059fb93f25 Mon Sep 17 00:00:00 2001 From: Ufuk Kayserilioglu Date: Fri, 3 Sep 2021 22:14:28 +0300 Subject: [PATCH 1/2] Stop monkey-patching Class to add descendants This causes a confusion with the `Class#descendants` method that Active Support adds and the signature on this method leaks out into the Tapioca generated RBI and causes type-checking problems in Core because of it. This is arguably a reflection method, so I moved it to `Tapioca::Reflection` module which is already included in the Base DSL class and SymbolGenerator class. --- .../dsl/action_controller_helpers.rb | 2 +- lib/tapioca/compilers/dsl/action_mailer.rb | 2 +- lib/tapioca/compilers/dsl/active_job.rb | 2 +- .../dsl/active_record_associations.rb | 2 +- .../compilers/dsl/active_record_columns.rb | 2 +- .../compilers/dsl/active_record_enum.rb | 2 +- .../compilers/dsl/active_record_scope.rb | 2 +- .../dsl/active_record_typed_store.rb | 4 +-- lib/tapioca/compilers/dsl/active_resource.rb | 2 +- lib/tapioca/compilers/dsl/active_storage.rb | 2 +- .../dsl/active_support_current_attributes.rb | 2 +- lib/tapioca/compilers/dsl/frozen_record.rb | 2 +- lib/tapioca/compilers/dsl/identity_cache.rb | 2 +- lib/tapioca/compilers/dsl/state_machines.rb | 2 -- lib/tapioca/compilers/dsl_compiler.rb | 4 +-- .../symbol_table/symbol_generator.rb | 4 +-- lib/tapioca/core_ext/class.rb | 28 ------------------- lib/tapioca/loader.rb | 3 +- lib/tapioca/reflection.rb | 22 +++++++++++++++ tasks/generator_documentation.rake | 1 - 20 files changed, 40 insertions(+), 52 deletions(-) delete mode 100644 lib/tapioca/core_ext/class.rb diff --git a/lib/tapioca/compilers/dsl/action_controller_helpers.rb b/lib/tapioca/compilers/dsl/action_controller_helpers.rb index 0493d6e62..905fd52e0 100644 --- a/lib/tapioca/compilers/dsl/action_controller_helpers.rb +++ b/lib/tapioca/compilers/dsl/action_controller_helpers.rb @@ -125,7 +125,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActionController::Base.descendants.reject(&:abstract?).select(&:name) + descendants_of(::ActionController::Base).reject(&:abstract?).select(&:name) end private diff --git a/lib/tapioca/compilers/dsl/action_mailer.rb b/lib/tapioca/compilers/dsl/action_mailer.rb index 757e05f4c..616d3b790 100644 --- a/lib/tapioca/compilers/dsl/action_mailer.rb +++ b/lib/tapioca/compilers/dsl/action_mailer.rb @@ -54,7 +54,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActionMailer::Base.descendants.reject(&:abstract?) + descendants_of(::ActionMailer::Base).reject(&:abstract?) end end end diff --git a/lib/tapioca/compilers/dsl/active_job.rb b/lib/tapioca/compilers/dsl/active_job.rb index b4ab23359..f092f32f1 100644 --- a/lib/tapioca/compilers/dsl/active_job.rb +++ b/lib/tapioca/compilers/dsl/active_job.rb @@ -67,7 +67,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActiveJob::Base.descendants + descendants_of(::ActiveJob::Base) end end end diff --git a/lib/tapioca/compilers/dsl/active_record_associations.rb b/lib/tapioca/compilers/dsl/active_record_associations.rb index f4d6e4c2b..7648984de 100644 --- a/lib/tapioca/compilers/dsl/active_record_associations.rb +++ b/lib/tapioca/compilers/dsl/active_record_associations.rb @@ -122,7 +122,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ActiveRecord::Base.descendants.reject(&:abstract_class?) + descendants_of(::ActiveRecord::Base).reject(&:abstract_class?) end private diff --git a/lib/tapioca/compilers/dsl/active_record_columns.rb b/lib/tapioca/compilers/dsl/active_record_columns.rb index f6bdfb24a..8611c2f0d 100644 --- a/lib/tapioca/compilers/dsl/active_record_columns.rb +++ b/lib/tapioca/compilers/dsl/active_record_columns.rb @@ -127,7 +127,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ActiveRecord::Base.descendants.reject(&:abstract_class?) + descendants_of(::ActiveRecord::Base).reject(&:abstract_class?) end private diff --git a/lib/tapioca/compilers/dsl/active_record_enum.rb b/lib/tapioca/compilers/dsl/active_record_enum.rb index 8170c1302..dee43b398 100644 --- a/lib/tapioca/compilers/dsl/active_record_enum.rb +++ b/lib/tapioca/compilers/dsl/active_record_enum.rb @@ -80,7 +80,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActiveRecord::Base.descendants.reject(&:abstract_class?) + descendants_of(::ActiveRecord::Base).reject(&:abstract_class?) end private diff --git a/lib/tapioca/compilers/dsl/active_record_scope.rb b/lib/tapioca/compilers/dsl/active_record_scope.rb index d8084f5c4..638bb89aa 100644 --- a/lib/tapioca/compilers/dsl/active_record_scope.rb +++ b/lib/tapioca/compilers/dsl/active_record_scope.rb @@ -68,7 +68,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActiveRecord::Base.descendants.reject(&:abstract_class?) + descendants_of(::ActiveRecord::Base).reject(&:abstract_class?) end private diff --git a/lib/tapioca/compilers/dsl/active_record_typed_store.rb b/lib/tapioca/compilers/dsl/active_record_typed_store.rb index b5bc8b54e..4318a0896 100644 --- a/lib/tapioca/compilers/dsl/active_record_typed_store.rb +++ b/lib/tapioca/compilers/dsl/active_record_typed_store.rb @@ -1,8 +1,6 @@ # typed: strict # frozen_string_literal: true -require "tapioca/core_ext/class" - begin require "activerecord-typedstore" rescue LoadError @@ -115,7 +113,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActiveRecord::Base.descendants.select do |klass| + descendants_of(::ActiveRecord::Base).select do |klass| klass.include?(ActiveRecord::TypedStore::Behavior) end end diff --git a/lib/tapioca/compilers/dsl/active_resource.rb b/lib/tapioca/compilers/dsl/active_resource.rb index 132c5c8bc..96a5dd530 100644 --- a/lib/tapioca/compilers/dsl/active_resource.rb +++ b/lib/tapioca/compilers/dsl/active_resource.rb @@ -74,7 +74,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActiveResource::Base.descendants + descendants_of(::ActiveResource::Base) end private diff --git a/lib/tapioca/compilers/dsl/active_storage.rb b/lib/tapioca/compilers/dsl/active_storage.rb index db6b9e5ef..49d7600d6 100644 --- a/lib/tapioca/compilers/dsl/active_storage.rb +++ b/lib/tapioca/compilers/dsl/active_storage.rb @@ -72,7 +72,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ActiveRecord::Base.descendants + descendants_of(::ActiveRecord::Base) .reject(&:abstract_class?) .grep(::ActiveStorage::Reflection::ActiveRecordExtensions::ClassMethods) end diff --git a/lib/tapioca/compilers/dsl/active_support_current_attributes.rb b/lib/tapioca/compilers/dsl/active_support_current_attributes.rb index 4a7871d4f..a87e62766 100644 --- a/lib/tapioca/compilers/dsl/active_support_current_attributes.rb +++ b/lib/tapioca/compilers/dsl/active_support_current_attributes.rb @@ -95,7 +95,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActiveSupport::CurrentAttributes.descendants + descendants_of(::ActiveSupport::CurrentAttributes) end private diff --git a/lib/tapioca/compilers/dsl/frozen_record.rb b/lib/tapioca/compilers/dsl/frozen_record.rb index 8154dc989..468e90680 100644 --- a/lib/tapioca/compilers/dsl/frozen_record.rb +++ b/lib/tapioca/compilers/dsl/frozen_record.rb @@ -86,7 +86,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::FrozenRecord::Base.descendants.reject(&:abstract_class?) + descendants_of(::FrozenRecord::Base).reject(&:abstract_class?) end end end diff --git a/lib/tapioca/compilers/dsl/identity_cache.rb b/lib/tapioca/compilers/dsl/identity_cache.rb index 40b1a95c5..1daa5a043 100644 --- a/lib/tapioca/compilers/dsl/identity_cache.rb +++ b/lib/tapioca/compilers/dsl/identity_cache.rb @@ -99,7 +99,7 @@ def decorate(root, constant) sig { override.returns(T::Enumerable[Module]) } def gather_constants - ::ActiveRecord::Base.descendants.select do |klass| + descendants_of(::ActiveRecord::Base).select do |klass| klass < ::IdentityCache::WithoutPrimaryIndex end end diff --git a/lib/tapioca/compilers/dsl/state_machines.rb b/lib/tapioca/compilers/dsl/state_machines.rb index ce6e2197d..b63f0b7ba 100644 --- a/lib/tapioca/compilers/dsl/state_machines.rb +++ b/lib/tapioca/compilers/dsl/state_machines.rb @@ -1,8 +1,6 @@ # typed: strict # frozen_string_literal: true -require "tapioca/core_ext/class" - begin require "state_machines" rescue LoadError diff --git a/lib/tapioca/compilers/dsl_compiler.rb b/lib/tapioca/compilers/dsl_compiler.rb index 72126e12f..cb8cc01d3 100644 --- a/lib/tapioca/compilers/dsl_compiler.rb +++ b/lib/tapioca/compilers/dsl_compiler.rb @@ -62,12 +62,12 @@ def run(&blk) ).returns(T::Enumerable[Dsl::Base]) end def gather_generators(requested_generators, excluded_generators) - generator_klasses = Dsl::Base.descendants.select do |klass| + generator_klasses = ::Tapioca::Reflection.descendants_of(Dsl::Base).select do |klass| (requested_generators.empty? || requested_generators.include?(klass)) && !excluded_generators.include?(klass) end.sort_by { |klass| T.must(klass.name) } - T.cast(generator_klasses.map(&:new), T::Enumerable[Dsl::Base]) + generator_klasses.map(&:new) end sig { params(requested_constants: T::Array[Module]).returns(T::Set[Module]) } diff --git a/lib/tapioca/compilers/symbol_table/symbol_generator.rb b/lib/tapioca/compilers/symbol_table/symbol_generator.rb index e419f6bcc..bf23c658b 100644 --- a/lib/tapioca/compilers/symbol_table/symbol_generator.rb +++ b/lib/tapioca/compilers/symbol_table/symbol_generator.rb @@ -54,8 +54,8 @@ def symbols def engine_symbols(symbols) return Set.new unless Object.const_defined?("Rails::Engine") - engine = Object.const_get("Rails::Engine") - .descendants.reject(&:abstract_railtie?) + engine = descendants_of(Object.const_get("Rails::Engine")) + .reject(&:abstract_railtie?) .find do |klass| name = name_of(klass) !name.nil? && symbols.include?(name) diff --git a/lib/tapioca/core_ext/class.rb b/lib/tapioca/core_ext/class.rb deleted file mode 100644 index c8c00714c..000000000 --- a/lib/tapioca/core_ext/class.rb +++ /dev/null @@ -1,28 +0,0 @@ -# typed: strict -# frozen_string_literal: true - -class Class - extend T::Sig - - # Returns an array with all classes that are < than its receiver. - # - # class C; end - # C.descendants # => [] - # - # class B < C; end - # C.descendants # => [B] - # - # class A < B; end - # C.descendants # => [B, A] - # - # class D < C; end - # C.descendants # => [B, A, D] - sig { returns(T::Array[Class]) } - def descendants - result = ObjectSpace.each_object(singleton_class).reject do |k| - T.cast(k, Module).singleton_class? || k == self - end - - T.cast(result, T::Array[Class]) - end -end diff --git a/lib/tapioca/loader.rb b/lib/tapioca/loader.rb index 5fbb24ec2..99e1f2620 100644 --- a/lib/tapioca/loader.rb +++ b/lib/tapioca/loader.rb @@ -1,8 +1,6 @@ # typed: strict # frozen_string_literal: true -require "tapioca/core_ext/class" - module Tapioca class Loader extend(T::Sig) @@ -50,6 +48,7 @@ def require_helper(file) def rails_engines return [] unless Object.const_defined?("Rails::Engine") + # We can use `Class#descendants` here, since we know Rails is loaded Object.const_get("Rails::Engine").descendants.reject(&:abstract_railtie?) end diff --git a/lib/tapioca/reflection.rb b/lib/tapioca/reflection.rb index a6d477931..933e97e73 100644 --- a/lib/tapioca/reflection.rb +++ b/lib/tapioca/reflection.rb @@ -105,5 +105,27 @@ def signature_of(method) def name_of_type(type) type.to_s.gsub(/\bAttachedClass\b/, "T.attached_class") end + + # Returns an array with all classes that are < than the supplied class. + # + # class C; end + # descendants_of(C) # => [] + # + # class B < C; end + # descendants_of(C) # => [B] + # + # class A < B; end + # descendants_of(C) # => [B, A] + # + # class D < C; end + # descendants_of(C) # => [B, A, D] + sig { type_parameters(:U).params(klass: T.type_parameter(:U)).returns(T::Array[T.type_parameter(:U)]) } + def descendants_of(klass) + result = ObjectSpace.each_object(klass.singleton_class).reject do |k| + T.cast(k, Module).singleton_class? || T.unsafe(k) == klass + end + + T.unsafe(result) + end end end diff --git a/tasks/generator_documentation.rake b/tasks/generator_documentation.rake index 7b0b56270..02b689034 100644 --- a/tasks/generator_documentation.rake +++ b/tasks/generator_documentation.rake @@ -2,7 +2,6 @@ require "yard" require "tapioca" -require "tapioca/core_ext/class" YARD::Rake::YardocTask.new(:yard_for_generate_documentation) do |task| task.files = ["lib/tapioca/compilers/dsl/**/*.rb"] From ffa085b43ffb42c8acdd1f6f3432a832bd17b8e6 Mon Sep 17 00:00:00 2001 From: Ufuk Kayserilioglu Date: Fri, 3 Sep 2021 22:16:53 +0300 Subject: [PATCH 2/2] Stop monkey-patching String to add underscore This causes a confusion with the `String#underscore` method that Active Support adds and causes problems like https://discourse.shopify.io/t/file-naming-inflection-issues-with-tapioca-dsl-verify-in-ci/19663 I moved the method onto the `Generator` class and also duplicated it into the DSL test base class. --- lib/tapioca/core_ext/string.rb | 18 ------------------ lib/tapioca/generator.rb | 17 ++++++++++++++--- spec/dsl_spec.rb | 15 +++++++++++++-- 3 files changed, 27 insertions(+), 23 deletions(-) delete mode 100644 lib/tapioca/core_ext/string.rb diff --git a/lib/tapioca/core_ext/string.rb b/lib/tapioca/core_ext/string.rb deleted file mode 100644 index ffdd7ff67..000000000 --- a/lib/tapioca/core_ext/string.rb +++ /dev/null @@ -1,18 +0,0 @@ -# typed: strict -# frozen_string_literal: true - -class String - extend T::Sig - - sig { returns(String) } - def underscore - return self unless /[A-Z-]|::/.match?(self) - - word = to_s.gsub("::", "/") - word.gsub!(/([A-Z\d]+)([A-Z][a-z])/, '\1_\2') - word.gsub!(/([a-z\d])([A-Z])/, '\1_\2') - word.tr!("-", "_") - word.downcase! - word - end -end diff --git a/lib/tapioca/generator.rb b/lib/tapioca/generator.rb index a2532ad78..afc123d78 100644 --- a/lib/tapioca/generator.rb +++ b/lib/tapioca/generator.rb @@ -4,7 +4,6 @@ require "pathname" require "thor" require "rake" -require "tapioca/core_ext/string" module Tapioca class Generator < ::Thor::Shell::Color @@ -372,7 +371,7 @@ def expected_rbis sig { params(constant_name: String).returns(Pathname) } def dsl_rbi_filename(constant_name) - config.outpath / "#{constant_name.underscore}.rbi" + config.outpath / "#{underscore(constant_name)}.rbi" end sig { params(gem_name: String, version: String).returns(Pathname) } @@ -568,7 +567,7 @@ def compile_gem_rbi(gem) def compile_dsl_rbi(constant_name, contents, outpath: config.outpath, quiet: false) return if contents.nil? - rbi_name = constant_name.underscore + ".rbi" + rbi_name = underscore(constant_name) + ".rbi" filename = outpath / rbi_name out = String.new @@ -702,5 +701,17 @@ def abort_if_pending_migrations! Rails.application.load_tasks Rake::Task["db:abort_if_pending_migrations"].invoke if Rake::Task.task_defined?("db:abort_if_pending_migrations") end + + sig { params(class_name: String).returns(String) } + def underscore(class_name) + return class_name unless /[A-Z-]|::/.match?(class_name) + + word = class_name.to_s.gsub("::", "/") + word.gsub!(/([A-Z\d]+)([A-Z][a-z])/, '\1_\2') + word.gsub!(/([a-z\d])([A-Z])/, '\1_\2') + word.tr!("-", "_") + word.downcase! + word + end end end diff --git a/spec/dsl_spec.rb b/spec/dsl_spec.rb index 552484fa5..6babf8f87 100644 --- a/spec/dsl_spec.rb +++ b/spec/dsl_spec.rb @@ -6,7 +6,6 @@ require "content_helper" require "template_helper" require "isolation_helper" -require "tapioca/core_ext/string" class DslSpec < Minitest::Spec extend T::Sig @@ -46,7 +45,19 @@ def target_class_name sig { returns(String) } def target_class_file - target_class_name.underscore + underscore(target_class_name) + end + + sig { params(class_name: String).returns(String) } + def underscore(class_name) + return class_name unless /[A-Z-]|::/.match?(class_name) + + word = class_name.to_s.gsub("::", "/") + word.gsub!(/([A-Z\d]+)([A-Z][a-z])/, '\1_\2') + word.gsub!(/([a-z\d])([A-Z])/, '\1_\2') + word.tr!("-", "_") + word.downcase! + word end sig { params(str: String, indent: Integer).returns(String) }