Added TypeCheck as strict for file_parser

dependabot · Jan 13, 2025 · 55b5351 · 55b5351
1 parent eb81114
commit 55b5351
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 34 deletions.
diff --git a/python/lib/dependabot/python/file_parser.rb b/python/lib/dependabot/python/file_parser.rb
@@ -1,4 +1,4 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 require "dependabot/dependency"
@@ -18,12 +18,13 @@
 module Dependabot
   module Python
     class FileParser < Dependabot::FileParsers::Base
+      extend T::Sig
       require_relative "file_parser/pipfile_files_parser"
       require_relative "file_parser/pyproject_files_parser"
       require_relative "file_parser/setup_file_parser"
       require_relative "file_parser/python_requirement_parser"
 
-      DEPENDENCY_GROUP_KEYS = [
+      DEPENDENCY_GROUP_KEYS = T.let([
         {
           pipfile: "packages",
           lockfile: "default"
@@ -32,7 +33,7 @@ class FileParser < Dependabot::FileParsers::Base
           pipfile: "dev-packages",
           lockfile: "develop"
         }
-      ].freeze
+      ].freeze, T::Array[T::Hash[T.untyped, T.untyped]])
       REQUIREMENT_FILE_EVALUATION_ERRORS = %w(
         InstallationError RequirementsFileParseError InvalidMarker
         InvalidRequirement ValueError RecursionError
@@ -43,6 +44,7 @@ class FileParser < Dependabot::FileParsers::Base
       # in any way if any metric collection exception start happening
       UNDETECTED_PACKAGE_MANAGER_VERSION = "0.0"
 
+      sig { override.returns(T::Array[Dependabot::Dependency]) }
       def parse
         # TODO: setup.py from external dependencies is evaluated. Provide guards before removing this.
         raise Dependabot::UnexpectedExternalCode if @reject_external_code
@@ -57,7 +59,7 @@ def parse
         dependency_set.dependencies
       end
 
-      sig { returns(Ecosystem) }
+      sig { override.returns(Ecosystem) }
       def ecosystem
         @ecosystem ||= T.let(
           Ecosystem.new(
@@ -71,18 +73,16 @@ def ecosystem
 
       private
 
+      sig { returns(Dependabot::Python::LanguageVersionManager) }
       def language_version_manager
-        @language_version_manager ||=
-          LanguageVersionManager.new(
-            python_requirement_parser: python_requirement_parser
-          )
+        @language_version_manager ||= T.let(LanguageVersionManager.new(python_requirement_parser:
+                                        python_requirement_parser), T.nilable(LanguageVersionManager))
       end
 
+      sig { returns(Dependabot::Python::FileParser::PythonRequirementParser) }
       def python_requirement_parser
-        @python_requirement_parser ||=
-          FileParser::PythonRequirementParser.new(
-            dependency_files: dependency_files
-          )
+        @python_requirement_parser ||= T.let(FileParser::PythonRequirementParser.new(dependency_files: dependency_files),
+                                             T.nilable(FileParser::PythonRequirementParser))
       end
 
       sig { returns(Ecosystem::VersionManager) }
@@ -91,7 +91,7 @@ def package_manager
           Dependabot.logger.info("Detected package manager : #{detected_package_manager.name}")
         end
 
-        @package_manager ||= detected_package_manager
+        @package_manager ||= T.let(detected_package_manager, T.nilable(Dependabot::Ecosystem::VersionManager))
       end
 
       sig { returns(Ecosystem::VersionManager) }
@@ -188,7 +188,7 @@ def package_manager_version(package_manager)
       end
 
       # setup python local setup on file parser stage
-      sig { void }
+      sig { returns(T.nilable(String)) }
       def setup_python_environment
         language_version_manager.install_required_python
 
@@ -231,24 +231,24 @@ def language
         )
       end
 
+      sig { returns(T::Array[Dependabot::DependencyFile]) }
       def requirement_files
         dependency_files.select { |f| f.name.end_with?(".txt", ".in") }
       end
 
+      sig { returns(T.nilable(PipfileFilesParser)) }
       def pipenv_dependencies
-        @pipenv_dependencies ||=
-          PipfileFilesParser
-          .new(dependency_files: dependency_files)
-          .dependency_set
+        @pipenv_dependencies ||= T.let(PipfileFilesParser.new(dependency_files:
+                                    dependency_files).dependency_set, T.nilable(PipfileFilesParser))
       end
 
+      sig { returns(T.nilable(PyprojectFilesParser))}
       def pyproject_file_dependencies
-        @pyproject_file_dependencies ||=
-          PyprojectFilesParser
-          .new(dependency_files: dependency_files)
-          .dependency_set
+        @pyproject_file_dependencies ||= T.let(PyprojectFilesParser.new(dependency_files:
+                                          dependency_files).dependency_set, T.nilable(PyprojectFilesParser))
       end
 
+      sig { returns(DependencySet)}
       def requirement_dependencies
         dependencies = DependencySet.new
         parsed_requirement_files.each do |dep|
@@ -286,20 +286,23 @@ def requirement_dependencies
         dependencies
       end
 
+      sig { params(name: String, version: String).returns(T::Boolean) }
       def old_pyyaml?(name, version)
-        major_version = version&.split(".")&.first
+        major_version = version.split(".").first
         return false unless major_version
 
         name == "pyyaml" && major_version < "6"
       end
 
+      sig { params(filename: String).returns(T::Array[String]) }
       def group_from_filename(filename)
         if filename.include?("dev") then ["dev-dependencies"]
         else
           ["dependencies"]
         end
       end
 
+      sig { params(dep: T.untyped).returns(T::Boolean) }
       def blocking_marker?(dep)
         return false if dep["markers"] == "None"
 
@@ -316,6 +319,7 @@ def blocking_marker?(dep)
         end
       end
 
+      sig { params(marker: T.untyped, python_version: T.nilable(T.any(String, Integer, Gem::Version))).returns(T::Boolean) }
       def marker_satisfied?(marker, python_version)
         conditions = marker.split(/\s+(and|or)\s+/)
 
@@ -356,13 +360,15 @@ def evaluate_condition(condition, python_version)
         end
       end
 
+      sig { void }
       def setup_file_dependencies
-        @setup_file_dependencies ||=
+        @setup_file_dependencies ||= T.let(
           SetupFileParser
           .new(dependency_files: dependency_files)
-          .dependency_set
+          .dependency_set, T.untyped)
       end
 
+      sig { returns(T.untyped) }
       def parsed_requirement_files
         SharedHelpers.in_a_temporary_directory do
           write_temporary_dependency_files
@@ -383,6 +389,7 @@ def parsed_requirement_files
         raise Dependabot::DependencyFileNotEvaluatable, e.message
       end
 
+      sig { params(requirements: T.untyped).returns(T.untyped) }
       def check_requirements(requirements)
         requirements.each do |dep|
           next unless dep["requirement"]
@@ -393,18 +400,22 @@ def check_requirements(requirements)
         end
       end
 
+      sig { returns(T::Boolean) }
       def pipcompile_in_file
         requirement_files.any? { |f| f.name.end_with?(PipCompilePackageManager::MANIFEST_FILENAME) }
       end
 
+      sig { returns(T::Boolean) }
       def pipenv_files
         dependency_files.any? { |f| f.name == PipenvPackageManager::LOCKFILE_FILENAME }
       end
 
+      sig { returns(T.nilable(TrueClass)) }
       def poetry_files
         true if get_original_file(PoetryPackageManager::LOCKFILE_NAME)
       end
 
+      sig { returns(T::Array[Dependabot::DependencyFile]) }
       def write_temporary_dependency_files
         dependency_files
           .reject { |f| f.name == ".python-version" }
@@ -415,6 +426,7 @@ def write_temporary_dependency_files
           end
       end
 
+      sig { params(file: T.untyped).returns(T.untyped) }
       def remove_imports(file)
         return file.content if file.path.end_with?(".tar.gz", ".whl", ".zip")
 
@@ -424,10 +436,12 @@ def remove_imports(file)
             .join
       end
 
+      sig { params(name: String, extras: T::Array[String]).returns(String) }
       def normalised_name(name, extras = [])
         NameNormaliser.normalise_including_extras(name, extras)
       end
 
+      sig { override.void }
       def check_required_files
         filenames = dependency_files.map(&:name)
         return if filenames.any? { |name| name.end_with?(".txt", ".in") }
@@ -439,37 +453,45 @@ def check_required_files
         raise "Missing required files!"
       end
 
+      sig { returns(T.nilable(Dependabot::DependencyFile)) }
       def pipfile
-        @pipfile ||= get_original_file("Pipfile")
+        @pipfile ||= T.let(get_original_file("Pipfile"), T.nilable(Dependabot::DependencyFile))
       end
 
+      sig { returns(T.nilable(Dependabot::DependencyFile)) }
       def pipfile_lock
-        @pipfile_lock ||= get_original_file("Pipfile.lock")
+        @pipfile_lock ||= T.let(get_original_file("Pipfile.lock"), T.nilable(Dependabot::DependencyFile))
       end
 
+      sig { returns(T.nilable(Dependabot::DependencyFile)) }
       def pyproject
-        @pyproject ||= get_original_file("pyproject.toml")
+        @pyproject ||= T.let(get_original_file("pyproject.toml"), T.nilable(Dependabot::DependencyFile))
       end
 
+      sig { returns(T.nilable(Dependabot::DependencyFile)) }
       def poetry_lock
-        @poetry_lock ||= get_original_file("poetry.lock")
+        @poetry_lock ||= T.let(get_original_file("poetry.lock"), T.nilable(Dependabot::DependencyFile))
       end
 
+      sig { returns(T.nilable(Dependabot::DependencyFile))}
       def setup_file
-        @setup_file ||= get_original_file("setup.py")
+        @setup_file ||= T.let(get_original_file("setup.py"), T.nilable(Dependabot::DependencyFile))
       end
 
+      sig { returns(T.nilable(Dependabot::DependencyFile)) }
       def setup_cfg_file
-        @setup_cfg_file ||= get_original_file("setup.cfg")
+        @setup_cfg_file ||= T.let(get_original_file("setup.cfg"), T.nilable(Dependabot::DependencyFile))
       end
 
+      sig { returns(T::Array[Dependabot::DependencyFile]) }
       def pip_compile_files
-        @pip_compile_files ||=
-          dependency_files.select { |f| f.name.end_with?(".in") }
+        @pip_compile_files ||= T.let(dependency_files.select { |f| f.name.end_with?(".in") }, T.untyped)
       end
 
+      sig { returns(Dependabot::Python::PipCompileFileMatcher) }
       def pip_compile_file_matcher
-        @pip_compile_file_matcher ||= PipCompileFileMatcher.new(pip_compile_files)
+        @pip_compile_file_matcher ||= T.let(PipCompileFileMatcher.new(pip_compile_files),
+                                            T.nilable(Dependabot::Python::PipCompileFileMatcher))
       end
     end
   end

diff --git a/python/lib/dependabot/python/pipenv_runner.rb b/python/lib/dependabot/python/pipenv_runner.rb
@@ -26,6 +26,7 @@ def run_upgrade(constraint)
         run(command, fingerprint: "pyenv exec pipenv upgrade --verbose <dependency_name><constraint>")
       end
 
+      sig { params(constraint: String).void }
       def run_upgrade_and_fetch_version(constraint)
         run_upgrade(constraint)