Merge pull request #455 from Shopify/at/rbs-comment-signatures

Migrate Sorbet signatures to RBS comments

Author: rafaelfranca

.gitignore

@@ -10,4 +10,3 @@
 /tmp/
 .rubocop-*
 .byebug_history
-sorbet/rbi/hidden-definitions/errors.txt

.rubocop.yml

@@ -13,6 +13,12 @@ AllCops:
   Exclude:
     - 'test/fixtures/**/*'
 
+Layout/LeadingCommentSpace:
+  AllowRBSInlineAnnotation: true
+
+Layout/LineLength:
+  AllowRBSInlineAnnotation: true
+
 Sorbet/ConstantsFromStrings:
   Enabled: true
 

Gemfile

@@ -23,5 +23,5 @@ gem("minitest-mock")
 
 gem("m")
 gem("rake")
-gem("sorbet-static-and-runtime")
+gem("sorbet-static")
 gem("zeitwerk")

Gemfile.lock

@@ -11,7 +11,6 @@ PATH
       parallel (< 2)
       parser
       prism (>= 1.4.0)
-      sorbet-runtime (>= 0.5.9914)
       zeitwerk (>= 2.6.1)
 
 GEM
@@ -206,15 +205,15 @@ GEM
       thor (>= 0.19.2)
     spring (4.0.0)
     stringio (3.1.7)
-    tapioca (0.19.0)
+    tapioca (0.19.1)
       benchmark
       bundler (>= 2.2.25)
       netrc (>= 0.11.0)
       parallel (>= 1.21.0)
       rbi (>= 0.3.7)
       require-hooks (>= 0.2.2)
       rubydex (>= 0.1.0.beta10)
-      sorbet-static-and-runtime (>= 0.5.11087)
+      sorbet-static-and-runtime (>= 0.6.12698)
       spoom (>= 1.7.9)
       thor (>= 1.2.0)
       tsort
@@ -233,6 +232,7 @@ PLATFORMS
   arm64-darwin-21
   arm64-darwin-22
   arm64-darwin-23
+  arm64-darwin-25
   x86_64-darwin
   x86_64-darwin-20
   x86_64-linux
@@ -251,7 +251,7 @@ DEPENDENCIES
   rubocop-performance
   rubocop-shopify
   rubocop-sorbet
-  sorbet-static-and-runtime
+  sorbet-static
   spring
   tapioca
   zeitwerk

USAGE.md

@@ -302,24 +302,28 @@ class MyChecker
   include Packwerk::Checker
   # implement the `Checker` interface
 
-  sig { override.returns(String) }
+  # @override
+  #: -> String
   def violation_type
     'my_custom_violation_type'
   end
 
-  sig { override.params(listed_offense: ReferenceOffense).returns(T::Boolean) }
+  # @override
+  #: (ReferenceOffense listed_offense) -> bool
   def strict_mode_violation?(listed_offense)
     # This will allow "strict mode" to be supported in your checker
     referencing_package = listed_offense.reference.package
     referencing_package.config["enforce_custom"] == "strict"
   end
 
-  sig { override.params(reference: Reference).returns(T::Boolean) }
+  # @override
+  #: (Reference reference) -> bool
   def invalid_reference?(reference)
     # your logic here
   end
 
-  sig { override.params(reference: Reference).returns(String) }
+  # @override
+  #: (Reference reference) -> String
   def message(reference)
     # your message here
   end
@@ -345,12 +349,14 @@ class MyValidator
   include Packwerk::Validator
   # implement the `Validator` interface
 
-  sig { override.returns(T::Array[String]) }
+  # @override
+  #: -> Array[String]
   def permitted_keys
     ['enforce_my_custom_checker']
   end
 
-  sig { override.params(package_set: PackageSet, configuration: Configuration).returns(ApplicationValidator::Result) }
+  # @override
+  #: (PackageSet package_set, Configuration configuration) -> ApplicationValidator::Result
   def call(package_set, configuration)
     # your logic here
   end

exe/packwerk

@@ -1,10 +1,6 @@
 #!/usr/bin/env ruby
 # frozen_string_literal: true
 
-unless defined?(Spring)
-  require "packwerk/disable_sorbet"
-end
-
 require "packwerk"
 
 # Needs to be run in test environment in order to have test helper paths available in the autoload paths

lib/packwerk.rb

@@ -1,7 +1,6 @@
 # typed: strict
 # frozen_string_literal: true
 
-require "sorbet-runtime"
 require "active_support"
 require "fileutils"
 require "stringio"

lib/packwerk/application_validator.rb

@@ -10,18 +10,18 @@ module Packwerk
   # correct results.
   class ApplicationValidator
     include Validator
-    extend T::Sig
     extend ActiveSupport::Autoload
 
     autoload :Helpers
 
-    sig { params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) }
+    #: (PackageSet package_set, Configuration configuration) -> Validator::Result
     def check_all(package_set, configuration)
       results = Validator.all.flat_map { |validator| validator.call(package_set, configuration) }
       merge_results(results)
     end
 
-    sig { override.params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) }
+    # @override
+    #: (PackageSet package_set, Configuration configuration) -> Validator::Result
     def call(package_set, configuration)
       results = [
         check_package_manifest_syntax(configuration),
@@ -33,14 +33,15 @@ def call(package_set, configuration)
       merge_results(results, separator: "\n❓ ")
     end
 
-    sig { override.returns(T::Array[String]) }
+    # @override
+    #: -> Array[String]
     def permitted_keys
       [
         "metadata",
       ]
     end
 
-    sig { params(configuration: Configuration).returns(Validator::Result) }
+    #: (Configuration configuration) -> Validator::Result
     def check_package_manifest_syntax(configuration)
       errors = []
 
@@ -68,7 +69,7 @@ def check_package_manifest_syntax(configuration)
       end
     end
 
-    sig { params(configuration: Configuration).returns(Validator::Result) }
+    #: (Configuration configuration) -> Validator::Result
     def check_application_structure(configuration)
       resolver = ConstantResolver.new(
         root_path: configuration.root_path.to_s,
@@ -84,7 +85,7 @@ def check_application_structure(configuration)
       end
     end
 
-    sig { params(configuration: Configuration).returns(Validator::Result) }
+    #: (Configuration configuration) -> Validator::Result
     def check_package_manifest_paths(configuration)
       all_package_manifests = package_manifests(configuration, "**/")
       package_paths_package_manifests = package_manifests(configuration, package_glob(configuration))
@@ -105,7 +106,7 @@ def check_package_manifest_paths(configuration)
       end
     end
 
-    sig { params(configuration: Configuration).returns(Validator::Result) }
+    #: (Configuration configuration) -> Validator::Result
     def check_root_package_exists(configuration)
       root_package_path = File.join(configuration.root_path, "package.yml")
       all_packages_manifests = package_manifests(configuration, package_glob(configuration))
@@ -124,12 +125,12 @@ def check_root_package_exists(configuration)
 
     private
 
-    sig { params(list: T.untyped).returns(T.untyped) }
+    #: (untyped list) -> untyped
     def format_yaml_strings(list)
       list.sort.map { |p| "- \"#{p}\"" }.join("\n")
     end
 
-    sig { params(configuration: Configuration, paths: T::Array[String]).returns(T::Array[Pathname]) }
+    #: (Configuration configuration, Array[String] paths) -> Array[Pathname]
     def relative_paths(configuration, paths)
       paths.map { |path| relative_path(configuration, path) }
     end

lib/packwerk/association_inspector.rb

@@ -4,39 +4,28 @@
 module Packwerk
   # Extracts the implicit constant reference from an active record association
   class AssociationInspector
-    extend T::Sig
     include ConstantNameInspector
 
-    CustomAssociations = T.type_alias { T.any(T::Array[Symbol], T::Set[Symbol]) }
-
-    RAILS_ASSOCIATIONS = T.let(
-      [
-        :belongs_to,
-        :has_many,
-        :has_one,
-        :has_and_belongs_to_many,
-      ].to_set,
-      CustomAssociations
-    )
-
-    sig do
-      params(
-        inflector: T.class_of(ActiveSupport::Inflector),
-        custom_associations: CustomAssociations,
-        excluded_files: T::Set[String]
-      ).void
-    end
+    RAILS_ASSOCIATIONS = [
+      :belongs_to,
+      :has_many,
+      :has_one,
+      :has_and_belongs_to_many,
+    ].to_set #: Set[Symbol]
+
+    #: (
+    #|   inflector: singleton(ActiveSupport::Inflector),
+    #|   ?custom_associations: Set[Symbol],
+    #|   ?excluded_files: Set[String]
+    #| ) -> void
     def initialize(inflector:, custom_associations: Set.new, excluded_files: Set.new)
       @inflector = inflector
-      @associations = T.let(RAILS_ASSOCIATIONS + custom_associations, CustomAssociations)
-      @excluded_files = T.let(excluded_files, T::Set[String])
+      @associations = RAILS_ASSOCIATIONS + custom_associations #: Set[Symbol]
+      @excluded_files = excluded_files #: Set[String]
     end
 
-    sig do
-      override
-        .params(node: AST::Node, ancestors: T::Array[AST::Node], relative_file: String)
-        .returns(T.nilable(String))
-    end
+    # @override
+    #: (AST::Node node, ancestors: Array[AST::Node], relative_file: String) -> String?
     def constant_name_from_node(node, ancestors:, relative_file:)
       return unless NodeHelpers.method_call?(node)
       return if excluded?(relative_file)
@@ -56,28 +45,28 @@ def constant_name_from_node(node, ancestors:, relative_file:)
 
     private
 
-    sig { params(relative_file: String).returns(T::Boolean) }
+    #: (String relative_file) -> bool
     def excluded?(relative_file)
       @excluded_files.include?(relative_file)
     end
 
-    sig { params(node: AST::Node).returns(T::Boolean) }
+    #: (AST::Node node) -> bool
     def association?(node)
       method_name = NodeHelpers.method_name(node)
       @associations.include?(method_name)
     end
 
-    sig { params(arguments: T::Array[AST::Node]).returns(T.nilable(AST::Node)) }
+    #: (Array[AST::Node] arguments) -> AST::Node?
     def custom_class_name(arguments)
       association_options = arguments.detect { |n| NodeHelpers.hash?(n) }
       return unless association_options
 
       NodeHelpers.value_from_hash(association_options, :class_name)
     end
 
-    sig { params(arguments: T::Array[AST::Node]).returns(T.any(T.nilable(Symbol), T.nilable(String))) }
+    #: (Array[AST::Node] arguments) -> (Symbol | String)?
     def association_name(arguments)
-      association_name_node = T.must(arguments[0])
+      association_name_node = arguments[0] #: as !nil
       return unless NodeHelpers.symbol?(association_name_node)
 
       NodeHelpers.literal_value(association_name_node)