Merge pull request #11716 from dependabot/markhallen/strictly-type-el…

…m-cli-parser Add strict typing for the elm update checker
dependabot · Mar 4, 2025 · d6085b3 · d6085b3
2 parents 73b7b40 + 3ad27bf
commit d6085b3
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 26 deletions.
diff --git a/elm/lib/dependabot/elm/update_checker.rb b/elm/lib/dependabot/elm/update_checker.rb
@@ -1,4 +1,4 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 require "excon"
@@ -10,15 +10,19 @@
 module Dependabot
   module Elm
     class UpdateChecker < Dependabot::UpdateCheckers::Base
+      extend T::Sig
+
       require_relative "update_checker/requirements_updater"
       require_relative "update_checker/elm_19_version_resolver"
 
+      sig { override.returns(T.nilable(Dependabot::Version)) }
       def latest_version
-        @latest_version ||= candidate_versions.max
+        @latest_version ||= T.let(candidate_versions.max, T.nilable(Dependabot::Version))
       end
 
       # Overwrite the base class to allow multi-dependency update PRs for
       # dependencies for which we don't have a version.
+      sig { override.params(requirements_to_unlock: T.nilable(Symbol)).returns(T::Boolean) }
       def can_update?(requirements_to_unlock:)
         if dependency.appears_in_lockfile?
           version_can_update?(requirements_to_unlock: requirements_to_unlock)
@@ -28,22 +32,28 @@ def can_update?(requirements_to_unlock:)
           requirements_can_update?
         elsif requirements_to_unlock == :all
           updated_dependencies_after_full_unlock.any?
+        else
+          false
         end
       end
 
+      sig { override.returns(T.nilable(Dependabot::Version)) }
       def latest_resolvable_version
-        @latest_resolvable_version ||=
+        @latest_resolvable_version ||= T.let(
           version_resolver
-          .latest_resolvable_version(unlock_requirement: :own)
+          .latest_resolvable_version(unlock_requirement: :own), T.nilable(Dependabot::Version)
+        )
       end
 
+      sig { override.returns(T.nilable(Dependabot::Version)) }
       def latest_resolvable_version_with_no_unlock
         # Irrelevant, since Elm has a single dependency file (well, there's
         # also `exact-dependencies.json`, but it's not recommended that that
         # is committed).
         nil
       end
 
+      sig { override.returns(T::Array[T::Hash[Symbol, T.nilable(String)]]) }
       def updated_requirements
         RequirementsUpdater.new(
           requirements: dependency.requirements,
@@ -53,8 +63,9 @@ def updated_requirements
 
       private
 
+      sig { returns(Elm19VersionResolver) }
       def version_resolver
-        @version_resolver ||=
+        @version_resolver ||= T.let(
           begin
             unless dependency.requirements.any? { |r| r.fetch(:file) == MANIFEST_FILE }
               raise Dependabot::DependencyFileNotResolvable, "No #{MANIFEST_FILE} found"
@@ -64,18 +75,22 @@ def version_resolver
               dependency: dependency,
               dependency_files: dependency_files
             )
-          end
+          end, T.nilable(Elm19VersionResolver)
+        )
       end
 
+      sig { override.returns(T::Array[Dependabot::Dependency]) }
       def updated_dependencies_after_full_unlock
         version_resolver.updated_dependencies_after_full_unlock
       end
 
+      sig { override.returns(T::Boolean) }
       def latest_version_resolvable_with_full_unlock?
         latest_version == version_resolver
                           .latest_resolvable_version(unlock_requirement: :all)
       end
 
+      sig { returns(T::Array[Dependabot::Version]) }
       def candidate_versions
         filtered = all_versions
                    .reject { |v| ignore_requirements.any? { |r| r.satisfied_by?(v) } }
@@ -87,17 +102,20 @@ def candidate_versions
         filtered
       end
 
+      sig { params(versions_array: T::Array[Dependabot::Version]).returns(T::Array[Dependabot::Version]) }
       def filter_lower_versions(versions_array)
         return versions_array unless current_version
 
         versions_array
           .select { |version| version > current_version }
       end
 
+      sig { returns(T::Array[Dependabot::Version]) }
       def all_versions
-        @all_versions ||= fetch_all_versions
+        @all_versions ||= T.let(fetch_all_versions, T.nilable(T::Array[Dependabot::Version]))
       end
 
+      sig { returns(T::Array[Dependabot::Version]) }
       def fetch_all_versions
         response = Dependabot::RegistryClient.get(
           url: "https://package.elm-lang.org/packages/#{dependency.name}/releases.json"
@@ -113,6 +131,7 @@ def fetch_all_versions
 
       # Overwrite the base class's requirements_up_to_date? method to instead
       # check whether the latest version is allowed
+      sig { override.returns(T::Boolean) }
       def requirements_up_to_date?
         return false unless latest_version
 

diff --git a/elm/lib/dependabot/elm/update_checker/cli_parser.rb b/elm/lib/dependabot/elm/update_checker/cli_parser.rb
@@ -1,4 +1,4 @@
-# typed: true
+# typed: strong
 # frozen_string_literal: true
 
 require "dependabot/elm/version"
@@ -8,9 +8,12 @@ module Dependabot
   module Elm
     class UpdateChecker
       class CliParser
+        extend T::Sig
+
         INSTALL_DEPENDENCY_REGEX = %r{([^\s]+\/[^\s]+)\s+(\d+\.\d+\.\d+)}
         UPGRADE_DEPENDENCY_REGEX = %r{([^\s]+\/[^\s]+) \(\d+\.\d+\.\d+ => (\d+\.\d+\.\d+)\)}
 
+        sig { params(text: String).returns(T::Hash[String, Elm::Version]) }
         def self.decode_install_preview(text)
           installs = {}
 

diff --git a/elm/lib/dependabot/elm/update_checker/elm_19_version_resolver.rb b/elm/lib/dependabot/elm/update_checker/elm_19_version_resolver.rb
@@ -1,4 +1,4 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 require "open3"
@@ -28,6 +28,9 @@ class UnrecoverableState < StandardError; end
         def initialize(dependency:, dependency_files:)
           @dependency = dependency
           @dependency_files = dependency_files
+
+          @install_metadata = T.let(nil, T.nilable(T::Hash[String, Dependabot::Elm::Version]))
+          @original_dependency_details ||= T.let(nil, T.nilable(T::Array[Dependabot::Dependency]))
         end
 
         sig { params(unlock_requirement: Symbol).returns(T.nilable(Dependabot::Elm::Version)) }
@@ -113,22 +116,26 @@ def check_install_result(changed_deps)
 
         sig { returns(T::Hash[String, Dependabot::Elm::Version]) }
         def install_metadata
-          @install_metadata ||=
-            SharedHelpers.in_a_temporary_directory do
-              write_temporary_dependency_files
-
-              # Elm package install outputs a preview of the actions to be
-              # performed. We can use this preview to calculate whether it
-              # would do anything funny
-              dependency_name = Shellwords.escape(dependency.name)
-              command = "yes n | elm19 install #{dependency_name}"
-              response = run_shell_command(command)
-
-              CliParser.decode_install_preview(response)
-            rescue SharedHelpers::HelperSubprocessFailed => e
-              # 5) We bump our dep but elm blows up
-              handle_elm_errors(e)
-            end
+          @install_metadata ||= parse_install_metadata
+        end
+
+        sig { returns(T.any(T::Hash[String, Dependabot::Elm::Version], T.noreturn)) }
+        def parse_install_metadata
+          SharedHelpers.in_a_temporary_directory do
+            write_temporary_dependency_files
+
+            # Elm package install outputs a preview of the actions to be
+            # performed. We can use this preview to calculate whether it
+            # would do anything funny
+            dependency_name = Shellwords.escape(dependency.name)
+            command = "yes n | elm19 install #{dependency_name}"
+            response = run_shell_command(command)
+
+            CliParser.decode_install_preview(response)
+          rescue SharedHelpers::HelperSubprocessFailed => e
+            # 5) We bump our dep but elm blows up
+            handle_elm_errors(e)
+          end
         end
 
         sig { params(command: String).returns(::String) }
@@ -151,7 +158,7 @@ def run_shell_command(command)
           )
         end
 
-        sig { params(error: Dependabot::DependabotError).void }
+        sig { params(error: Dependabot::DependabotError).returns(T.noreturn) }
         def handle_elm_errors(error)
           if error.message.include?("OLD DEPENDENCIES") ||
              error.message.include?("BAD JSON")