Apply strict sorbet typings for python files

Author: kbukum1

python/lib/dependabot/python/authed_url_builder.rb

@@ -1,9 +1,14 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
+require "sorbet-runtime"
+
 module Dependabot
   module Python
     class AuthedUrlBuilder
+      extend T::Sig
+
+      sig { params(credential: Credential).returns(T.nilable(String)) }
       def self.authed_url(credential:)
         token = credential.fetch("token", nil)
         url = credential.fetch("index-url", nil)

python/lib/dependabot/python/file_updater.rb

@@ -135,12 +135,13 @@ def updated_requirement_based_files
       sig { returns(T::Array[String]) }
       def pip_compile_index_urls
         if credentials.any?(&:replaces_base?)
-          credentials.select(&:replaces_base?).map { |cred| AuthedUrlBuilder.authed_url(credential: cred) }
+          credentials.select(&:replaces_base?)
+                     .filter_map { |cred| AuthedUrlBuilder.authed_url(credential: cred) }
         else
-          urls = credentials.map { |cred| AuthedUrlBuilder.authed_url(credential: cred) }
+          urls = credentials.filter_map { |cred| AuthedUrlBuilder.authed_url(credential: cred) }
           # If there are no credentials that replace the base, we need to
           # ensure that the base URL is included in the list of extra-index-urls.
-          [nil, *urls]
+          urls
         end
       end
 

python/lib/dependabot/python/file_updater/pip_compile_file_updater.rb

@@ -12,6 +12,7 @@
 require "dependabot/python/native_helpers"
 require "dependabot/python/name_normaliser"
 require "dependabot/python/authed_url_builder"
+require "sorbet-runtime"
 
 module Dependabot
   module Python
@@ -303,16 +304,20 @@ def setup_cfg(file)
 
         sig { params(file: Dependabot::DependencyFile).returns(T.nilable(String)) }
         def freeze_dependency_requirement(file)
-          return file.content unless file.name.end_with?(".in")
+          content = file.content
+          return content unless file.name.end_with?(".in")
 
           old_req = T.must(T.must(dependency).previous_requirements)
                      .find { |r| r[:file] == file.name }
 
-          return file.content unless old_req
-          return file.content if old_req == "==#{T.must(dependency).version}"
+          content = file.content
+          return content unless old_req
+          return content if old_req == "==#{T.must(dependency).version}"
+
+          return unless content
 
           RequirementReplacer.new(
-            content: file.content,
+            content: content,
             dependency_name: T.must(dependency).name,
             old_requirement: old_req[:requirement],
             new_requirement: "==#{T.must(dependency).version}",
@@ -322,17 +327,20 @@ def freeze_dependency_requirement(file)
 
         sig { params(file: Dependabot::DependencyFile).returns(T.nilable(String)) }
         def update_dependency_requirement(file)
-          return file.content unless file.name.end_with?(".in")
+          content = file.content
+          return content unless file.name.end_with?(".in")
 
           old_req = T.must(T.must(dependency).previous_requirements)
                      .find { |r| r[:file] == file.name }
           new_req = T.must(dependency).requirements
                      .find { |r| r[:file] == file.name }
-          return file.content unless old_req&.fetch(:requirement)
-          return file.content if old_req == new_req
+          return content unless old_req&.fetch(:requirement)
+          return content if old_req == new_req
+
+          return unless content
 
           RequirementReplacer.new(
-            content: file.content,
+            content: content,
             dependency_name: T.must(dependency).name,
             old_requirement: old_req[:requirement],
             new_requirement: T.must(new_req)[:requirement],
@@ -449,7 +457,7 @@ def deps_to_augment_hashes_for(updated_content, original_content)
 
         sig { params(name: String, version: String, algorithm: String).returns(T::Array[String]) }
         def package_hashes_for(name:, version:, algorithm:)
-          index_urls = @index_urls || [nil]
+          index_urls = @index_urls&.any? ? @index_urls : [nil]
           hashes = []
 
           index_urls.each do |index_url|

python/lib/dependabot/python/file_updater/pipfile_preparer.rb

@@ -75,8 +75,11 @@ def pipfile_sources
         end
 
         sig do
-          params(source: T::Hash[String, String],
-                 credentials: T::Array[Dependabot::Credential]).returns(T.nilable(T::Hash[String, String]))
+          params(
+            source: T::Hash[String, String],
+            credentials: T::Array[Dependabot::Credential]
+          )
+            .returns(T.nilable(T::Hash[String, String]))
         end
         def sub_auth_url(source, credentials)
           if source["url"]&.include?("${")
@@ -88,7 +91,9 @@ def sub_auth_url(source, credentials)
 
             return nil if source_cred.nil?
 
-            source["url"] = AuthedUrlBuilder.authed_url(credential: source_cred)
+            source_url = AuthedUrlBuilder.authed_url(credential: source_cred)
+
+            source["url"] = source_url if source_url
           end
 
           source

python/lib/dependabot/python/file_updater/requirement_file_updater.rb

@@ -1,69 +1,104 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 require "dependabot/python/requirement_parser"
 require "dependabot/python/file_updater"
 require "dependabot/shared_helpers"
 require "dependabot/python/native_helpers"
+require "sorbet-runtime"
 
 module Dependabot
   module Python
     class FileUpdater
       class RequirementFileUpdater
+        extend T::Sig
+
         require_relative "requirement_replacer"
 
-        attr_reader :dependencies
+        sig { returns(T::Array[Dependabot::DependencyFile]) }
         attr_reader :dependency_files
+
+        sig { returns(T::Array[Dependabot::Credential]) }
         attr_reader :credentials
 
+        sig { returns(T::Array[Dependabot::Dependency]) }
+        attr_reader :dependencies
+
+        sig do
+          params(
+            dependencies: T::Array[Dependabot::Dependency],
+            dependency_files: T::Array[Dependabot::DependencyFile],
+            credentials: T::Array[Dependabot::Credential],
+            index_urls: T.nilable(T::Array[String])
+          ).void
+        end
         def initialize(dependencies:, dependency_files:, credentials:, index_urls: nil)
           @dependencies = dependencies
           @dependency_files = dependency_files
           @credentials = credentials
           @index_urls = index_urls
         end
 
+        sig { returns(T::Array[Dependabot::DependencyFile]) }
         def updated_dependency_files
-          @updated_dependency_files ||= fetch_updated_dependency_files
+          @updated_dependency_files ||= T.let(
+            fetch_updated_dependency_files,
+            T.nilable(T::Array[DependencyFile])
+          )
         end
 
         private
 
+        sig { returns(Dependabot::Dependency) }
         def dependency
           # For now, we'll only ever be updating a single dependency
-          dependencies.first
+          T.must(dependencies.first)
         end
 
+        sig { returns(T::Array[Dependabot::DependencyFile]) }
         def fetch_updated_dependency_files
-          reqs = dependency.requirements.zip(dependency.previous_requirements)
+          reqs = dependency.requirements.zip(dependency.previous_requirements || [])
 
           reqs.filter_map do |(new_req, old_req)|
             next if new_req == old_req
 
             file = get_original_file(new_req.fetch(:file)).dup
             updated_content =
               updated_requirement_or_setup_file_content(new_req, old_req)
-            next if updated_content == file.content
+            next if updated_content == file&.content
 
-            file.content = updated_content
+            file&.content = updated_content
             file
           end
         end
 
+        sig do
+          params(
+            new_req: T::Hash[Symbol, T.untyped],
+            old_req: T.nilable(T::Hash[Symbol, T.untyped])
+          ).returns(T.nilable(String))
+        end
         def updated_requirement_or_setup_file_content(new_req, old_req)
           original_file = get_original_file(new_req.fetch(:file))
           raise "Could not find a dependency file for #{new_req}" unless original_file
 
+          original_content = original_file.content
+          return original_content if original_content.nil?
+
           RequirementReplacer.new(
-            content: original_file.content,
+            content: original_content,
             dependency_name: dependency.name,
-            old_requirement: old_req.fetch(:requirement),
+            old_requirement: old_req&.fetch(:requirement),
             new_requirement: new_req.fetch(:requirement),
             new_hash_version: dependency.version,
             index_urls: @index_urls
           ).updated_content
         end
 
+        sig do
+          params(filename: String)
+            .returns(T.nilable(Dependabot::DependencyFile))
+        end
         def get_original_file(filename)
           dependency_files.find { |f| f.name == filename }
         end