Sorbet: add strict typing for GemfileUpdater and GemspecDependencyNam…

…eFinder
dependabot · Jan 16, 2025 · 899227a · 899227a
1 parent e0c8d70
commit 899227a
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 11 deletions.
diff --git a/bundler/lib/dependabot/bundler/file_updater/gemfile_updater.rb b/bundler/lib/dependabot/bundler/file_updater/gemfile_updater.rb
@@ -1,4 +1,4 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 require "dependabot/bundler/file_updater"
@@ -7,19 +7,23 @@ module Dependabot
   module Bundler
     class FileUpdater
       class GemfileUpdater
+        extend T::Sig
+
         GEMFILE_FILENAMES = %w(Gemfile gems.rb).freeze
 
         require_relative "git_pin_replacer"
         require_relative "git_source_remover"
         require_relative "requirement_replacer"
 
+        sig { params(dependencies: T::Array[Dependabot::Dependency], gemfile: Dependabot::DependencyFile).void }
         def initialize(dependencies:, gemfile:)
           @dependencies = dependencies
           @gemfile = gemfile
         end
 
+        sig { returns(String) }
         def updated_gemfile_content
-          content = gemfile.content
+          content = T.must(gemfile.content)
 
           dependencies.each do |dependency|
             content = replace_gemfile_version_requirement(
@@ -38,21 +42,25 @@ def updated_gemfile_content
 
         private
 
+        sig { returns(T::Array[Dependabot::Dependency]) }
         attr_reader :dependencies
+
+        sig { returns(Dependabot::DependencyFile) }
         attr_reader :gemfile
 
+        sig { params(dependency: Dependabot::Dependency, file: Dependabot::DependencyFile, content: String).returns(String) }
         def replace_gemfile_version_requirement(dependency, file, content)
           return content unless requirement_changed?(file, dependency)
 
           updated_requirement =
             dependency.requirements
                       .find { |r| r[:file] == file.name }
-                      .fetch(:requirement)
+                      &.fetch(:requirement)
 
           previous_requirement =
             dependency.previous_requirements
-                      .find { |r| r[:file] == file.name }
-                      .fetch(:requirement)
+                      &.find { |r| r[:file] == file.name }
+                      &.fetch(:requirement)
 
           RequirementReplacer.new(
             dependency: dependency,
@@ -62,27 +70,30 @@ def replace_gemfile_version_requirement(dependency, file, content)
           ).rewrite(content)
         end
 
+        sig { params(file: Dependabot::DependencyFile, dependency: Dependabot::Dependency).returns(T::Boolean) }
         def requirement_changed?(file, dependency)
           changed_requirements =
-            dependency.requirements - dependency.previous_requirements
+            dependency.requirements - T.must(dependency.previous_requirements)
 
           changed_requirements.any? { |f| f[:file] == file.name }
         end
 
+        sig { params(dependency: Dependabot::Dependency).returns(T::Boolean) }
         def remove_git_source?(dependency)
           old_gemfile_req =
             dependency.previous_requirements
-                      .find { |f| GEMFILE_FILENAMES.include?(f[:file]) }
+                      &.find { |f| GEMFILE_FILENAMES.include?(f[:file]) }
 
           return false unless old_gemfile_req&.dig(:source, :type) == "git"
 
           new_gemfile_req =
             dependency.requirements
                       .find { |f| GEMFILE_FILENAMES.include?(f[:file]) }
 
-          new_gemfile_req[:source].nil?
+          T.must(new_gemfile_req)[:source].nil?
         end
 
+        sig { params(dependency: Dependabot::Dependency, file: Dependabot::DependencyFile).returns(T::Boolean) }
         def update_git_pin?(dependency, file)
           new_gemfile_req =
             dependency.requirements
@@ -91,18 +102,23 @@ def update_git_pin?(dependency, file)
 
           # If the new requirement is a git dependency with a ref then there's
           # no harm in doing an update
-          new_gemfile_req.dig(:source, :ref)
+          !T.must(new_gemfile_req).dig(:source, :ref).nil?
         end
 
+        sig { params(dependency: Dependabot::Dependency, content: String).returns(String) }
         def remove_gemfile_git_source(dependency, content)
           GitSourceRemover.new(dependency: dependency).rewrite(content)
         end
 
+        sig do
+          params(dependency: Dependabot::Dependency, file: Dependabot::DependencyFile, content: String).returns(String)
+        end
         def update_gemfile_git_pin(dependency, file, content)
           new_pin =
             dependency.requirements
                       .find { |f| f[:file] == file.name }
-                      .fetch(:source).fetch(:ref)
+                      &.fetch(:source)
+                      &.fetch(:ref)
 
           GitPinReplacer
             .new(dependency: dependency, new_pin: new_pin)

diff --git a/bundler/lib/dependabot/bundler/file_updater/gemspec_dependency_name_finder.rb b/bundler/lib/dependabot/bundler/file_updater/gemspec_dependency_name_finder.rb
@@ -1,4 +1,4 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 require "parser/current"
@@ -8,13 +8,20 @@ module Dependabot
   module Bundler
     class FileUpdater
       class GemspecDependencyNameFinder
+        extend T::Sig
+
+        ChildNode = T.type_alias { T.nilable(T.any(Parser::AST::Node, Symbol, String)) }
+
+        sig { returns(String) }
         attr_reader :gemspec_content
 
+        sig { params(gemspec_content: String).void }
         def initialize(gemspec_content:)
           @gemspec_content = gemspec_content
         end
 
         # rubocop:disable Security/Eval
+        sig { returns(T.nilable(String)) }
         def dependency_name
           ast = Parser::CurrentRuby.parse(gemspec_content)
           dependency_name_node = find_dependency_name_node(ast)
@@ -30,6 +37,7 @@ def dependency_name
 
         private
 
+        sig { params(node: ChildNode).returns(T.nilable(Parser::AST::Node)) }
         def find_dependency_name_node(node)
           return unless node.is_a?(Parser::AST::Node)
           return node if declares_dependency_name?(node)
@@ -40,6 +48,7 @@ def find_dependency_name_node(node)
           end
         end
 
+        sig { params(node: ChildNode).returns(T::Boolean) }
         def declares_dependency_name?(node)
           return false unless node.is_a?(Parser::AST::Node)