Merge pull request #19195 from Homebrew/sorbet-strict-more-rubocops

More Sorbet `typed: strict` RuboCops

Library/Homebrew/rubocops/blank.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 module RuboCop
@@ -44,6 +44,7 @@ class Blank < Base
           )
         PATTERN
 
+        sig { params(node: RuboCop::AST::Node).void }
         def on_or(node)
           nil_or_empty?(node) do |var1, var2|
             return if var1 != var2
@@ -57,14 +58,16 @@ def on_or(node)
 
         private
 
+        sig { params(corrector: RuboCop::Cop::Corrector, node: RuboCop::AST::Node).void }
         def autocorrect(corrector, node)
           variable1, _variable2 = nil_or_empty?(node)
           range = node.source_range
           corrector.replace(range, replacement(variable1))
         end
 
+        sig { params(node: T.nilable(RuboCop::AST::Node)).returns(String) }
         def replacement(node)
-          node.respond_to?(:source) ? "#{node.source}.blank?" : "blank?"
+          node.respond_to?(:source) ? "#{node&.source}.blank?" : "blank?"
         end
       end
     end

Library/Homebrew/rubocops/bottle.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "rubocops/extend/formula_cop"
@@ -174,12 +174,14 @@ def audit_formula(formula_nodes)
           end
         end
 
+        sig { params(nodes: T::Array[RuboCop::AST::SendNode]).returns(T::Array[T.any(String, Symbol)]) }
         def sha256_order(nodes)
           nodes.map do |node|
             sha256_bottle_tag node
           end
         end
 
+        sig { params(node: AST::SendNode).returns(T.any(String, Symbol)) }
         def sha256_bottle_tag(node)
           hash_pair = node.last_argument.pairs.last
           if hash_pair.key.sym_type?

Library/Homebrew/rubocops/cask/array_alphabetization.rb

@@ -7,6 +7,7 @@ module Cask
       class ArrayAlphabetization < Base
         extend AutoCorrector
 
+        sig { params(node: RuboCop::AST::SendNode).void }
         def on_send(node)
           return unless [:zap, :uninstall].include?(node.method_name)
 

Library/Homebrew/rubocops/checksum.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "rubocops/extend/formula_cop"
@@ -23,6 +23,7 @@ def audit_formula(formula_nodes)
           end
         end
 
+        sig { params(checksum: T.nilable(RuboCop::AST::Node)).void }
         def audit_sha256(checksum)
           return if checksum.nil?
 
@@ -37,7 +38,7 @@ def audit_sha256(checksum)
 
           return unless regex_match_group(checksum, /[^a-f0-9]+/i)
 
-          add_offense(@offensive_source_range, message: "sha256 contains invalid characters")
+          add_offense(T.must(@offensive_source_range), message: "sha256 contains invalid characters")
         end
       end
 
@@ -54,9 +55,9 @@ def audit_formula(formula_nodes)
             next unless regex_match_group(checksum, /[A-F]+/)
 
             add_offense(@offensive_source_range, message: "sha256 should be lowercase") do |corrector|
-              correction = @offensive_node.source.downcase
-              corrector.insert_before(@offensive_node.source_range, correction)
-              corrector.remove(@offensive_node.source_range)
+              correction = T.must(@offensive_node).source.downcase
+              corrector.insert_before(T.must(@offensive_node).source_range, correction)
+              corrector.remove(T.must(@offensive_node).source_range)
             end
           end
         end

Library/Homebrew/rubocops/compact_blank.rb

@@ -62,6 +62,7 @@ class CompactBlank < Base
               (sym :blank?)))
         PATTERN
 
+        sig { params(node: RuboCop::AST::SendNode).void }
         def on_send(node)
           return unless bad_method?(node)
 
@@ -74,6 +75,7 @@ def on_send(node)
 
         private
 
+        sig { params(node: RuboCop::AST::SendNode).returns(T::Boolean) }
         def bad_method?(node)
           return true if reject_with_block_pass?(node)
 
@@ -93,6 +95,7 @@ def use_hash_value_block_argument?(arguments, receiver_in_block)
           arguments.length == 2 && arguments[1].source == receiver_in_block.source
         end
 
+        sig { params(node: RuboCop::AST::SendNode).returns(Parser::Source::Range) }
         def offense_range(node)
           end_pos = if node.parent&.block_type? && node.parent&.send_node == node
             node.parent.source_range.end_pos
@@ -103,6 +106,7 @@ def offense_range(node)
           range_between(node.loc.selector.begin_pos, end_pos)
         end
 
+        sig { params(node: RuboCop::AST::SendNode).returns(String) }
         def preferred_method(node)
           node.method?(:reject) ? "compact_blank" : "compact_blank!"
         end

Library/Homebrew/rubocops/conflicts.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "rubocops/extend/formula_cop"
@@ -22,7 +22,7 @@ def audit_formula(formula_nodes)
 
             reason = parameters(conflicts_with_call).last.values.first
             offending_node(reason)
-            name = Regexp.new(@formula_name, Regexp::IGNORECASE)
+            name = Regexp.new(T.must(@formula_name), Regexp::IGNORECASE)
             reason_text = string_content(reason).sub(name, "")
             first_word = reason_text.split.first
 
@@ -45,7 +45,7 @@ def audit_formula(formula_nodes)
           if !tap_style_exception?(:versioned_formulae_conflicts_allowlist) && method_called_ever?(body_node,
                                                                                                    :conflicts_with)
             problem MSG do |corrector|
-              corrector.replace(@offensive_node.source_range, "keg_only :versioned_formula")
+              corrector.replace(T.must(@offensive_node).source_range, "keg_only :versioned_formula")
             end
           end
         end

Library/Homebrew/rubocops/deprecate_disable.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "rubocops/extend/formula_cop"
@@ -61,13 +61,13 @@ def audit_formula(formula_nodes)
 
               if reason_string.start_with?("it ")
                 problem "Do not start the reason with `it`" do |corrector|
-                  corrector.replace(@offensive_node.source_range, "\"#{reason_string[3..]}\"")
+                  corrector.replace(T.must(@offensive_node).source_range, "\"#{reason_string[3..]}\"")
                 end
               end
 
               if PUNCTUATION_MARKS.include?(reason_string[-1])
                 problem "Do not end the reason with a punctuation mark" do |corrector|
-                  corrector.replace(@offensive_node.source_range, "\"#{reason_string.chop}\"")
+                  corrector.replace(T.must(@offensive_node).source_range, "\"#{reason_string.chop}\"")
                 end
               end
             end

Library/Homebrew/rubocops/desc.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "rubocops/extend/formula_cop"
@@ -17,7 +17,7 @@ class Desc < FormulaCop
         def audit_formula(formula_nodes)
           body_node = formula_nodes.body_node
 
-          @name = @formula_name
+          @name = T.let(@formula_name, T.nilable(String))
           desc_call = find_node_method_by_name(body_node, :desc)
           offending_node(formula_nodes.class_node) if body_node.nil?
           audit_desc(:formula, @name, desc_call)

Library/Homebrew/rubocops/extend/formula_cop.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "rubocops/shared/helper_functions"
@@ -14,9 +14,10 @@ class FormulaCop < Base
       abstract!
       exclude_from_registry
 
+      sig { returns(T.nilable(String)) }
       attr_accessor :file_path
 
-      @registry = Registry.global
+      @registry = T.let(Registry.global, RuboCop::Cop::Registry)
 
       class FormulaNodes < T::Struct
         prop :node, RuboCop::AST::ClassNode
@@ -26,15 +27,18 @@ class FormulaNodes < T::Struct
       end
 
       # This method is called by RuboCop and is the main entry point.
+      sig { params(node: RuboCop::AST::ClassNode).void }
       def on_class(node)
-        @file_path = processed_source.file_path
+        @file_path = T.let(processed_source.file_path, T.nilable(String))
         return unless file_path_allowed?
         return unless formula_class?(node)
 
-        class_node, parent_class_node, @body = *node
-        @formula_name = Pathname.new(@file_path).basename(".rb").to_s
-        @tap_style_exceptions = nil
-        audit_formula(FormulaNodes.new(node:, class_node:, parent_class_node:, body_node: @body))
+        class_node, parent_class_node, body = *node
+        @body = T.let(body, T.nilable(RuboCop::AST::Node))
+
+        @formula_name = T.let(Pathname.new(@file_path).basename(".rb").to_s, T.nilable(String))
+        @tap_style_exceptions = T.let(nil, T.nilable(T::Hash[Symbol, T::Array[String]]))
+        audit_formula(FormulaNodes.new(node:, class_node:, parent_class_node:, body_node: T.must(@body)))
       end
 
       sig { abstract.params(formula_nodes: FormulaNodes).void }
@@ -44,7 +48,13 @@ def audit_formula(formula_nodes); end
       #
       # @param urls [Array] url/mirror method call nodes
       # @param regex [Regexp] pattern to match URLs
-      def audit_urls(urls, regex)
+      sig {
+        params(
+          urls: T::Array[RuboCop::AST::Node], regex: Regexp,
+          _block: T.proc.params(arg0: T::Array[RuboCop::AST::Node], arg1: String, arg2: Integer).void
+        ).void
+      }
+      def audit_urls(urls, regex, &_block)
         urls.each_with_index do |url_node, index|
           url_string_node = parameters(url_node).first
           url_string = string_content(url_string_node)
@@ -59,6 +69,7 @@ def audit_urls(urls, regex)
       # Returns if the formula depends on dependency_name.
       #
       # @param dependency_name dependency's name
+      sig { params(dependency_name: T.any(String, Symbol), types: Symbol).returns(T::Boolean) }
       def depends_on?(dependency_name, *types)
         return false if @body.nil?
 
@@ -69,13 +80,20 @@ def depends_on?(dependency_name, *types)
         end
         return false if idx.nil?
 
-        @offensive_node = dependency_nodes[idx]
+        @offensive_node = T.let(dependency_nodes[idx], T.nilable(RuboCop::AST::Node))
 
         true
       end
 
       # Returns true if given dependency name and dependency type exist in given dependency method call node.
       # TODO: Add case where key of hash is an array
+      sig {
+        params(
+          node: RuboCop::AST::Node, name: T.nilable(T.any(String, Symbol)), type: Symbol,
+        ).returns(
+          T::Boolean,
+        )
+      }
       def depends_on_name_type?(node, name = nil, type = :required)
         name_match = !name # Match only by type when name is nil
 
@@ -88,8 +106,8 @@ def depends_on_name_type?(node, name = nil, type = :required)
           name_match ||= dependency_name_hash_match?(node, name) if type_match
         when :any
           type_match = true
-          name_match ||= required_dependency_name?(node, name)
-          name_match ||= dependency_name_hash_match?(node, name)
+          name_match ||= required_dependency_name?(node, name) || false
+          name_match ||= dependency_name_hash_match?(node, name) || false
         else
           type_match = false
         end
@@ -115,13 +133,15 @@ def depends_on_name_type?(node, name = nil, type = :required)
       EOS
 
       # Return all the caveats' string nodes in an array.
+      sig { returns(T::Array[RuboCop::AST::Node]) }
       def caveats_strings
         return [] if @body.nil?
 
-        find_strings(find_method_def(@body, :caveats))
+        find_strings(find_method_def(@body, :caveats)).to_a
       end
 
       # Returns the sha256 str node given a sha256 call node.
+      sig { params(call: RuboCop::AST::Node).returns(T.nilable(RuboCop::AST::Node)) }
       def get_checksum_node(call)
         return if parameters(call).empty? || parameters(call).nil?
 
@@ -144,28 +164,35 @@ def get_checksum_node(call)
       end
 
       # Yields to a block with comment text as parameter.
-      def audit_comments
+      sig { params(_block: T.proc.params(arg0: String).void).void }
+      def audit_comments(&_block)
         processed_source.comments.each do |comment_node|
           @offensive_node = comment_node
           yield comment_node.text
         end
       end
 
       # Returns true if the formula is versioned.
+      sig { returns(T::Boolean) }
       def versioned_formula?
+        return false if @formula_name.nil?
+
         @formula_name.include?("@")
       end
 
       # Returns the formula tap.
+      sig { returns(T.nilable(String)) }
       def formula_tap
-        return unless (match_obj = @file_path.match(%r{/(homebrew-\w+)/}))
+        return unless (match_obj = @file_path&.match(%r{/(homebrew-\w+)/}))
 
         match_obj[1]
       end
 
       # Returns the style exceptions directory from the file path.
+      sig { returns(T.nilable(String)) }
       def style_exceptions_dir
-        file_directory = File.dirname(@file_path)
+        file_directory = File.dirname(@file_path) if @file_path
+        return unless file_directory
 
         # if we're in a sharded subdirectory, look below that.
         directory_name = File.basename(file_directory)
@@ -189,6 +216,7 @@ def style_exceptions_dir
 
       # Returns whether the given formula exists in the given style exception list.
       # Defaults to the current formula being checked.
+      sig { params(list: Symbol, formula: T.nilable(String)).returns(T::Boolean) }
       def tap_style_exception?(list, formula = nil)
         if @tap_style_exceptions.nil? && !formula_tap.nil?
           @tap_style_exceptions = {}
@@ -209,11 +237,12 @@ def tap_style_exception?(list, formula = nil)
         return false if @tap_style_exceptions.nil? || @tap_style_exceptions.count.zero?
         return false unless @tap_style_exceptions.key? list
 
-        @tap_style_exceptions[list].include?(formula || @formula_name)
+        T.must(@tap_style_exceptions[list]).include?(formula || @formula_name)
       end
 
       private
 
+      sig { params(node: RuboCop::AST::Node).returns(T::Boolean) }
       def formula_class?(node)
         _, class_node, = *node
         class_names = %w[
@@ -223,19 +252,24 @@ def formula_class?(node)
           AmazonWebServicesFormula
         ]
 
-        class_node && class_names.include?(string_content(class_node))
+        !!(class_node && class_names.include?(string_content(class_node)))
       end
 
+      sig { returns(T::Boolean) }
       def file_path_allowed?
         return true if @file_path.nil? # file_path is nil when source is directly passed to the cop, e.g. in specs
 
         !@file_path.include?("/Library/Homebrew/test/")
       end
 
+      sig { returns(T::Array[Symbol]) }
       def on_system_methods
-        @on_system_methods ||= [:intel, :arm, :macos, :linux, :system, *MacOSVersion::SYMBOLS.keys].map do |m|
-          :"on_#{m}"
-        end
+        @on_system_methods ||= T.let(
+          [:intel, :arm, :macos, :linux, :system, *MacOSVersion::SYMBOLS.keys].map do |m|
+            :"on_#{m}"
+          end,
+          T.nilable(T::Array[Symbol]),
+        )
       end
     end
   end